[openstreetmap/openstreetmap-website] Use `where()` instead of `find()` to prevent RecordNotFound (PR #4203)

[Tom Hughes]

@tomhughes requested changes on this pull request.



> @@ -419,13 +419,15 @@ def test_index
       check_json_details(js["users"][1], user3, false, false)
 
       get api_users_path, :params => { :users => create(:user, :suspended).id }
-      assert_response :not_found
+      assert_response :success

You should add the same assertion here that you use later to check that no users are returned.


>  
       get api_users_path, :params => { :users => create(:user, :deleted).id }
-      assert_response :not_found
+      assert_response :success

Same here - this should check no users are returned.

> @@ -1,4 +1,4 @@
 xml.instruct! :xml, :version => "1.0"
 xml.osm(OSM::API.new.xml_root_attributes) do |osm|
-  osm << render(@users)
+  osm << (render(@users) || "")

Is this actually necessary? Won't an empty set render sensibly? If it is needed that what about the JSON version?

> @@ -506,13 +508,15 @@ def test_index_oauth1
       check_json_details(js["users"][1], user3, false, false)
 
       signed_get api_users_path, :params => { :users => create(:user, :suspended).id }, :oauth => { :token => good_token }
-      assert_response :not_found
+      assert_response :success

This should check the set is empty.

>  
       signed_get api_users_path, :params => { :users => create(:user, :deleted).id }, :oauth => { :token => good_token }
-      assert_response :not_found
+      assert_response :success

This should check the set is empty.

> @@ -593,13 +597,15 @@ def test_index_oauth2
       check_json_details(js["users"][1], user3, false, false)
 
       get api_users_path, :params => { :users => create(:user, :suspended).id }, :headers => bearer_authorization_header(good_token.token)
-      assert_response :not_found
+      assert_response :success

This should check the set is empty.

>  
       get api_users_path, :params => { :users => create(:user, :deleted).id }, :headers => bearer_authorization_header(good_token.token)
-      assert_response :not_found
+      assert_response :success

This should check the set is empty.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/4203#pullrequestreview-1596828985
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/4203/review/1596828985 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20230826/a3d072be/attachment.htm>