-
Notifications
You must be signed in to change notification settings - Fork 501
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
Support deletion of saved searches #10198
Changes from 14 commits
2dc84db
ac80e0a
a50f963
98f7b53
4fabfec
0dc2ea9
b6f4b7f
614da60
6836366
633cd6c
92e61fc
4061c08
5f2de4a
008507b
8acfdc9
4022c9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
### Saved search deletion | ||
|
||
Saved searches can now be removed using API `/api/admin/savedsearches/$id`. See PR #10198. | ||
This is reflected in the [Saved Search Native API section](https://dataverse-guide--10198.org.readthedocs.build/en/10198/api/native-api.html#saved-search) of the Guide. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5749,7 +5749,8 @@ The ``$identifier`` should start with an ``@`` if it's a user. Groups start with | |
Saved Search | ||
~~~~~~~~~~~~ | ||
|
||
The Saved Search, Linked Dataverses, and Linked Datasets features shipped with Dataverse 4.0, but as a "`superuser-only <https://github.com/IQSS/dataverse/issues/90#issuecomment-86094663>`_" because they are **experimental** (see `#1364 <https://github.com/IQSS/dataverse/issues/1364>`_, `#1813 <https://github.com/IQSS/dataverse/issues/1813>`_, `#1840 <https://github.com/IQSS/dataverse/issues/1840>`_, `#1890 <https://github.com/IQSS/dataverse/issues/1890>`_, `#1939 <https://github.com/IQSS/dataverse/issues/1939>`_, `#2167 <https://github.com/IQSS/dataverse/issues/2167>`_, `#2186 <https://github.com/IQSS/dataverse/issues/2186>`_, `#2053 <https://github.com/IQSS/dataverse/issues/2053>`_, and `#2543 <https://github.com/IQSS/dataverse/issues/2543>`_). The following API endpoints were added to help people with access to the "admin" API make use of these features in their current form. There is a known issue (`#1364 <https://github.com/IQSS/dataverse/issues/1364>`_) that once a link to a Dataverse collection or dataset is created, it cannot be removed (apart from database manipulation and reindexing) which is why a ``DELETE`` endpoint for saved searches is neither documented nor functional. The Linked Dataverse collections feature is `powered by Saved Search <https://github.com/IQSS/dataverse/issues/1852>`_ and therefore requires that the "makelinks" endpoint be executed on a periodic basis as well. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a conversation going on in #10628 about how these docs used to explain to people that they might have to wait up to 24 hours for their saved search to show results. Should we add that back as part of this pull request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated with The update of all saved search is run by a timer once a week (See :ref: |
||
The Saved Search, Linked Dataverses, and Linked Datasets features are only accessible to superusers except for linking a dataset. The following API endpoints were added to help people with access to the "admin" API make use of these features in their current form. Keep in mind that they are partially experimental. | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
The update of all saved search is run by a timer once a week (See :ref:`saved-search-timer`) so if you just created a saved search, you can run manually ``makelinks`` endpoint that will find new dataverses and datasets that match the saved search and then link the search results to the dataverse in which the saved search is defined. | ||
|
||
List all saved searches. :: | ||
|
||
|
@@ -5759,6 +5760,12 @@ List a saved search by database id. :: | |
|
||
GET http://$SERVER/api/admin/savedsearches/$id | ||
|
||
Delete a saved search by database id. | ||
|
||
The ``unlink=true`` query parameter unlinks all links (linked dataset or Dataverse collection) associated with the deleted saved search. Use of this parameter should be well considered as you cannot know if the links were created manually or by the saved search. After deleting a saved search with ``unlink=true``, we recommend running ``/makelinks/all`` just in case there was a dataset that was linked by another saved search. (Saved searches can link the same dataset.) Reindexing might be necessary as well.:: | ||
|
||
DELETE http://$SERVER/api/admin/savedsearches/$id?unlink=true | ||
|
||
Execute a saved search by database id and make links to Dataverse collections and datasets that are found. The JSON response indicates which Dataverse collections and datasets were newly linked versus already linked. The ``debug=true`` query parameter adds to the JSON response extra information about the saved search being executed (which you could also get by listing the saved search). :: | ||
|
||
PUT http://$SERVER/api/admin/savedsearches/makelinks/$id?debug=true | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package edu.harvard.iq.dataverse.api; | ||
|
||
import edu.harvard.iq.dataverse.Dataverse; | ||
import edu.harvard.iq.dataverse.api.auth.AuthRequired; | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; | ||
import edu.harvard.iq.dataverse.engine.command.DataverseRequest; | ||
import edu.harvard.iq.dataverse.engine.command.exception.CommandException; | ||
|
@@ -24,6 +25,8 @@ | |
import jakarta.ws.rs.Path; | ||
import jakarta.ws.rs.PathParam; | ||
import jakarta.ws.rs.QueryParam; | ||
import jakarta.ws.rs.container.ContainerRequestContext; | ||
import jakarta.ws.rs.core.Context; | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import jakarta.ws.rs.core.Response; | ||
import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; | ||
import static jakarta.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; | ||
|
@@ -55,7 +58,7 @@ public Response list() { | |
savedSearchesBuilder.add(thisSavedSearch); | ||
} | ||
JsonObjectBuilder response = Json.createObjectBuilder(); | ||
response.add("saved searches", savedSearchesBuilder); | ||
response.add("savedSearches", savedSearchesBuilder); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return ok(response); | ||
} | ||
|
||
|
@@ -90,7 +93,6 @@ private JsonObjectBuilder toJson(SavedSearch savedSearch) { | |
|
||
@POST | ||
public Response add(JsonObject body) { | ||
|
||
if (body == null) { | ||
return error(BAD_REQUEST, "JSON is expected."); | ||
} | ||
|
@@ -159,7 +161,7 @@ public Response add(JsonObject body) { | |
|
||
try { | ||
SavedSearch persistedSavedSearch = savedSearchSvc.add(toPersist); | ||
return ok("Added: " + persistedSavedSearch); | ||
return ok("Added: " + persistedSavedSearch, Json.createObjectBuilder().add("id", persistedSavedSearch.getId())); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} catch (EJBException ex) { | ||
StringBuilder errors = new StringBuilder(); | ||
Throwable throwable = ex.getCause(); | ||
|
@@ -173,16 +175,18 @@ public Response add(JsonObject body) { | |
|
||
@DELETE | ||
@Path("{id}") | ||
public Response delete(@PathParam("id") long doomedId) { | ||
boolean disabled = true; | ||
if (disabled) { | ||
return error(BAD_REQUEST, "Saved Searches can not safely be deleted because links can not safely be deleted. See https://github.com/IQSS/dataverse/issues/1364 for details."); | ||
} | ||
public Response delete(@PathParam("id") long doomedId, @QueryParam("unlink") boolean unlink) { | ||
SavedSearch doomed = savedSearchSvc.find(doomedId); | ||
if (doomed == null) { | ||
return error(NOT_FOUND, "Could not find saved search id " + doomedId); | ||
} | ||
boolean wasDeleted = savedSearchSvc.delete(doomedId); | ||
boolean wasDeleted; | ||
try { | ||
wasDeleted = savedSearchSvc.delete(doomedId, unlink); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to put this in a command? There's a "CreateSavedSearchCommand" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @sekmiller, if my understanding is correct I note that It could be prematured to create a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One reason that I suggested the command was because commands also check for permissions. I didn't notice that this is a superuser only feature - so that permission check wouldn't be available in the command. |
||
} catch (Exception e) { | ||
return error(INTERNAL_SERVER_ERROR, "Problem while trying to unlink links of saved search id " + doomedId); | ||
luddaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
if (wasDeleted) { | ||
return ok(Json.createObjectBuilder().add("Deleted", doomedId)); | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to leave comments below about backward incompatible changes. I'm pretty sure my preference is to NOT introduce any backward-incompatible changes but if we do the should be listed in
doc/sphinx-guides/source/api/changelog.rst
and the release notes should give an overview of backward-incompatible changes and tell people to check that changelog.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I removed every authentication requirement for
/api/admin/savedsearches