forked from forem/forem
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Extract article markdown rendering to service (forem#18754)
* Beginning, needs cleanup & tests * Minor refactor/cleanup * Wondering if this factory ever worked? * Minor tweak for blank body processing * Better error handling for ContentRenderer * Added some tests around ContentRenderer error conditions * This test seems fine? * Mocking internal dependencies * Use new extraction for has_frontmatter? * FeatureFlag :consistent_rendering * Try to enable per-actor feature flag
- Loading branch information
Showing
8 changed files
with
241 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
class ContentRenderer | ||
class_attribute :fixer, default: MarkdownProcessor::Fixer::FixAll | ||
class_attribute :front_matter_parser, default: FrontMatterParser::Parser.new(:md) | ||
class_attribute :processor, default: MarkdownProcessor::Parser | ||
|
||
class ContentParsingError < StandardError | ||
end | ||
|
||
delegate :calculate_reading_time, to: :processed | ||
delegate :content, :front_matter, to: :parsed_input | ||
|
||
attr_reader :input, :source, :user | ||
|
||
def initialize(input, source:, user:) | ||
@input = input || "" | ||
@source = source | ||
@user = user | ||
end | ||
|
||
def processed | ||
@processed ||= processor.new(content, source: source, user: user) | ||
# TODO: Replicating prior behaviour, but this swallows errors we probably shouldn't | ||
rescue StandardError => e | ||
raise ContentParsingError, e.message | ||
end | ||
|
||
def finalize | ||
processed.finalize | ||
# TODO: Replicating prior behaviour, but this swallows errors we probably shouldn't | ||
rescue StandardError => e | ||
raise ContentParsingError, e.message | ||
end | ||
|
||
private | ||
|
||
def fix(markdown) | ||
fixer.call(markdown) | ||
# TODO: Replicating prior behaviour, but this swallows errors we probably shouldn't | ||
rescue StandardError => e | ||
raise ContentParsingError, e.message | ||
end | ||
|
||
def parse_front_matter(markdown) | ||
front_matter_parser.call(markdown) | ||
# TODO: Replicating prior behaviour, but this swallows errors we probably shouldn't | ||
rescue StandardError => e | ||
raise ContentParsingError, e.message | ||
end | ||
|
||
def parsed_input | ||
@parsed_input = parse_front_matter(fix(input)) | ||
# TODO: Replicating prior behaviour, but this swallows errors we probably shouldn't | ||
rescue StandardError => e | ||
raise ContentParsingError, e.message | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
require "rails_helper" | ||
|
||
RSpec.describe ContentRenderer do | ||
describe "#finalize" do | ||
let(:markdown) { "hello, hey" } | ||
let(:expected_result) { "<p>hello, hey</p>\n\n" } | ||
let(:mock_fixer) { class_double MarkdownProcessor::Fixer::FixAll } | ||
let(:mock_front_matter_parser) { instance_double FrontMatterParser::Parser } | ||
let(:mock_processor) { class_double MarkdownProcessor::Parser } | ||
let(:fixed_markdown) { :fixed_markdown } | ||
let(:parsed_contents) { Struct.new(:content).new(:parsed_content) } | ||
let(:processed_contents) { instance_double MarkdownProcessor::Parser } | ||
|
||
# rubocop:disable RSpec/InstanceVariable | ||
before do | ||
allow(mock_fixer).to receive(:call).and_return(fixed_markdown) | ||
allow(mock_front_matter_parser).to receive(:call).with(fixed_markdown).and_return(parsed_contents) | ||
allow(mock_processor).to receive(:new).and_return(processed_contents) | ||
allow(processed_contents).to receive(:finalize).and_return(expected_result) | ||
|
||
@original_fixer = described_class.fixer | ||
@original_parser = described_class.front_matter_parser | ||
@original_processor = described_class.processor | ||
|
||
described_class.fixer = mock_fixer | ||
described_class.front_matter_parser = mock_front_matter_parser | ||
described_class.processor = mock_processor | ||
end | ||
|
||
after do | ||
described_class.fixer = @original_fixer | ||
described_class.front_matter_parser = @original_parser | ||
described_class.processor = @original_processor | ||
end | ||
# rubocop:enable RSpec/InstanceVariable | ||
|
||
it "is the result of fixing, parsing, and processing" do | ||
result = described_class.new(markdown, source: nil, user: nil).finalize | ||
expect(result).to eq(expected_result) | ||
expect(mock_fixer).to have_received(:call) | ||
expect(mock_front_matter_parser).to have_received(:call).with(fixed_markdown) | ||
expect(mock_processor).to have_received(:new) | ||
expect(processed_contents).to have_received(:finalize) | ||
end | ||
end | ||
|
||
context "when markdown is valid" do | ||
let(:markdown) { "# Hey\n\nHi, hello there, what's up?" } | ||
let(:expected_result) { <<~RESULT } | ||
<h1> | ||
<a name="hey" href="#hey"> | ||
</a> | ||
Hey | ||
</h1> | ||
<p>Hi, hello there, what's up?</p> | ||
RESULT | ||
|
||
it "processes markdown" do | ||
result = described_class.new(markdown, source: nil, user: nil).finalize | ||
expect(result).to eq(expected_result) | ||
end | ||
end | ||
|
||
context "when markdown has liquid tags that aren't allowed for user" do | ||
let(:markdown) { "hello hey hey hey {% poll 123 %}" } | ||
let(:article) { build(:article) } | ||
let(:user) { instance_double(User) } | ||
|
||
before do | ||
allow(user).to receive(:any_admin?).and_return(false) | ||
end | ||
|
||
it "raises ContentParsingError" do | ||
expect do | ||
described_class.new(markdown, source: article, user: user).finalize | ||
end.to raise_error(ContentRenderer::ContentParsingError, /User is not permitted to use this liquid tag/) | ||
end | ||
end | ||
|
||
context "when markdown has liquid tags that aren't allowed for source" do | ||
let(:markdown) { "hello hey hey hey {% poll 123 %}" } | ||
let(:source) { build(:comment) } | ||
let(:user) { instance_double(User) } | ||
|
||
before do | ||
allow(user).to receive(:any_admin?).and_return(true) | ||
end | ||
|
||
it "raises ContentParsingError" do | ||
expect do | ||
described_class.new(markdown, source: source, user: user).finalize | ||
end.to raise_error(ContentRenderer::ContentParsingError, /This liquid tag can only be used in Articles/) | ||
end | ||
end | ||
|
||
context "when markdown has invalid frontmatter" do | ||
let(:markdown) { "---\ntitle: Title\npublished: false\npublished_at:2022-12-05 18:00 +0300---\n\n" } | ||
|
||
it "raises ContentParsingError" do | ||
expect do | ||
described_class.new(markdown, source: nil, user: nil).front_matter | ||
end.to raise_error(ContentRenderer::ContentParsingError, /while scanning a simple key/) | ||
end | ||
end | ||
end |