Fix CMSHelper unit test

After creating the ArticleBlock view test at ff15413b2a8dca45d9f64e57270f294e351a0098 this test got broken. The root cause was the inclusion of ApplicationHelper for the article_block_test which led to `custom_options_article` for CMSHelper tests to fail as the BlogHelper (included by ApplicationHelper) was overriding the method. This has been fized by remove the include to ApplicationHelper and extract the necessary methods to separate helpers which led to two new methods for ArticleHelper and one new helper the ButtonsHelper.

Fix CMSHelper unit test
After creating the ArticleBlock view test at ff15413b2a8dca45d9f64e57270f294e351a0098 this test got broken. The root cause was the inclusion of ApplicationHelper for the article_block_test which led to `custom_options_article` for CMSHelper tests to fail as the BlogHelper (included by ApplicationHelper) was overriding the method. This has been fized by remove the include to ApplicationHelper and extract the necessary methods to separate helpers which led to two new methods for ArticleHelper and one new helper the ButtonsHelper.
Rafael Reggiani Manzo
1 parent beb5c42b
Showing 7 changed files with 80 additions and 71 deletions Show diff stats
app/helpers/application_helper.rb
app/helpers/article_helper.rb
app/helpers/buttons_helper.rb
test/unit/application_helper_test.rb
test/unit/article_block_test.rb
test/unit/buttons_helper_test.rb
test/unit/manage_products_helper_test.rb
@@ -48,6 +48,8 @@ module ApplicationHelper
   include PluginsHelper
+  include ButtonsHelper
+
   def locale
     (@page && !@page.language.blank?) ? @page.language : FastGettext.locale
   end
@@ -209,52 +211,6 @@ module ApplicationHelper
     result
   end
-  def button(type, label, url, html_options = {})
-    html_options ||= {}
-    the_class = 'with-text'
-    if html_options.has_key?(:class)
-      the_class << ' ' << html_options[:class]
-    end
-    button_without_text type, label, url, html_options.merge(:class => the_class)
-  end
-
-  def button_without_text(type, label, url, html_options = {})
-    the_class = "button icon-#{type}"
-    if html_options.has_key?(:class)
-      the_class << ' ' << html_options[:class]
-    end
-    the_title = html_options[:title] || label
-    if html_options[:disabled]
-      content_tag('a', '&nbsp;'+content_tag('span', label), html_options.merge(:class => the_class, :title => the_title))
-    else
-      link_to('&nbsp;'+content_tag('span', label), url, html_options.merge(:class => the_class, :title => the_title))
-    end
-  end
-
-  def button_to_function(type, label, js_code, html_options = {}, &block)
-    html_options[:class] = "button with-text" unless html_options[:class]
-    html_options[:class] << " icon-#{type}"
-    link_to_function(label, js_code, html_options, &block)
-  end
-
-  def button_to_function_without_text(type, label, js_code, html_options = {}, &block)
-    html_options[:class] = "" unless html_options[:class]
-    html_options[:class] << " button icon-#{type}"
-    link_to_function(content_tag('span', label), js_code, html_options, &block)
-  end
-
-  def button_to_remote(type, label, options, html_options = {})
-    html_options[:class] = "button with-text" unless html_options[:class]
-    html_options[:class] << " icon-#{type}"
-    link_to_remote(label, options, html_options)
-  end
-
-  def button_to_remote_without_text(type, label, options, html_options = {})
-    html_options[:class] = "" unless html_options[:class]
-    html_options[:class] << " button icon-#{type}"
-    link_to_remote(content_tag('span', label), options, html_options.merge(:title => label))
-  end
-
   def icon(icon_name, html_options = {})
     the_class = "button #{icon_name}"
     if html_options.has_key?(:class)
@@ -934,13 +890,6 @@ module ApplicationHelper
     content_for(:head) { stylesheet_link_tag(*args) }
   end
