From ff76cdd3c517c01b5beb0492fb08fc758b098741 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Thu, 7 Jul 2022 17:32:49 +0300 Subject: [PATCH] Scheduling articles (#17939) * Article query spec for scheduled articles * Added scheduled article badge on the user dashboard * Added published_at field to editor options * Accept and validate published_at from editor * Refactor published_at validation * Allow 1-minute difference in published_at * Notice on an unpublished article page * Added specs for 'Click to edit' link on scheduled article preview page * ContextNotification model * Articles::Publish worker * Added specs for articles publish worker * Schedule publish articles worker * Added tests to check for scheduled posts in feeds * Don't allow managing scheduled articles * Don't send notifications for scheduled articles * Set published_at in Articles::Updater when publishing * Published_at value in post options * Pass timezone and set published_at accordingly * Limit setting published_at to the future * Readonly published_at for articles that were already published * Chagning published_at format in editor v1 (start) * Changed published_at format in frontmatter, specs * Added specs for updating published_at from frontmatter * Fixed accepting past published_at for articles published_from_feed * Enabled published_at validation: don't allow updating published_at for already published articles * Validate published_at on create * Added a spec for updating published_at for exported articles * Fixed specs related to creating articles with past published_at * Fixed specs related to past published_at for articles * Added a hack so that admins would be able to update published_at * Switch button text schedule/publish when changin publishedAt * Fixed saving published_at with timezone * Added a feature flag for scheduling articles * Default text in markdown editor depends on feature flag * Enable article editor cache again * Fixed the default value in the markdown editor * Fix sitemaps spec * Removed tooltip * Fixed articles update specs * Added missing locales * Fixed article create specs * Fixed spec * Removed commented code * Returned enabling extensions in the schema * Returned accidentally deleted constraint * Make articles query spec more stable Co-authored-by: Jeremy Friesen * Removed commented code * Removed unused code * A clearer policy Co-authored-by: Jeremy Friesen * Use StringInquirer for article current state * Added a note and todo to articles factory past trait * Remove duplicated PropType Co-authored-by: Suzanne Aitchison * Refactor query in the Articles::PublishWorker Co-authored-by: Mac Siri * Refactor articleForm.jsx Co-authored-by: Mac Siri * Removed specs that are no longer relevant * Removed useless onKeyUp on a hidden input * Refactored articleForm * Hide scheduling from post options when published_at is readonly * Run sends notifications worker every 5 minutes instead of every minute Co-authored-by: Jeremy Friesen Co-authored-by: Suzanne Aitchison Co-authored-by: Mac Siri --- app/controllers/admin/articles_controller.rb | 2 +- app/controllers/articles_controller.rb | 15 ++- app/controllers/stories_controller.rb | 2 +- app/decorators/article_decorator.rb | 13 ++- app/javascript/article-form/articleForm.jsx | 13 ++- .../article-form/components/EditorActions.jsx | 17 ++- .../article-form/components/Options.jsx | 58 ++++++++++ app/javascript/packs/articleForm.jsx | 5 +- app/models/article.rb | 42 +++++-- app/policies/application_policy.rb | 4 - app/policies/article_policy.rb | 4 + app/services/articles/attributes.rb | 9 +- app/services/articles/builder.rb | 4 +- app/services/articles/creator.rb | 3 - app/services/articles/updater.rb | 17 +-- app/views/articles/_v2_form.html.erb | 5 +- app/views/articles/new.html.erb | 2 +- app/views/articles/show.html.erb | 6 +- .../_dashboard_article_row.html.erb | 6 +- app/workers/articles/publish_worker.rb | 20 ++++ config/locales/models/en.yml | 2 + config/locales/models/fr.yml | 2 + config/locales/views/articles/en.yml | 4 + config/locales/views/articles/fr.yml | 4 + config/locales/views/dashboard/en.yml | 2 + config/locales/views/dashboard/fr.yml | 2 + config/schedule.yml | 4 + ...3333_add_schedule_articles_feature_flag.rb | 7 ++ spec/decorators/article_decorator_spec.rb | 6 + spec/factories/articles.rb | 10 ++ ...add_schedule_articles_feature_flag_spec.rb | 24 ++++ .../goal_conversion_handler_spec.rb | 16 +-- spec/models/article_spec.rb | 35 ++++-- .../articles/active_threads_query_spec.rb | 15 +-- spec/queries/homepage/articles_query_spec.rb | 6 + spec/requests/admin/overview_spec.rb | 18 +-- spec/requests/api/v0/articles_spec.rb | 25 +---- spec/requests/api/v1/articles_spec.rb | 21 ---- .../requests/articles/articles_create_spec.rb | 65 ++++++++++- spec/requests/articles/articles_show_spec.rb | 21 ++++ spec/requests/articles/articles_spec.rb | 10 ++ .../requests/articles/articles_update_spec.rb | 95 ++++++++++++++-- spec/requests/dashboard_spec.rb | 19 ++++ spec/requests/sitemaps_spec.rb | 2 +- spec/requests/stories/tagged_articles_spec.rb | 10 +- spec/requests/stories_index_spec.rb | 6 + spec/services/admin/charts_data_spec.rb | 5 +- spec/services/articles/attributes_spec.rb | 20 ++++ spec/services/articles/builder_spec.rb | 19 +++- spec/services/articles/creator_spec.rb | 26 ----- spec/services/articles/feeds/basic_spec.rb | 6 +- .../feeds/large_forem_experimental_spec.rb | 6 +- spec/services/articles/feeds/latest_spec.rb | 6 +- .../services/articles/feeds/timeframe_spec.rb | 4 +- spec/services/articles/updater_spec.rb | 80 ++++++++++++-- spec/services/badges/award_streak_spec.rb | 8 +- spec/services/search/article_spec.rb | 5 +- spec/system/articles/feeds/basic_spec.rb | 2 +- .../articles/user_visits_an_article_spec.rb | 25 +++++ .../user_visits_articles_by_tag_spec.rb | 6 +- .../user_visits_articles_by_timeframe_spec.rb | 8 +- spec/workers/articles/publish_worker_spec.rb | 104 ++++++++++++++++++ 62 files changed, 797 insertions(+), 211 deletions(-) create mode 100644 app/workers/articles/publish_worker.rb create mode 100644 lib/data_update_scripts/20220613093333_add_schedule_articles_feature_flag.rb create mode 100644 spec/lib/data_update_scripts/add_schedule_articles_feature_flag_spec.rb create mode 100644 spec/workers/articles/publish_worker_spec.rb diff --git a/app/controllers/admin/articles_controller.rb b/app/controllers/admin/articles_controller.rb index bdd196609..197375c74 100644 --- a/app/controllers/admin/articles_controller.rb +++ b/app/controllers/admin/articles_controller.rb @@ -40,7 +40,7 @@ module Admin def update article = Article.find(params[:id]) - if article.update(article_params) + if article.update(article_params.merge(admin_update: true)) flash[:success] = I18n.t("admin.articles_controller.saved") else flash[:danger] = article.errors_as_sentence diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index c22caf85c..3654f4f0c 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -146,7 +146,6 @@ class ArticlesController < ApplicationController def create authorize Article - @user = current_user article = Articles::Creator.call(@user, article_params_json) @@ -160,7 +159,6 @@ class ArticlesController < ApplicationController def update authorize @article @user = @article.user || current_user - updated = Articles::Updater.call(@user, @article, article_params_json) respond_to do |format| @@ -303,6 +301,8 @@ class ArticlesController < ApplicationController # TODO: refactor all of this update logic into the Articles::Updater possibly, # ideally there should only be one place to handle the update logic def article_params_json + return @article_params_json if @article_params_json + params.require(:article) # to trigger the correct exception in case `:article` is missing params["article"].transform_keys!(&:underscore) @@ -312,7 +312,7 @@ class ArticlesController < ApplicationController else %i[ title body_markdown main_image published description video_thumbnail_url - tag_list canonical_url series collection_id archived + tag_list canonical_url series collection_id archived published_at timezone ] end @@ -325,7 +325,14 @@ class ArticlesController < ApplicationController allowed_params << :organization_id end - params.require(:article).permit(allowed_params) + time_zone_str = params["article"].delete("timezone") + if params["article"]["published_at"] + time_zone = Time.find_zone(time_zone_str) + time_zone ||= Time.find_zone("UTC") + params["article"]["published_at"] = time_zone.parse(params["article"]["published_at"]) + end + + @article_params_json = params.require(:article).permit(allowed_params) end def allowed_to_change_org_id? diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index d973d0ca3..2016df528 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -274,7 +274,7 @@ class StoriesController < ApplicationController end def permission_denied? - !@article.published && params[:preview] != @article.password + (!@article.published || @article.scheduled?) && params[:preview] != @article.password end def assign_co_authors diff --git a/app/decorators/article_decorator.rb b/app/decorators/article_decorator.rb index 7fe4f930b..f5b31751c 100644 --- a/app/decorators/article_decorator.rb +++ b/app/decorators/article_decorator.rb @@ -10,8 +10,19 @@ class ArticleDecorator < ApplicationDecorator DataInfo.to_json(object: cached_user, class_name: "User", id: user_id, style: "full") end + def current_state + state = if !published? + "unpublished" + elsif scheduled? + "scheduled" + else + "published" + end + ActiveSupport::StringInquirer.new(state) + end + def current_state_path - published ? "/#{username}/#{slug}" : "/#{username}/#{slug}?preview=#{password}" + current_state.published? ? "/#{username}/#{slug}" : "/#{username}/#{slug}?preview=#{password}" end def processed_canonical_url diff --git a/app/javascript/article-form/articleForm.jsx b/app/javascript/article-form/articleForm.jsx index 94f3e9a30..9838e3793 100644 --- a/app/javascript/article-form/articleForm.jsx +++ b/app/javascript/article-form/articleForm.jsx @@ -18,7 +18,7 @@ import { getOSKeyboardModifierKeyString } from '@utilities/runtime'; /* global activateRunkitTags */ /* - Although the state fields: id, description, canonicalUrl, series, allSeries and + Although the state fields: id, description, canonicalUrl, publishedAt, series, allSeries and editing are not used in this file, they are important to the editor. */ @@ -63,6 +63,7 @@ export class ArticleForm extends Component { article: PropTypes.string.isRequired, organizations: PropTypes.string, siteLogo: PropTypes.string.isRequired, + schedulingEnabled: PropTypes.bool.isRequired, }; static defaultProps = { @@ -71,7 +72,7 @@ export class ArticleForm extends Component { constructor(props) { super(props); - const { article, version, siteLogo } = this.props; + const { article, version, siteLogo, schedulingEnabled } = this.props; let { organizations } = this.props; this.article = JSON.parse(article); organizations = organizations ? JSON.parse(organizations) : null; @@ -102,10 +103,13 @@ export class ArticleForm extends Component { tagList: this.article.cached_tag_list || '', description: '', // eslint-disable-line react/no-unused-state canonicalUrl: this.article.canonical_url || '', // eslint-disable-line react/no-unused-state + publishedAt: this.article.published_at || '', // eslint-disable-line react/no-unused-state + timezone: Intl.DateTimeFormat().resolvedOptions().timeZone || '', // eslint-disable-line react/no-unused-state series: this.article.series || '', // eslint-disable-line react/no-unused-state allSeries: this.article.all_series || [], // eslint-disable-line react/no-unused-state bodyMarkdown: this.article.body_markdown || '', published: this.article.published || false, + schedulingEnabled, previewShowing: false, previewLoading: false, previewResponse: { processed_html: '' }, @@ -331,6 +335,7 @@ export class ArticleForm extends Component { tagList: this.article.cached_tag_list || '', description: '', // eslint-disable-line react/no-unused-state canonicalUrl: this.article.canonical_url || '', // eslint-disable-line react/no-unused-state + publishedAt: this.article.published_at || '', // eslint-disable-line react/no-unused-state series: this.article.series || '', // eslint-disable-line react/no-unused-state allSeries: this.article.all_series || [], // eslint-disable-line react/no-unused-state bodyMarkdown: this.article.body_markdown || '', @@ -392,9 +397,11 @@ export class ArticleForm extends Component { tagList, bodyMarkdown, published, + publishedAt, previewShowing, previewLoading, previewResponse, + schedulingEnabled, submitting, organizations, organizationId, @@ -491,6 +498,8 @@ export class ArticleForm extends Component { now; + + const saveButtonText = schedule + ? 'Schedule' + : published || isVersion1 + ? 'Save changes' + : 'Publish'; + return (
{!(published || isVersion1) && ( @@ -59,6 +71,7 @@ export const EditorActions = ({ {isVersion2 && ( { let publishedField = ''; let existingSeries = ''; + let publishedAtField = ''; + const publishedDate = new Date(publishedAt); + const currentDate = new Date(); + + const localPublishedAt = toISOStringLocal(publishedDate); + const minPublishedAt = toISOStringLocal(currentDate); + + const readonlyPublishedAt = published && publishedDate < currentDate; if (allSeries.length > 0) { const seriesNames = allSeries.map((name, index) => { @@ -66,6 +92,34 @@ export const Options = ({
); } + if (schedulingEnabled && !readonlyPublishedAt) { + publishedAtField = ( +
+ + + +
+ ); + } + return (
+ {publishedAtField}