diff --git a/docs/llmo-brandalf-apis/brands-api.md b/docs/llmo-brandalf-apis/brands-api.md index 9479f3efb..eb502ca35 100644 --- a/docs/llmo-brandalf-apis/brands-api.md +++ b/docs/llmo-brandalf-apis/brands-api.md @@ -35,6 +35,8 @@ All brand endpoints return and accept the same shape: { "id": "019cb903-1184-742b-9a16-bc7a8696962f", "name": "Adobe", + "baseSiteId": "c2473d89-e997-458d-a86d-b4096649c12b", + "baseUrl": "https://adobe.com", "status": "active", "origin": "human", "description": "Adobe Inc.", @@ -65,7 +67,9 @@ All brand endpoints return and accept the same shape: ``` **Field notes:** -- `status` — `active` (default), `pending`, or `deleted`; use `pending` for brands awaiting review +- `baseSiteId` — UUID of the brand's primary site. Optional on create; **immutable** once set (ignored on PATCH). Must be unique per organization (409 if already taken). Brands created without `baseSiteId` are forced to `pending` status. +- `baseUrl` — read-only, resolved from `baseSiteId` (the site's `base_url`). Not accepted on write. +- `status` — `active` (default), `pending`, or `deleted`; use `pending` for brands awaiting review. Forced to `pending` if `baseSiteId` is not provided on create. - `origin` — `human` (default) or `ai` - `region` — ISO 3166-1 alpha-2 country codes (e.g. `US`, `GB`) - `urls` — brand site URLs, optionally with paths (e.g. `https://adobe.com/products`); matched against the org's known sites to populate `siteIds`. Multiple paths under the same base URL share one `brand_sites` row. @@ -114,7 +118,8 @@ Creates a brand. Uses `organization_id + name` as the upsert conflict key — po | Field | Required | Description | |-------|----------|-------------| | `name` | yes | Brand name (unique per org) | -| `status` | no | `active` (default) or `pending` | +| `baseSiteId` | no | UUID of the primary site for this brand. Immutable once set. Unique per org (409 if taken). If omitted, status is forced to `pending`. | +| `status` | no | `active` (default) or `pending`. Forced to `pending` if `baseSiteId` is not provided. | | `origin` | no | `human` (default) or `ai` | | `description` | no | Free-text description | | `vertical` | no | Industry vertical | @@ -137,7 +142,9 @@ Partially updates a brand. Only fields present in the request body are modified. **Important:** All child arrays (`brandAliases`, `competitors`, `socialAccounts`, `earnedContent`, `urls`) use **full replace semantics** — when a field is present in the request body, all existing entries for that field are deleted and replaced with the submitted list. Omit a field entirely to leave it unchanged. -**Request body:** Any subset of the brand fields listed in the POST section above. +**Note:** `baseSiteId` is immutable — if the brand already has a `baseSiteId`, any value sent via PATCH is silently ignored. If the brand does not yet have a `baseSiteId`, it can be set via PATCH (subject to uniqueness validation). `baseUrl` is read-only and stripped from PATCH payloads. + +**Request body:** Any subset of the brand fields listed in the POST section above (except `baseSiteId` which is immutable once set, and `baseUrl` which is read-only). **Response:** Updated brand object (`200`), or `404` if not found. @@ -166,6 +173,7 @@ Prompt management endpoints are documented separately. See [Prompts Management A | 400 | Missing or invalid `spaceCatId` (not a UUID); missing `brandId` or `name` | | 403 | User does not have access to the organization | | 404 | Organization not found; brand not found | +| 409 | `baseSiteId` is already assigned as the primary URL for another brand in this org | | 503 | PostgREST service unavailable (V2 config requires Postgres) | | 500 | Unexpected storage error | diff --git a/docs/openapi/schemas.yaml b/docs/openapi/schemas.yaml index 85aa9bdca..958491fb9 100644 --- a/docs/openapi/schemas.yaml +++ b/docs/openapi/schemas.yaml @@ -3565,6 +3565,14 @@ Brand: name: description: The name of the brand type: string + baseSiteId: + description: UUID of the brand's primary site. Immutable once set. Unique per organization. + type: string + format: uuid + baseUrl: + description: Read-only. The base URL of the primary site, resolved from baseSiteId. + type: string + readOnly: true imsOrgId: description: The IMS Organization ID of the brand $ref: '#/ImsOrganizationId' diff --git a/docs/reference/llmo-brand-presence-schema-and-v2-config-writeup.md b/docs/reference/llmo-brand-presence-schema-and-v2-config-writeup.md index 872837e60..f33d46d2d 100644 --- a/docs/reference/llmo-brand-presence-schema-and-v2-config-writeup.md +++ b/docs/reference/llmo-brand-presence-schema-and-v2-config-writeup.md @@ -10,7 +10,7 @@ Postgres schema for brand presence. V2 customer config brands are synced to `bra |-------------------|-----------|----------|--------| | id | uuid | no | Default uuid_generate_v7() | | organization_id | uuid | yes | FK organizations(id). Unique (organization_id, name). | -| site_id | uuid | no | Deprecated; nullable. FK sites(id). | +| site_id | uuid | no | Base site ID (primary URL). FK sites(id). Immutable once set; unique per organization. Brands without a `site_id` are forced to `pending` status on creation. Exposed in the API as `baseSiteId`. | | name | text | yes | Unique per organization. | | status | text | yes | Default 'active'. CHECK: 'pending' \| 'active' \| 'deleted'. | | origin | category_origin | yes | Default 'human'. Enum: 'human' \| 'ai'. | diff --git a/src/controllers/brands.js b/src/controllers/brands.js index 2c1b94eea..5c877c853 100644 --- a/src/controllers/brands.js +++ b/src/controllers/brands.js @@ -1130,6 +1130,9 @@ function BrandsController(ctx, log, env) { return notFound(`Brand not found: ${brandId}`); } + // baseUrl is read-only (resolved from baseSiteId) — strip from updates. + delete updates.baseUrl; + const updated = await updateBrand({ organizationId: spaceCatId, brandId: brandUuid, diff --git a/src/controllers/llmo/llmo-onboarding.js b/src/controllers/llmo/llmo-onboarding.js index 304e3ac33..d8b90d9ec 100644 --- a/src/controllers/llmo/llmo-onboarding.js +++ b/src/controllers/llmo/llmo-onboarding.js @@ -1362,6 +1362,7 @@ export async function performLlmoOnboarding(params, context, say = () => {}) { brand: { name: brandName.trim(), status: 'active', + baseSiteId: site.getId(), urls: [{ value: baseURL, type: 'url' }], brandAliases: [{ name: brandName.trim(), regions: ['gl'] }], }, diff --git a/src/support/brands-storage.js b/src/support/brands-storage.js index c925a08cb..21806e477 100644 --- a/src/support/brands-storage.js +++ b/src/support/brands-storage.js @@ -17,6 +17,7 @@ import { composeBaseURL, hasText } from '@adobe/spacecat-shared-utils'; */ const BRAND_SELECT = [ '*', + 'base_site:sites!site_id(id, base_url)', 'brand_aliases(alias, regions)', 'brand_social_accounts(url, regions)', 'brand_earned_sources(name, url, regions)', @@ -69,6 +70,8 @@ function mapDbBrandToV2(row) { return { id: row.id, name: row.name, + baseSiteId: row.base_site?.id || row.site_id || null, + baseUrl: row.base_site?.base_url || null, status: row.status || 'active', origin: row.origin || 'human', description: row.description || null, @@ -362,10 +365,27 @@ export async function upsertBrand({ const regions = (brand.region || []) .map((r) => (typeof r === 'string' ? r : String(r))).filter(hasText); + // Check if the brand already exists with a base site set. + // This prevents silently downgrading an active brand to pending when a caller + // re-upserts by name without passing baseSiteId. + const { data: existing } = await postgrestClient + .from('brands') + .select('site_id') + .eq('organization_id', organizationId) + .eq('name', brand.name) + .maybeSingle(); + + // A brand cannot be active without a base site ID — but respect persisted state + // on the update path (the DB row may already have site_id set). + const hasBaseSite = hasText(brand.baseSiteId) || hasText(existing?.site_id); + const status = (!hasBaseSite && (brand.status || 'active') === 'active') + ? 'pending' + : (brand.status || 'active'); + const row = { organization_id: organizationId, name: brand.name, - status: brand.status || 'active', + status, origin: brand.origin || 'human', description: brand.description || null, vertical: brand.vertical || null, @@ -376,6 +396,11 @@ export async function upsertBrand({ updated_by: updatedBy, }; + // Set base site ID if provided. + if (hasText(brand.baseSiteId)) { + row.site_id = brand.baseSiteId; + } + const { data: upserted, error } = await postgrestClient .from('brands') .upsert(row, { onConflict: 'organization_id,name' }) @@ -383,6 +408,11 @@ export async function upsertBrand({ .single(); if (error) { + if (error.code === '23505' && error.message?.includes('brands_base_site_unique')) { + const err = new Error('This site is already the primary URL for another brand'); + err.status = 409; + throw err; + } throw new Error(`Failed to upsert brand: ${error.message}`); } @@ -442,6 +472,21 @@ export async function updateBrand({ patch.vertical = updates.vertical; } + // baseSiteId is immutable once set — only allow setting from NULL. + // The DB partial unique index (brands_base_site_unique) enforces uniqueness. + if (hasText(updates.baseSiteId)) { + const { data: current } = await postgrestClient + .from('brands') + .select('site_id') + .eq('id', brandId) + .maybeSingle(); + + if (!current?.site_id) { + patch.site_id = updates.baseSiteId; + } + // If site_id is already set, silently ignore the update (immutable). + } + if (updates.region !== undefined) { patch.regions = (updates.region || []) .map((r) => (typeof r === 'string' ? r : String(r))).filter(hasText); @@ -460,6 +505,11 @@ export async function updateBrand({ .maybeSingle(); if (error) { + if (error.code === '23505' && error.message?.includes('brands_base_site_unique')) { + const err = new Error('This site is already the primary URL for another brand'); + err.status = 409; + throw err; + } throw new Error(`Failed to update brand: ${error.message}`); } if (!data) { diff --git a/test/support/brands-storage.test.js b/test/support/brands-storage.test.js index 41b5696cf..d2ceb1e33 100644 --- a/test/support/brands-storage.test.js +++ b/test/support/brands-storage.test.js @@ -269,6 +269,20 @@ describe('brands-storage', () => { expect(result.urls).to.deep.equal([{ value: 'https://x.com' }]); }); + it('uses base_site join for baseSiteId and baseUrl when available', async () => { + const dbRow = makeBrandRow({ + base_site: { id: 'joined-site-id', base_url: 'https://joined.com' }, + site_id: 'fallback-site-id', + }); + + const query = createChainableQuery({ data: dbRow, error: null }); + const postgrestClient = { from: sinon.stub().returns(query) }; + + const result = await getBrandById(ORG_ID, BRAND_ID, postgrestClient); + expect(result.baseSiteId).to.equal('joined-site-id'); + expect(result.baseUrl).to.equal('https://joined.com'); + }); + it('returns null when brand not found', async () => { const query = createChainableQuery({ data: null, error: null }); const postgrestClient = { from: sinon.stub().returns(query) }; @@ -387,6 +401,63 @@ describe('brands-storage', () => { })).to.be.rejectedWith('Failed to upsert brand: upsert failed'); }); + it('throws 409 when baseSiteId violates unique constraint on upsert', async () => { + const postgrestClient = createTableMockClient({ + brands: { data: null, error: { code: '23505', message: 'brands_base_site_unique' } }, + }); + + const err = await upsertBrand({ + organizationId: ORG_ID, + brand: { name: 'Test', baseSiteId: 'some-site-id' }, + postgrestClient, + }).catch((e) => e); + + expect(err.message).to.equal('This site is already the primary URL for another brand'); + expect(err.status).to.equal(409); + }); + + it('does not downgrade active brand to pending when re-upserting without baseSiteId', async () => { + const fullBrandRow = makeBrandRow({ name: 'Test', status: 'active', site_id: 'existing-site-id' }); + + const postgrestClient = createTableMockClient({ + brands: [ + // existing brand lookup — row already has site_id + { data: { site_id: 'existing-site-id' }, error: null }, + // upsert result + { data: { id: BRAND_ID, name: 'Test' }, error: null }, + // getBrandById result + { data: fullBrandRow, error: null }, + ], + }); + + const result = await upsertBrand({ + organizationId: ORG_ID, + brand: { name: 'Test' }, // no baseSiteId + postgrestClient, + }); + + expect(result.status).to.equal('active'); + }); + + it('sets site_id in upsert row when baseSiteId is provided', async () => { + const fullBrandRow = makeBrandRow({ name: 'Test', site_id: 'site-uuid' }); + + const postgrestClient = createTableMockClient({ + brands: [ + { data: { id: BRAND_ID, name: 'Test' }, error: null }, + { data: fullBrandRow, error: null }, + ], + }); + + const result = await upsertBrand({ + organizationId: ORG_ID, + brand: { name: 'Test', baseSiteId: 'site-uuid' }, + postgrestClient, + }); + + expect(result).to.include({ id: BRAND_ID, name: 'Test' }); + }); + it('successfully upserts a minimal brand with no aliases, competitors, or urls', async () => { const fullBrandRow = makeBrandRow({ name: 'Test' }); @@ -1080,6 +1151,75 @@ describe('brands-storage', () => { })).to.be.rejectedWith('Failed to update brand: update failed'); }); + it('throws 409 when baseSiteId violates unique constraint', async () => { + const postgrestClient = createTableMockClient({ + brands: [ + // 1st call: select current site_id (null → allow setting) + { data: { site_id: null }, error: null }, + // 2nd call: update fails with unique constraint + { data: null, error: { code: '23505', message: 'brands_base_site_unique' } }, + ], + }); + + const err = await updateBrand({ + organizationId: ORG_ID, + brandId: BRAND_ID, + updates: { baseSiteId: 'some-site-id' }, + postgrestClient, + }).catch((e) => e); + + expect(err.message).to.equal('This site is already the primary URL for another brand'); + expect(err.status).to.equal(409); + }); + + it('sets baseSiteId when brand has no site_id yet', async () => { + const fullBrandRow = makeBrandRow({ site_id: 'new-site-id' }); + + const postgrestClient = createTableMockClient({ + brands: [ + // 1st call: select current site_id (null → allow setting) + { data: { site_id: null }, error: null }, + // 2nd call: update succeeds + { data: { id: BRAND_ID }, error: null }, + // 3rd call: getBrandById re-fetch + { data: fullBrandRow, error: null }, + ], + }); + + const result = await updateBrand({ + organizationId: ORG_ID, + brandId: BRAND_ID, + updates: { baseSiteId: 'new-site-id' }, + postgrestClient, + }); + + expect(result).to.not.be.null; + }); + + it('ignores baseSiteId when brand already has a site_id (immutable)', async () => { + const fullBrandRow = makeBrandRow({ site_id: 'existing-site-id' }); + + const postgrestClient = createTableMockClient({ + brands: [ + // 1st call: select current site_id (already set → ignore) + { data: { site_id: 'existing-site-id' }, error: null }, + // 2nd call: update succeeds (without site_id in patch) + { data: { id: BRAND_ID }, error: null }, + // 3rd call: getBrandById re-fetch + { data: fullBrandRow, error: null }, + ], + }); + + const result = await updateBrand({ + organizationId: ORG_ID, + brandId: BRAND_ID, + updates: { baseSiteId: 'different-site-id' }, + postgrestClient, + }); + + expect(result).to.not.be.null; + }); + it('successfully updates scalar fields (name, status, origin, description, vertical)', async () => { const fullBrandRow = makeBrandRow({ name: 'NewName',