From: Tom Hughes <tom@compton.nu>
Date: Sat, 10 Jan 2015 12:35:52 +0000 (+0000)
Subject: Don't pass ActiveRecord objects to find and exists?
X-Git-Tag: live~4298
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/316ebb7570d1e894d6889481ec3a56276cb3e88b?hp=6af8f8a9e37e92c139779fbb5fb2590ad040993e;ds=sidebyside

Don't pass ActiveRecord objects to find and exists?
---

diff --git a/app/controllers/changeset_controller.rb b/app/controllers/changeset_controller.rb
index adda8c20c..224c2c488 100644
--- a/app/controllers/changeset_controller.rb
+++ b/app/controllers/changeset_controller.rb
@@ -341,7 +341,7 @@ class ChangesetController < ApplicationController
     end
 
     # Add the commenter to the subscribers if necessary
-    changeset.subscribers << @user unless changeset.subscribers.exists?(@user)
+    changeset.subscribers << @user unless changeset.subscribers.exists?(@user.id)
 
     # Return a copy of the updated changeset
     render :text => changeset.to_xml.to_s, :content_type => "text/xml"
@@ -359,7 +359,7 @@ class ChangesetController < ApplicationController
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
     raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
-    raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user)
+    raise OSM::APIChangesetAlreadySubscribedError.new(changeset) if changeset.subscribers.exists?(@user.id)
 
     # Add the subscriber
     changeset.subscribers << @user
@@ -380,7 +380,7 @@ class ChangesetController < ApplicationController
     # Find the changeset and check it is valid
     changeset = Changeset.find(id)
     raise OSM::APIChangesetNotYetClosedError.new(changeset) if changeset.is_open?
-    raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user)
+    raise OSM::APIChangesetNotSubscribedError.new(changeset) unless changeset.subscribers.exists?(@user.id)
 
     # Remove the subscriber
     changeset.subscribers.delete(@user)
diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb
index bbe227921..efb7bb7f9 100644
--- a/app/views/browse/changeset.html.erb
+++ b/app/views/browse/changeset.html.erb
@@ -15,7 +15,7 @@
   
   <div class="buttons clearfix subscribe-buttons">
     <form action="#" class="hide_unless_logged_in">
-      <% if @changeset.subscribers.exists?(@user) %>
+      <% if @user and @changeset.subscribers.exists?(@user.id) %>
         <input class="action-button" type="submit" name="unsubscribe" value="<%= t('javascripts.changesets.show.unsubscribe') %>" data-method="POST" data-url="<%= changeset_unsubscribe_url(@changeset) %>" />
       <% else %>
         <input class="action-button" type="submit" name="subscribe" value="<%= t('javascripts.changesets.show.subscribe') %>" data-method="POST" data-url="<%= changeset_subscribe_url(@changeset) %>" />
diff --git a/test/models/changeset_comment_test.rb b/test/models/changeset_comment_test.rb
index 5f8efdbbd..c13fde8e2 100644
--- a/test/models/changeset_comment_test.rb
+++ b/test/models/changeset_comment_test.rb
@@ -36,6 +36,6 @@ class ChangesetCommentTest < ActiveSupport::TestCase
   end
 
   def test_comments_of_changeset_count
-    assert_equal 3, Changeset.find(changesets(:normal_user_closed_change)).comments.count
+    assert_equal 3, Changeset.find(changesets(:normal_user_closed_change).id).comments.count
   end
 end
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 7acc6168f..612bff703 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -137,7 +137,7 @@ class NodeTest < ActiveSupport::TestCase
   end
 
   def test_delete
-    node_template = Node.find(current_nodes(:visible_node))
+    node_template = Node.find(current_nodes(:visible_node).id)
     assert_not_nil node_template
 
     assert_equal OldNode.where(:node_id => node_template.id).count, 1