Skip to content

chore: Improve handling of custom ORD content#443

Open
mlakov wants to merge 16 commits into
mainfrom
chore/improve-handling-of-custom-ord-content
Open

chore: Improve handling of custom ORD content#443
mlakov wants to merge 16 commits into
mainfrom
chore/improve-handling-of-custom-ord-content

Conversation

@mlakov
Copy link
Copy Markdown
Contributor

@mlakov mlakov commented May 14, 2026

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_STRATEGIES map. This improves correctness, predictability, and eliminates the need for workarounds like cleanNullProperties.

Changes

  • lib/extend-ord-with-custom.js: Major refactor of the merging logic:

    • Removed cleanNullProperties and the old patchGeneratedOrdResources functions.
    • Introduced a MERGE_STRATEGIES map defining explicit per-property merge behavior (scalar replace, object assign, or smart array merge by key).
    • Added prune helper to remove null/undefined values from merged entities.
    • Added merge helper for smart array-of-objects merging using configurable identity keys (e.g., ordId, groupId).
    • getCustomORDContent now consistently returns {} instead of undefined when no custom file is configured or the file is missing.
    • compareAndHandleCustomORDContentWithExistingContent now uses the strategy map to handle properties exclusive to either side or present in both.
    • Removed the warning for top-level primitive properties — they are now handled via the strategy map.
    • Removed the CONTENT_MERGE_KEY dependency from constants.
  • __tests__/unit/extend-ord-with-custom.test.js: Updated test descriptions and expectations to reflect {} return values instead of undefined; 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 corrected localId values.

  • __tests__/unit/__snapshots__/ord.e2e.test.js.snap: Updated snapshots to reflect corrected ordering and content of merged packages, including fixed shortDescription and version values from the improved merge strategy.

  • 🔄 Regenerate and Update Summary
PR Bot Information

Version: 1.20.51

  • Event Trigger: issue_comment.edited
  • Correlation ID: 6c3ac90f-9ddf-4fa7-ae63-ac5685b83b3a

@mlakov mlakov requested review from swennemers and zongqichen May 14, 2026 12:10
hyperspace-insights[bot]

This comment was marked as outdated.

@mlakov mlakov added ready for review run-e2e Trigger e2e test pipeline labels May 20, 2026
@mlakov mlakov marked this pull request as ready for review May 20, 2026 11:23
@mlakov
Copy link
Copy Markdown
Contributor Author

mlakov commented May 20, 2026

@zongqichen, @swennemers - can you, please, review ?

hyperspace-insights[bot]

This comment was marked as outdated.

Comment thread lib/extend-ord-with-custom.js
Comment thread lib/extend-ord-with-custom.js
Comment thread lib/extend-ord-with-custom.js Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review run-e2e Trigger e2e test pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants