Merge branch 'refactor_display_content' into 'master'

Refactor DisplayContentPlugin Removes HTML generation from class into views. This is the last plugin requiring such refactors allowing backports to get removed. See merge request !927

Merge branch 'refactor_display_content' into 'master'
Refactor DisplayContentPlugin Removes HTML generation from class into views. This is the last plugin requiring such refactors allowing backports to get removed. See merge request !927
Braulio Bhavamitra
2 parents 0834f87c 203192d0
Showing 7 changed files with 168 additions and 166 deletions Show diff stats
plugins/display_content/lib/display_content_block.rb
plugins/display_content/test/unit/display_content_block_test.rb
plugins/display_content/views/blocks/_document.slim
plugins/display_content/views/blocks/_read_more.slim
plugins/display_content/views/blocks/_section.slim
plugins/display_content/views/blocks/_tag.slim
plugins/display_content/views/blocks/display_content.slim
@@ -118,69 +118,28 @@ class DisplayContentBlock &lt; Block
     holder.articles.where(type: types, parent_id: if parent.nil? then nil else parent end)
   end
-  def content(args={})
-    block = self
-
+  def docs
     order_string = "published_at"
     order_string += " DESC" if order_by_recent
     limit_final = [limit_to_show, 0].max
-    docs = owner.articles.order(order_string)
+    documents = owner.articles.order(order_string)
       .where(articles: {type: self.types})
       .includes(:profile, :image, :tags)
     if nodes.present?
       nodes_conditions  = 'articles.id IN(:nodes)'
       nodes_conditions << ' OR articles.parent_id IN(:nodes) ' if display_folder_children
-      docs = docs.where nodes_conditions, nodes: nodes
+      documents = documents.where nodes_conditions, nodes: nodes
     end
-    docs = docs.limit limit_final if display_folder_children
+    documents = documents.limit limit_final if display_folder_children
     if content_with_translations
-      docs = docs.native_translations
-      docs.replace docs.map{ |p| p.get_translation_to(FastGettext.locale) }.compact
+      documents = documents.native_translations
+      documents.replace documents.map{ |p| p.get_translation_to(FastGettext.locale) }.compact
     end
-    proc do
-      block.block_title(block.title, block.subtitle) +
-        content_tag('ul', docs.map {|item|
-        if !item.folder? && item.class != RssFeed
-          content_sections = ''
-          read_more_section = ''
-          tags_section = ''
-
-          block.sections.select { |section|
-            case section[:value]
-            when 'publish_date'
-              content_sections += (block.display_section?(section) ? (content_tag('div', show_date(item.published_at, false), :class => 'published-at') ) : '')
-            when 'title'
-              content_sections += (block.display_section?(section) ? (content_tag('div', link_to(h(item.title), item.url), :class => 'title') ) : '')
-            when 'abstract'
-              content_sections += (block.display_section?(section) ? (content_tag('div', item.abstract.html_safe , :class => 'lead')) : '' )
-              if block.display_section?(section)
-                read_more_section = content_tag('div', link_to(_('Read more'), item.url), :class => 'read_more')
-              end
-            when 'body'
-              content_sections += (block.display_section?(section) ? (content_tag('div', item.body.html_safe ,:class => 'body')) : '' )
-            when 'image'
-              image_section = image_tag item.image.public_filename if item.image
-              if !image_section.blank?
-                content_sections += (block.display_section?(section) ? (content_tag('div', link_to( image_section, item.url ) ,:class => 'image')) : '' )
-              end
-            when 'tags'
-              if !item.tags.empty?
-                tags_section = item.tags.map { |t| content_tag('span', t.name) }.join("")
-                content_sections += (block.display_section?(section) ? (content_tag('div', tags_section, :class => 'tags')) : '')
-              end
-            end
-          }
-
-          content_sections += read_more_section if !read_more_section.blank?
-#raise sections.inspect
-          content_tag('li', content_sections.html_safe)
-        end
-      }.join(" ").html_safe)
-    end
+    documents
   end
   def url_params
@@ -347,53 +347,6 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
   end
