Skip to content

feat: opt-in editable paragraph component (fix Visual Builder 'Invalid CSLP tag') - #18

Open
vaishnavi-kedar-cstk wants to merge 6 commits into
mainfrom
feat/editable-paragraph
Open

vaishnavi-kedar-cstk wants to merge 6 commits into
mainfrom
feat/editable-paragraph

Conversation

@vaishnavi-kedar-cstk

Copy link
Copy Markdown
Contributor

Problem

Seeded starter-pack content (cms_paragraph_component, simple_banner_component, product_carousel_component) renders through stock Spartacus components (cx-paragraph, cx-banner, cx-product-carousel). For stock components the connector can only stamp a coarse entry-level data-cslp tag on the wrapper ({content_type}.{entry}.{locale}, via the ComponentDecorator). Contentstack Visual Builder needs a field-level tag (…​.{field}) to inline-edit a field, so it reports "Invalid CSLP tag / incorrectly generated" on those sections. Confirmed by reading the live DOM — every island had only the 3-part tag.

Fix (opt-in, paragraph first)

  • ContentstackCmsComponentNormalizer now preserves the Live Preview field-tag map (entry.$) on the converted component data. It's only present on preview builds (added by tagEntryTree when livePreview is on) and omitted otherwise — so stock components and production delivery are byte-for-byte unchanged.
  • ContentstackEditableParagraphComponent (new) renders identically to the stock paragraph (cx-paragraph class + innerHTML) but binds [csEditable] to data.$.content, emitting the 4-part field tag {content_type}.{entry}.{locale}.content.
  • ContentstackEditableComponentsModule (new, opt-in) registers it for CMSParagraphComponent. It is not pulled in by ContentstackCmsFeatureModule, so default behavior is unchanged; an app opts in with one import:
    imports: [ContentstackCmsFeatureModule, ContentstackEditableComponentsModule]

Verified live

Built + packed the connector, installed into my-storefront, opted in, and read the rendered DOM in a real browser. The hero paragraph now emits:

cms_paragraph_component.blt1522f06173db68e1.en-us.content   ← 4-part field tag (new)

(previously only the 3-part entry-level tag). All three paragraphs on the page now carry the .content field tag.

Scope / consequences

  • No change to SAP/commerce, data fetching, hybrid mode, access control, or OCC fallback. Rendering-only, and only for CMSParagraphComponent when opted in.
  • Preview-only effect: the edit tags are inert outside preview builds (no $ ⇒ the directive removes the attribute).
  • Banner and product-carousel are intentionally deferred. The carousel additionally hydrates live SAP product data (ProductService), which its editable renderer must preserve — separate follow-up.

Tests

  • Normalizer specs: $ is preserved when present, and omitted entirely when the entry wasn't tagged (production/non-preview unchanged).
  • lint, format:check, typecheck, test:all (169 unit + 8 schematics), build all green.

🤖 Generated with Claude Code

…iting

Seeded starter-pack content (cms_paragraph_component, …) renders through
stock Spartacus components, so the connector could only stamp a coarse
entry-level data-cslp tag on the wrapper — which Contentstack Visual
Builder reports as an "Invalid CSLP tag" because there is no field-level
tag to inline-edit.

This adds an OPT-IN path to fix that for paragraphs:

- ContentstackCmsComponentNormalizer now preserves the Live Preview
  field-tag map (entry.$) on the converted component data (only present
  on preview builds; omitted otherwise, so stock components and
  production delivery are unchanged).
- New ContentstackEditableParagraphComponent renders identically to the
  stock paragraph (cx-paragraph class + innerHTML) but binds
  [csEditable] to data.$.content, emitting the 4-part field-level tag
  `{content_type}.{entry}.{locale}.content`.
- New ContentstackEditableComponentsModule (opt-in) registers it for
  CMSParagraphComponent. Not imported by ContentstackCmsFeatureModule, so
  default behavior is unchanged; apps opt in by importing it.

Verified live against my-storefront in a real browser: the paragraph now
emits `cms_paragraph_component.<uid>.en-us.content` (was 3-part
entry-level only). Banner and product-carousel are intentionally left to
follow-ups (the carousel must preserve its live SAP product hydration).

