feat(wordpress): migrate the post featured image and its alt text - #1154
umesh-more-cstk wants to merge 1 commit into
Conversation
WordPress stores a post's featured image as a `_thumbnail_id` postmeta pointing at an attachment item, but the connector never read it. Attachments were downloaded (getAllAssets pulls every attachment in the export) yet nothing referenced them, and the generated content type had no field for one — so the featured image was absent from the Field Mapping step and orphaned in the stack. Three additions: - extractItems now emits a `featured_image` file field when an item in the post type declares a `_thumbnail_id`, so the field is there to map. - saveEntry resolves that id to the downloaded asset. saveAsset already registers every attachment under `assets_<wp:post_id>`, which is exactly what `_thumbnail_id` holds, so it is a one-hop lookup. A thumbnail whose download failed is absent from assetData and is left unset rather than written as a dangling uid. - saveAsset prefers the attachment's `_wp_attachment_image_alt` postmeta for the asset description, which is where WordPress keeps alt text. It surfaces as `featured_image.description` on the entry. The previous description/content/excerpt fallbacks are unchanged and still apply when alt is empty. Ported as a standalone slice of the equivalent work on feature/wordpress-acf, without that branch's ACF, Yoast SEO, excerpt and lifecycle-field changes. Reported via support ticket 00062373.
✅ 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
🟠 High Severity - SLA Breached Issues (with fixes)Showing 5 issue(s) that have exceeded the 30-day SLA threshold:
ℹ️ 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:
❌ BUILD FAILED - Security checks failed Please review and fix the security vulnerabilities before merging. |
umesh-more-cstk
left a comment
There was a problem hiding this comment.
Automated review of head d6ea3d1. I traced the featured-image path end to end — extractItems → field mapping → buildSchemaTree / convertToSchemaFormate → saveEntry → entry JSON — and the mechanism holds up. 0 blockers · 1 question · 2 nits, all inline.
What checks out
- The asset lookup is the right contract.
saveAssetregisters every attachment asassets_<wp:post_id>(wordpress.service.ts:1848) and_thumbnail_idholds exactly that id, so the one-hop resolve is correct. Returning the whole asset object rather than a uid is also right: the existing WordPressfilecase doesformatted = asset(wordpress.service.ts:1082), and Drupal'sfilecase does the sameassets_${value}lookup, returns the object, and likewise leaves the field unset on a miss (drupal/entries.service.ts:442-476). The deliberate "no dangling uid" choice in the reviewer notes matches the sibling connector exactly. - The schema side lands.
contentstackFieldType: 'file'reachescase "file"incontent-type-creator.utils.ts:756;contentstackField: 'Featured Image'becomesdisplay_nameviacontent-type-creator.utils.ts:428; andisDeleted: falseis harmless since only=== trueis filtered (line 136). The pushed field object carries the same key set as the canonicaltitlefield atextractItems.ts:264. - Placement is correct. The new block sits outside the per-item loop alongside the
terms/authorpushes, so it is emitted once per content type, and the!isAllContentEmptyguard matches its siblings atextractItems.ts:525/535/551. The reviewer note explaining that guard is accurate. wp:postmetadoes survive the XML→JSON step — the parser atupload-api/migration-wordpress/utils/helper.ts:16applies no element whitelist, so_thumbnail_idis genuinely reachable insaveEntry.- Scope matches the stated Affected Areas (
api+upload-api). No lockfile or generated churn, no secrets, no leftover debug code.
The disclosed behaviour change is real and slightly wider than the feature name suggests: getAttachmentAltText is consulted in saveAsset for every attachment, so alt text now wins over description / content:encoded / excerpt:encoded for all migrated assets, not only featured images. It is documented in the PR body and only applies when alt is non-empty, so I am not filing it as a finding — just flagging it for whoever signs off, since it touches assets that already migrate fine today.
Not verified
I did not run either test suite — node_modules is not installed in this environment. I can neither confirm nor dispute the stated 632 / 303 pass counts; the 19 new tests read as covering the right cases, including the single-object postmeta shape and the failed-download fallback.
Process, not code
The PR's own template flags the Jira ticket as outstanding ("Needs a Jira ticket linked before review"), and docs/ was intentionally left out. Both are the author's call — carrying them forward so they are not lost at merge. The PR is also currently behind dev.
Generated by Claude Code
| entryData[uid]['author'] = authorData; | ||
| const featuredImage = resolveFeaturedImageAsset(item, assetData); | ||
| if (featuredImage) { | ||
| entryData[uid]['featured_image'] = featuredImage; |
There was a problem hiding this comment.
question: saveEntry writes featured_image without consulting fields (the content type's fieldMapping), while the schema side only emits the field when hasFeaturedImage && !isAllContentEmpty (upload-api/migration-wordpress/libs/extractItems.ts:580). The two can disagree.
Concrete case: a post type whose items all have empty content:encoded. extractItems then skips the featured_image push (along with title/url), but if any of those items carries _thumbnail_id and the attachment downloaded, this line still puts featured_image on the entry — an entry key with no matching field in the generated content type. The same holds if the field is marked deleted at the Field Mapping step: buildFieldSchema drops isDeleted === true fields (api/src/utils/content-type-creator.utils.ts:136), but nothing filters the entry side.
The author / tags writes on the lines just above have the same shape, so this may well be pre-existing and tolerated by the importer — which is why this is a question rather than a blocker. Worth confirming an orphan key is genuinely ignored on import; if it isn't, gating this on a lookup for a featured_image entry in fields would close it.
Generated by Claude Code
| (meta: any) => meta?.["wp:meta_key"] === metaKey && meta?.["wp:meta_value"] | ||
| ); | ||
| const value = match?.["wp:meta_value"]; | ||
| return typeof value === "string" ? value : (value != null ? String(value) : ""); |
There was a problem hiding this comment.
nit: String(value) yields "[object Object]" if wp:meta_value ever arrives as an xml2js node rather than a plain string. The parser is configured with attrkey: 'attributes' / charkey: 'text' (upload-api/migration-wordpress/utils/helper.ts:16), so any element carrying an attribute becomes { text, attributes } — which is exactly why saveEntry reads tag?.text and cat?.attributes?.nicename further down this file.
Standard WXR writes <wp:meta_value> with no attributes, so this is unlikely rather than broken. It just fails quietly if it does happen: the lookup misses and the warning reads assets_[object Object]. Cheap guard:
| return typeof value === "string" ? value : (value != null ? String(value) : ""); | |
| if (typeof value === "string") return value; | |
| const raw = value?.text ?? value; | |
| return raw != null ? String(raw) : ""; |
Generated by Claude Code
| "isDeleted": false, | ||
| "uid": 'featured_image', | ||
| "backupFieldUid": 'featured_image', | ||
| "otherCmsField": '_thumbnail_id', |
There was a problem hiding this comment.
nit: otherCmsField: '_thumbnail_id' becomes a user-visible label in one path. updateContentType resets a field with contentstackField: field?.otherCmsField (api/src/services/contentMapper.service.ts:1010), and contentstackField is what buildSchemaTree turns into the Contentstack display_name (api/src/utils/content-type-creator.utils.ts:428). A field mapping that goes through that reset therefore produces a field literally named _thumbnail_id in the destination stack instead of "Featured Image".
The sibling fields sidestep this by putting a human label in otherCmsField — the author push at line 551 uses 'Author' rather than dc:creator, and terms uses 'terms'. Using 'Featured Image' here would match them and keep the reset path honest; the _thumbnail_id provenance is already captured in the comment above and in backupFieldUid.
Generated by Claude Code
🔗 Jira Ticket
📋 PR Type
📝 Description
What changed?
extractItemsnow emits afeatured_imagefile field on the generated content type, so the featured image is actually available to map at the Field Mapping step. Added only when an item in that post type declares a_thumbnail_id, so exports without featured images are unaffected.saveEntryresolves that thumbnail to the downloaded asset via a newresolveFeaturedImageAssethelper.saveAssetalready registers every attachment underassets_<wp:post_id>, which is exactly what_thumbnail_idholds — so it is a one-hop lookup, no URL matching needed.saveAssetnow prefers the attachment's_wp_attachment_image_altpostmeta for the asset description, via a newgetAttachmentAltTexthelper. That is where WordPress keeps alt text, and it surfaces asfeatured_image.descriptionon the entry.Why?
A customer reported that their WordPress featured images were downloaded into the stack but never attached to the post entry, and that no featured image field appeared at the Field Mapping step.
Both symptoms trace to the same gap: the connector never read
_thumbnail_idat all — there were zero references to it in the codebase.getAllAssetsdownloads everyattachmentitem in the export, so the image file did arrive; but entries only referenced assets found inside the Gutenberg blocks ofcontent:encoded, and a featured image lives outside the post body. The result was an uploaded, orphaned asset and a content type with nowhere to put it.This is a standalone port of the equivalent work already on
feature/wordpress-acf. That branch has a working implementation, but it also rewriteswordpress.service.tsfrom ~2,670 to 8,000+ lines and bundles in ACF support, a Yoast SEO group,excerpt, andstatus/created_atlifecycle fields. Only the featured-image slice is taken here, so the fix can ship without that surface area.Behaviour note: the asset
descriptionfallback order changes fromdescription → content → excerpttoalt → description → content → excerpt. Alt is only preferred when non-empty, so the existing fallbacks still apply unchanged for attachments with no alt text.🧩 Affected Areas
api— Node.js backendui— React frontendupload-api— Upload API serverdocker/docker-compose🧪 How to Test
_thumbnail_idpostmeta and the referencedattachmentitem is present in the same file. (A "Posts only" WordPress export omits the media items — the thumbnail cannot be resolved in that case, by design.)Expected result: the entry's
featured_imagefield holds the migrated asset, and the asset's description carries the image's WordPress alt text when the source had any. A post with no featured image gets nofeatured_imagevalue, and an export with no featured images anywhere gets nofeatured_imagefield at all.📸 Screenshots / Recordings
Not applicable — no UI changes. The new field renders through the existing Field Mapping table.
🔗 Related PRs / Dependencies
feature/wordpress-acffor featured-image support.main, which is where the reporting customer is running from.devandmaindiffer by only ~131 lines across the touched files, so it should apply cleanly.✅ Author Checklist
feature/,bugfix/, orhotfix/+ 5–30 lowercase chars.env/example.envupdated if new environment variables were added — n/a, none addedapi632 passed,upload-api303 passed; both were green before the change too (619 / 297, the deltas being the new tests below).tsc --noEmiterror count onapiis unchanged at 49 pre-existing errorsextractItems.featuredImage.test.ts(6, covering the field being added, single-object postmeta as xml2js emits it, added once across mixed items, and three negative cases) andwordpress.service.featuredImage.test.ts(13, covering thumbnail resolution, numeric ids, failed-download fallback, and alt-text extraction). Fixtures mirror the real shapes from the reporting export.README.md/ docs updated if behaviour changed — not done; flagging below👀 Reviewer Notes
hasFeaturedImage && !isAllContentEmpty, matching how the existingtermsandauthorreference fields are gated. Without theisAllContentEmptyguard, a post type whose items all have empty bodies would produce a content type holding only a file field and notitle, which would not import.assetDataonly contains attachments that downloaded successfully, and a dangling asset uid would fail the entry import. The helper logs a warning naming the missingassets_<id>.docs/was not updated. The public WordPress-to-Contentstack doc walks through field mapping; it may want a line about the featured image now appearing. Happy to add that here or separately.feature/wordpress-acfdoes — but the customer's wording asked for "featured_image and it's alt on the post content type", which could also be read as wanting a dedicatedfeatured_image_altfield. Easy to add if reviewers prefer that reading.🤖 Generated with Claude Code