-  def article_to_html(article, options = {})
-    options.merge!(:page => params[:npage])
-    content = article.to_html(options)
-    content = content.kind_of?(Proc) ? self.instance_exec(&content).html_safe : content.html_safe
-    filter_html(content, article)
-  end
-
   # Please, use link_to by default!
   # This method was created to work around to inexplicable
   # chain of problems when display_short_format was called
@@ -1365,16 +1314,6 @@ module ApplicationHelper
     @no_design_blocks = true
   end
-  def filter_html(html, source)
-    if @plugins && source && source.has_macro?
-      html = convert_macro(html, source) unless @plugins.enabled_macros.blank?
-      #TODO This parse should be done through the macro infra, but since there
-      #     are old things that do not support it we are keeping this hot spot.
-      html = @plugins.pipeline(:parse_content, html, source).first
-    end
-    html && html.html_safe
-  end
-
   def convert_macro(html, source)
     doc = Nokogiri::HTML.fragment html
     #TODO This way is more efficient but do not support macro inside of
@@ -181,4 +181,21 @@ module ArticleHelper
     end
   end
+  def filter_html(html, source)
+    if @plugins && source && source.has_macro?
+      html = convert_macro(html, source) unless @plugins.enabled_macros.blank?
+      #TODO This parse should be done through the macro infra, but since there
+      #     are old things that do not support it we are keeping this hot spot.
+      html = @plugins.pipeline(:parse_content, html, source).first
+    end
+    html && html.html_safe
+  end
+
+  def article_to_html(article, options = {})
+    options.merge!(:page => params[:npage])
+    content = article.to_html(options)
+    content = content.kind_of?(Proc) ? self.instance_exec(&content).html_safe : content.html_safe
+    filter_html(content, article)
+  end
+
 end
@@ -0,0 +1,47 @@
+module ButtonsHelper
+  def button(type, label, url, html_options = {})
+    html_options ||= {}
+    the_class = 'with-text'
+    if html_options.has_key?(:class)
+      the_class << ' ' << html_options[:class]
+    end
+    button_without_text type, label, url, html_options.merge(:class => the_class)
+  end
+
+  def button_without_text(type, label, url, html_options = {})
+    the_class = "button icon-#{type}"
+    if html_options.has_key?(:class)
+      the_class << ' ' << html_options[:class]
+    end
+    the_title = html_options[:title] || label
+    if html_options[:disabled]
+      content_tag('a', '&nbsp;'+content_tag('span', label), html_options.merge(:class => the_class, :title => the_title))
+    else
+      link_to('&nbsp;'+content_tag('span', label), url, html_options.merge(:class => the_class, :title => the_title))
+    end
+  end
+
+  def button_to_function(type, label, js_code, html_options = {}, &block)
+    html_options[:class] = "button with-text" unless html_options[:class]
+    html_options[:class] << " icon-#{type}"
+    link_to_function(label, js_code, html_options, &block)
+  end
+
+  def button_to_function_without_text(type, label, js_code, html_options = {}, &block)
+    html_options[:class] = "" unless html_options[:class]
+    html_options[:class] << " button icon-#{type}"
+    link_to_function(content_tag('span', label), js_code, html_options, &block)
+  end
+
+  def button_to_remote(type, label, options, html_options = {})
+    html_options[:class] = "button with-text" unless html_options[:class]
+    html_options[:class] << " icon-#{type}"
+    link_to_remote(label, options, html_options)
+  end
+
+  def button_to_remote_without_text(type, label, options, html_options = {})
+    html_options[:class] = "" unless html_options[:class]
+    html_options[:class] << " button icon-#{type}"
+    link_to_remote(content_tag('span', label), options, html_options.merge(:title => label))
+  end
+end
@@ -80,12 +80,6 @@ class ApplicationHelperTest &lt; ActionView::TestCase
     assert_equal '', show_date(nil)
   end
