Retract DOI#2542
Conversation
fbacall
left a comment
There was a problem hiding this comment.
Looks good, some minor comments, the main thing is: lets just use AssetDoiLog::DELETE instead of coming up with a new action.
| klass = controller_name.singularize.camelize | ||
| version = params[:version] | ||
|
|
||
| retract_log = AssetDoiLog.where(asset_type: klass, asset_id: params[:id], asset_version: version, action: AssetDoiLog::RETRACT).last |
There was a problem hiding this comment.
I wonder if we should also check for the existence of a AssetDoiLog without the asset_version too?
For example if there is a DOI on a "parent" resource which gets retracted, and all its versions are thus deleted, but then someone visits e.g. /workflows/123?version=1 somehow.
(No idea if this can happen - disregard if not)
| @@ -0,0 +1,58 @@ | |||
| module Seek | |||
| module Doi | |||
| module Retracting | |||
There was a problem hiding this comment.
I would just put all this code in Seek::Doi::Minting, since you can't really have one without the other.
| MINT = 1 | ||
| DELETE = 2 | ||
| UNPUBLISH = 3 | ||
| RETRACT = 4 |
There was a problem hiding this comment.
I've thought a bit about this, and think we should use DELETE.
From looking at the two places where DELETE and UNPUBLISH are used - it seems their intent was to handle cases where something was deleted and something was made private (as in, deleted or unpublished from SEEK, not from the DOI provider), respectively. In our case we're deleting the resource from SEEK, so lets use that.
b45ef8e to
c17cb85
Compare
| <h2>The requested <%= asset_type %> does not exist anymore.</h2> | ||
|
|
||
| <p>The <%= asset_type %> with identifier <%= doi_link(retract_log[:doi]) %> has been retracted.</p> | ||
| <p><%= render_doi_citation(retract_log[:doi], selected_citation_style) %></p> |
There was a problem hiding this comment.
Not super happy with this at the moment....
I tried https://github.com/datacite/bolognese to get citeproc hash from the metadata to then render the citation, but there were issues with conflicting versions of dependencies.
There was a problem hiding this comment.
I have replaced it with extracting information from the metadata into a Datacite::Metadata and then serializing this .
| </ul> | ||
|
|
||
| <div class="form-group"> | ||
| <%= label_tag :retraction_reason, 'Reason for retraction (optional)' %> |
| retract_log = AssetDoiLog.where(asset_type: klass, asset_id: params[:id], asset_version: version, action: AssetDoiLog::DELETE).last | ||
| end | ||
| if retract_log.present? | ||
| metadata = JSON.parse(retract_log.datacite_metadata) if retract_log.datacite_metadata.present? |
There was a problem hiding this comment.
I think we should handle the JSON parsing/serializing in the AssetDoiLog model. You can define some custom getter/setter methods e.g. metadata/metadata= and do it in there
|
|
||
| def retract_log(retraction_reason = nil, metadata = nil) | ||
| AssetDoiLog.create(asset_type: doi_resource.class.name, asset_id: doi_resource_id, asset_version: doi_resource_suffix, | ||
| doi: suggested_doi, action: AssetDoiLog::DELETE, user_id: User.current_user.try(:id), comment: retraction_reason, datacite_metadata: metadata) |
There was a problem hiding this comment.
You can just assign user: User.current_user (the way it was done in create_log is also wrong)
| metadata = Datacite::MetadataReader.new(endpoint.metadata(doi)).parse | ||
| endpoint.inactivate(doi) | ||
|
|
||
| retract_log(retraction_reason, metadata.to_json) |
There was a problem hiding this comment.
Validate retraction_reason is present
| end | ||
|
|
||
| def can_retract_doi? | ||
| Seek::Config.doi_minting_enabled && has_doi? && visible?(nil) |
There was a problem hiding this comment.
The visible?(nil) check is probably unnecessary
Resolves #2497
Allows a user to send a DELETE DOI request to datacite.
This will mark the resource as no longer findable.
After retracting, an
AssetDoiLogentry is created to log the retraction and the resource is deleted.If a user follows the DOI - the AssetDoiLog is queried and a tombstone page is displayed.