From 41ab84a25d84272f1174e3b0007e870a7b14ed58 Mon Sep 17 00:00:00 2001 From: Anna Buianova Date: Fri, 3 Nov 2023 18:42:12 +0300 Subject: [PATCH] Remove feature flag and use consistent rendering by default (#20305) --- app/controllers/articles_controller.rb | 16 ++----- app/controllers/comments_controller.rb | 12 ++---- app/models/article.rb | 43 +------------------ app/models/comment.rb | 16 ------- spec/models/article_spec.rb | 2 - spec/models/comment_spec.rb | 2 - .../requests/articles/articles_create_spec.rb | 1 - spec/requests/articles/articles_show_spec.rb | 2 - .../requests/articles/articles_update_spec.rb | 1 - spec/requests/comments_spec.rb | 22 ---------- spec/requests/editor_spec.rb | 29 ------------- spec/services/articles/creator_spec.rb | 1 - spec/services/articles/updater_spec.rb | 1 - 13 files changed, 8 insertions(+), 140 deletions(-) diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index b96acd481..50150aec1 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -111,18 +111,10 @@ class ArticlesController < ApplicationController authorize Article begin - if FeatureFlag.enabled?(:consistent_rendering, FeatureFlag::Actor[current_user]) - renderer = ContentRenderer.new(params[:article_body], source: Article.new, user: current_user) - result = renderer.process_article - processed_html = result.processed_html - front_matter = result.front_matter.to_h - else - fixed_body_markdown = MarkdownProcessor::Fixer::FixForPreview.call(params[:article_body]) - parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown) - parsed_markdown = MarkdownProcessor::Parser.new(parsed.content, source: Article.new, user: current_user) - processed_html = parsed_markdown.finalize - front_matter = parsed.front_matter.to_h - end + renderer = ContentRenderer.new(params[:article_body], source: Article.new, user: current_user) + result = renderer.process_article + processed_html = result.processed_html + front_matter = result.front_matter.to_h rescue StandardError => e @article = Article.new(body_markdown: params[:article_body]) @article.errors.add(:base, ErrorMessages::Clean.call(e.message)) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 2ad70c09e..7980000c4 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -187,14 +187,8 @@ class CommentsController < ApplicationController skip_authorization begin permitted_body_markdown = permitted_attributes(Comment)[:body_markdown] - if FeatureFlag.enabled?(:consistent_rendering, FeatureFlag::Actor[current_user]) - renderer = ContentRenderer.new(permitted_body_markdown, source: self, user: current_user) - processed_html = renderer.process.processed_html - else - fixed_body_markdown = MarkdownProcessor::Fixer::FixForPreview.call(permitted_body_markdown) - parsed_markdown = MarkdownProcessor::Parser.new(fixed_body_markdown, source: Comment.new, user: current_user) - processed_html = parsed_markdown.finalize - end + renderer = ContentRenderer.new(permitted_body_markdown, source: self, user: current_user) + processed_html = renderer.process.processed_html rescue StandardError => e processed_html = I18n.t("comments_controller.markdown_html", error: e) end @@ -223,7 +217,7 @@ class CommentsController < ApplicationController if success @comment&.commentable&.update_column(:any_comments_hidden, true) if params[:hide_children] == "1" - @comment.descendants.includes(:user, :commentable).each do |c| + @comment.descendants.includes(:user, :commentable).find_each do |c| c.update(hidden_by_commentable_user: true) end end diff --git a/app/models/article.rb b/app/models/article.rb index 6eb624fbe..627d5c3de 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -478,22 +478,7 @@ class Article < ApplicationRecord end def has_frontmatter? - if FeatureFlag.enabled?(:consistent_rendering, FeatureFlag::Actor[user]) - processed_content.has_front_matter? - else - original_has_frontmatter? - end - end - - def original_has_frontmatter? - fixed_body_markdown = MarkdownProcessor::Fixer::FixAll.call(body_markdown) - begin - parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown) - parsed.front_matter["title"].present? - rescue Psych::SyntaxError, Psych::DisallowedClass - # if frontmatter is invalid, still render editor with errors instead of 500ing - true - end + processed_content.has_front_matter? end def class_name @@ -687,14 +672,6 @@ class Article < ApplicationRecord end def evaluate_markdown - if FeatureFlag.enabled?(:consistent_rendering, FeatureFlag::Actor[user]) - extracted_evaluate_markdown - else - original_evaluate_markdown - end - end - - def extracted_evaluate_markdown content_renderer = processed_content return unless content_renderer @@ -716,24 +693,6 @@ class Article < ApplicationRecord errors.add(:base, ErrorMessages::Clean.call(e.message)) end - def original_evaluate_markdown - fixed_body_markdown = MarkdownProcessor::Fixer::FixAll.call(body_markdown || "") - parsed = FrontMatterParser::Parser.new(:md).call(fixed_body_markdown) - parsed_markdown = MarkdownProcessor::Parser.new(parsed.content, source: self, user: user) - self.reading_time = parsed_markdown.calculate_reading_time - self.processed_html = parsed_markdown.finalize - - if parsed.front_matter.any? - evaluate_front_matter(parsed.front_matter) - elsif tag_list.any? - set_tag_list(tag_list) - end - - self.description = processed_description if description.blank? - rescue StandardError => e - errors.add(:base, ErrorMessages::Clean.call(e.message)) - end - def set_tag_list(tags) self.tag_list = [] # overwrite any existing tag with those from the front matter tag_list.add(tags, parse: true) diff --git a/app/models/comment.rb b/app/models/comment.rb index 9b77b4f0e..6cd9dfc38 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -229,14 +229,6 @@ class Comment < ApplicationRecord end def evaluate_markdown - if FeatureFlag.enabled?(:consistent_rendering, FeatureFlag::Actor[user]) - extracted_evaluate_markdown - else - original_evaluate_markdown - end - end - - def extracted_evaluate_markdown return unless user renderer = ContentRenderer.new(body_markdown, source: self, user: user) @@ -247,14 +239,6 @@ class Comment < ApplicationRecord errors.add(:base, ErrorMessages::Clean.call(e.message)) end - def original_evaluate_markdown - fixed_body_markdown = MarkdownProcessor::Fixer::FixForComment.call(body_markdown) - parsed_markdown = MarkdownProcessor::Parser.new(fixed_body_markdown, source: self, user: user) - self.processed_html = parsed_markdown.finalize(link_attributes: { rel: "nofollow" }) - wrap_timestamps_if_video_present! if commentable - shorten_urls! - end - def adjust_comment_parent_based_on_depth self.parent_id = parent.descendant_ids.last if parent_exists? && (parent.depth > 1 && parent.has_children?) end diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb index b2d1c5eb6..957089366 100644 --- a/spec/models/article_spec.rb +++ b/spec/models/article_spec.rb @@ -7,8 +7,6 @@ RSpec.describe Article do article end - before { allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) } - let(:user) { create(:user) } let!(:article) { create(:article, user: user) } diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 571cb4a65..12419140b 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -5,8 +5,6 @@ RSpec.describe Comment do let(:article) { create(:article, user: user) } let(:comment) { create(:comment, user: user, commentable: article) } - before { allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) } - include_examples "#sync_reactions_count", :article_comment describe "validations" do diff --git a/spec/requests/articles/articles_create_spec.rb b/spec/requests/articles/articles_create_spec.rb index e040e444f..eb3d1c068 100644 --- a/spec/requests/articles/articles_create_spec.rb +++ b/spec/requests/articles/articles_create_spec.rb @@ -7,7 +7,6 @@ RSpec.describe "ArticlesCreate" do before do sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) end it "creates ordinary article with proper params" do diff --git a/spec/requests/articles/articles_show_spec.rb b/spec/requests/articles/articles_show_spec.rb index 30bb2d043..0730fee1d 100644 --- a/spec/requests/articles/articles_show_spec.rb +++ b/spec/requests/articles/articles_show_spec.rb @@ -65,8 +65,6 @@ RSpec.describe "ArticlesShow" do end describe "GET /:username/:slug (scheduled)" do - before { allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) } - let(:scheduled_article) { create(:article, published: true, published_at: Date.tomorrow) } let(:query_params) { "?preview=#{scheduled_article.password}" } let(:scheduled_article_path) { scheduled_article.path + query_params } diff --git a/spec/requests/articles/articles_update_spec.rb b/spec/requests/articles/articles_update_spec.rb index 3e2428a82..af0b511a3 100644 --- a/spec/requests/articles/articles_update_spec.rb +++ b/spec/requests/articles/articles_update_spec.rb @@ -15,7 +15,6 @@ RSpec.describe "ArticlesUpdate" do before do sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) end it "updates ordinary article with proper params" do diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index d0a3fe913..e80706ef4 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -254,7 +254,6 @@ RSpec.describe "Comments" do describe "PUT /comments/:id" do before do - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) sign_in user end @@ -293,25 +292,6 @@ RSpec.describe "Comments" do context "when logged-in and consistent rendering" do before do sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) - post "/comments/preview", - params: { comment: { body_markdown: "hi" } }, - headers: { HTTP_ACCEPT: "application/json" } - end - - it "returns 200 on good request" do - expect(response).to have_http_status(:ok) - end - - it "returns json" do - expect(response.media_type).to eq("application/json") - end - end - - context "when logged-in and inconsistent rendering" do - before do - sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(false) post "/comments/preview", params: { comment: { body_markdown: "hi" } }, headers: { HTTP_ACCEPT: "application/json" } @@ -339,8 +319,6 @@ RSpec.describe "Comments" do } end - before { allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) } - context "when a user is comment_suspended" do before do sign_in user diff --git a/spec/requests/editor_spec.rb b/spec/requests/editor_spec.rb index 37a3aeaf2..e752dda0b 100644 --- a/spec/requests/editor_spec.rb +++ b/spec/requests/editor_spec.rb @@ -57,7 +57,6 @@ RSpec.describe "Editor" do context "when logged-in" do before do sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(false) end it "returns json" do @@ -81,33 +80,5 @@ RSpec.describe "Editor" do expect(response).to be_successful end end - - context "when logged-in + consistent rendering" do - before do - sign_in user - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) - end - - it "returns json" do - post "/articles/preview", headers: headers - expect(response.media_type).to eq("application/json") - end - - it "returns successfully" do - article_body = <<~MARKDOWN - --- - --- - - Hello - MARKDOWN - - post "/articles/preview", - headers: headers, - params: { article_body: article_body }, - as: :json - - expect(response).to be_successful - end - end end end diff --git a/spec/services/articles/creator_spec.rb b/spec/services/articles/creator_spec.rb index 458d403be..07e3413a0 100644 --- a/spec/services/articles/creator_spec.rb +++ b/spec/services/articles/creator_spec.rb @@ -4,7 +4,6 @@ RSpec.describe Articles::Creator, type: :service do let(:user) { create(:user) } before do - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) allow(SegmentedUserRefreshWorker).to receive(:perform_async) end diff --git a/spec/services/articles/updater_spec.rb b/spec/services/articles/updater_spec.rb index 2c9d3e349..d44f19033 100644 --- a/spec/services/articles/updater_spec.rb +++ b/spec/services/articles/updater_spec.rb @@ -7,7 +7,6 @@ RSpec.describe Articles::Updater, type: :service do let(:draft) { create(:article, user: user, published: false, published_at: nil) } before do - allow(FeatureFlag).to receive(:enabled?).with(:consistent_rendering, any_args).and_return(true) allow(SegmentedUserRefreshWorker).to receive(:perform_async) end