411 Removes dependency on immature "alternate_ids" product property #414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the value in question is a string, and is not guaranteed to be a LIDVID string (could be a LID string)
…specified parent collection's LIDVID, rather than attempting to resolve alternate_ids alternate_id is an immature property/feature and may not yet be relied upon for its assumed purpose of enumerating all identities across name-changes
…PdsLidVids instead of Strings
…s directly when attempting to resolve member non-aggregate products, rather than attempting to resolve collection alternate_ids during the process alternate_id is an immature property/feature and may not yet be relied upon for its assumed purpose of enumerating all identities across name-changes
alexdunnjpl
requested review from
tloubrieu-jpl and
al-niessner
as code owners
tloubrieu-jpl
approved these changes
Feb 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🗒️ Summary
alternate_idsproperty was used when resolving collection lidvids for both collection member and bundle member/member queries.This property is not currently maintained in a way that fulfills its assumed purpose. This PR removes use of the property, and (because only LIDVIDs are supported in this context) replaces use of string identifiers with
PdsLidVidobjects instead. If any exceptions are experienced as a result of this change, it indicates an issue elsewhere in the code and exposing that issue is desirable.⚙️ Test Data and/or Report
Tested hits count for both a bundle/member/member query and collection/member query and confirmed same number of hits before/after change.
Tested with both LID and LIDVID identifier in path parameter to confirm that LIDs are correctly resolved to latest-LIDVID as expected.
Tested failing example from #411 to ensure it is now succeeding (with zero hits, as expected, as referenced products are not harvested per @jordanpadams )
♻️ Related Issues
Fixes #411