-
-  should 'append with-text class and keep existing classes' do
-    expects(:button_without_text).with('type', 'label', 'url', { :class => 'with-text class1'})
-    button('type', 'label', 'url', { :class => 'class1' })
-  end
-
   should 'generate correct link to category' do
     cat = mock
     cat.expects(:path).returns('my-category/my-subcatagory')
@@ -87,7 +87,8 @@ require &#39;block_helper&#39;
 class ArticleBlockViewTest < ActionView::TestCase
   include BoxesHelper
-  ActionView::Base.send :include, ApplicationHelper
+  ActionView::Base.send :include, ArticleHelper
+  ActionView::Base.send :include, ButtonsHelper
   ActionView::Base.send :include, BlockHelper
   should "take article's content" do
@@ -157,6 +158,5 @@ class ArticleBlockViewTest &lt; ActionView::TestCase
     UploadedFile.any_instance.stubs(:url).returns('myhost.mydomain/path/to/file')
     assert_tag_in_string render_block_content(block), :tag => 'a', :content => _('Download')
-
   end
 end
@@ -0,0 +1,11 @@
+# encoding: UTF-8
+require_relative "../test_helper"
+
+class ButtonsHelperTest < ActionView::TestCase
+  include ButtonsHelper
+
+  should 'append with-text class and keep existing classes' do
+    expects(:button_without_text).with('type', 'label', 'url', { :class => 'with-text class1'})
+    button('type', 'label', 'url', { :class => 'class1' })
+  end
+end
 \ No newline at end of file
@@ -5,6 +5,7 @@ class ManageProductsHelperTest &lt; ActionView::TestCase
   include ManageProductsHelper
   include ContentViewerHelper
+  include ArticleHelper
   include ActionView::Helpers::AssetTagHelper
   include ApplicationHelper
	@@ -48,6 +48,8 @@ module ApplicationHelper		@@ -48,6 +48,8 @@ module ApplicationHelper
48		48
49	include PluginsHelper	49	include PluginsHelper
50		50
		51	+ include ButtonsHelper
		52	+
51	def locale	53	def locale
52	(@page && !@page.language.blank?) ? @page.language : FastGettext.locale	54	(@page && !@page.language.blank?) ? @page.language : FastGettext.locale
53	end	55	end
	@@ -209,52 +211,6 @@ module ApplicationHelper		@@ -209,52 +211,6 @@ module ApplicationHelper
209	result	211	result
210	end	212	end
211		213
212	- def button(type, label, url, html_options = {})
213	- html_options \|\|= {}
214	- the_class = 'with-text'
215	- if html_options.has_key?(:class)
216	- the_class << ' ' << html_options[:class]
217	- end
218	- button_without_text type, label, url, html_options.merge(:class => the_class)
219	- end
220	-
221	- def button_without_text(type, label, url, html_options = {})
222	- the_class = "button icon-#{type}"
223	- if html_options.has_key?(:class)
224	- the_class << ' ' << html_options[:class]
225	- end
226	- the_title = html_options[:title] \|\| label
227	- if html_options[:disabled]
228	- content_tag('a', ' '+content_tag('span', label), html_options.merge(:class => the_class, :title => the_title))
229	- else
230	- link_to(' '+content_tag('span', label), url, html_options.merge(:class => the_class, :title => the_title))
231	- end
232	- end
233	-
234	- def button_to_function(type, label, js_code, html_options = {}, &block)
235	- html_options[:class] = "button with-text" unless html_options[:class]
236	- html_options[:class] << " icon-#{type}"
237	- link_to_function(label, js_code, html_options, &block)
238	- end
239	-
240	- def button_to_function_without_text(type, label, js_code, html_options = {}, &block)
241	- html_options[:class] = "" unless html_options[:class]
242	- html_options[:class] << " button icon-#{type}"
243	- link_to_function(content_tag('span', label), js_code, html_options, &block)
244	- end
245	-
246	- def button_to_remote(type, label, options, html_options = {})
247	- html_options[:class] = "button with-text" unless html_options[:class]
248	- html_options[:class] << " icon-#{type}"
249	- link_to_remote(label, options, html_options)
250	- end
251	-
252	- def button_to_remote_without_text(type, label, options, html_options = {})
253	- html_options[:class] = "" unless html_options[:class]
254	- html_options[:class] << " button icon-#{type}"
255	- link_to_remote(content_tag('span', label), options, html_options.merge(:title => label))
256	- end
257	-
258	def icon(icon_name, html_options = {})	214	def icon(icon_name, html_options = {})
259	the_class = "button #{icon_name}"	215	the_class = "button #{icon_name}"
260	if html_options.has_key?(:class)	216	if html_options.has_key?(:class)
	@@ -934,13 +890,6 @@ module ApplicationHelper		@@ -934,13 +890,6 @@ module ApplicationHelper
