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

Unconsistent behaviour of dataset delete API #11253

Open
vera opened this issue Feb 13, 2025 · 2 comments
Open

Unconsistent behaviour of dataset delete API #11253

vera opened this issue Feb 13, 2025 · 2 comments
Labels
Type: Bug a defect

Comments

@vera
Copy link
Contributor

vera commented Feb 13, 2025

While testing the dataset delete API, I just noticed the following behavior, which was unexpected to me:

When logged in as a superuser, and then calling the delete API on a dataset with exactly one published version and no draft currently existing, the request seems to be automatically "upgraded" to a destroy action. The request succeeds and the dataset is deleted.

I expected the request to fail, since the delete API is not meant for deleting published datasets (at least according to the docs where the API is titled "Delete Unpublished Dataset" and the destroy API exists, which is in contrast meant for deleting published datasets as a superuser).

What makes the behavior even more confusing to me, as soon as more than one published version exists, the delete API always fails even when I'm a superuser (message: This is a published dataset with multiple versions. This API can only delete the latest version if it is a DRAFT).

When not logged in as a superuser, everything works as I expected (delete API succeeds for drafts but fails for published versions).

Here's the related code:

if (doomed.getVersions().size() == 1) {
if (doomed.isReleased() && (!(u instanceof AuthenticatedUser) || !u.isSuperuser())) {
throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, "Only superusers can delete published datasets"));
}
destroy = true;
} else {
if (!doomedVersion.isDraft()) {
throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, "This is a published dataset with multiple versions. This API can only delete the latest version if it is a DRAFT"));
}
}

And my question: Is this intended behavior? If yes, I think the special case (superuser + exactly one published version allows delete API to succeed) should be documented in the API docs.

@pdurbin
Copy link
Member

pdurbin commented Feb 13, 2025

@vera yes, I'd say you're finding some unexpected behavior when using a superuser token with that "Delete Unpublished Dataset" API:

  • it should never destroy the dataset
  • it should work on drafts when there are previously published versions

I'll go ahead and flag this as a bug. Please feel free to make a PR if you'd like! ❤

@pdurbin pdurbin added the Type: Bug a defect label Feb 13, 2025
@qqmyers
Copy link
Member

qqmyers commented Feb 19, 2025

+1 for simply calling the delete draft version method for this endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug a defect
Projects
None yet
Development

No branches or pull requests

3 participants