-  should 'list links for all articles title defined in nodes' do
-    profile = create_user('testuser').person
-    Article.delete_all
-    a1 = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id)
-    a2 = fast_create(TextileArticle, :name => 'test article 2', :profile_id => profile.id)
-
-    block = DisplayContentBlock.new
-    block.sections = [{:value => 'title', :checked => true}]
-    block.nodes = [a1.id, a2.id]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /.*<a.*>#{a1.title}<\/a>/, instance_eval(&block.content)
-    assert_match /.*<a.*>#{a2.title}<\/a>/, instance_eval(&block.content)
-  end
-
-  should 'list content for all articles lead defined in nodes' do
-    profile = create_user('testuser').person
-    Article.delete_all
-    a1 = fast_create(TinyMceArticle, :name => 'test article 1', :profile_id => profile.id, :abstract => 'abstract article 1')
-    a2 = fast_create(TinyMceArticle, :name => 'test article 2', :profile_id => profile.id, :abstract => 'abstract article 2')
-
-    block = DisplayContentBlock.new
-    block.sections = [{:value => 'abstract', :checked => true}]
-    block.nodes = [a1.id, a2.id]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /<div class="lead">#{a1.lead}<\/div>/, instance_eval(&block.content)
-    assert_match /<div class="lead">#{a2.lead}<\/div>/, instance_eval(&block.content)
-  end
-
-  should 'not crash when referenced article is removed' do
-    profile = create_user('testuser').person
-    a1 = fast_create(TextArticle, :name => 'test article 1', :profile_id => profile.id)
-
-    block = DisplayContentBlock.new
-    block.nodes = [a1.id]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    Article.delete_all
-    assert_match /<ul><\/ul>/, instance_eval(&block.content)
-  end
   include ActionView::Helpers
   include Rails.application.routes.url_helpers
@@ -420,48 +373,6 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     assert_equal params, block.url_params
   end
-  should 'show title if defined by user' do
-    profile = create_user('testuser').person
-    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id)
-
-    block = DisplayContentBlock.new
-    block.nodes = [a.id]
-    block.sections = [{:value => 'title', :checked => true}]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /.*<a.*>#{a.title}<\/a>/, instance_eval(&block.content)
-  end
-
-  should 'show abstract if defined by user' do
-    profile = create_user('testuser').person
-    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id, :abstract => 'some abstract')
-
-    block = DisplayContentBlock.new
-    block.nodes = [a.id]
-    block.sections = [{:value => 'abstract', :checked => true}]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /#{a.abstract}/, instance_eval(&block.content)
-  end
-
-  should 'show body if defined by user' do
-    profile = create_user('testuser').person
-    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id, :body => 'some body')
-
-    block = DisplayContentBlock.new
-    block.nodes = [a.id]
-    block.sections = [{:value => 'body', :checked => true}]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /#{a.body}/, instance_eval(&block.content)
-  end
-
   should 'display_attribute be true for title by default' do
     profile = create_user('testuser').person
@@ -489,20 +400,6 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     assert block.display_section?({:value => 'publish_date', :checked => true})
   end
-  should 'show publishd date if defined by user' do
-    profile = create_user('testuser').person
-    a = fast_create(TextArticle, :name => 'test article 1', :profile_id => profile.id, :body => 'some body')
-
-    block = DisplayContentBlock.new
-    block.nodes = [a.id]
-    block.sections = [{:value => 'publish_date', :checked => true}]
-    box = mock()
-    block.stubs(:box).returns(box)
-    box.stubs(:owner).returns(profile)
-
-    assert_match /#{a.published_at}/, instance_eval(&block.content)
-  end
-
   should 'do not save children if a folder is checked' do
     profile = create_user('testuser').person
     Article.delete_all
@@ -648,6 +545,117 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     assert_equal [], block.parent_nodes
   end
