search

DSpace / RestContract

REST Contract for DSpace 7-8
https://wiki.lyrasis.org/display/DSDOC8x/
37 stars 48 forks source link

[TLC-380] REST Contract for /api/discover/browselinks #205

Closed kshepherd closed 1 year ago

kshepherd commented 1 year ago

Related to PRs that add support for browse links (using pre-7 webui.browse.link configuration): https://github.com/DSpace/DSpace/pull/8589 https://github.com/DSpace/dspace-angular/pull/1972

tdonohue commented 1 year ago

@kshepherd : There was some discussion about this feature (and its REST API design) in last week's DevMtg. During that meeting, @artlowel and @abollini had mentioned that the design of these REST endpoints seems to not align well with how other (similar) endpoints are structured.

Namely, these new /api/discover/browselinks endpoints seem to provide access to the same Browse Indexes that are already available under /api/discover/browses. So, it was suggested in the meeting that we might want to investigate whether it'd be easier to refactor these /api/discover/browselinks/ endpoints to be something like /api/discover/browses/search/findByField?<:metadata-field>

The current design looks odd because it's a completely separate endpoint that seems to simply filter the set of browse indexes which are already returned by /api/discover/browses.

For more information/examples of these /search subpaths, see https://github.com/DSpace/RestContract/blob/main/search-rels.md There are a number of other endpoints which already use /search subpaths, including /api/core/collections, /api/config/submissiondefinitions/, /api/workflow/workflowitems, etc.

kshepherd commented 1 year ago

@tdonohue I have refactored this as requested. The /browses/ endpoint, controller and data services are now used.

kshepherd commented 1 year ago

@tdonohue and @abollini I believe I've address all of these as requested. (see also the DSpace PR which has all the endpoint removals / changes pushed as well as new integration tests

kshepherd commented 1 year ago

Sorry, I didn't fully understand the "old URL" requests, I thought this was related to my path or parameter usage, as I'd copied the base URL from elsewhere in the same file

tdonohue commented 1 year ago

Merging, all feedback has been addressed & the implementation has been tested and approved. Thanks again @kshepherd !