fix: adds base site id to brand creation

[igor-grubic]

LLMO-4172: Add baseSiteId to brand API

Summary

• Add baseSiteId and baseUrl fields to the brand API response (resolved via PostgREST join on brands.site_id → sites)
• Accept optional baseSiteId on brand creation (POST) — resolves to brands.site_id
• Enforce per-org uniqueness: return 409 if another brand already claims the same site
• Enforce immutability on PATCH: once set, baseSiteId cannot be changed (can still be set from NULL)
• Force status = 'pending' when creating a brand without baseSiteId
• Strip read-only baseUrl from PATCH payloads
• Update API docs, OpenAPI schema, and schema reference

Test plan

• POST brand with baseSiteId → response includes baseSiteId + baseUrl
• POST brand without baseSiteId → created as pending
• POST brand with already-taken baseSiteId → 409
• PATCH brand that has baseSiteId with a different value → silently ignored
• PATCH brand with NULL baseSiteId to set it → succeeds
• GET brand → includes baseSiteId and baseUrl

[irenelagno]

Cross-reviewed end-to-end with the UI, DRS, and DB PRs. Direction is right — repurposing site_id, immutable + unique per org, force-pend without a base site. But there are three things I'd fix before merge, plus one cross-cutting concern that belongs in this PR.

1. The onboarding seeder bypasses this PR's force-pend check

src/controllers/llmo/llmo-onboarding.js:1360-1372 calls upsertBrand directly from the storage layer, skipping the createBrand controller and therefore skipping the new if (!baseSiteId) status = 'pending' logic added here. It passes status: 'active' but not baseSiteId. Net effect post-rollout: every newly onboarded brand is in the state status='active', site_id=NULL, which this PR says should be impossible.

Fix: add baseSiteId: site.getId() to the seed payload (site.getId() is already in scope 4 lines above in a log statement). See also the layering concern on brands.js below.

2. Uniqueness check has a self-collision on re-POST

See line comment on src/support/brands-storage.js. This interacts with fix #1 — making the seeder pass baseSiteId is unsafe until this is fixed, because onboarding is idempotent.

3. Uniqueness is not backed by the DB

Pair this with a partial unique index in adobe/mysticat-data-service#329 so the SELECT-then-INSERT race window goes away and the JS check can simplify (or be removed entirely in favor of catching the unique-violation error).

Downstream consequence worth knowing

The UI PR adobe/project-elmo-ui#1479 gates its editable Primary URL ComboBox on isPending && !hasBaseUrl. Combined with fact #1 above, every seeded brand is active-without-baseSiteId, and the UI provides no user-reachable way to fix it. Either this PR fixes the seeder (preferred) or the UI PR needs a wider edit gate or a backfill script.

Good news for DRS

I traced the DRS brand presence analyzer (spacecat_client.build_brand_config_from_v2, runner._get_brand_config_v2, brand_analyzer._check_citation, analyze_with_llm) and it does not read baseSiteId or baseUrl anywhere. Zero regression risk on brand presence analysis from this PR set.

— Review based on cross-repo analysis; happy to discuss any of the above.

[irenelagno]

Self-collision bug. upsertBrand uses onConflict: 'organization_id,name', so a re-POST of the same brand is a legitimate update path. In that path this pre-check finds the brand's own row and returns 409 against itself.

Fix: look up the existing brand id by (organization_id, name) first and add .neq('id', existingId) to this pre-check. Or drop the JS check entirely once the partial unique index lands in adobe/mysticat-data-service#329 and just catch the unique-violation error.

[igor-grubic]

Good catch — removed the JS pre-check SELECT entirely. Uniqueness is now enforced solely by the DB constraint (brands_base_site_unique), which correctly handles upserts on the same row without self-collision.

[irenelagno]

.maybeSingle() throws if the DB ever has >1 row matching — which is exactly the invariant this PR is trying to enforce but currently isn't backed by a DB constraint. Defensive choice: use .limit(1) until the partial unique index exists in the DB.

[igor-grubic]

Agreed — removed the pre-check queries that used .maybeSingle() on a non-unique column in both upsertBrand and updateBrand. The DB constraint handles uniqueness enforcement now instead.

[irenelagno]

This invariant lives in the wrong layer. The HTTP controller is only one of the upsertBrand call sites. The llmo onboarding handler in src/controllers/llmo/llmo-onboarding.js:1360 imports upsertBrand from src/support/brands-storage.js directly and bypasses this force-pend check entirely. Net effect: the seeded brand is created as active with site_id=NULL, violating the PR's stated invariant.

