From 25ebf87e5aaa2fbe979dcf47d59cfebe08e43ad8 Mon Sep 17 00:00:00 2001 From: Andy Allan Date: Wed, 20 Nov 2019 16:09:48 +0100 Subject: [PATCH] Rename UserPreference controller methods to follow rails conventions more closely --- app/abilities/api_ability.rb | 2 +- app/abilities/api_capability.rb | 4 +- .../api/user_preferences_controller.rb | 10 +-- config/routes.rb | 10 +-- test/abilities/api_capability_test.rb | 12 ++-- .../api/user_preferences_controller_test.rb | 70 +++++++++---------- 6 files changed, 54 insertions(+), 54 deletions(-) diff --git a/app/abilities/api_ability.rb b/app/abilities/api_ability.rb index 6d62ece51..217fe9713 100644 --- a/app/abilities/api_ability.rb +++ b/app/abilities/api_ability.rb @@ -34,7 +34,7 @@ class ApiAbility can [:new, :create], Report can [:create, :show, :update, :destroy, :data], Trace can [:details, :gpx_files], User - can [:read, :read_one, :update, :update_one, :delete_one], UserPreference + can [:index, :show, :update, :update_all, :destroy], UserPreference if user.terms_agreed? can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset diff --git a/app/abilities/api_capability.rb b/app/abilities/api_capability.rb index 7d8a13364..64861f1d6 100644 --- a/app/abilities/api_capability.rb +++ b/app/abilities/api_capability.rb @@ -10,8 +10,8 @@ class ApiCapability can [:create, :update, :destroy], Trace if capability?(token, :allow_write_gpx) can [:details], User if capability?(token, :allow_read_prefs) can [:gpx_files], User if capability?(token, :allow_read_gpx) - can [:read, :read_one], UserPreference if capability?(token, :allow_read_prefs) - can [:update, :update_one, :delete_one], UserPreference if capability?(token, :allow_write_prefs) + can [:index, :show], UserPreference if capability?(token, :allow_read_prefs) + can [:update, :update_all, :destroy], UserPreference if capability?(token, :allow_write_prefs) if token&.user&.terms_agreed? can [:create, :update, :upload, :close, :subscribe, :unsubscribe, :expand_bbox], Changeset if capability?(token, :allow_write_api) diff --git a/app/controllers/api/user_preferences_controller.rb b/app/controllers/api/user_preferences_controller.rb index 39e0dff30..9b117a4d9 100644 --- a/app/controllers/api/user_preferences_controller.rb +++ b/app/controllers/api/user_preferences_controller.rb @@ -9,7 +9,7 @@ module Api ## # return all the preferences as an XML document - def read + def index doc = OSM::API.new.get_xml_doc prefs = current_user.preferences @@ -26,14 +26,14 @@ module Api ## # return the value for a single preference - def read_one + def show pref = UserPreference.find([current_user.id, params[:preference_key]]) render :plain => pref.v.to_s end # update the entire set of preferences - def update + def update_all old_preferences = current_user.preferences.each_with_object({}) do |preference, preferences| preferences[preference.k] = preference end @@ -63,7 +63,7 @@ module Api ## # update the value of a single preference - def update_one + def update begin pref = UserPreference.find([current_user.id, params[:preference_key]]) rescue ActiveRecord::RecordNotFound @@ -80,7 +80,7 @@ module Api ## # delete a single preference - def delete_one + def destroy UserPreference.find([current_user.id, params[:preference_key]]).delete render :plain => "" diff --git a/config/routes.rb b/config/routes.rb index 58c05da17..d936072d7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,11 +67,11 @@ OpenStreetMap::Application.routes.draw do get "user/gpx_files" => "api/users#gpx_files" get "users" => "api/users#index", :as => :api_users - get "user/preferences" => "api/user_preferences#read" - get "user/preferences/:preference_key" => "api/user_preferences#read_one" - put "user/preferences" => "api/user_preferences#update" - put "user/preferences/:preference_key" => "api/user_preferences#update_one" - delete "user/preferences/:preference_key" => "api/user_preferences#delete_one" + get "user/preferences" => "api/user_preferences#index" + get "user/preferences/:preference_key" => "api/user_preferences#show" + put "user/preferences" => "api/user_preferences#update_all" + put "user/preferences/:preference_key" => "api/user_preferences#update" + delete "user/preferences/:preference_key" => "api/user_preferences#destroy" post "gpx/create" => "api/traces#create" get "gpx/:id" => "api/traces#show", :id => /\d+/ diff --git a/test/abilities/api_capability_test.rb b/test/abilities/api_capability_test.rb index 8d0e682f6..8a98f29e0 100644 --- a/test/abilities/api_capability_test.rb +++ b/test/abilities/api_capability_test.rb @@ -100,33 +100,33 @@ class UserApiCapabilityTest < ApiCapabilityTest test "user preferences" do # a user with no tokens capability = ApiCapability.new nil - [:read, :read_one, :update, :update_one, :delete_one].each do |act| + [:index, :show, :update_all, :update, :destroy].each do |act| assert capability.cannot? act, UserPreference end # A user with empty tokens capability = ApiCapability.new tokens - [:read, :read_one, :update, :update_one, :delete_one].each do |act| + [:index, :show, :update_all, :update, :destroy].each do |act| assert capability.cannot? act, UserPreference end capability = ApiCapability.new tokens(:allow_read_prefs) - [:update, :update_one, :delete_one].each do |act| + [:update_all, :update, :destroy].each do |act| assert capability.cannot? act, UserPreference end - [:read, :read_one].each do |act| + [:index, :show].each do |act| assert capability.can? act, UserPreference end capability = ApiCapability.new tokens(:allow_write_prefs) - [:read, :read_one].each do |act| + [:index, :show].each do |act| assert capability.cannot? act, UserPreference end - [:update, :update_one, :delete_one].each do |act| + [:update_all, :update, :destroy].each do |act| assert capability.can? act, UserPreference end end diff --git a/test/controllers/api/user_preferences_controller_test.rb b/test/controllers/api/user_preferences_controller_test.rb index 99dad9597..ac377848c 100644 --- a/test/controllers/api/user_preferences_controller_test.rb +++ b/test/controllers/api/user_preferences_controller_test.rb @@ -7,38 +7,38 @@ module Api def test_routes assert_routing( { :path => "/api/0.6/user/preferences", :method => :get }, - { :controller => "api/user_preferences", :action => "read" } + { :controller => "api/user_preferences", :action => "index" } ) assert_routing( { :path => "/api/0.6/user/preferences", :method => :put }, - { :controller => "api/user_preferences", :action => "update" } + { :controller => "api/user_preferences", :action => "update_all" } ) assert_routing( { :path => "/api/0.6/user/preferences/key", :method => :get }, - { :controller => "api/user_preferences", :action => "read_one", :preference_key => "key" } + { :controller => "api/user_preferences", :action => "show", :preference_key => "key" } ) assert_routing( { :path => "/api/0.6/user/preferences/key", :method => :put }, - { :controller => "api/user_preferences", :action => "update_one", :preference_key => "key" } + { :controller => "api/user_preferences", :action => "update", :preference_key => "key" } ) assert_routing( { :path => "/api/0.6/user/preferences/key", :method => :delete }, - { :controller => "api/user_preferences", :action => "delete_one", :preference_key => "key" } + { :controller => "api/user_preferences", :action => "destroy", :preference_key => "key" } ) end ## - # test read action - def test_read + # test showing all preferences + def test_index # first try without auth - get :read + get :index assert_response :unauthorized, "should be authenticated" # authenticate as a user with no preferences basic_authorization create(:user).email, "test" # try the read again - get :read + get :index assert_select "osm" do assert_select "preferences", :count => 1 do assert_select "preference", :count => 0 @@ -52,7 +52,7 @@ module Api basic_authorization user.email, "test" # try the read again - get :read + get :index assert_response :success assert_equal "application/xml", @response.content_type assert_select "osm" do @@ -65,39 +65,39 @@ module Api end ## - # test read_one action - def test_read_one + # test showing one preference + def test_show user = create(:user) create(:user_preference, :user => user, :k => "key", :v => "value") # try a read without auth - get :read_one, :params => { :preference_key => "key" } + get :show, :params => { :preference_key => "key" } assert_response :unauthorized, "should be authenticated" # authenticate as a user with preferences basic_authorization user.email, "test" # try the read again - get :read_one, :params => { :preference_key => "key" } + get :show, :params => { :preference_key => "key" } assert_response :success assert_equal "text/plain", @response.content_type assert_equal "value", @response.body # try the read again for a non-existent key - get :read_one, :params => { :preference_key => "unknown_key" } + get :show, :params => { :preference_key => "unknown_key" } assert_response :not_found end ## - # test update action - def test_update + # test bulk update action + def test_update_all user = create(:user) create(:user_preference, :user => user, :k => "key", :v => "value") create(:user_preference, :user => user, :k => "some_key", :v => "some_value") # try a put without auth assert_no_difference "UserPreference.count" do - put :update, :body => "" + put :update_all, :body => "" end assert_response :unauthorized, "should be authenticated" assert_equal "value", UserPreference.find([user.id, "key"]).v @@ -111,7 +111,7 @@ module Api # try the put again assert_no_difference "UserPreference.count" do - put :update, :body => "" + put :update_all, :body => "" end assert_response :success assert_equal "text/plain", @response.content_type @@ -124,7 +124,7 @@ module Api # try a put with duplicate keys assert_no_difference "UserPreference.count" do - put :update, :body => "" + put :update_all, :body => "" end assert_response :bad_request assert_equal "text/plain", @response.content_type @@ -133,20 +133,20 @@ module Api # try a put with invalid content assert_no_difference "UserPreference.count" do - put :update, :body => "nonsense" + put :update_all, :body => "nonsense" end assert_response :bad_request end ## - # test update_one action - def test_update_one + # test update action + def test_update user = create(:user) create(:user_preference, :user => user) # try a put without auth assert_no_difference "UserPreference.count" do - put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value" + put :update, :params => { :preference_key => "new_key" }, :body => "new_value" end assert_response :unauthorized, "should be authenticated" assert_raises ActiveRecord::RecordNotFound do @@ -158,7 +158,7 @@ module Api # try adding a new preference assert_difference "UserPreference.count", 1 do - put :update_one, :params => { :preference_key => "new_key" }, :body => "new_value" + put :update, :params => { :preference_key => "new_key" }, :body => "new_value" end assert_response :success assert_equal "text/plain", @response.content_type @@ -167,7 +167,7 @@ module Api # try changing the value of a preference assert_no_difference "UserPreference.count" do - put :update_one, :params => { :preference_key => "new_key" }, :body => "newer_value" + put :update, :params => { :preference_key => "new_key" }, :body => "newer_value" end assert_response :success assert_equal "text/plain", @response.content_type @@ -176,14 +176,14 @@ module Api end ## - # test delete_one action - def test_delete_one + # test destroy action + def test_destroy user = create(:user) create(:user_preference, :user => user, :k => "key", :v => "value") # try a delete without auth assert_no_difference "UserPreference.count" do - delete :delete_one, :params => { :preference_key => "key" } + delete :destroy, :params => { :preference_key => "key" } end assert_response :unauthorized, "should be authenticated" assert_equal "value", UserPreference.find([user.id, "key"]).v @@ -193,7 +193,7 @@ module Api # try the delete again assert_difference "UserPreference.count", -1 do - get :delete_one, :params => { :preference_key => "key" } + get :destroy, :params => { :preference_key => "key" } end assert_response :success assert_equal "text/plain", @response.content_type @@ -204,7 +204,7 @@ module Api # try the delete again for the same key assert_no_difference "UserPreference.count" do - get :delete_one, :params => { :preference_key => "key" } + get :destroy, :params => { :preference_key => "key" } end assert_response :not_found assert_raises ActiveRecord::RecordNotFound do @@ -214,7 +214,7 @@ module Api # Ensure that a valid access token with correct capabilities can be used to # read preferences - def test_read_one_using_token + def test_show_using_token user = create(:user) token = create(:access_token, :user => user, :allow_read_prefs => true) create(:user_preference, :user => user, :k => "key", :v => "value") @@ -224,14 +224,14 @@ module Api @request.env["oauth.strategies"] = [:token] @request.env["oauth.token"] = token - get :read_one, :params => { :preference_key => "key" } + get :show, :params => { :preference_key => "key" } assert_response :success end # Ensure that a valid access token with incorrect capabilities can't be used # to read preferences even, though the owner of that token could read them # by other methods. - def test_read_one_using_token_fail + def test_show_using_token_fail user = create(:user) token = create(:access_token, :user => user, :allow_read_prefs => false) create(:user_preference, :user => user, :k => "key", :v => "value") @@ -239,7 +239,7 @@ module Api @request.env["oauth.strategies"] = [:token] @request.env["oauth.token"] = token - get :read_one, :params => { :preference_key => "key" } + get :show, :params => { :preference_key => "key" } assert_response :forbidden end end -- 2.43.2