MWPW-197349: Bulk version/revert follow-up — snapshot recording at project save time - #1039
MWPW-197349: Bulk version/revert follow-up — snapshot recording at project save time#1039seanchoi0 wants to merge 24 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-time snapshots Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rget in worker Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ct-save trigger Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1039 +/- ##
==========================================
- Coverage 90.16% 90.13% -0.03%
==========================================
Files 303 304 +1
Lines 96759 96932 +173
==========================================
+ Hits 87246 87373 +127
- Misses 9513 9559 +46
... and 12 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Reviewed the snapshot-at-save flow end to end — all 50 IO tests pass and the serialize/deserialize contract with revertSnapshot is consistent. Requesting two tests for scenarios where a future change could silently regress the revert target. Non-blocking.
…nd await snapshot
- recordSnapshot now collects per-fragment failures instead of throwing,
returning { entries, failures } to match the revertSnapshot pattern
- save-snapshot-action writes lastError to project when fragments fail
- bulk-publish-worker destructures { entries, failures } from recordSnapshot
- mas-bulk-publish-editor awaits saveSnapshot with try/catch instead of
fire-and-forget to prevent race with Publish button
- Add tests: Pre-bulk-publish as non-translation, publishComplete: true fallthrough
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…shotError in final update - Wrap recordSnapshot batch callback in try/catch so 500/network errors from getFragmentByPath or findNonTranslationVersion are captured as per-fragment failures instead of aborting the whole batch - Track snapshotError across worker branches and use it in the final updateProject call instead of lastError: '' so SAVE_SNAPSHOT warnings are not silently discarded after publish completes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Analogous to the fix applied to recordSnapshot where a single failure would reject the whole batch of fragments, please wrap this in a try/catch as well.
There was a problem hiding this comment.
Fixed — applied the same { entries, failures } pattern as recordSnapshot. Each fragment callback is now wrapped in try/catch so a single failure returns { path, error } instead of aborting the whole batch. createSnapshot now returns { entries, failures } and the worker accumulates any failures as a CREATE_SNAPSHOT: prefix in snapshotError, surfaced in lastError the same way SAVE_SNAPSHOT: failures are.
…{ entries, failures }
Single-fragment failure in createSnapshot's processBatchWithConcurrency callback
would previously abort the entire batch. Apply the same { entries, failures } pattern
already used by recordSnapshot so partial failures are collected per-fragment without
stopping other paths. Worker accumulates CREATE_SNAPSHOT errors into snapshotError
alongside SAVE_SNAPSHOT errors; snapshot-action updated to match new return shape.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Please resolve conflicts. |
- Keep snapshot helper functions (hasPendingSnapshot, addPendingMarker, removePendingMarker) from project.js import; remove local duplicates - Merge updateProject try/catch from main with snapshotError from branch - Import both saveSnapshot and resetToDraft in bulk-publish-store.test.js
| Store.bulkPublishProjects.inEdit.set(new FragmentStore(new Fragment(raw))); | ||
| this.hasChanges = false; | ||
| showToast('Project created successfully.', 'positive'); | ||
| const { saveSnapshot } = await import('./bulk-publish-store.js'); |
There was a problem hiding this comment.
Duplicated with the identical block at line 590 (same dynamic import + try/catch around saveSnapshot({...}), only project and the error message differ).
Maybe we can extract into a private method (#recordSnapshotAfterSave(project, context)) and call it from both places.
| } | ||
| }); | ||
|
|
||
| const failures = results.filter((r) => r?.error); |
There was a problem hiding this comment.
revertSnapshot only walks Object.entries(snapshot.fragments)
A fragment that failed during recordSnapshot (line 183) is never added to snap.fragments, so it's absent from entries here too, it ends up in neither failures nor skipped, with no trace at revert time. Consider having recordSnapshot persist failed paths alongside the snapshot (or have the caller track them) so revertSnapshot/the UI can surface "N fragments have no revert target" instead of silently skipping them.
|
|
||
| Take the highest `mas-js-v*` tag as the current version. Then read `web-components/package.json` and compare: | ||
|
|
||
| - If `package.json` is **behind** the latest release tag, it drifted (a previous bump commit was never committed/pushed). Warn the user, and treat the release tag — not `package.json` — as the baseline for choosing the next version. |
There was a problem hiding this comment.
please do not revert this section. I guess this file is not supposed to be changed in this PR.
|
|
||
| async function findNonTranslationVersion(odinEndpoint, fragmentId, authToken) { | ||
| const versions = await fetchVersionHistory(odinEndpoint, fragmentId, authToken); | ||
| const found = versions.find((v) => !isTranslationVersion(v)); |
There was a problem hiding this comment.
.find takes the first non-translation entry, so the revert target assumes Odin returns /versions newest-first. If that order isn't guaranteed we could revert to an older version — worth pinning with an explicit sort or a comment noting the assumption.
7 participants
Record a snapshot (non-translation CF version ID) every time a bulk publish project is saved, so that bulk publish can use it as the revert target instead of the pre-publish state. Translation versions (identified by
createdBy === 'odin-cf-versioning-user'orcomment.startsWith('Pre-rollout snapshot')) are skipped when finding the revert target.Resolves https://jira.corp.adobe.com/browse/MWPW-197349
QA Checklist: https://wiki.corp.adobe.com/display/adobedotcom/M@S+Engineering+QA+Use+Cases
Test URLs: