Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

record: request deletion button #2419

Closed
wants to merge 1 commit into from

Conversation

mitsosf
Copy link
Contributor

@mitsosf mitsosf commented Dec 5, 2022

image

image

@mitsosf mitsosf force-pushed the record_request_deletion branch 6 times, most recently from 91eadea to a267bb9 Compare December 6, 2022 15:10
@mitsosf mitsosf changed the title wip-record: request deletion button record: request deletion button Dec 6, 2022
@mitsosf mitsosf marked this pull request as ready for review December 6, 2022 15:51
@@ -109,3 +154,45 @@
</ul>
</div>
{%- endif %}

<script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, with the changes discussed IRL, we can completely remove JS since:

  • there's no UI validation for the DOI typing
  • the action is stored using cookies that expire in 1 week

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am doing the cookie check with JS, because if I didn't I would have to modify the landing page view to check for the cookie and it seems excessive to me, let's discuss IRL, maybe I'm forgetting parts of our last discussion.

Comment on lines +583 to +588
if request.form['expected-doi'] != request.form['record-doi']:
flash(_("The DOI you typed did not match the record's DOI."),
category='warning')
return redirect(url_for("invenio_records_ui.recid", pid_value=record[
'recid']))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary as discussed IRL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you remind me why we decided to skip this? :)

@mitsosf
Copy link
Contributor Author

mitsosf commented Jan 16, 2023

Before this goes live we need to see how we will be handling DOIs not managed by us, eg provide a perpetual option for deletion. Or simplify things code-wise by having the users go through the support line if the record is older than 30 days?

@mitsosf mitsosf closed this Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants