From 3b87f879386587a9440de76ced6b3db965fdc058 Mon Sep 17 00:00:00 2001 From: Kieran Pilkington Date: Tue, 31 Mar 2009 12:51:23 +1300 Subject: [PATCH] bugfix: fixing test failures that occurred after recent changes in code and database. --- app/helpers/application_helper.rb | 35 +++++----------------- lib/item_privacy.rb | 21 +++++++++++-- lib/item_privacy_test_helper.rb | 16 +++++----- test/functional/baskets_controller_test.rb | 12 ++++---- 4 files changed, 39 insertions(+), 45 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 86b66cdca..df445f1b4 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -656,36 +656,15 @@ def tag_cloud(tags, classes) def tags_for(item) html_string = String.new - return html_string if item.raw_tag_list.nil? - - raw_tag_array = Array.new - # Get the raw tag list, split, squish (removed whitespace), and add each to raw_tag_array - # Make sure we skip if the array already has that tag name (remove any duplicates that occur) - item.raw_tag_list.split(',').each do |raw_tag| - next if raw_tag_array.include?(raw_tag.squish) - raw_tag_array << raw_tag.squish - end + return html_string if item.tags.blank? - # grab all the tag objects - tags_out_of_order = Tag.find_all_by_name(raw_tag_array) - if tags_out_of_order.size > 0 - tags = Array.new - # resort them to match raw_tag_list order - raw_tag_array.each do |tag_name| - tag = tags_out_of_order.select { |tag| tag.name == tag_name } - tags << tag - end - # at this point, we have an array, with arrays of object [[tag], [tag], [tag]] - # use compact to remove any nil objects, and flatten to convert it to [tag, tag, tag] - tags = tags.compact.flatten - - html_string = "

Tags: " - tags.each_with_index do |tag,index| - html_string += link_to_tagged(tag, item.class.name) - html_string += ", " unless tags.size == (index + 1) - end - html_string += "

" + html_string = "

Tags: " + item_tags = item.tags + item_tags.each_with_index do |tag,index| + html_string += link_to_tagged(tag, item.class.name) + html_string += ", " unless item_tags.size == (index + 1) end + html_string += "

" html_string end diff --git a/lib/item_privacy.rb b/lib/item_privacy.rb index 9a9954ad2..1bc05bbb5 100644 --- a/lib/item_privacy.rb +++ b/lib/item_privacy.rb @@ -231,9 +231,26 @@ def self.included(klass) module InstanceMethods # Transparently map tags for the current item to the tags of the correct - # privacy. + # privacy, and sort them according to raw_tag_list def tags - private? ? private_tags : public_tags + tags_out_of_order = private? ? private_tags : public_tags + + return tags_out_of_order if raw_tag_list.blank? + + # Get the raw tag list, split, squish (removed whitespace), and add each to raw_tag_array + # Make sure we skip if the array already has that tag name (remove any duplicates that occur) + raw_tag_array = Array.new + raw_tag_list.split(',').each do |raw_tag| + next if raw_tag_array.include?(raw_tag.squish) + raw_tag_array << raw_tag.squish + end + + tags = Array.new + if tags_out_of_order.size > 0 + # resort them to match raw_tag_list order + tags = tags_out_of_order.sort { |a, b| raw_tag_array.index(a.name).to_i <=> raw_tag_array.index(b.name).to_i } + end + tags end def tag_list diff --git a/lib/item_privacy_test_helper.rb b/lib/item_privacy_test_helper.rb index 19bfc5c80..335451b53 100644 --- a/lib/item_privacy_test_helper.rb +++ b/lib/item_privacy_test_helper.rb @@ -589,7 +589,7 @@ def test_instances_respond_to_instance_methods_as_expected end def test_tags_are_preserved_on_public_items - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => false, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => false, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d.reload assert_equal 1, d.versions.size @@ -601,7 +601,7 @@ def test_tags_are_preserved_on_public_items end def test_tags_are_preserved_on_private_items - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) assert_equal 0, d.tags.size @@ -616,7 +616,7 @@ def test_tags_are_preserved_on_private_items end def test_tags_on_private_items_are_kept_private - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d.reload # Check there are no tags on public version @@ -650,7 +650,7 @@ def test_tags_on_private_items_are_kept_private end def test_tags_on_private_items_are_kept_private_on_re_find - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d = eval(@base_class).find(d.id) @@ -680,11 +680,11 @@ def test_tags_on_private_items_are_kept_private_on_re_find def test_tags_are_preserved_separately_by_privacy_setting # Create a private version - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d.reload # Create a public version with different tags - d.update_attributes!(:private => false, :title => "A public version", :description => "Version 3", :tag_list => "four, five, six") + d.update_attributes!(:private => false, :title => "A public version", :description => "Version 3", :tag_list => "four, five, six", :raw_tag_list => "four, five, six") # Create a second private version without tags d.private_version! @@ -712,7 +712,7 @@ def test_tags_are_preserved_separately_by_privacy_setting end def test_tags_on_private_items_are_of_private_context - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => true, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d.reload d.private_version! @@ -725,7 +725,7 @@ def test_tags_on_private_items_are_of_private_context end def test_tags_on_public_items_are_of_public_context - d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => false, :tag_list => "one, two, three" })) + d = eval(@base_class).create(@new_model.merge({ :description => "Version 1", :private => false, :tag_list => "one, two, three", :raw_tag_list => "one, two, three" })) d.reload assert_equal false, d.private? diff --git a/test/functional/baskets_controller_test.rb b/test/functional/baskets_controller_test.rb index 78535eee3..4c229be78 100644 --- a/test/functional/baskets_controller_test.rb +++ b/test/functional/baskets_controller_test.rb @@ -40,9 +40,7 @@ def test_redirect_to_basket_all def test_index_and_list get :index, index_path - assert_viewing_template 'baskets/list' - assert_var_assigned true - assert_equal 4, assigns(:baskets).size + assert_redirect_to( :action => 'list' ) get :list, index_path({ :action => 'list' }) assert_viewing_template 'baskets/list' @@ -126,14 +124,14 @@ def test_contact def test_basket_accessable_by_site_admin_when_status_not_approved basket = Basket.create(@new_model.merge({ :name => 'Test' })) - get :index, :urlified_name => 'test', :controller => 'index_page', :action => 'index' + get :list, :urlified_name => 'test', :controller => 'index_page', :action => 'index' assert_response :success end def test_basket_not_accessable_by_non_site_admin_when_status_not_approved logout basket = Basket.create(@new_model.merge({ :name => 'Test' })) - get :index, :urlified_name => 'test', :controller => 'index_page', :action => 'index' + get :list, :urlified_name => 'test', :controller => 'index_page', :action => 'index' assert_response :redirect assert_redirected_to "/site" assert_equal 'The basket Test is not approved for public viewing', flash[:error] @@ -141,14 +139,14 @@ def test_basket_not_accessable_by_non_site_admin_when_status_not_approved def test_basket_accessable_by_site_admin_when_approved basket = Basket.create(@new_model.merge({ :name => 'Test', :status => 'approved' })) - get :index, :urlified_name => 'test', :controller => 'index_page', :action => 'index' + get :list, :urlified_name => 'test', :controller => 'index_page', :action => 'index' assert_response :success end def test_basket_accessable_by_non_site_admin_when_approved logout basket = Basket.create(@new_model.merge({ :name => 'Test', :status => 'approved' })) - get :index, :urlified_name => 'test', :controller => 'index_page', :action => 'index' + get :list, :urlified_name => 'test', :controller => 'index_page', :action => 'index' assert_response :success end