934	content_for(:head) { stylesheet_link_tag(*args) }	890	content_for(:head) { stylesheet_link_tag(*args) }
935	end	891	end
936		892
937	- def article_to_html(article, options = {})
938	- options.merge!(:page => params[:npage])
939	- content = article.to_html(options)
940	- content = content.kind_of?(Proc) ? self.instance_exec(&content).html_safe : content.html_safe
941	- filter_html(content, article)
942	- end
943	-
944	# Please, use link_to by default!	893	# Please, use link_to by default!
945	# This method was created to work around to inexplicable	894	# This method was created to work around to inexplicable
946	# chain of problems when display_short_format was called	895	# chain of problems when display_short_format was called
	@@ -1365,16 +1314,6 @@ module ApplicationHelper		@@ -1365,16 +1314,6 @@ module ApplicationHelper
1365	@no_design_blocks = true	1314	@no_design_blocks = true
1366	end	1315	end
1367		1316
1368	- def filter_html(html, source)
1369	- if @plugins && source && source.has_macro?
1370	- html = convert_macro(html, source) unless @plugins.enabled_macros.blank?
1371	- #TODO This parse should be done through the macro infra, but since there
1372	- # are old things that do not support it we are keeping this hot spot.
1373	- html = @plugins.pipeline(:parse_content, html, source).first
1374	- end
1375	- html && html.html_safe
1376	- end
1377	-
1378	def convert_macro(html, source)	1317	def convert_macro(html, source)
1379	doc = Nokogiri::HTML.fragment html	1318	doc = Nokogiri::HTML.fragment html
1380	#TODO This way is more efficient but do not support macro inside of	1319	#TODO This way is more efficient but do not support macro inside of
	@@ -181,4 +181,21 @@ module ArticleHelper		@@ -181,4 +181,21 @@ module ArticleHelper
181	end	181	end
182	end	182	end
183		183
		184	+ def filter_html(html, source)
		185	+ if @plugins && source && source.has_macro?
		186	+ html = convert_macro(html, source) unless @plugins.enabled_macros.blank?
		187	+ #TODO This parse should be done through the macro infra, but since there
		188	+ # are old things that do not support it we are keeping this hot spot.
		189	+ html = @plugins.pipeline(:parse_content, html, source).first
		190	+ end
		191	+ html && html.html_safe
		192	+ end
		193	+
		194	+ def article_to_html(article, options = {})
		195	+ options.merge!(:page => params[:npage])
		196	+ content = article.to_html(options)
		197	+ content = content.kind_of?(Proc) ? self.instance_exec(&content).html_safe : content.html_safe
		198	+ filter_html(content, article)
		199	+ end
		200	+