169 unit + 8 schematics tests pass; lint, format, typecheck, build green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vaishnavi-kedar-cstk
vaishnavi-kedar-cstk requested a review from a team as a code owner September 16, 2026 06:56
Copilot AI lite review requested due to automatic review settings September 16, 2026 06:56
@snyk-io

snyk-io Bot commented Sep 16, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Adapter-fetched entries may lose field tags, and preview-only behavior needs correction.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an opt-in editable paragraph renderer for Contentstack Visual Builder field-level editing.

Changes:

  • Preserve Live Preview field-tag metadata during normalization.
  • Add and export an editable paragraph component and opt-in module.
  • Add normalizer tests for tagged and untagged entries.
File summaries
File Summary and findings
src/public-api.ts Exports the editable renderer APIs.
src/cms/converters/contentstack-cms-component.normalizer.ts Preserves $ metadata. Moderate findings: preview-only gating may be incomplete (2 votes), and standalone fetch paths may lack tags (1 vote).
src/cms/converters/contentstack-cms-component.normalizer.spec.ts Tests tag preservation and omission.
src/cms/components/contentstack-editable-paragraph.component.ts Renders field-tagged paragraph content. Moderate finding: adapter-fetched entries may lose editability (1 vote); nit: missing component-level template coverage (3 votes).
src/cms/components/contentstack-editable-components.module.ts Registers the opt-in renderer. Moderate finding: adapter-loaded entries may silently lose field-level editing (1 vote).
Review details

Suppressed comments (3)

src/cms/components/contentstack-editable-components.module.ts:33

  • This global override also handles CMSParagraphComponent instances loaded through ContentstackCmsComponentAdapter (for example after a language switch or a standalone lookup), but those client fetch paths do not call tagEntryTree. The normalizer then receives no $, so [csEditable] removes the attribute and field-level editing silently disappears after that reload. Tag adapter-fetched entries as well, or avoid selecting this renderer when no field-tag data is available.
        CMSParagraphComponent: {
          component: ContentstackEditableParagraphComponent,
        },

src/cms/components/contentstack-editable-paragraph.component.ts:47

  • $ is only attached by the page/global fetch paths. The component adapter's getEntryByUid and getEntriesByUids paths return raw entries without calling tagForLivePreview, so an unresolved/shared paragraph (or a component reloaded on a language switch) reaches this binding without $ and loses field-level editing even with Live Preview enabled. Tag standalone fetch results with the same per-entry content type before normalization, or explicitly limit this opt-in mapping to inline page entries.
      [csEditable]="data.$?.['content']"

src/cms/converters/contentstack-cms-component.normalizer.ts:78

  • This only preserves $ when the raw entry already has tags, but the standalone component-loading paths do not create them: ContentstackClientService.getEntryByUid and getEntriesByUids return fetched entries without calling tagEntryTree. An unresolved reference or adapter-loaded paragraph therefore reaches this opt-in renderer with no $, so CsEditableDirective removes data-cslp and the field remains non-editable in Visual Builder. Tag entries in live-preview mode in both component fetch paths before caching/normalization.
      ...(source['$'] ? { $: source['$'] } : {}),
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

