bugfix: fixing test failures that occurred after recent changes in co…

…de and database.

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 = "<p>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 += "</p>"
+    html_string = "<p>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 += "</p>"
 
     html_string
   end

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

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?

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,29 +124,29 @@ 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]
   end
 
   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