+end
+
+require 'boxes_helper'
+
+class DisplayContentBlockViewTest < ActionView::TestCase
+  include BoxesHelper
+
+  should 'list links for all articles title defined in nodes' do
+    profile = create_user('testuser').person
+    Article.delete_all
+    a1 = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id)
+    a2 = fast_create(TextileArticle, :name => 'test article 2', :profile_id => profile.id)
+
+    block = DisplayContentBlock.new
+    block.sections = [{:value => 'title', :checked => true}]
+    block.nodes = [a1.id, a2.id]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /.*<a.*>#{a1.title}<\/a>/, render_block_content(block)
+    assert_match /.*<a.*>#{a2.title}<\/a>/, render_block_content(block)
+  end
+
+  should 'list content for all articles lead defined in nodes' do
+    profile = create_user('testuser').person
+    Article.delete_all
+    a1 = fast_create(TinyMceArticle, :name => 'test article 1', :profile_id => profile.id, :abstract => 'abstract article 1')
+    a2 = fast_create(TinyMceArticle, :name => 'test article 2', :profile_id => profile.id, :abstract => 'abstract article 2')
+
+    block = DisplayContentBlock.new
+    block.sections = [{:value => 'abstract', :checked => true}]
+    block.nodes = [a1.id, a2.id]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /<div class="lead">#{a1.lead}<\/div>/, render_block_content(block)
+    assert_match /<div class="lead">#{a2.lead}<\/div>/, render_block_content(block)
+  end
+
+  should 'not crash when referenced article is removed' do
+    profile = create_user('testuser').person
+    a1 = fast_create(TextArticle, :name => 'test article 1', :profile_id => profile.id)
+
+    block = DisplayContentBlock.new
+    block.nodes = [a1.id]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    Article.delete_all
+    assert_match /<ul><\/ul>/, render_block_content(block)
+  end
+
+  should 'show title if defined by user' do
+    profile = create_user('testuser').person
+    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id)
+
+    block = DisplayContentBlock.new
+    block.nodes = [a.id]
+    block.sections = [{:value => 'title', :checked => true}]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /.*<a.*>#{a.title}<\/a>/, render_block_content(block)
+  end
+
+  should 'show abstract if defined by user' do
+    profile = create_user('testuser').person
+    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id, :abstract => 'some abstract')
+
+    block = DisplayContentBlock.new
+    block.nodes = [a.id]
+    block.sections = [{:value => 'abstract', :checked => true}]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /#{a.abstract}/, render_block_content(block)
+  end
+
+  should 'show body if defined by user' do
+    profile = create_user('testuser').person
+    a = fast_create(TextileArticle, :name => 'test article 1', :profile_id => profile.id, :body => 'some body')
+
+    block = DisplayContentBlock.new
+    block.nodes = [a.id]
+    block.sections = [{:value => 'body', :checked => true}]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /#{a.body}/, render_block_content(block)
+  end
+
+  should 'show publishd date if defined by user' do
+    profile = create_user('testuser').person
+    a = fast_create(TextArticle, :name => 'test article 1', :profile_id => profile.id, :body => 'some body')
+
+    block = DisplayContentBlock.new
+    block.nodes = [a.id]
+    block.sections = [{:value => 'publish_date', :checked => true}]
+    box = mock()
+    block.stubs(:box).returns(box)
+    box.stubs(:owner).returns(profile)
+
+    assert_match /#{a.published_at}/, render_block_content(block)
+  end
+
   should 'show articles in recent order' do
     profile = create_user('testuser').person
     Article.delete_all
@@ -663,8 +671,8 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     block.order_by_recent = true
-    a1_index = instance_eval(&block.content).index(a1.name)
-    a2_index = instance_eval(&block.content).index(a2.name)
+    a1_index = render_block_content(block).index(a1.name)
+    a2_index = render_block_content(block).index(a2.name)
     assert a2_index < a1_index
   end
@@ -684,8 +692,8 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     block.order_by_recent = false
-    a1_index = instance_eval(&block.content).index(a1.name)
-    a2_index = instance_eval(&block.content).index(a2.name)
+    a1_index = render_block_content(block).index(a1.name)
+    a2_index = render_block_content(block).index(a2.name)
     assert a1_index < a2_index
   end
@@ -706,7 +714,7 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     block.order_by_recent = true
     block.limit_to_show = 1
-    a1_index = instance_eval(&block.content).index(a1.name)
+    a1_index = render_block_content(block).index(a1.name)
     assert a1_index.nil?
   end
@@ -730,15 +738,15 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     block.stubs(:box).returns(box)
     box.stubs(:owner).returns(profile)
-    assert_nil instance_eval(&block.content).index(en_article.name)
-    assert_nil instance_eval(&block.content).index(en_article2.name)
-    assert instance_eval(&block.content).index(pt_article.name).present?
+    assert_nil render_block_content(block).index(en_article.name)
+    assert_nil render_block_content(block).index(en_article2.name)
+    assert render_block_content(block).index(pt_article.name).present?
     FastGettext.stubs(:locale).returns('en')