// components can bind a per-field `data-cslp` via CsEditableDirective.
// Only present on preview builds; absent (and omitted) otherwise, so stock
// components and production delivery are byte-for-byte unaffected.
...(source['$'] ? { $: source['$'] } : {}),
Comment on lines +43 to +49
template: `
<div
*ngIf="data$ | async as data"
class="cx-paragraph"
[csEditable]="data.$?.['content']"
[innerHTML]="data.content"
></div>
Wire ContentstackEditableComponentsModule into ContentstackCmsFeatureModule
so consuming apps get field-level Visual Builder editing for seeded
component types with no opt-in import. Safe by default: the editable
renderers render identically to the stock Spartacus components and the
edit tags are inert outside preview builds, so normal delivery/production
rendering is unchanged.

Verified live against my-storefront with NO app-side import: the seeded
paragraphs still emit `cms_paragraph_component.<uid>.en-us.content`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 16, 2026 08:45
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

…der)

Adds ContentstackEditableBannerComponent and registers it for the banner
typeCodes (SimpleBannerComponent, SimpleResponsiveBannerComponent) in
ContentstackEditableComponentsModule (on by default).

It renders the same building blocks as the stock Spartacus banner —
cx-media (elementType 'picture') inside cx-generic-link — passing
getImage-equivalent data.media to cx-media, and binds [csEditable] to the
media field so Contentstack Visual Builder can edit the banner image
inline. As a global cmsComponents override it also renders OCC-sourced
banners; it's a faithful drop-in (same cx-media/cx-generic-link), and the
edit tag is inert on untagged (OCC/non-preview) banners.

typings/spartacus.d.ts: declare MediaModule/GenericLinkModule/MediaComponent
(real @spartacus/storefront exports) so the offline typecheck resolves the
imports.

Verified live against my-storefront: the Contentstack banner renders its
Contentstack image (images.contentstack.io) with tag
`simple_banner_component.<uid>.en-us.media`; all OCC banners still render.

lint, typecheck, test (169+8), build all green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved issues remain around default module registration, component-fetch tagging, and rendering-path test coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/cms/components/contentstack-editable-paragraph.component.ts:48

  • There is no component/module test for this new rendering path: the normalizer specs do not exercise the template binding or CsEditableDirective, so a regression could still pass all current tests. Add a TestBed spec (as done for custom-hero.component.spec.ts) covering both the four-part tag from data.$.content and removal of the attribute when $ is absent.
      [csEditable]="data.$?.['content']"

src/cms/components/contentstack-editable-paragraph.component.ts:48

  • This binding only works when the component data came from the page normalizer with a tagged raw entry. ContentstackCmsComponentAdapter.load and findComponentsByIds fetch through getEntryByUid/getEntriesByUids without calling tagEntryTree, so standalone paragraph loads and component reloads on a language switch normalize entries without $; CsEditableDirective then removes the field tag and Visual Builder editing silently disappears. Apply live-preview tagging to all component-fetch paths before normalization (including locale reloads).
      [csEditable]="data.$?.['content']"

src/cms/converters/contentstack-cms-component.normalizer.ts:78

  • Preserving $ here only helps when the raw entry was tagged by tagEntryTree, but ContentstackClientService.getEntryByUid and getEntriesByUids do not tag their results when Live Preview is enabled. Components loaded through those paths (for example unresolved references or the per-UID reload on language changes) therefore reach this renderer without $, so the field-level data-cslp attribute disappears. Tag component-fetch results as well, or the opt-in renderer will not remain editable on those paths.
      ...(source['$'] ? { $: source['$'] } : {}),
  • Files reviewed: 7/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

imports: [
ContentstackCmsModule,
ContentstackLivePreviewModule,
ContentstackEditableComponentsModule,
Copilot AI review requested due to automatic review settings September 16, 2026 09:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Four moderate review issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/cms/components/contentstack-editable-paragraph.component.ts:49

  • The new acceptance behavior is not covered by a component/config test: the added specs only assert that $ survives normalization, but none renders ContentstackEditableParagraphComponent to verify the DOM data-cslp attribute or verifies the cmsComponents registration. A selector/binding or mapping regression could therefore pass the current suite; the existing TestBed coverage in src/examples/hero-banner/custom-hero.component.spec.ts provides a pattern for this check.
      *ngIf="data$ | async as data"
      class="cx-paragraph"
      [csEditable]="data.$?.['content']"
      [innerHTML]="data.content"

src/contentstack-cms-feature.module.ts:65

  • This import makes the supposedly opt-in renderer module part of every ContentstackCmsFeatureModule consumer, so CMSParagraphComponent (and the new banner mappings) replace Spartacus's stock components by default. That contradicts the stated opt-in contract and changes the default rendering path; remove this import and require apps that want field tags to import ContentstackEditableComponentsModule explicitly.
    ContentstackEditableComponentsModule,
  • Files reviewed: 7/8 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/cms/components/contentstack-editable-banner.component.ts Outdated
// components can bind a per-field `data-cslp` via CsEditableDirective.
// Only present on preview builds; absent (and omitted) otherwise, so stock
// components and production delivery are byte-for-byte unaffected.
...(source['$'] ? { $: source['$'] } : {}),
…eserved)

Adds ContentstackEditableProductCarouselComponent and registers it for
ProductCarouselComponent in ContentstackEditableComponentsModule.

The stock carousel renders its title inside cx-carousel (not taggable), so
this renders its own [csEditable] <h3> for the title (4-part data-cslp on
the `title` field) and passes an empty title to cx-carousel to avoid a
duplicate heading. Products are unchanged: each SKU in productCodes is
hydrated live from SAP via ProductService ([LIST, STOCK] scopes, same as
stock) and rendered with the stock cx-product-carousel-item, so
name/price/stock/image + add-to-cart still come from SAP OCC.

typings/spartacus.d.ts: declare CarouselModule/ProductCarouselModule (+
CarouselComponent/ProductCarouselItemComponent) so the offline typecheck
resolves the imports.

Verified live against my-storefront: carousel title carries
`product_carousel_component.<uid>.en-us.title`, and all 5 products render
with live SAP names + prices ($59.00, $183.00, …).

lint, typecheck, test (169+8), build all green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 17, 2026 06:30
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved carousel rendering, banner tagging, preview propagation, and opt-in scope issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/cms/components/contentstack-editable-banner.component.ts:44

  • The tag map is keyed by the raw Contentstack field UID, but this renderer always looks up media. For banners using the starter-pack's media_container or media_<breakpoint> fields, data.$.media is absent, so CsEditableDirective removes the tag and Visual Builder still cannot edit the image field. The plain-link branch repeats the same lookup.
          [csEditable]="data.$?.['media']"
        ></cx-media>

src/cms/converters/contentstack-cms-component.normalizer.ts:78

  • This only preserves $ when the entry was already tagged. The standalone component adapter fetches entries through getEntryByUid/getEntriesByUids, but those client paths do not call tagEntryTree; components loaded outside a tagged page (or reloaded through that adapter) therefore reach the editable renderer without $ and emit no field tag. Apply the same live-preview tagging to those fetch paths or narrow the renderer's supported path.
      // Preserve the Live Preview field-tag map (`entry.$`, added by
      // tagEntryTree when livePreview is on) so connector-provided editable
      // components can bind a per-field `data-cslp` via CsEditableDirective.
      // Only present on preview builds; absent (and omitted) otherwise, so stock
      // components and production delivery are byte-for-byte unaffected.
      ...(source['$'] ? { $: source['$'] } : {}),

src/contentstack-cms-feature.module.ts:65

  • Importing this module here makes the editable mappings active for every app that imports the documented feature module, so the change is not opt-in and default component resolution is no longer unchanged. Keep ContentstackEditableComponentsModule out of this root module and require the separate consumer import described in the PR.
    ContentstackEditableComponentsModule,
  • Files reviewed: 8/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment on lines +82 to +85
readonly items$: Observable<Observable<Product | undefined>[]> = this.data$.pipe(
map((data) =>
(data.productCodes ?? '')
.trim()
Comment on lines +32 to +36
SimpleBannerComponent: {
component: ContentstackEditableBannerComponent,
},
SimpleResponsiveBannerComponent: {
component: ContentstackEditableBannerComponent,
Visual Builder flagged the banner's field tag as an "invalid / incorrectly
generated" CSLP tag because it was bound on <cx-media> (a custom element);
the working paragraph/carousel tags sit on standard elements (div/h3). Wrap
cx-media in a <div [csEditable]> so the media field tag lands on a standard
element, matching the other editable renderers. Image still renders from
Contentstack; SAP/OCC banners unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 17, 2026 06:49
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved renderer, tagging, opt-in integration, and test coverage issues block approval.

Review details

Suppressed comments (8)

src/cms/components/contentstack-editable-banner.component.ts:43

  • On the page preview path, CmsComponentData.data$ is fed properties.data: entry (the raw Contentstack entry), not the normalized component. This renderer therefore sees url_link and raw media/media_container; data.urlLink is empty and the media is not the normalized responsive shape, so preview banners can lose their link/media (and the media tag is absent when media_container is used). Feed normalized data together with $, or normalize the raw preview shape before rendering.
      <cx-generic-link *ngIf="data.urlLink; else plain" [url]="data.urlLink">
        <div [csEditable]="data.$?.['media']">
          <cx-media [container]="$any(data.media)" [elementType]="'picture'"></cx-media>
        </div>
      </cx-generic-link>

src/cms/components/contentstack-editable-banner.component.ts:40

  • The new renderer is registered for both banner type codes, but the starter-pack normalizer can populate media from media_container or from media_mobile/media_tablet/media_desktop/media_widescreen. Those entries have field tags under those original UIDs, not under media, so this binding emits no field-level tag for the supported responsive/reference forms. Bind the actual authoring fields (with an appropriate per-breakpoint strategy) or leave the banner mapping deferred.
        <div [csEditable]="data.$?.['media']">

src/cms/components/contentstack-editable-components.module.ts:36

  • These banner mappings are not correct for the starter-pack data shape this module claims to support. The banner normalizer turns media_container and media_desktop/other breakpoint fields into data.media, but the CSLP map remains keyed by the original field UID, so [csEditable] bound to data.$?.['media'] emits no field tag for the common media_container path. Bind the tag for the selected source field (preserving its provenance), or do not register this renderer until that is implemented.
        SimpleBannerComponent: {
          component: ContentstackEditableBannerComponent,
        },
        SimpleResponsiveBannerComponent: {
          component: ContentstackEditableBannerComponent,

src/cms/components/contentstack-editable-components.module.ts:39

  • The PR description explicitly defers ProductCarouselComponent, but this mapping ships in the same opt-in module. An app opting in for paragraph editing therefore also replaces the stock carousel renderer; remove this registration until the carousel follow-up is complete, otherwise the documented opt-in changes carousel behavior too.
        ProductCarouselComponent: {
          component: ContentstackEditableProductCarouselComponent,

src/cms/components/contentstack-editable-paragraph.component.ts:49

  • The central behavior added here—rendering content while applying the normalized $ field tag, and removing the attribute when $ is absent—is not covered by a component test. The normalizer specs only verify data propagation; add a fixture test for this template/directive in both preview and non-preview shapes to guard the Visual Builder fix.
      [csEditable]="data.$?.['content']"
      [innerHTML]="data.content"

src/cms/components/contentstack-editable-product-carousel.component.ts:88

  • The page preview payload is the raw entry, where the starter-pack carousel stores SKUs in products; productCodes exists only on the normalized component. Consequently items$ resolves an empty list in preview and the editable carousel renders no products—the path this renderer is meant to fix. Consume normalized data with $, or explicitly map the raw products field before enabling this renderer.
      (data.productCodes ?? '')
        .trim()
        .split(/\s+/)
        .filter(Boolean)
        .map((code) => this.productService.get(code, [ProductScope.LIST, ProductScope.STOCK])),

src/cms/converters/contentstack-cms-component.normalizer.ts:78

  • Preserving $ here only helps when the upstream entry was tagged. ContentstackClientService.getEntryByUid and getEntriesByUids return preview entries without calling tagForLivePreview, so components loaded through the adapter (including unresolved references or reloads) are normalized without $ and CsEditableDirective removes the field tag. Tag those fetch results before normalization, or carry the tagged entry through the adapter.
      ...(source['$'] ? { $: source['$'] } : {}),

src/contentstack-cms-feature.module.ts:65

  • Importing this module here makes the editable mappings unconditional: every app that imports ContentstackCmsFeatureModule now replaces the stock CMSParagraphComponent (and the banner/carousel mappings added in this PR). That contradicts the stated opt-in contract and changes default hybrid/OCC rendering; leave this module out and have consuming apps import it explicitly.
    ContentstackEditableComponentsModule,
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…uilder

Visual Builder rejects an inline data-cslp on a file/asset field (media)
as 'invalid or incorrectly generated'; it only supports inline editing for
text-type fields (as with the paragraph 'content' and carousel 'title').

Point the banner's edit tag at the url_link text field so the component
carries a valid field-level tag (clearing the Invalid CSLP tag error).
The banner image remains editable via the entry form panel, which is the
Contentstack pattern for asset fields.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 17, 2026 07:11
@github-actions

Copy link
Copy Markdown

🔒 Security Scan Results

ℹ️ Note: Only vulnerabilities with available fixes (upgrades or patches) are counted toward thresholds.

Check Type Count (with fixes) Without fixes Threshold Result
🔴 Critical Severity 0 0 10 ✅ Passed
🟠 High Severity 0 0 25 ✅ Passed
🟡 Medium Severity 0 2 500 ✅ Passed
🔵 Low Severity 0 0 1000 ✅ Passed

⏱️ SLA Breach Summary

✅ No SLA breaches detected. All vulnerabilities are within acceptable time thresholds.

Severity Breaches (with fixes) Breaches (no fixes) SLA Threshold (with/no fixes) Status
🔴 Critical 0 0 15 / 30 days ✅ Passed
🟠 High 0 0 30 / 120 days ✅ Passed
🟡 Medium 0 0 90 / 365 days ✅ Passed
🔵 Low 0 0 180 / 365 days ✅ Passed

ℹ️ 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:

  • Critical without fixes: 0
  • High without fixes: 0
  • Medium without fixes: 2
  • Low without fixes: 0

✅ BUILD PASSED - All security checks passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Moderate unresolved issues remain in module opt-in behavior, preview tagging paths, carousel rendering, and paragraph test coverage.

Review details

Suppressed comments (7)

src/cms/components/contentstack-editable-components.module.ts:23

  • The PR explicitly defers banner and product-carousel support, but this module registers both editable renderers (and the public API exports them). Importing the documented opt-in module would therefore replace those stock components with unverified behavior beyond the paragraph fix; keep this module paragraph-only until those follow-ups are ready.
 * Covers `CMSParagraphComponent`, the banner typeCodes (`SimpleBannerComponent`,
 * `SimpleResponsiveBannerComponent`), and `ProductCarouselComponent` (whose
 * editable renderer keeps the live SAP product hydration and only adds an
 * editable tag on the carousel title).

src/cms/components/contentstack-editable-paragraph.component.ts:49

  • This is the core field-level rendering path, but no Angular fixture test covers it; the added specs only verify normalizer $ copying. Add a test that supplies a tagged and an untagged CmsComponentData value and asserts the 4-part data-cslp attribute and its removal, otherwise template/directive regressions can pass test:all.
      [csEditable]="data.$?.['content']"
      [innerHTML]="data.content"

src/cms/components/contentstack-editable-product-carousel.component.ts:66

  • items$ is an array of Observable<Product | undefined> values, but this binding passes each Observable directly to cx-product-carousel-item. The item renderer therefore receives no Product fields; unwrap the stream in the template (item | async) before binding it.
      <cx-product-carousel-item [item]="item" [itemIndex]="itemIndex"></cx-product-carousel-item>

src/cms/components/contentstack-editable-product-carousel.component.ts:61

  • The normalized product-carousel data deliberately includes scroll and popup (see ContentstackFieldMapper), but this replacement drops both values and hardcodes only the item width/title inputs. Any authored non-default carousel behavior is therefore lost, so this is not equivalent to the stock renderer; reuse the stock component's model/input handling before registering this replacement.
      <cx-carousel
        *ngIf="items$ | async as items"
        [items]="items"
        [template]="carouselItem"
        itemWidth="285px"
        [title]="''"

src/cms/components/contentstack-editable-product-carousel.component.ts:53

  • $ is keyed by the raw Contentstack field UID, but the starter-pack carousel title is cms_title and the mapper only renames it to title in the normalized data (contentstack-field-mapper.ts:35). Binding only data.$?.['title'] therefore yields no data-cslp tag for the actual carousel title; use the raw key (with a fallback if both schemas are supported).
      <h3 *ngIf="data.title" class="cs-editable-carousel-title" [csEditable]="data.$?.['title']">

src/cms/converters/contentstack-cms-component.normalizer.ts:78

  • Preserving $ here only helps entries tagged before conversion. The getEntryByUid and getEntriesByUids paths used by ContentstackCmsComponentAdapter do not call tagEntryTree, so components loaded through those paths receive no $ in preview and the new directive removes its field tag (for example on standalone or reloaded components). Tag those fetch paths before normalization as well.
      ...(source['$'] ? { $: source['$'] } : {}),

src/contentstack-cms-feature.module.ts:65

  • Importing this module from the root feature makes the editable mappings active for every consumer of ContentstackCmsFeatureModule, replacing the stock renderers by default. That contradicts the PR's opt-in contract and means apps that only import the feature module no longer get unchanged default behavior; leave this module out here and require the explicit second import.
    ContentstackEditableComponentsModule,
  • Files reviewed: 8/9 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants