Allow Indexes and IDs in getDatasetItem API endpoint

DigitalCommons / mykomap-monolith

A web application for mapping initiatives in the Solidarity Economy

0 stars 0 forks source link

Allow Indexes and IDs in getDatasetItem API endpoint #35

Closed wu-lee closed 3 weeks ago

wu-lee commented 4 weeks ago

What? Why?

Related to issue #25

In essence, instead of having two endpoints, one for getting items by their index, and one for getting them by ID, just adapt the existing one.

Allow the getDatasetItem endpoint, which has the route /dataset/:id/item, to accept indexes and ids
But in a way which it can be determined which is which
Return the appropriate item, obtained via the index or the ID, in the usual format as before.

What should we test?

Check the reasoning makes sense, and the tests test what they are meant to.

Checklist

[x] ~~Updated or added any necessary docs in docs/~~
[x] Added UTs
[ ] Checked that all automated tests pass

Deployment notes

None

wu-lee commented 4 weeks ago

Note, this branch is also appended to some prior branches so the PR is not nearly as long as it seems! These branches will all be merged separately.

rogup commented 3 weeks ago

Hi @wu-lee, I found it difficult to review this due to the PR's changes being mixed in with lots of other commits (I usually do reviews directly on GitHub, unless I need to do some checks in my IDE). But it looks generally fine, and I'm happy for it to be merged so that stuff can be progressed, and we can fix anything that comes up later. I just have 1 comment about decoding the IDs/indexes

wu-lee commented 3 weeks ago

Having answered @rogup question directly, the reviewer seems to be happy for this to be merged, which is now done. (See above for the merge commit, which references this PR. There was some rebasing to do since it was chained onto another branch for operational reasons, hence the merge done in git outside of GitHub, which makes it think this was closed rather than merged.)