Consider moving this check into upsertBrand itself in brands-storage.js so it applies uniformly. Otherwise it's enforcement-by-coincidence — any new internal seed path silently reintroduces the bug.

[igor-grubic]

Fixed both parts:

1. Moved the force-pend logic into upsertBrand in brands-storage.js so it applies uniformly to all callers.
2. Added baseSiteId: site.getId() to the onboarding seeder payload — site is already in scope so seeded brands now correctly get a base site and stay active.

[github-actions]

This PR will trigger no release when merged.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[irenelagno]

Cross-PR review notes (from walking through DRS #1267 / UI #1479 / mysticat #329 together). Three things I'd want resolved before merge:

1. upsertBrand can silently downgrade an active brand to pending

In src/support/brands-storage.js:

const status = (!hasText(brand.baseSiteId) && (brand.status || 'active') === 'active')
    ? 'pending'
    : (brand.status || 'active');

Because the upsert conflict key is (organization_id, name) and status is always written into the row object, any caller that re-upserts an existing brand by name without passing baseSiteId will overwrite an existing active row's status to pending — even if the row already has site_id set in the DB. The guard only inspects the incoming payload, not the persisted state.

Today's known callers (onboarding + DRS brandalf sync) both pass baseSiteId, so it's fine now, but it's a landmine for the next caller. Two options:

• Read the current row's status/site_id inside upsertBrand and only force-pend on the insert path (not the update path), or
• Don't force status in JS at all — add a DB CHECK (status <> 'active' OR site_id IS NOT NULL) in the same migration as fix(deps): update dependency @adobe/spacecat-shared-rum-api-client to v2 #329 so the invariant is enforced once, at the right layer.

I'd lean toward the DB CHECK — it's a few lines and matches the stated invariant ("an active brand must have a base site") exactly.

2. Sub-brands are all forced to pending — confirm downstream is OK

baseSiteId is unique per org, so by definition only the primary brand in a brandalf set can have one. Every sub-brand (idx > 0 in DRS sync) will now land as pending. Two things to verify before merge:

• Does the brand presence pipeline filter on status='active'? If yes, sub-brands silently stop being analyzed after this lands. (Worth a grep in src/pipelines/brand_presence/ on the DRS side.)
• Does the brands listing / UI hide pending brands by default? Customers may perceive sub-brands as "disappeared".

If this is intentional and the pipeline handles it, a one-time backfill for existing active sub-brands may still be needed so nothing flips to pending on the next sync.

3. llmo-onboarding.js re-onboarding will now throw 409

performLlmoOnboarding unconditionally sets baseSiteId: site.getId(). If the same site is re-onboarded under a different brand name (or a second onboarding path touches a site that already has a brand), the partial unique index raises 23505 → upsertBrand throws a 409 Error. Please confirm the onboarding caller either catches this and falls through, or that re-onboarding is not a supported flow. Otherwise this is a regression on idempotency.

---

Everything else on this PR looks good — the force-pend move into upsertBrand, DB-backed uniqueness, 23505 → 409 mapping, read-only baseUrl strip, and the base_site:sites!site_id(id, base_url) join are all the right shape.

[igor-grubic]

Thanks for the thorough cross-PR review!

1. upsertBrand silently downgrading active → pending

Good catch — fixed. upsertBrand now queries the existing brand row by (organization_id, name) before deciding the status. If the DB row already has site_id set, we respect the persisted state and don't force-pend:

const { data: existing } = await postgrestClient
  .from('brands')
  .select('site_id')
  .eq('organization_id', organizationId)
  .eq('name', brand.name)
  .maybeSingle();

const hasBaseSite = hasText(brand.baseSiteId) || hasText(existing?.site_id);
const status = (!hasBaseSite && (brand.status || 'active') === 'active')
  ? 'pending'
  : (brand.status || 'active');

.maybeSingle() is safe here since (organization_id, name) has a unique constraint. Added a dedicated test for the re-upsert scenario — coverage stays at 100%.

2. Sub-brands forced to pending

Handled correctly in the ui

3. Re-onboarding 409

The upsertBrand call in llmo-onboarding.js is already wrapped in a try/catch that logs a warning and continues — so the onboarding flow won't break. Same-name re-onboard hits the (org_id, name) conflict key and updates in place (no 409). Different-name re-onboard for the same site would 409 but is caught gracefully — arguably correct since the site already has a primary brand.

[solaris007]

🎉 This PR is included in version 1.436.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