Skip to content

Commit

Permalink
Fix additional element placement on empty articles
Browse files Browse the repository at this point in the history
If an article contains no elements but additional elements are place on
it, it previously introduced a `nil` element into the article elements.
This in turn then led the exporters to crash.

Now we check if there are any elements in the original article and if
not, the element place simply returns the additional elements.
  • Loading branch information
nicolas-fricke committed Nov 29, 2017
1 parent 5c6cbc1 commit 7f14ceb
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## WIP
- Fix additional element placement on empty articles

## 0.3.0 - 2017/11/21
In this third bigger release we **added support**:
- For exporting articles in the Facebook Instant Article format
Expand Down
3 changes: 2 additions & 1 deletion lib/article_json/utils/additional_element_placer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class AdditionalElementPlacer
# @param [Array[ArticleJSON::Article::Elements::Base]] elements
# @param [Array[Object]] additional_elements
def initialize(elements, additional_elements)
@elements = elements.dup
@elements = elements&.dup
@additional_elements = additional_elements
end

Expand Down Expand Up @@ -39,6 +39,7 @@ def initialize(elements, additional_elements)
#
# @return [Array[ArticleJSON::Elements::Base|Object]]
def merge_elements
return @additional_elements if @elements.nil? || @elements.empty?
remaining_elements = @additional_elements.dup
next_in = insert_next_element_in(0, remaining_elements)
characters_passed = 0
Expand Down
26 changes: 26 additions & 0 deletions spec/article_json/utils/additional_element_placer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,31 @@
end
end
end

context 'when the original article only contains one element' do
let(:text) { ArticleJSON::Elements::Text.new(content: 'Lorem Ipsum ') }
let(:paragraph) { ArticleJSON::Elements::Paragraph.new(content: [text]) }
let(:article_elements) { [paragraph] }
let(:element_1) { double('additional_element_1', type: :element_1) }
let(:element_2) { double('additional_element_2', type: :element_2) }
let(:additional_elements) { [element_1, element_2] }
it { should eq [*article_elements, *additional_elements] }
end

context 'when the original article is empty' do
let(:article_elements) { [] }
let(:element_1) { double('additional_element_1', type: :element_1) }
let(:element_2) { double('additional_element_2', type: :element_2) }
let(:additional_elements) { [element_1, element_2] }
it { should eq additional_elements }
end

context 'when the original article elements are `nil`' do
let(:article_elements) { nil }
let(:element_1) { double('additional_element_1', type: :element_1) }
let(:element_2) { double('additional_element_2', type: :element_2) }
let(:additional_elements) { [element_1, element_2] }
it { should eq additional_elements }
end
end
end

0 comments on commit 7f14ceb

Please sign in to comment.