Skip to content

fix(demo-url): resolve tenant slug from IMS, not the brand name - #362

Open
tkotthakota-adobe wants to merge 1 commit into
mainfrom
fix/demo-url-tenant-sitesinternal
Open

tkotthakota-adobe wants to merge 1 commit into
mainfrom
fix/demo-url-tenant-sitesinternal

Conversation

@tkotthakota-adobe

Copy link
Copy Markdown
Collaborator

What & why

The onboarding "Access your environment here" demo URL was rendering the wrong Experience Cloud tenant slug (e.g. #/@daveandbusters/... instead of #/@sitesinternal/...), producing a broken deep link that forced users to manually switch IMS orgs.

Root cause in src/tasks/demo-url-processor/handler.js: when the IMS lookup threw, getImsTenantId fell back to slugifying the SpaceCat org name (name.toLowerCase().replace(/\s+/g,'')). Internally onboarded sites all live under the shared Sites Internal IMS org, so the org name is the customer brand ("Dave and Busters") → a fabricated tenant daveandbusters that does not exist in Experience Cloud.

Change

Resolve the tenant slug in order:

  1. IMS_ORG_TENANT_ID_MAPPINGS[imsOrgId] — explicit ops-curated override (this secret was referenced by the tests but the handler never actually read it — the wiring had been dropped).
  2. imsClient.getImsOrganizationDetails(imsOrgId).tenantId — now only used when truthy.
  3. DEFAULT_TENANT_ID — known-good fallback.
  • Removed the brand-name slugification entirely — worst case is now a real default, never an invented brand slug.
  • Removed the dead organization.tenantId read (the Organization data model has no such attribute).
  • Tenant resolution now depends only on the IMS org id, not the org record.

Reviewer notes / operational follow-up

The code guarantees it will not emit a brand slug, but for prod to render sitesinternal you need, in dx_mysticat/prod/task-processor, one of:

  • IMS_ORG_TENANT_ID_MAPPINGS mapping the Sites Internal IMS org id → "sitesinternal", and/or
  • DEFAULT_TENANT_ID = sitesinternal, and/or
  • confirmation of why getImsOrganizationDetails was throwing/empty for that org.

Testing

npm test — 426 passing; demo-url-processor/handler.js at 100% stmts/branch/funcs/lines. Rewrote the handler's unit tests to cover the new precedence (mapping override, IMS fallback, malformed mapping, IMS-throws → default, no-tenantId → default, org-not-found, findById-throws).

Related Issues

None linked.

Change Management

cm-assessment: v1
changeType: standard
impact: unnoticeable
risk: minor
scope: single-repo
relatedPRs: []
rationale: "Refactors tenant-slug resolution for a link inside an onboarding Slack message; a failure only yields a wrong/default tenant in that link, is bounded, touches no data/auth/API contract, and is reversible by redeploy."
recommendations: "Populate IMS_ORG_TENANT_ID_MAPPINGS and/or set DEFAULT_TENANT_ID=sitesinternal in Vault (dx_mysticat/prod/task-processor) so internal onboarding renders the correct tenant."
backout: "Redeploy the previous release."

🤖 Generated with Claude Code

The onboarding "Access your environment here" demo URL derived the
Experience Cloud tenant slug by slugifying the SpaceCat org's name when
the IMS lookup failed (name.toLowerCase().replace(/\s+/g,'')). Internally
onboarded sites live under the shared "Sites Internal" IMS org, so the org
name is the customer brand (e.g. "Dave and Busters") and slugifying it
produced a non-existent tenant ("daveandbusters") -> a broken deep link
that forced users to manually switch IMS orgs.

Resolve the tenant slug in order:
  1. IMS_ORG_TENANT_ID_MAPPINGS[imsOrgId] - explicit ops-curated override
     (this secret was referenced by tests but the handler never read it)
  2. imsClient.getImsOrganizationDetails(imsOrgId).tenantId (only when truthy)
  3. DEFAULT_TENANT_ID - known-good fallback

Remove the brand-name slugification entirely and the dead
organization.tenantId read (the Organization model has no such attribute).
Tenant resolution now depends only on the IMS org id, not the org record.

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

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@rpapani

rpapani commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Reviewed thoroughly and verified every load-bearing claim against source — this is a correct, well-tested fix that strictly improves the failure mode (worst case is now a real default, never an invented brand slug).

Root cause is real: the old getImsTenantId fell back to slugifying the SpaceCat org name, and since internally-onboarded sites share the "Sites Internal" IMS org, that name is the customer brand → a fabricated tenant (daveandbusters) that doesn't exist in Experience Cloud → broken deep link. The new order (mapping override → getImsOrganizationDetails().tenantId when truthy → DEFAULT_TENANT_ID) removes the brand-slug path entirely.

Verified:

  • Organization model has no tenantId — its attributes are config, name, imsOrgId, llmBackend, fulfillableItems, semrushWorkspaceId. So the removed organization.tenantId read was always undefined — genuine dead-code removal, not a behaviour change. ✅
  • env/log/imsClient still in scope after dropping the organization param — const { log, env, imsClient } = context;. ✅
  • IMS_ORG_TENANT_ID_MAPPINGS really was dropped wiring — the removed references are all in the test setup; the old handler never read the secret. Now it does. Good catch. ✅
  • Safe fall-through on malformed mapping (JSON.parse in try/catch → undefined), IMS throw (catch → default), and no-tenantId (imsOrgDetails?.tenantId guard). ✅

Tests cover all branches (mapping override, IMS fallback, malformed mapping, IMS-throws→default, no-tenantId→default, org-not-found, findById-throws); 100% coverage; CI green.

Three non-blocking notes:

  1. It's a config-gated fix. The code guarantees no brand slug, but rendering the correct tenant now depends entirely on ops populating IMS_ORG_TENANT_ID_MAPPINGS and/or DEFAULT_TENANT_ID in dx_mysticat/prod/task-processor (as your reviewer notes say). Until then internal onboardings render the default — and if DEFAULT_TENANT_ID is itself unset, the final fallback returns undefined, so the link would render #/@undefined/.... Worth guaranteeing that env var is set as part of this rollout (a guard/log when it's falsy wouldn't hurt).
  2. Slack noise: the :warning: Using default tenant ID say(...) fires on every default fallback, so it'll post on every internal onboarding until the secret is populated — informative, just potentially chatty in the interim.
  3. Minor edge: an external org that previously got a coincidentally-correct brand slug and whose getImsOrganizationDetails returns no tenantId now falls to DEFAULT. Defensible (the brand slug was always fragile), but it does lean the whole non-default path on IMS/mapping — and since your notes flag uncertainty about why IMS was empty/throwing for the Sites Internal org, it's worth confirming IMS resolution is reliable for the broader org population.

LGTM.

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