[openstreetmap/openstreetmap-website] More user factory replacements (#1481)

Tom Hughes notifications at github.com
Thu Mar 9 13:43:53 UTC 2017


tomhughes requested changes on this pull request.



>      assert_response :not_found
 
     # Now with some other user, which should work since the trace is public
-    get :picture, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user) }
+    get :picture, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user) }

That's not doing the same thing any more as it is now authenticating as a different user to the one that nominally owns the non-existent trace.

In the previous test that did the same thing you created the deleted trace up front and used that user throughout.

>      assert_response :not_found
 
     # Now with some other user
-    get :icon, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user) }
+    get :icon, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user) }

Same issue here as above.

>      assert_response :forbidden
 
     # Now with a trace which doesn't exist
-    get :edit, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user) }
+    get :edit, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user) }

Same again...

>      assert_response :forbidden
 
     # Now with a trace which doesn't exist
-    post :edit, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user) }
+    post :edit, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user) }

Guess what ;-)

>      assert_response :forbidden
 
     # Now with a trace which doesn't exist
-    post :edit, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user), :trace => new_details }
+    post :edit, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user), :trace => new_details }

♻️ 

>      assert_response :forbidden
 
     # Now with a trace which doesn't exist
-    post :delete, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user) }
+    post :delete, { :display_name => create(:user).display_name, :id => 0 }, { :user => create(:user) }

♻️ 

>      assert_response :unauthorized
 
     # Now with a trace that has never existed
-    basic_authorization(users(:public_user).display_name, "test")
-    get :api_data, :display_name => users(:public_user).display_name, :id => 0
+    basic_authorization(create(:user).display_name, "test")
+    get :api_data, :display_name => create(:user).display_name, :id => 0

♻️ 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1481#pullrequestreview-26027619
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20170309/84256272/attachment.html>


More information about the rails-dev mailing list