From 65af185847e9d204c57e847fc091bc7123b2e498 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Mon, 6 Jun 2016 08:34:15 +0200 Subject: [PATCH] Streamline of rest api (added missing destroy backends). --- app/controllers/application_controller.rb | 44 ++++++++++++++----- app/controllers/groups_controller.rb | 19 ++++---- app/controllers/organizations_controller.rb | 24 +++++----- .../ticket_priorities_controller.rb | 1 + app/controllers/ticket_states_controller.rb | 1 + app/controllers/users_controller.rb | 23 ++++++---- config/routes/group.rb | 9 ++-- config/routes/organization.rb | 9 ++-- config/routes/ticket.rb | 2 + config/routes/user.rb | 1 + lib/models.rb | 1 + .../user_organization_controller_test.rb | 12 ++--- 12 files changed, 95 insertions(+), 51 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index d8f5fd348..010826119 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -379,7 +379,7 @@ class ApplicationController < ActionController::Base end # model helper - def model_create_render (object, params) + def model_create_render(object, params) # create object generic_object = object.new(object.param_cleanup(params[object.to_app_model_url], true )) @@ -397,11 +397,11 @@ class ApplicationController < ActionController::Base render json: model_match_error(e.message), status: :unprocessable_entity end - def model_create_render_item (generic_object) + def model_create_render_item(generic_object) render json: generic_object.attributes_with_associations, status: :created end - def model_update_render (object, params) + def model_update_render(object, params) # find object generic_object = object.find(params[:id]) @@ -419,11 +419,11 @@ class ApplicationController < ActionController::Base render json: model_match_error(e.message), status: :unprocessable_entity end - def model_update_render_item (generic_object) + def model_update_render_item(generic_object) render json: generic_object.attributes_with_associations, status: :ok end - def model_destory_render (object, params) + def model_destory_render(object, params) generic_object = object.find(params[:id]) generic_object.destroy model_destory_render_item() @@ -453,12 +453,17 @@ class ApplicationController < ActionController::Base render json: model_match_error(e.message), status: :unprocessable_entity end - def model_show_render_item (generic_object) + def model_show_render_item(generic_object) render json: generic_object.attributes_with_associations, status: :ok end - def model_index_render (object, _params) - generic_objects = object.all + def model_index_render(object, params) + if params[:page] && params[:per_page] + offset = (params[:page].to_i - 1) * params[:per_page].to_i + generic_objects = object.limit(params[:per_page]).offset(offset) + else + generic_objects = object.all + end if params[:full] assets = {} @@ -485,11 +490,11 @@ class ApplicationController < ActionController::Base render json: model_match_error(e.message), status: :unprocessable_entity end - def model_index_render_result (generic_objects) + def model_index_render_result(generic_objects) render json: generic_objects, status: :ok end - def model_match_error (error) + def model_match_error(error) data = { error: error } @@ -499,6 +504,25 @@ class ApplicationController < ActionController::Base data end + def model_references_check(object, params) + generic_object = object.find(params[:id]) + result = Models.references(object, generic_object.id) + return false if result.empty? + render json: { error: 'Can\'t delete, object has references.' }, status: :unprocessable_entity + true + rescue => e + logger.error e.message + logger.error e.backtrace.inspect + render json: model_match_error(e.message), status: :unprocessable_entity + end + + def not_found(e) + respond_to do |format| + format.json { render json: { error: e.message }, status: :not_found } + format.any { render text: "Error: #{e.message}", status: :not_found } + end + end + # check maintenance mode def check_maintenance_only(user) return false if Setting.get('maintenance_mode') != true diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index d3311c069..0966040bf 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -27,7 +27,7 @@ Example: =begin Resource: -GET /api/v1/groups.json +GET /api/v1/groups Response: [ @@ -44,7 +44,7 @@ Response: ] Test: -curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} +curl http://localhost/api/v1/groups -v -u #{login}:#{password} =end @@ -55,7 +55,7 @@ curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} =begin Resource: -GET /api/v1/groups/#{id}.json +GET /api/v1/groups/#{id} Response: { @@ -65,7 +65,7 @@ Response: } Test: -curl http://localhost/api/v1/groups/#{id}.json -v -u #{login}:#{password} +curl http://localhost/api/v1/groups/#{id} -v -u #{login}:#{password} =end @@ -76,7 +76,7 @@ curl http://localhost/api/v1/groups/#{id}.json -v -u #{login}:#{password} =begin Resource: -POST /api/v1/groups.json +POST /api/v1/groups Payload: { @@ -96,7 +96,7 @@ Response: } Test: -curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} -H "Content-Type: application/json" -X POST -d '{"name": "some_name","active": true, "note": "some note"}' +curl http://localhost/api/v1/groups -v -u #{login}:#{password} -H "Content-Type: application/json" -X POST -d '{"name": "some_name","active": true, "note": "some note"}' =end @@ -108,7 +108,7 @@ curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} -H "Content- =begin Resource: -PUT /api/v1/groups/{id}.json +PUT /api/v1/groups/{id} Payload: { @@ -128,7 +128,7 @@ Response: } Test: -curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} -H "Content-Type: application/json" -X PUT -d '{"name": "some_name","active": true, "note": "some note"}' +curl http://localhost/api/v1/groups -v -u #{login}:#{password} -H "Content-Type: application/json" -X PUT -d '{"name": "some_name","active": true, "note": "some note"}' =end @@ -140,10 +140,13 @@ curl http://localhost/api/v1/groups.json -v -u #{login}:#{password} -H "Content- =begin Resource: +DELETE /api/v1/groups/{id} Response: +{} Test: +curl http://localhost/api/v1/groups/{id} -v -u #{login}:#{password} -H "Content-Type: application/json" -X DELETE -d '{}' =end diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 69dcbc0aa..95c222bda 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -25,7 +25,7 @@ Example: =begin Resource: -GET /api/v1/organizations.json +GET /api/v1/organizations Response: [ @@ -42,7 +42,7 @@ Response: ] Test: -curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} +curl http://localhost/api/v1/organizations -v -u #{login}:#{password} =end @@ -63,7 +63,7 @@ curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} =begin Resource: -GET /api/v1/organizations/#{id}.json +GET /api/v1/organizations/#{id} Response: { @@ -73,7 +73,7 @@ Response: } Test: -curl http://localhost/api/v1/organizations/#{id}.json -v -u #{login}:#{password} +curl http://localhost/api/v1/organizations/#{id} -v -u #{login}:#{password} =end @@ -101,7 +101,7 @@ curl http://localhost/api/v1/organizations/#{id}.json -v -u #{login}:#{password} =begin Resource: -POST /api/v1/organizations.json +POST /api/v1/organizations Payload: { @@ -119,7 +119,7 @@ Response: } Test: -curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} -H "Content-Type: application/json" -X POST -d '{"name": "some_name","active": true,"shared": true,"note": "some note"}' +curl http://localhost/api/v1/organizations -v -u #{login}:#{password} -H "Content-Type: application/json" -X POST -d '{"name": "some_name","active": true,"shared": true,"note": "some note"}' =end @@ -131,7 +131,7 @@ curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} -H "C =begin Resource: -PUT /api/v1/organizations/{id}.json +PUT /api/v1/organizations/{id} Payload: { @@ -150,7 +150,7 @@ Response: } Test: -curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} -H "Content-Type: application/json" -X PUT -d '{"id": 1,"name": "some_name","active": true,"shared": true,"note": "some note"}' +curl http://localhost/api/v1/organizations -v -u #{login}:#{password} -H "Content-Type: application/json" -X PUT -d '{"id": 1,"name": "some_name","active": true,"shared": true,"note": "some note"}' =end @@ -162,15 +162,19 @@ curl http://localhost/api/v1/organizations.json -v -u #{login}:#{password} -H "C =begin Resource: +DELETE /api/v1/organization/{id} Response: +{} Test: +curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Content-Type: application/json" -X DELETE -d '{}' =end def destroy - return if deny_if_not_role('Agent') + return if deny_if_not_role(Z_ROLENAME_AGENT) + return if model_references_check(Organization, params) model_destory_render(Organization, params) end @@ -184,7 +188,7 @@ Test: end # get organization data - organization = Organization.find( params[:id] ) + organization = Organization.find(params[:id]) # get history of organization history = organization.history_get(true) diff --git a/app/controllers/ticket_priorities_controller.rb b/app/controllers/ticket_priorities_controller.rb index 6056abdfa..d7f698adf 100644 --- a/app/controllers/ticket_priorities_controller.rb +++ b/app/controllers/ticket_priorities_controller.rb @@ -28,6 +28,7 @@ class TicketPrioritiesController < ApplicationController # DELETE /ticket_priorities/1 def destroy return if deny_if_not_role(Z_ROLENAME_ADMIN) + return if model_references_check(Ticket::Priority, params) model_destory_render(Ticket::Priority, params) end end diff --git a/app/controllers/ticket_states_controller.rb b/app/controllers/ticket_states_controller.rb index f0bd2b2c7..c200cafb9 100644 --- a/app/controllers/ticket_states_controller.rb +++ b/app/controllers/ticket_states_controller.rb @@ -28,6 +28,7 @@ class TicketStatesController < ApplicationController # DELETE /ticket_states/1 def destroy return if deny_if_not_role(Z_ROLENAME_ADMIN) + return if model_references_check(Ticket::State, params) model_destory_render(Ticket::State, params) end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a43f3e39f..c6c72f6b9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -65,7 +65,11 @@ class UsersController < ApplicationController # @response_message 200 [User] Created User record. # @response_message 401 Invalid session. def create - user = User.new( User.param_cleanup(params, true) ) + + # in case of authentication, set current_user to access later + authentication_check_only({}) + + user = User.new(User.param_cleanup(params, true)) begin # check if it's first user @@ -76,13 +80,13 @@ class UsersController < ApplicationController # check if feature is enabled if !Setting.get('user_create_account') - render json: { error_human: 'Feature not enabled!' }, status: :unprocessable_entity + render json: { error: 'Feature not enabled!' }, status: :unprocessable_entity return end # check signup option only after admin account is created if count > 2 && !params[:signup] - render json: { error_human: 'Only signup is possible!' }, status: :unprocessable_entity + render json: { error: 'Only signup with not authenticate user possible!' }, status: :unprocessable_entity return end user.updated_by_id = 1 @@ -127,7 +131,7 @@ class UsersController < ApplicationController if user.email exists = User.where(email: user.email.downcase).first if exists - render json: { error_human: 'User already exists!' }, status: :unprocessable_entity + render json: { error: 'User already exists!' }, status: :unprocessable_entity return end end @@ -233,6 +237,7 @@ class UsersController < ApplicationController # @response_message 401 Invalid session. def destroy return if deny_if_not_role(Z_ROLENAME_ADMIN) + return if model_references_check(User, params) model_destory_render(User, params) end @@ -462,12 +467,12 @@ curl http://localhost/api/v1/users/email_verify_send.json -v -u #{login}:#{passw # check is verify is possible to send user = User.find_by(email: params[:email].downcase) if !user - render json: { error_human: 'No such user!' }, status: :unprocessable_entity + render json: { error: 'No such user!' }, status: :unprocessable_entity return end #if user.verified == true - # render json: { error_human: 'Already verified!' }, status: :unprocessable_entity + # render json: { error: 'Already verified!' }, status: :unprocessable_entity # return #end @@ -917,13 +922,13 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content params[:role_ids].each {|role_id| role_local = Role.lookup(id: role_id) if !role_local - render json: { error_human: 'Invalid role_ids!' }, status: :unauthorized + render json: { error: 'Invalid role_ids!' }, status: :unauthorized logger.info "Invalid role_ids for current_user_id: #{current_user.id} role_ids #{role_id}" return false end role_name = role_local.name next if role_name != 'Admin' && role_name != 'Agent' - render json: { error_human: 'This role assignment is only allowed by admin!' }, status: :unauthorized + render json: { error: 'This role assignment is only allowed by admin!' }, status: :unauthorized logger.info "This role assignment is only allowed by admin! current_user_id: #{current_user.id} assigned to #{role_name}" return false } @@ -934,7 +939,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content params[:group_ids] = [params[:group_ids]] end if !params[:group_ids].empty? - render json: { error_human: 'Group relation is only allowed by admin!' }, status: :unauthorized + render json: { error: 'Group relation is only allowed by admin!' }, status: :unauthorized logger.info "Group relation is only allowed by admin! current_user_id: #{current_user.id} group_ids #{params[:group_ids].inspect}" return false end diff --git a/config/routes/group.rb b/config/routes/group.rb index 3b2051160..c2a0ba8d2 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -2,9 +2,10 @@ Zammad::Application.routes.draw do api_path = Rails.configuration.api_path # groups - match api_path + '/groups', to: 'groups#index', via: :get - match api_path + '/groups/:id', to: 'groups#show', via: :get - match api_path + '/groups', to: 'groups#create', via: :post - match api_path + '/groups/:id', to: 'groups#update', via: :put + match api_path + '/groups', to: 'groups#index', via: :get + match api_path + '/groups/:id', to: 'groups#show', via: :get + match api_path + '/groups', to: 'groups#create', via: :post + match api_path + '/groups/:id', to: 'groups#update', via: :put + match api_path + '/groups/:id', to: 'groups#destroy', via: :delete end diff --git a/config/routes/organization.rb b/config/routes/organization.rb index ead3b0193..8163443bc 100644 --- a/config/routes/organization.rb +++ b/config/routes/organization.rb @@ -2,10 +2,11 @@ Zammad::Application.routes.draw do api_path = Rails.configuration.api_path # organizations - match api_path + '/organizations', to: 'organizations#index', via: :get - match api_path + '/organizations/:id', to: 'organizations#show', via: :get - match api_path + '/organizations', to: 'organizations#create', via: :post - match api_path + '/organizations/:id', to: 'organizations#update', via: :put + match api_path + '/organizations', to: 'organizations#index', via: :get + match api_path + '/organizations/:id', to: 'organizations#show', via: :get + match api_path + '/organizations', to: 'organizations#create', via: :post + match api_path + '/organizations/:id', to: 'organizations#update', via: :put + match api_path + '/organizations/:id', to: 'organizations#destroy', via: :delete match api_path + '/organizations/history/:id', to: 'organizations#history', via: :get end diff --git a/config/routes/ticket.rb b/config/routes/ticket.rb index 11bdf30d0..c8ac1fbce 100644 --- a/config/routes/ticket.rb +++ b/config/routes/ticket.rb @@ -24,12 +24,14 @@ Zammad::Application.routes.draw do match api_path + '/ticket_priorities/:id', to: 'ticket_priorities#show', via: :get match api_path + '/ticket_priorities', to: 'ticket_priorities#create', via: :post match api_path + '/ticket_priorities/:id', to: 'ticket_priorities#update', via: :put + match api_path + '/ticket_priorities/:id', to: 'ticket_priorities#destroy', via: :delete # ticket state match api_path + '/ticket_states', to: 'ticket_states#index', via: :get match api_path + '/ticket_states/:id', to: 'ticket_states#show', via: :get match api_path + '/ticket_states', to: 'ticket_states#create', via: :post match api_path + '/ticket_states/:id', to: 'ticket_states#update', via: :put + match api_path + '/ticket_states/:id', to: 'ticket_states#destroy', via: :delete # ticket articles match api_path + '/ticket_articles', to: 'ticket_articles#index', via: :get diff --git a/config/routes/user.rb b/config/routes/user.rb index 429c798a7..0c0da5c5d 100644 --- a/config/routes/user.rb +++ b/config/routes/user.rb @@ -20,6 +20,7 @@ Zammad::Application.routes.draw do match api_path + '/users/history/:id', to: 'users#history', via: :get match api_path + '/users', to: 'users#create', via: :post match api_path + '/users/:id', to: 'users#update', via: :put + match api_path + '/users/:id', to: 'users#destroy', via: :delete match api_path + '/users/image/:hash', to: 'users#image', via: :get match api_path + '/users/email_verify', to: 'users#email_verify', via: :post diff --git a/lib/models.rb b/lib/models.rb index 357a6d495..d9c218949 100644 --- a/lib/models.rb +++ b/lib/models.rb @@ -97,6 +97,7 @@ returns =end def self.references(object_name, object_id) + object_name = object_name.to_s # check if model exists object_model = load_adapter(object_name) diff --git a/test/controllers/user_organization_controller_test.rb b/test/controllers/user_organization_controller_test.rb index f32444f59..20b3f264c 100644 --- a/test/controllers/user_organization_controller_test.rb +++ b/test/controllers/user_organization_controller_test.rb @@ -81,8 +81,8 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest post '/api/v1/users', params.to_json, @headers assert_response(422) result = JSON.parse(@response.body) - assert(result['error_human']) - assert_equal('Feature not enabled!', result['error_human']) + assert(result['error']) + assert_equal('Feature not enabled!', result['error']) Setting.set('user_create_account', true) @@ -91,16 +91,16 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest post '/api/v1/users', params.to_json, @headers assert_response(422) result = JSON.parse(@response.body) - assert(result['error_human']) - assert_equal('Only signup is possible!', result['error_human']) + assert(result['error']) + assert_equal('Only signup with not authenticate user possible!', result['error']) # already existing user with enabled feature params = { email: 'rest-customer1@example.com', signup: true } post '/api/v1/users', params.to_json, @headers assert_response(422) result = JSON.parse(@response.body) - assert(result['error_human']) - assert_equal('User already exists!', result['error_human']) + assert(result['error']) + assert_equal('User already exists!', result['error']) # create user with enabled feature params = { firstname: 'Me First', lastname: 'Me Last', email: 'new_here@example.com', signup: true }