Conversation
added 3 commits
July 31, 2026 11:43
…alue, including UMM-C and ISO processing levels. I also fixed ISO deletion to remove only the matching processing-level wrapper instead of all sibling values.
📝 WalkthroughWalkthroughThe editors now match duplicate XML values by normalized old content, serialize hierarchical ISO 19115 keywords consistently, remove enclosing acquisition wrappers, and support nested instrument paths. Tests add end-to-end replacement and deletion coverage for five metadata formats. ChangesMetadata correction mappings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CorrectionMapping
participant FormatDelegate
participant MetadataEditor
participant MetadataDocument
CorrectionMapping->>FormatDelegate: submit replacement or deletion mapping
FormatDelegate->>MetadataEditor: resolve target using old value
MetadataEditor->>MetadataDocument: select matching metadata node
MetadataEditor->>MetadataDocument: update or remove node and wrapper
FormatDelegate-->>CorrectionMapping: return correction counts and serialized metadata
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
==========================================
+ Coverage 99.71% 99.73% +0.01%
==========================================
Files 234 234
Lines 6346 6391 +45
Branches 1866 1907 +41
==========================================
+ Hits 6328 6374 +46
Misses 16 16
+ Partials 2 1 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json (1)
237-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo 435-line UMM-C before fixtures differ only in
ProcessingLevel. The shared root cause is that a full fixture was copied to vary four lines, becauseProcessingLevel.Idis a scalar and cannot carry a clean value and a_SHOULD_DELETEvalue in one document. Every other one of the 431 lines is identical, so the two files will drift when one is edited.
scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json#L237-L240: keep this file as the deletion-case input, and record in the end-to-end suite or a short README that it exists only to markProcessingLevelDescriptionandId. Also confirm which of the two files the suite loads for the UMM-C deletion case, and confirm thatdeletions/ummc.after.jsonencodes the intended result for the requiredProcessingLevel.Idscalar.scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/ummc.before.json#L237-L240: alternatively, delete the duplicate and have the deletion test load this file and override onlyProcessingLevelin code, so one document stays the single source for all other markers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json` around lines 237 - 240, Eliminate drift between the duplicate UMM-C fixtures by deleting scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json (lines 237-240 and the rest), updating the deletion test to load scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/ummc.before.json (lines 237-240) and override only ProcessingLevel in code, and verify deletions/ummc.after.json preserves the intended scalar ProcessingLevel.Id result.scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json (1)
45-78: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueNo change needed. The Echo10 update path mutates a single matched node and returns on the first change, so the two duplicate
instrumentscorrections are not needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json` around lines 45 - 78, Remove the duplicate instruments correction entry in the Echo10 corrections fixture, retaining only one replacement mapping for the AMSR-E keyword and long name. Keep the remaining correction unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/iso19115.corrections.json`:
- Around line 85-91: Update the ISO19115 dataformat deletion entry in the
corrections fixture to identify the keyword with the Value key instead of
ShortName, matching the editor’s gmx:Anchor/gco:CharacterString object format
and the existing ECHO10 mapping.
In `@serverless/src/shared/XmlMetadataPathEditor.js`:
- Around line 762-767: Update the empty-value branch in the replacement flow to
pass the same expectedText selector to removeNestedElement that setNestedText
receives, using correction.oldKeywordObject and matchOldValueKey. Preserve the
existing unconditional removal behavior when no matchOldValueKey is provided.
---
Nitpick comments:
In
`@scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json`:
- Around line 237-240: Eliminate drift between the duplicate UMM-C fixtures by
deleting
scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json
(lines 237-240 and the rest), updating the deletion test to load
scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/ummc.before.json
(lines 237-240) and override only ProcessingLevel in code, and verify
deletions/ummc.after.json preserves the intended scalar ProcessingLevel.Id
result.
In
`@scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json`:
- Around line 45-78: Remove the duplicate instruments correction entry in the
Echo10 corrections fixture, retaining only one replacement mapping for the
AMSR-E keyword and long name. Keep the remaining correction unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1aaf4cb0-aeb3-4bef-bf19-1ad20990d85e
📒 Files selected for processing (35)
scripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/dif10.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/dif10.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/echo10.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/echo10.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/iso19115.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/iso19115.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/isosmap.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/isosmap.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.after.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/deletions/ummc.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/dif10.before.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/echo10.before.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/iso19115.before.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/isosmap.before.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/ummc.before.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/dif10.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/echo10.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/iso19115.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/iso19115.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xmlscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/isosmap.corrections.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/ummc.after.jsonscripts/local/fixtures/native_metadata_correction_mappings/end_to_end/updates/ummc.corrections.jsonserverless/src/shared/Iso19115DomEditor.jsserverless/src/shared/Iso19115MetadataPathEditor.jsserverless/src/shared/XmlMetadataPathEditor.jsserverless/src/shared/__tests__/Iso19115MetadataPathEditor.test.jsserverless/src/shared/__tests__/XmlMetadataPathEditor.test.jsserverless/src/shared/__tests__/applyEcho10MetadataCorrections.test.jsserverless/src/shared/__tests__/applyIso19115MetadataCorrections.test.jsserverless/src/shared/__tests__/applyNativeMetadataCorrectionMappings.test.jsserverless/src/shared/echo10DomEditor.js
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.json (1)
155-164: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the
rucontenttypeURLContentType with the type value.
VIEW RELATED INFORMATIONbelongs toPublicationURLin the GCMD related-URL hierarchy. The ECHO10 fixture in this same PR usesPublicationURLfor the identical type. DIF10 stores onlyTypeandSubtype, so this value does not change the assertion result. Align it anyway so the fixture stays a valid example.♻️ Proposed change
"oldKeywordObject": { - "URLContentType": "DistributionURL", + "URLContentType": "PublicationURL", "Type": "VIEW RELATED INFORMATION", "Subtype": "OpenSearch" }, "newKeywordObject": { - "URLContentType": "DistributionURL", + "URLContentType": "PublicationURL", "Type": "VIEW RELATED INFORMATION", "Subtype": "OpenSearch_CHANGED" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.json` around lines 155 - 164, Update the oldKeywordObject URLContentType in the dif10 correction fixture from DistributionURL to PublicationURL for the VIEW RELATED INFORMATION type, while leaving the Type and Subtype values unchanged.serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.corrections.json (1)
144-153: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe
dataformatscheme uses two different key names across fixtures.Here and in
updates/ummc.corrections.jsonthedataformatkeyword object usesShortName. Inupdates/echo10.corrections.jsonline 144 the same scheme usesValue.updateLeafNodereads the first key ofoldKeywordObject, so both work today. The inconsistency still leaves the payload contract fordataformatundefined. Pick one key name for the scheme across all fixtures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.corrections.json` around lines 144 - 153, Standardize the `dataformat` keyword object key across the correction fixtures, including `iso19115.corrections.json`, `ummc.corrections.json`, and `echo10.corrections.json`. Choose either `ShortName` or `Value` and update both `oldKeywordObject` and `newKeywordObject` consistently for every `dataformat` replacement.serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json (1)
45-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIdentical duplicated corrections make the expected counts depend on apply order. Three fixtures repeat the same correction so that each apply consumes a different duplicate node. The test asserts
result.correctionCount === request.corrections.length, so these fixtures pass only while a replace consumes exactly one node per call. A change to idempotent or all-occurrence matching breaks them for a reason unrelated to the code under test, and the ISO-SMAP expected output already contains a redundant keyword as a result.
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json#L45-L78: replace the two identicalinstrumentscorrections with one, or give the second entry a distinct old value.serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.corrections.json#L111-L132: replace the two byte-identicalproviderscorrections with one, or give the second entry a distinct old value.serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xml#L197-L218: after the correction list changes, regenerate this block soSMAP_PROVIDER_CHANGEDdoes not appear in two separate dataCentre keyword groups.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json` around lines 45 - 78, The end-to-end fixtures contain duplicate corrections that make expected counts and output depend on replacement order. In serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json:45-78, remove one duplicate instruments correction or give it a distinct old value; make the same change to the duplicate providers corrections in serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.corrections.json:111-132. Then regenerate serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xml:197-218 so SMAP_PROVIDER_CHANGED appears in only one dataCentre keyword group.serverless/src/shared/__tests__/Iso19115MetadataPathEditor.test.js (1)
279-304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the synthetic acquisition structure.
This fixture uses two non-standard shapes on purpose.
gmi:MI_Platformhas nogmi:platformwrapper, andgmi:MI_Operationsits inside a syntheticgmi:operationContainerelement. Both shapes forceupdateBlockNodeto remove the type node instead of the property wrapper. Add a short comment so a later reader does not treat these shapes as valid ISO 19139 structures.♻️ Proposed comment
+ <!-- Synthetic shapes: MI_Platform has no gmi:platform wrapper, and + MI_Operation is nested below gmi:operation, so updateBlockNode must + fall back to removing the type node itself. --> <gmi:acquisitionInformation>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@serverless/src/shared/__tests__/Iso19115MetadataPathEditor.test.js` around lines 279 - 304, Add a brief comment in the fixture near the synthetic acquisition structure explaining that MI_Platform lacks the standard platform wrapper and MI_Operation is nested under operationContainer, intentionally exercising updateBlockNode’s type-node removal behavior. Do not alter the XML structure or test behavior.serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json (1)
1-435: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce the 435-line duplication with
end_to_end/ummc.before.json.This fixture is byte-identical to
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/ummc.before.jsonexcept for Lines 238-239. Two near-identical fixtures drift silently: a future edit to the shared record will apply to only one deletion or update scenario.Consider keeping one base record and applying a small scenario override in the test loader, or add a header comment in both files that states they must stay in sync and lists the intended delta.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json` around lines 1 - 435, Reduce duplication between this deletion fixture and the shared end_to_end/ummc.before.json by reusing one base record in the test loader and applying only the intended Lines 238-239 scenario override. Ensure both deletion and update scenarios remain behaviorally equivalent while preventing future edits from drifting between duplicate fixtures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.after.xml`:
- Around line 405-425: Align duplicate-node replacement behavior across the ISO
19115, DIF10, and ECHO10 correction flows by deciding whether a correction
updates every matching platform/instrument node or only the first, then make the
implementation and expectations consistent. Update the ISO 19115 fixture at
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.after.xml:405-425,
DIF10 at
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.after.xml:51-62,
and ECHO10 at
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.after.xml:295-299;
use the behavior implemented by the ISO DOM iteration and DIF10/ECHO10
resolveNodeByFind/updateBlockNode paths as the contract.
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xml`:
- Around line 197-218: Remove the duplicate providers correction entry in
updates/isosmap.corrections.json so SMAP_PROVIDER_CHANGED is emitted only once,
then update the corresponding isosmap.after.xml fixture to contain a single
dataCentre block with that keyword.
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.corrections.json`:
- Around line 138-151: Align the UMM-C correction and expected output so the
provider role matches the corrected data center: in
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.corrections.json:138-151,
either set BucketLevel0 to PROCESSOR or target the GMU/CEOSR archiver entry;
then regenerate the DataCenters entries in
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.after.json:176-196
so the corrected DRSC record and its role match the correction.
---
Nitpick comments:
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.json`:
- Around line 1-435: Reduce duplication between this deletion fixture and the
shared end_to_end/ummc.before.json by reusing one base record in the test loader
and applying only the intended Lines 238-239 scenario override. Ensure both
deletion and update scenarios remain behaviorally equivalent while preventing
future edits from drifting between duplicate fixtures.
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.json`:
- Around line 155-164: Update the oldKeywordObject URLContentType in the dif10
correction fixture from DistributionURL to PublicationURL for the VIEW RELATED
INFORMATION type, while leaving the Type and Subtype values unchanged.
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json`:
- Around line 45-78: The end-to-end fixtures contain duplicate corrections that
make expected counts and output depend on replacement order. In
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.json:45-78,
remove one duplicate instruments correction or give it a distinct old value;
make the same change to the duplicate providers corrections in
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.corrections.json:111-132.
Then regenerate
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xml:197-218
so SMAP_PROVIDER_CHANGED appears in only one dataCentre keyword group.
In
`@serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.corrections.json`:
- Around line 144-153: Standardize the `dataformat` keyword object key across
the correction fixtures, including `iso19115.corrections.json`,
`ummc.corrections.json`, and `echo10.corrections.json`. Choose either
`ShortName` or `Value` and update both `oldKeywordObject` and `newKeywordObject`
consistently for every `dataformat` replacement.
In `@serverless/src/shared/__tests__/Iso19115MetadataPathEditor.test.js`:
- Around line 279-304: Add a brief comment in the fixture near the synthetic
acquisition structure explaining that MI_Platform lacks the standard platform
wrapper and MI_Operation is nested under operationContainer, intentionally
exercising updateBlockNode’s type-node removal behavior. Do not alter the XML
structure or test behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c894f50-f87d-4c08-90c8-a76cb4500e77
📒 Files selected for processing (29)
serverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/dif10.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/dif10.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/echo10.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/echo10.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/iso19115.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/iso19115.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/isosmap.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/isosmap.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.after.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.before.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/deletions/ummc.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/dif10.before.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/echo10.before.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/iso19115.before.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/isosmap.before.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/ummc.before.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/echo10.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/iso19115.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/isosmap.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.after.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.corrections.jsonserverless/src/shared/__tests__/Iso19115MetadataPathEditor.test.jsserverless/src/shared/__tests__/XmlMetadataPathEditor.test.jsserverless/src/shared/__tests__/applyNativeMetadataCorrectionMappings.test.js
🚧 Files skipped from review as they are similar to previous changes (2)
- serverless/src/shared/tests/XmlMetadataPathEditor.test.js
- serverless/src/shared/tests/applyNativeMetadataCorrectionMappings.test.js
added 2 commits
August 6, 2026 14:41
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@serverless/src/shared/__tests__/XmlMetadataPathEditor.test.js`:
- Around line 727-728: Update the test around editor.serialize() to query the
resulting ProcessingCenter nodes and assert exactly one node remains with the
value SOMEONE-ELSE. Replace the broad string containment checks so empty
elements or alternate replacement values cannot satisfy the assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c10f901-ec7b-4f69-bdf9-c6d05293a8c0
📒 Files selected for processing (6)
serverless/src/shared/XmlMetadataPathEditor.jsserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.after.xmlserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/dif10.corrections.jsonserverless/src/shared/__mocks__/native_metadata_correction_mappings/end_to_end/updates/ummc.corrections.jsonserverless/src/shared/__tests__/XmlMetadataPathEditor.test.jsserverless/src/shared/__tests__/applyNativeMetadataCorrectionMappings.test.js
🚧 Files skipped from review as they are similar to previous changes (3)
- serverless/src/shared/XmlMetadataPathEditor.js
- serverless/src/shared/mocks/native_metadata_correction_mappings/end_to_end/updates/dif10.after.xml
- serverless/src/shared/tests/applyNativeMetadataCorrectionMappings.test.js
added 3 commits
August 10, 2026 15:09
…R-11421 to prevent a feedback loop.
htranho
approved these changes
Aug 10, 2026
| expect(result.correctionCount).toBe(4) | ||
| // This triggers: if (parent.Organization.length === 0) { delete parent.Organization } | ||
| expect(result.correctedMetadata).not.toContain('<Contacts>') | ||
| expect(result.correctedMetadata).not.toContain('<ProcessingCenter>') |
Contributor
There was a problem hiding this comment.
Why are ProcessingCenter and ArchiveCenter being checked with this test?
Contributor
Author
There was a problem hiding this comment.
Correct me if I’m wrong, since you’re more familiar with the format, but the mapping shows that a PROCESSOR provider maps to ProcessingCenter, while an ARCHIVER provider maps to ArchiveCenter. Replacement synchronization already existed; this change adds equivalent deletion behavior and safer old-value matching. These assertions verify that deleting those provider roles also removes their corresponding ECHO10 center fields.
william-valencia
approved these changes
Aug 11, 2026
william-valencia
left a comment
william-valencia
left a comment
Contributor
There was a problem hiding this comment.
Looks good. just had one minor comment. not a show stopper.
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
4 participants
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.
Overview
What is the feature?
Add end-to-end regression coverage for native metadata correction mappings so unexpected changes to UMM-C, DIF10, ECHO10, ISO19115, or ISO-SMAP transformations fail automated tests. It also updates CMR writeback requests to include a KMS client identifier and configurable CMR validation headers, allowing CMR to recognize KMS-originated writes and helping prevent correction feedback loops while validation behavior can be adjusted during integration (This last part is new due to a bug that Ed and I found on Thursday).
What is the Solution?
_CHANGEDand_SHOULD_DELETEso intended transformations are easy to review.What areas of the application does this impact?
Testing
beforefixture under__tests__/fixtures/native_metadata_correction_mappings/end_to_end, right-click its tab, and select Select for Compare.afterfixture, right-click its tab, and select Compare with Selected._CHANGEDvalues appear only at the intended mapped fields,_SHOULD_DELETEvalues are removed, and unrelated metadata remains unchanged.Checklist
Summary by CodeRabbit
Bug Fixes
Tests