Add endpoints to lock and unlock a document #5107
Conversation
Add an endpoint to the export api to allow the document import process to flag a document that's being imported as "locked" so that users are not permitted to make any changes to it. Ensures that only users with export data permissions can access this endpoint.
| def lock; end | ||
| def lock | ||
| document = Document.find(params[:id]) | ||
| document.update(locked: true) |
There was a problem hiding this comment.
We should use the bang equivalent here so that it raises an error if the document fails validation rather than return a bool.
| @@ -20,6 +20,8 @@ def lock | |||
| document = Document.find(params[:id]) | |||
| document.update(locked: true) | |||
| head :no_content | |||
| rescue ActiveRecord::RecordNotFound | |||
| respond_with Hash.new, status: :not_found | |||
There was a problem hiding this comment.
We may not need this as ActiveRecord::RecordNotFound is an exception that Rails converts into a response. I've always found it hard to find a good link for this but you can find some details in the rescue responses section of https://guides.rubyonrails.org/configuring.html#configuring-action-dispatch
I'd suggest trying it out to see if it automatically does a 404 as I think it's unusual in the Whitehall codebase to rescue this.
There was a problem hiding this comment.
Yeah, it returns a 404 without the rescue block. I'll remove this code.
| assert document.reload.locked | ||
| assert_response :no_content |
|
|
||
| def no_content | ||
| head :no_content | ||
| end |
There was a problem hiding this comment.
I'd have thought you don't really need to dry this up as someone reading this then has to look this method up to understand quite what no_content means which seems a bit unnecessary for a 6 character saving.
Doing a bit of Googling you might not actually need to specify head :no_content as Rails seems to have started doing that by default since Rails 5: rails/rails#19377
| @@ -1,4 +1,5 @@ | |||
| class Admin::Export::DocumentController < Admin::Export::BaseController | |||
| skip_before_action :verify_authenticity_token | |||
A 204 No Content is returned if the lock request is successful. [Rails 5](rails/rails#19377) does this by default by automatically adding `:no_content` to the header if it can't find a template to respond with.
Add an endpoint to the export api to allow the document import process to "unlock" a document that's being imported if the import has failed for any reason. Ensure that only users with export data permissions can access this endpoint.
A 204 No Content is returned if the unlock request is successful. [Rails 5](rails/rails#19377) does this by default by automatically adding `:no_content` to the header if it can't find a template to respond with.
Authenticity token are generated as a hidden field in Whitehall forms. As it is not intended for these POST routes to be called from a form within in the application, these check should be skipped.
leenagupte
force-pushed
the
lock-unlock-actions
branch
from
2ba87a5
to
2bd2ad9
Compare
|
@kevindew Thanks for your comments. I've addressed them and fixed up. |
Trello: https://trello.com/c/V17DA7ma
Follows on from: #4903
What's changed?
Add two imports to the Document Export API to "lock" and "unlock" a document.
The lock and unlock requests are only allowed for authenticated users who have the export permission.
Why?
A while ago we added the ability to lock a document in Whitehall to prevent a document from being edited in Whitehall during, and after, a migration. Should a migration fail we need the ability to unlock the document.
Results from local testing
Locking
Unlocking