184	end	201	end
@@ -0,0 +1,47 @@		@@ -0,0 +1,47 @@
	1	+module ButtonsHelper
	2	+ def button(type, label, url, html_options = {})
	3	+ html_options \|\|= {}
	4	+ the_class = 'with-text'
	5	+ if html_options.has_key?(:class)
	6	+ the_class << ' ' << html_options[:class]
	7	+ end
	8	+ button_without_text type, label, url, html_options.merge(:class => the_class)
	9	+ end
	10	+
	11	+ def button_without_text(type, label, url, html_options = {})
	12	+ the_class = "button icon-#{type}"
	13	+ if html_options.has_key?(:class)
	14	+ the_class << ' ' << html_options[:class]
	15	+ end
	16	+ the_title = html_options[:title] \|\| label
	17	+ if html_options[:disabled]
	18	+ content_tag('a', ' '+content_tag('span', label), html_options.merge(:class => the_class, :title => the_title))
	19	+ else
	20	+ link_to(' '+content_tag('span', label), url, html_options.merge(:class => the_class, :title => the_title))
	21	+ end
	22	+ end
	23	+
	24	+ def button_to_function(type, label, js_code, html_options = {}, &block)
	25	+ html_options[:class] = "button with-text" unless html_options[:class]
	26	+ html_options[:class] << " icon-#{type}"
	27	+ link_to_function(label, js_code, html_options, &block)
	28	+ end
	29	+
	30	+ def button_to_function_without_text(type, label, js_code, html_options = {}, &block)
	31	+ html_options[:class] = "" unless html_options[:class]
	32	+ html_options[:class] << " button icon-#{type}"
	33	+ link_to_function(content_tag('span', label), js_code, html_options, &block)
	34	+ end
	35	+
	36	+ def button_to_remote(type, label, options, html_options = {})
	37	+ html_options[:class] = "button with-text" unless html_options[:class]
	38	+ html_options[:class] << " icon-#{type}"
	39	+ link_to_remote(label, options, html_options)
	40	+ end
	41	+
	42	+ def button_to_remote_without_text(type, label, options, html_options = {})
	43	+ html_options[:class] = "" unless html_options[:class]
	44	+ html_options[:class] << " button icon-#{type}"
	45	+ link_to_remote(content_tag('span', label), options, html_options.merge(:title => label))
	46	+ end
	47	+end
	@@ -80,12 +80,6 @@ class ApplicationHelperTest < ActionView::TestCase		@@ -80,12 +80,6 @@ class ApplicationHelperTest < ActionView::TestCase
80	assert_equal '', show_date(nil)	80	assert_equal '', show_date(nil)
81	end	81	end
82		82
83	-
84	- should 'append with-text class and keep existing classes' do
85	- expects(:button_without_text).with('type', 'label', 'url', { :class => 'with-text class1'})
86	- button('type', 'label', 'url', { :class => 'class1' })
87	- end
88	-
89	should 'generate correct link to category' do	83	should 'generate correct link to category' do
90	cat = mock	84	cat = mock
91	cat.expects(:path).returns('my-category/my-subcatagory')	85	cat.expects(:path).returns('my-category/my-subcatagory')
	@@ -0,0 +1,11 @@		@@ -0,0 +1,11 @@
		1	+# encoding: UTF-8
		2	+require_relative "../test_helper"
		3	+
		4	+class ButtonsHelperTest < ActionView::TestCase
		5	+ include ButtonsHelper
		6	+
		7	+ should 'append with-text class and keep existing classes' do
		8	+ expects(:button_without_text).with('type', 'label', 'url', { :class => 'with-text class1'})
		9	+ button('type', 'label', 'url', { :class => 'class1' })
		10	+ end
		11	+end
0	\ No newline at end of file	12	\ No newline at end of file
	@@ -5,6 +5,7 @@ class ManageProductsHelperTest < ActionView::TestCase		@@ -5,6 +5,7 @@ class ManageProductsHelperTest < ActionView::TestCase
5		5
6	include ManageProductsHelper	6	include ManageProductsHelper
7	include ContentViewerHelper	7	include ContentViewerHelper
		8	+ include ArticleHelper
8	include ActionView::Helpers::AssetTagHelper	9	include ActionView::Helpers::AssetTagHelper
9	include ApplicationHelper	10	include ApplicationHelper
10		11