-    assert instance_eval(&block.content).index(en_article.name)
-    assert instance_eval(&block.content).index(en_article2.name)
-    assert_nil instance_eval(&block.content).index(pt_article.name)
+    assert render_block_content(block).index(en_article.name)
+    assert render_block_content(block).index(en_article2.name)
+    assert_nil render_block_content(block).index(pt_article.name)
   end
   should 'replace article with its translation' do
@@ -758,12 +766,12 @@ class DisplayContentBlockTest &lt; ActiveSupport::TestCase
     block.stubs(:box).returns(box)
     box.stubs(:owner).returns(profile)
-    assert_nil instance_eval(&block.content).index(en_article.name)
-    assert instance_eval(&block.content).index(pt_article.name).present?
+    assert_nil render_block_content(block).index(en_article.name)
+    assert render_block_content(block).index(pt_article.name).present?
     FastGettext.stubs(:locale).returns('en')
-    assert instance_eval(&block.content).index(en_article.name).present?
-    assert_nil instance_eval(&block.content).index(pt_article.name)
+    assert render_block_content(block).index(en_article.name).present?
+    assert_nil render_block_content(block).index(pt_article.name)
   end
 end
@@ -0,0 +1,4 @@
+li
+  - unless item.folder? || item.class == RssFeed
+    = render partial: 'blocks/section', collection: block.sections, locals: { block: block, item: item }
+    = render partial: 'blocks/read_more', locals: { item: item, abstract_section: block.sections.bsearch { |section| section[:value] == 'abstract' }, block: block }
 \ No newline at end of file
@@ -0,0 +1,3 @@
+- if !abstract_section.nil? && block.display_section?(abstract_section)
+  div class='read_more'
+    = link_to(_('Read more'), item.url)
@@ -0,0 +1,22 @@
+- if block.display_section?(section)
+  - case section[:value]
+  - when 'publish_date'
+    div class='published-at'
+      = show_date(item.published_at, false)
+  - when 'title'
+    div class='title'
+      = link_to(h(item.title), item.url)
+  - when 'abstract'
+    div class='lead'
+      = item.abstract
+  - when 'body'
+    div class='body'
+      = item.body
+  - when 'image'
+    - unless item.image || item.image.public_filename
+      div class='image'
+        = link_to(image_tag(item.image.public_filename), item.url)
+  - when 'tags'
+    - unless item.tags.empty?
+      div class='tags'
+        = render partial: 'blocks/tag', collection: item.tags
@@ -0,0 +1,2 @@
+span
+  = tag.name
 \ No newline at end of file
@@ -0,0 +1,4 @@
+= block.block_title(block.title, block.subtitle)
+
+ul
+  = render partial: 'blocks/document', collection: block.docs, as: :item, locals: { block: block }
	@@ -118,69 +118,28 @@ class DisplayContentBlock < Block		@@ -118,69 +118,28 @@ class DisplayContentBlock < Block
118	holder.articles.where(type: types, parent_id: if parent.nil? then nil else parent end)	118	holder.articles.where(type: types, parent_id: if parent.nil? then nil else parent end)
119	end	119	end
120		120
121	- def content(args={})
122	- block = self
123	-	121	+ def docs
124	order_string = "published_at"	122	order_string = "published_at"
125	order_string += " DESC" if order_by_recent	123	order_string += " DESC" if order_by_recent
126		124
127	limit_final = [limit_to_show, 0].max	125	limit_final = [limit_to_show, 0].max
128		126
129	- docs = owner.articles.order(order_string)	127	+ documents = owner.articles.order(order_string)
130	.where(articles: {type: self.types})	128	.where(articles: {type: self.types})
131	.includes(:profile, :image, :tags)	129	.includes(:profile, :image, :tags)
132	if nodes.present?	130	if nodes.present?
133	nodes_conditions = 'articles.id IN(:nodes)'	131	nodes_conditions = 'articles.id IN(:nodes)'
134	nodes_conditions << ' OR articles.parent_id IN(:nodes) ' if display_folder_children	132	nodes_conditions << ' OR articles.parent_id IN(:nodes) ' if display_folder_children
135	- docs = docs.where nodes_conditions, nodes: nodes	133	+ documents = documents.where nodes_conditions, nodes: nodes
136	end	134	end
137	- docs = docs.limit limit_final if display_folder_children	135	+ documents = documents.limit limit_final if display_folder_children
138		136
139	if content_with_translations	137	if content_with_translations
140	- docs = docs.native_translations
141	- docs.replace docs.map{ \|p\| p.get_translation_to(FastGettext.locale) }.compact	138	+ documents = documents.native_translations
		139	+ documents.replace documents.map{ \|p\| p.get_translation_to(FastGettext.locale) }.compact
