Bugfix/cmg 1109 1004 fixes - #1150
Conversation
…sets extractAssets only read formal wp:post_type=attachment items, so images embedded in post content (no attachment item backing them) never got a row on the Map Entry Assets screen, even though the actual migration downloads and creates them as real Contentstack assets via wordpress.service.ts's content:encoded scan. Extends extraction to find those images too, matching the id scheme the real migration uses so uids still resolve, and dedupes against formal attachment items so an overlapping URL doesn't get two rows.
Editing the file path after selecting a CMS with a single fixed allowed format (e.g. Sitecore, always Zip) re-derived the displayed "File Format" from the new path's extension, showing e.g. "XML" instead of staying "Zip". Only CMS types with more than one allowed format (currently just stack-to-stack Contentstack) should have the label follow the actual uploaded extension; everyone else keeps their fixed format, with the existing validation effect flagging a real mismatch instead.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
umesh-more-cstk
left a comment
umesh-more-cstk
left a comment
There was a problem hiding this comment.
Reviewed at head 651d00f (both files, full diff). Traced the upload-api extractor against api/src/services/wordpress.service.ts (extractImageUrlsFromContent, saveAssetFromUrl, getAllAssets) and the mapper read/write path in api/src/services/contentMapper.service.ts, and the UI change against LoadSelectCms, LoadUploadFile, LegacyCms/index.tsx and ui/src/cmsData/legacyCms.json.
2 blockers, 2 questions, 2 nits.
Blockers (inline):
upload-api/migration-wordpress/libs/extractAssets.ts:88— the content scan only covers<img>src/data-src/srcset, while the migration run also collects<a href>image links,<audio>/<source>, and inline +<style>background images. Those assets migrate but still get no Map Entry row, so the CMG-1108/1109 symptom remains reproducible for that markup.ui/src/components/LegacyCms/Actions/LoadFileFormat.tsx:64— "no allowed formats known yet" shares the lock-and-return branch with "exactly one format", soDEFAULT_CMS_TYPE(allowed_file_formats: [], the first-visit state for multi-version Sitecore before a version card is clicked) now showsFile extension not foundon a valid.zipinstead of the extension-derivedZip.
Questions:
- Inline on
extractAssets.ts:206—file_size: ''plus the un-normalized filename make every content-embedded asset compare as changed on iteration ≥2, pre-selectingisUpdatefor untouched assets. Same shape as the existing attachment rows, so possibly a known gap — worth confirming. - question: how does this diff address the second half of CMG-1004 ("even after validation fails, the user can still proceed to the next step")?
isError/errorMessageinLoadFileFormat.tsxare local state rendered into onedivand read nowhere else, and the step-completion gate atui/src/components/LegacyCms/index.tsx:214-217keys offselectedFileFormat.title/fileformat_id,selectedCms.title,affixanduploadedFile.isValidated— never the format-vs-CMS mismatch. The blocking path today isLoadUploadFile.tsx:405-420settingisValidated: false, which this diff doesn't touch. If the intent is that the forcedselectedFileFormatkeeps that path reachable, please spell it out in the description; if progression is still possible, that half of the ticket looks unfixed.
Nits:
- Inline on
extractAssets.ts:142—?? ''vs the service's||/undefinedforbaseSiteUrlproduces rows the real run can't create whenwp:base_site_urlis absent. - nit: no tests added.
extractAssetsis pure andupload-apialready runs vitest (tests/unit/migration-wordpress/); one fixture with an embedded<img>+srcset, plus an image that is also a formal attachment, would lock in theotherCmsAssetUidcontract that the doc comments say must stay byte-identical tosaveAssetFromUrl'scustomId— currently only guarded by comments.
Scope: diff matches the stated Affected Areas (ui, upload-api); no lockfile or generated churn. Verified as correct: the content-asset uid derivation (nameWithoutExt.replace(/-/g,'_').toLowerCase(), query string stripped) matches saveAssetFromUrl's customId and the uid-mapping.json key exactly; the cheerio import matches the sibling extractItems.ts in the same package; and the srcset-variant rows are legitimate, since the run downloads each variant URL separately.
Generated by Claude Code
- extractAssets.ts: cover a[href] image links, audio/source, and CSS background-image (inline + <style>), not just img src/data-src/srcset, so the same set of embedded assets the real migration finds also get a row. Also: skip relative URLs when wp:base_site_url is missing instead of emitting a row for an asset the run can't actually resolve. - LoadFileFormat.tsx: lock the displayed format only when the CMS has exactly one allowed format. An empty allowed_file_formats (CMS not yet resolved, e.g. Sitecore before a version is picked) now correctly falls through to the extension-derived display instead of blanking the field.
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
3 participants
🔗 Jira Ticket
📋 PR Type
📝 Description
What changed?
extractAssets.tsnow also picks up images embedded in post content, not just formalwp:post_type=attachmentitems.LoadFileFormat.tsxno longer overwrites the displayed file format when the selected CMS only has one valid format (e.g. Sitecore = Zip).Why?
🧩 Affected Areas
api— Node.js backendui— React frontendupload-api— Upload API serverdocker/docker-compose🧪 How to Test
.xml/.sql— File Format stays "Zip"; validation error shows instead.cd upload-api && npx vitest run— passing.Expected result: content-embedded assets are visible for review pre-migration; Sitecore's file format label never changes regardless of what path is typed.
📸 Screenshots / Recordings
🔗 Related PRs / Dependencies
✅ Author Checklist
bugfix/cmg-1109-1004-fixes.env/example.envupdated — N/Anpx vitest runinupload-api/)LoadFileFormat.tsx, andextractAssets.tsverified manually against real extracted fixturesREADME.md/ docs updated — N/A👀 Reviewer Notes
extractAssets.ts's content-scan logic duplicates (and must stay in sync with)wordpress.service.ts'sextractImageUrlsFromContent/saveAssetFromUrl— they live in separate services (upload-apivsapi) with no shared package.LoadFileFormat.tsxfix only changes behavior for CMS types with a single allowed format; stack-to-stack Contentstack (2 allowed formats) still follows the actual uploaded extension, unchanged.