Skip to content

feat: scaffold PIC-1321 App Framework storage for Klaviyo field mappings - #11280

Open
david-shibley-contentful wants to merge 1 commit into
masterfrom
feat/klaviyo-app-framework-storage-scaffold
Open

feat: scaffold PIC-1321 App Framework storage for Klaviyo field mappings#11280
david-shibley-contentful wants to merge 1 commit into
masterfrom
feat/klaviyo-app-framework-storage-scaffold

Conversation

@david-shibley-contentful

@david-shibley-contentful david-shibley-contentful commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds the app-side half of the PIC-1321 App Framework storage RFC to the Klaviyo app: a storage block in contentful-app-manifest.json, a local type shim for the not-yet-shipped context.storage DSL, and two new Functions (getFieldMappings, setFieldMappings) written against that contract.
  • Purely additive/forward-looking: nothing is wired in yet. entrySyncFunction.ts and the existing frontend field-mapping UI (which currently store mappings as a stringified JSON blob in a klaviyoFieldMappings CMA entry, per the drift from ADR-0003) are untouched.
  • Known risk: the manifest change is confirmed safe against the local @contentful/app-scripts@2.3.0 build validation, but it's unverified whether the live Extensibility API's bundle-upload validation accepts the new top-level storage key. Do not run upload-ci/upload-staging/deploy with this change until that's confirmed against staging.

Test plan

  • npm run build:functions succeeds locally with the new manifest storage block and both new Functions bundle cleanly
  • npx tsc --noEmit passes with no type errors against the installed @contentful/node-apps-toolkit@3.15.0
  • npm test run (no existing test files touch these new files; pre-existing suite has zero test files, so this is a no-op check)
  • Confirm live Extensibility API bundle-upload validation accepts the new manifest storage key before ever running upload-ci/deploy on this branch

Generated with Claude Code

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚔️ I think we are exposing a bit too much about the innerworkings of app storage in this ADR. The idea was to use it for internal apps only.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call — removed the ADR entirely in eab1588, nothing else in the codebase referenced it. Updated the PR description to drop the dangling link too.

// the same fallback logic in entrySyncFunction.ts.
function toStorageRow(mapping: any): FieldMappingRow | null {
const contentfulFieldId = mapping.contentfulFieldId || mapping.id;
const klaviyoBlockName = mapping.klaviyoBlockName || mapping.name || contentfulFieldId;

@JuliRossi JuliRossi Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💭 I think we are doing this in entrySyncFunction.ts as well the contentfulFieldId/klaviyoBlockName/fieldType alias fallback in toStorageRow is basically the same logic. Perhaps this could be a shared helper both files import?

{ parameters: { entryId, mappings } }
);
} catch (error) {
logger.error('Error writing field mappings via Set Field Mappings action:', error);

@JuliRossi JuliRossi Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💭 Do we want to surface this failure to the caller somehow? Right now if the App Action call fails we just log and return none of the four call sites (ConfigScreen, FieldMappingScreen, FieldSelectDialog, Sidebar) can tell the save didn't go through.

Adds a storage block to contentful-app-manifest.json, a local type
shim for the not-yet-shipped context.storage DSL, and getFieldMappings/
setFieldMappings Functions written against that contract. Get/set now
try context.storage first and fall back to the existing
klaviyoFieldMappings CMA-entry logic (moved to field-mappings-legacy.ts)
when it's absent, which is true in every real environment today. The
frontend no longer touches that CMA entry directly - it calls the two
App Actions via field-mappings-client.ts, same exported names/
signatures as before so call sites only changed their import path.
getFieldMappings also bulk-migrates the legacy blob into the
field_mappings storage table the first time context.storage is
present, gated on the table being empty so it only runs once.
entrySyncFunction.ts still reads via the legacy path unconditionally;
wiring it onto context.storage is a follow-up once the platform ships.

Known risk: the manifest change is confirmed safe against the local
@contentful/app-scripts@2.3.0 build validation, but it's unverified
whether the live Extensibility API's bundle-upload validation accepts
the new top-level storage key. Do not run upload-ci/upload-staging/
deploy with this change until that's confirmed against staging.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@david-shibley-contentful
david-shibley-contentful force-pushed the feat/klaviyo-app-framework-storage-scaffold branch from eab1588 to 2bc0d32 Compare August 21, 2026 20:26
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