142	end	140	end
143		141
144	- proc do
145	- block.block_title(block.title, block.subtitle) +
146	- content_tag('ul', docs.map {\|item\|
147	- if !item.folder? && item.class != RssFeed
148	- content_sections = ''
149	- read_more_section = ''
150	- tags_section = ''
151	-
152	- block.sections.select { \|section\|
153	- case section[:value]
154	- when 'publish_date'
155	- content_sections += (block.display_section?(section) ? (content_tag('div', show_date(item.published_at, false), :class => 'published-at') ) : '')
156	- when 'title'
157	- content_sections += (block.display_section?(section) ? (content_tag('div', link_to(h(item.title), item.url), :class => 'title') ) : '')
158	- when 'abstract'
159	- content_sections += (block.display_section?(section) ? (content_tag('div', item.abstract.html_safe , :class => 'lead')) : '' )
160	- if block.display_section?(section)
161	- read_more_section = content_tag('div', link_to(_('Read more'), item.url), :class => 'read_more')
162	- end
163	- when 'body'
164	- content_sections += (block.display_section?(section) ? (content_tag('div', item.body.html_safe ,:class => 'body')) : '' )
165	- when 'image'
166	- image_section = image_tag item.image.public_filename if item.image
167	- if !image_section.blank?
168	- content_sections += (block.display_section?(section) ? (content_tag('div', link_to( image_section, item.url ) ,:class => 'image')) : '' )
169	- end
170	- when 'tags'
171	- if !item.tags.empty?
172	- tags_section = item.tags.map { \|t\| content_tag('span', t.name) }.join("")
173	- content_sections += (block.display_section?(section) ? (content_tag('div', tags_section, :class => 'tags')) : '')
174	- end
175	- end
176	- }
177	-
178	- content_sections += read_more_section if !read_more_section.blank?
179	-#raise sections.inspect
180	- content_tag('li', content_sections.html_safe)
181	- end
182	- }.join(" ").html_safe)
183	- end	142	+ documents
184	end	143	end
185		144
186	def url_params	145	def url_params
	@@ -0,0 +1,4 @@		@@ -0,0 +1,4 @@
		1	+li
		2	+ - unless item.folder? \|\| item.class == RssFeed
		3	+ = render partial: 'blocks/section', collection: block.sections, locals: { block: block, item: item }
		4	+ = render partial: 'blocks/read_more', locals: { item: item, abstract_section: block.sections.bsearch { \|section\| section[:value] == 'abstract' }, block: block }
0	\ No newline at end of file	5	\ No newline at end of file
@@ -0,0 +1,3 @@		@@ -0,0 +1,3 @@
	1	+- if !abstract_section.nil? && block.display_section?(abstract_section)
	2	+ div class='read_more'
	3	+ = link_to(_('Read more'), item.url)
@@ -0,0 +1,22 @@		@@ -0,0 +1,22 @@
	1	+- if block.display_section?(section)
	2	+ - case section[:value]
	3	+ - when 'publish_date'
	4	+ div class='published-at'
	5	+ = show_date(item.published_at, false)
	6	+ - when 'title'
	7	+ div class='title'
	8	+ = link_to(h(item.title), item.url)
	9	+ - when 'abstract'
	10	+ div class='lead'
	11	+ = item.abstract
	12	+ - when 'body'
	13	+ div class='body'
	14	+ = item.body
	15	+ - when 'image'
	16	+ - unless item.image \|\| item.image.public_filename
	17	+ div class='image'
	18	+ = link_to(image_tag(item.image.public_filename), item.url)
	19	+ - when 'tags'
	20	+ - unless item.tags.empty?
	21	+ div class='tags'
	22	+ = render partial: 'blocks/tag', collection: item.tags
	@@ -0,0 +1,2 @@		@@ -0,0 +1,2 @@
		1	+span
		2	+ = tag.name
0	\ No newline at end of file	3	\ No newline at end of file
@@ -0,0 +1,4 @@		@@ -0,0 +1,4 @@
	1	+= block.block_title(block.title, block.subtitle)
	2	+
	3	+ul
	4	+ = render partial: 'blocks/document', collection: block.docs, as: :item, locals: { block: block }