chore: Improve handling of custom ORD content#443
Open
mlakov wants to merge 16 commits into
Open
Conversation
Contributor
Author
|
@zongqichen, @swennemers - can you, please, review ? |
zongqichen
reviewed
May 22, 2026
zongqichen
reviewed
May 22, 2026
zongqichen
reviewed
May 22, 2026
This file contains hidden or 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
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.
Refactor Custom ORD Content Merging with Strategy-Based Approach
Refactor
♻️ Replaced the generic object traversal merge logic for custom ORD content with an explicit, strategy-based approach using a
MERGE_STRATEGIESmap. This improves correctness, predictability, and eliminates the need for workarounds likecleanNullProperties.Changes
lib/extend-ord-with-custom.js: Major refactor of the merging logic:cleanNullPropertiesand the oldpatchGeneratedOrdResourcesfunctions.MERGE_STRATEGIESmap defining explicit per-property merge behavior (scalar replace, object assign, or smart array merge by key).prunehelper to removenull/undefinedvalues from merged entities.mergehelper for smart array-of-objects merging using configurable identity keys (e.g.,ordId,groupId).getCustomORDContentnow consistently returns{}instead ofundefinedwhen no custom file is configured or the file is missing.compareAndHandleCustomORDContentWithExistingContentnow uses the strategy map to handle properties exclusive to either side or present in both.CONTENT_MERGE_KEYdependency from constants.__tests__/unit/extend-ord-with-custom.test.js: Updated test descriptions and expectations to reflect{}return values instead ofundefined; removed the deprecated "warn on top-level primitive" test case.__tests__/bookshop/ord/custom.ord.json: Removed conflicting top-level properties (namespace,openResourceDiscovery) that were only needed for the now-removed warning test.__tests__/unit/__snapshots__/extend-ord-with-custom.test.js.snap: Updated snapshots to reflect the removal of the deprecated test case and correctedlocalIdvalues.__tests__/unit/__snapshots__/ord.e2e.test.js.snap: Updated snapshots to reflect corrected ordering and content of merged packages, including fixedshortDescriptionandversionvalues from the improved merge strategy.PR Bot Information
Version:
1.20.51issue_comment.edited6c3ac90f-9ddf-4fa7-ae63-ac5685b83b3a