From 18a0cd0c81a1ecf35f7aa705ab26a7a472896593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Luka=C5=A1=C3=ADk?= Date: Tue, 28 Feb 2017 15:39:40 +0100 Subject: [PATCH 1/3] Refactor: Move logging and RBAC check closer to the actual action That way nobody runs the delete without the rbac check. --- app/controllers/api/base_controller/generic.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/base_controller/generic.rb b/app/controllers/api/base_controller/generic.rb index 2234a541d3a..de7c50f9725 100644 --- a/app/controllers/api/base_controller/generic.rb +++ b/app/controllers/api/base_controller/generic.rb @@ -98,8 +98,6 @@ def delete_resource(type, id = nil, _data = nil) klass = collection_class(type) id ||= @req.c_id raise BadRequestError, "Must specify an id for deleting a #{type} resource" unless id - api_log_info("Deleting #{type} id #{id}") - resource_search(id, type, klass) delete_resource_action(klass, type, id) end @@ -184,6 +182,8 @@ def add_subcollection_data_to_resource(resource, type, subcollection_data) end def delete_resource_action(klass, type, id) + api_log_info("Deleting #{type} id #{id}") + resource_search(id, type, klass) result = begin klass.destroy(id) action_result(true, "#{type} id: #{id} deleting") From 0a88102ac0da6e02800fffcb0d04c7a790ce8b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Luka=C5=A1=C3=ADk?= Date: Tue, 28 Feb 2017 15:42:47 +0100 Subject: [PATCH 2/3] Avoid double selects in row ActiveRecord.destroy selects the resource to memory to run all the callbacks and validations anyway. --- app/controllers/api/base_controller/generic.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/base_controller/generic.rb b/app/controllers/api/base_controller/generic.rb index de7c50f9725..f0de7d2ff89 100644 --- a/app/controllers/api/base_controller/generic.rb +++ b/app/controllers/api/base_controller/generic.rb @@ -183,9 +183,9 @@ def add_subcollection_data_to_resource(resource, type, subcollection_data) def delete_resource_action(klass, type, id) api_log_info("Deleting #{type} id #{id}") - resource_search(id, type, klass) + resource = resource_search(id, type, klass) result = begin - klass.destroy(id) + resource.destroy! action_result(true, "#{type} id: #{id} deleting") rescue => err action_result(false, err.to_s) From d3d5dcdfe3e1f6ed77c6f6ca24c06863aa959994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20Luka=C5=A1=C3=ADk?= Date: Tue, 28 Feb 2017 15:45:06 +0100 Subject: [PATCH 3/3] Improve usability of delete action - publish reason to the user Sometimes validations disallow user to delete a resource. Let's make him/her conscious about the reasons. --- app/controllers/api/base_controller/generic.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/base_controller/generic.rb b/app/controllers/api/base_controller/generic.rb index f0de7d2ff89..16122eb5c6f 100644 --- a/app/controllers/api/base_controller/generic.rb +++ b/app/controllers/api/base_controller/generic.rb @@ -188,7 +188,7 @@ def delete_resource_action(klass, type, id) resource.destroy! action_result(true, "#{type} id: #{id} deleting") rescue => err - action_result(false, err.to_s) + action_result(false, "#{err} - #{resource.errors.full_messages.join(', ')}") end add_href_to_result(result, type, id) log_result(result)