www.fgks.org   »   [go: up one dir, main page]

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

Added an API for deleting work #5433

Merged
merged 12 commits into from
Aug 4, 2021
Merged

Added an API for deleting work #5433

merged 12 commits into from
Aug 4, 2021

Conversation

BharatKalluri
Copy link
Collaborator
@BharatKalluri BharatKalluri commented Jul 16, 2021

I've proposed an idea to @cdrini that instead of deleting a work from the OL client by doing multiple http calls and getting all the editions, then making a bulk delete call to the server to delete editions of work + work. We can have an endpoint in OL server which can delete a work and all its references. This also allows us to have an option to delete work in the frontend for librarians in the future if that sounds like a good idea.

Technical

Fetches all the editions, deletes all the editions and works together. Can only be hit by admins and librarians.

Stakeholders

@cdrini

@BharatKalluri BharatKalluri marked this pull request as draft July 16, 2021 06:29
Copy link
Collaborator
@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Lgtm! Some small changes

openlibrary/plugins/openlibrary/api.py Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/api.py Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/api.py Outdated Show resolved Hide resolved
@cdrini cdrini marked this pull request as ready for review July 30, 2021 19:38
@cdrini
Copy link
Collaborator
cdrini commented Jul 30, 2021

Testing now...

@cdrini
Copy link
Collaborator
cdrini commented Jul 30, 2021

Tested by running in browser console:

  • fetch('/works/OL54120W', {method: 'DELETE'}) 403s when logged out
    • It 405s, not 403s...
  • fetch('/works/OL54120W', {method: 'DELETE'}) works when logged in as admin
    • It still 405s?

Hmm, how did you test this one? I'm having trouble getting this to run for me.

@cdrini cdrini self-assigned this Jul 30, 2021
BharatKalluri and others added 3 commits July 31, 2021 10:24
Co-authored-by: Drini Cami <cdrini@gmail.com>
Co-authored-by: Drini Cami <cdrini@gmail.com>
Co-authored-by: Drini Cami <cdrini@gmail.com>
@BharatKalluri
Copy link
Collaborator Author

Tested by running in browser console:

  • fetch('/works/OL54120W', {method: 'DELETE'}) 403s when logged out

    • It 405s, not 403s...
  • fetch('/works/OL54120W', {method: 'DELETE'}) works when logged in as admin

    • It still 405s?

Hmm, how did you test this one? I'm having trouble getting this to run for me.

yeah, I made the same err a couple of times too 😄
The catch is that, since the encoding type is json the endpoint should be suffixed by .json

An example would be fetch('/works/OL45310W.json', {method: 'DELETE'})

@mekarpeles mekarpeles added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Aug 2, 2021
Copy link
Collaborator
@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Ok! This lgtm! A few small readability changes, and then should be good to go :) Testing now...

openlibrary/plugins/openlibrary/api.py Outdated Show resolved Hide resolved
openlibrary/plugins/openlibrary/api.py Outdated Show resolved Hide resolved
Copy link
Collaborator
@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

  • Logged out
    • await fetch('/works/OL54120W/-/delete', {method: 'POST'}) 405s instead of 403s...
    • await fetch('/works/OL54120W/-/delete.json', {method: 'POST'}) 403s!
  • Logged in, admin
    • await fetch('/works/OL54120W/-/delete', {method: 'POST'}) 405s...
    • await fetch('/works/OL54120W/-/delete.json', {method: 'POST'}) Works! Status: ok
      • ✅ Work deleted
      • ✅ Editions deleted
      • ✅ Single batch edit made
    • ✅ await fetch('/works/OL18319A/-/delete.json', {method: 'POST'}) 405s
    • ✅ await fetch('/authors/OL18319A/-/delete.json', {method: 'POST'}) 405s
  • Logged in, non-admin
    • await fetch('/works/OL61982W/-/delete.json', {method: 'POST'}) 403s

Ok, no clue why it's not working without .json :/ But I'll take it! Lgtm!

@cdrini cdrini merged commit 1d34c3d into internetarchive:master Aug 4, 2021
@BharatKalluri BharatKalluri deleted the feature/work_delete_api branch August 4, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants