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

DDFHER-83 - implement new holdings endpoint /external/agencyid/catalog/holdingsLogistics/v1 #1542

Merged

Conversation

kasperbirch1
Copy link
Contributor

@kasperbirch1 kasperbirch1 commented Nov 15, 2024

Link to issue

https://reload.atlassian.net/browse/DDFHER-83
https://ciceroconnect.zendesk.com/hc/da/article_attachments/16168497399324
danskernesdigitalebibliotek/dpl-fbs-adapter-tool#1

Description

This pull request replaces /external/agencyid/catalog/holdings/v3 with /external/agencyid/catalog/holdingsLogistics/v1.

The new endpoint, holdingsLogistics/v1, provides more precise information about the location of materials, which is particularly important for the FindOnShelf modal.

There are very few code changes. The key API change is that we now use location, sublocation, department, and section under the new lmsPlacement key.

The most significant work was in updating fixtures. I’ve made an effort to refresh the data used in the fixtures and correct any issues in the tests. However, this was not possible for all tests, as some require specific circumstances and were therefore edited or fully manual.

Test

https://varnish.pr-1761.dpl-cms.dplplat01.dpl.reload.dk/

`/holdingsLogistics/v1` providing better precision in tracking material locations.
 
- Utilized a generated `fbs-adapter.yaml` via the [dpl-fbs-adapter-tool](https://github.com/danskernesdigitalebibliotek/dpl-fbs-adapter-tool) 
- Executed `yarn codegen:client:fbs` to regenerate the types and hooks.
…sPlacement`

The fields `department`, `location`, and `sublocation` are now located within `lmsPlacement` after replacing the "holdings" API.
…ldingsLogistics/v1`

I have updated the fixtures with fresh data to ensure that the data is as close to the reality as possible. Therefore, I have updated the test to align with the new fixture.

Since there was no instant loan for the material at the moment, I manually changed one of the branches. Because comments are not allowed in JSON, I added a key `dev_comment` with an explanation.

I also removed a non-helpful comment in the test.
Update due to API change from `holdings/v3` to `holdingsLogistics/v1`, causing the fixture to be out of date.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
…to `/holdingsLogistics/v1`

I have updated the fixtures with fresh data to ensure that the data is as close to the reality as possible. Therefore, I have updated the test to align with the new fixture.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
No need to update fixtures as no holdings in `material/holdings.json` are necessary for this test.
…oldingsLogistics/v1`

The current test fixtures appear to be manually created. I’ve updated them to reflect the new structure, but I suggest refactoring this test in the future. Ideally, it should be tested directly from the material app instead of relying on this "artificial" environment.
…Text`

Due to placement data now being available in either `logisticsPlacement` or `lmsPlacement`, I created `getLocationArray`. This function first checks `lmsPlacement` as it provides the most precise location. If `logisticsPlacement` is used, the first string (always the library name) is removed. Otherwise, for `lmsPlacement`, it uses the hierarchy: department -> location -> sublocation.
@kasperbirch1 kasperbirch1 merged commit 2f0a5f3 into develop Nov 28, 2024
21 checks passed
@kasperbirch1 kasperbirch1 deleted the DDFHER-83-implementer-nyt-beholdnings-endpoint-v2 branch November 28, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants