diff --git a/packages/spacecat-shared-data-access/src/models/opportunity/index.d.ts b/packages/spacecat-shared-data-access/src/models/opportunity/index.d.ts index ad49445a0..83af384c6 100644 --- a/packages/spacecat-shared-data-access/src/models/opportunity/index.d.ts +++ b/packages/spacecat-shared-data-access/src/models/opportunity/index.d.ts @@ -21,27 +21,31 @@ export interface Opportunity extends BaseModel { getAuditId(): string; getData(): object; getDescription(): string; + getFixEntities(): Promise; getGuidance(): string; + getLastAuditedAt(): string; getOrigin(): string; getRunbook(): string; + getScopeId(): string | undefined; + getScopeType(): string | undefined; getSite(): Promise; getSiteId(): string; getStatus(): string; - getFixEntities(): Promise; getSuggestions(): Promise; getSuggestionsByStatus(status: string): Promise; getSuggestionsByStatusAndRank(status: string, rank: string): Promise; - getLastAuditedAt(): string; getTags(): string[]; getTitle(): string; getType(): string; setAuditId(auditId: string): Opportunity; setData(data: object): Opportunity; setDescription(description: string): Opportunity; - setLastAuditedAt(lastAuditedAt: string): Opportunity; setGuidance(guidance: string): Opportunity; + setLastAuditedAt(lastAuditedAt: string): Opportunity; setOrigin(origin: string): Opportunity; setRunbook(runbook: string): Opportunity; + setScopeId(scopeId: string | null | undefined): Opportunity; + setScopeType(scopeType: 'brand' | null | undefined): Opportunity; setSiteId(siteId: string): Opportunity; setStatus(status: string): Opportunity; setTags(tags: string[]): Opportunity; @@ -49,6 +53,7 @@ export interface Opportunity extends BaseModel { } export interface OpportunityCollection extends BaseCollection { + allByScope(scopeType: string, scopeId: string): Promise; allByAuditId(auditId: string): Promise; allByAuditIdAndUpdatedAt(auditId: string, updatedAt: string): Promise; allBySiteId(siteId: string): Promise; diff --git a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.collection.js b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.collection.js index 185c2aae1..21d19ba9c 100644 --- a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.collection.js +++ b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.collection.js @@ -10,6 +10,9 @@ * governing permissions and limitations under the License. */ +import { hasText } from '@adobe/spacecat-shared-utils'; + +import { ValidationError } from '../../errors/index.js'; import BaseCollection from '../base/base.collection.js'; /** @@ -22,7 +25,91 @@ import BaseCollection from '../base/base.collection.js'; class OpportunityCollection extends BaseCollection { static COLLECTION_NAME = 'OpportunityCollection'; - // add custom methods here + /** + * Validates and creates a new Opportunity. Enforces that scopeType and scopeId + * must both be present or both be absent — a half-scoped record is invalid. + * + * @param {object} item - The opportunity data. + * @param {object} [options] - Optional create options (e.g. { upsert: true }). + * @returns {Promise} The created opportunity instance. + */ + async create(item, options) { + const { scopeType, scopeId } = item || {}; + if (hasText(scopeType) !== hasText(scopeId)) { + throw new ValidationError('scopeType and scopeId must both be set or both be absent', this); + } + return super.create(item, options); + } + + /** + * Validates and bulk-creates Opportunities. Enforces the scopeType/scopeId + * co-presence invariant on every item — half-scoped records are invalid. + * + * Overrides BaseCollection.createMany() which would otherwise bypass the + * single-item create() guard and persist invalid scope tuples directly. + * + * @param {object[]} items - The opportunity items to create. + * @param {...*} rest - Additional arguments forwarded to BaseCollection.createMany. + * @returns {Promise<*>} The result from BaseCollection.createMany. + */ + async createMany(items, ...rest) { + if (Array.isArray(items)) { + for (const item of items) { + const { scopeType, scopeId } = item || {}; + if (hasText(scopeType) !== hasText(scopeId)) { + throw new ValidationError('scopeType and scopeId must both be set or both be absent', this); + } + } + } + return super.createMany(items, ...rest); + } + + /** + * Validates and updates an Opportunity by its keys. Enforces the scopeType/scopeId + * co-presence invariant: if either field appears in the update payload, both must + * be set or both must be absent — a half-scoped update is invalid. + * + * Note: this guard enforces co-presence but allows scope re-attribution + * (moving an opportunity from one scopeId to another within the same scopeType). + * scopeId is the tenant boundary; callers must verify authorization before mutating it. + * + * @param {object} keys - The key attributes identifying the record. + * @param {object} updates - The fields to update. + * @returns {Promise} + */ + async updateByKeys(keys, updates) { + const hasScopeType = updates != null && 'scopeType' in updates; + const hasScopeId = updates != null && 'scopeId' in updates; + // If either field is included in the update, both must be included together, + // and their combined values must satisfy co-presence (both set or both absent). + if (hasScopeType !== hasScopeId) { + throw new ValidationError('scopeType and scopeId must both be set or both be absent', this); + } + if (hasScopeType && hasScopeId) { + const { scopeType, scopeId } = updates; + if (hasText(scopeType) !== hasText(scopeId)) { + throw new ValidationError('scopeType and scopeId must both be set or both be absent', this); + } + } + return super.updateByKeys(keys, updates); + } + + /** + * Returns all opportunities matching a given scope type and scope ID. + * + * @param {string} scopeType - The scope type (e.g. 'brand'). + * @param {string} scopeId - The scope entity UUID. + * @returns {Promise} The matching opportunities. + */ + async allByScope(scopeType, scopeId) { + if (!hasText(scopeType)) { + throw new Error('allByScope: scopeType is required'); + } + if (!hasText(scopeId)) { + throw new Error('allByScope: scopeId is required'); + } + return this.allByIndexKeys({ scopeType, scopeId }); + } } export default OpportunityCollection; diff --git a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.model.js b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.model.js index 1fc1af718..b4d585aef 100755 --- a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.model.js +++ b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.model.js @@ -10,6 +10,9 @@ * governing permissions and limitations under the License. */ +import { hasText } from '@adobe/spacecat-shared-utils'; + +import { ValidationError } from '../../errors/index.js'; import BaseModel from '../base/base.model.js'; /** @@ -37,6 +40,27 @@ class Opportunity extends BaseModel { RESOLVED: 'RESOLVED', }; + static SCOPE_TYPES = { + BRAND: 'brand', + }; + + /** + * Overrides BaseModel.save() to enforce the co-presence invariant: scopeType and scopeId + * must both be set or both be absent. Guards the setter+save path in addition to create(). + * + * @returns {Promise} The saved opportunity. + * @throws {ValidationError} When only one of scopeType / scopeId is set. + */ + async save() { + if (hasText(this.getScopeType()) !== hasText(this.getScopeId())) { + throw new ValidationError( + 'scopeType and scopeId must both be set or both be absent', + this, + ); + } + return super.save(); + } + /** * Adds the given suggestions to this Opportunity. Sets this opportunity as the parent * of each suggestion, as such the opportunity ID does not need to be provided. diff --git a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.schema.js b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.schema.js index 04241ff19..3f456f836 100644 --- a/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.schema.js +++ b/packages/spacecat-shared-data-access/src/models/opportunity/opportunity.schema.js @@ -12,7 +12,9 @@ /* c8 ignore start */ -import { isIsoDate, isNonEmptyObject, isValidUrl } from '@adobe/spacecat-shared-utils'; +import { + isIsoDate, isNonEmptyObject, isValidUrl, isValidUUID, +} from '@adobe/spacecat-shared-utils'; import SchemaBuilder from '../base/schema.builder.js'; import Opportunity from './opportunity.model.js'; @@ -64,6 +66,15 @@ const schema = new SchemaBuilder(Opportunity, OpportunityCollection) .addAttribute('lastAuditedAt', { type: 'string', validate: (value) => !value || isIsoDate(value), - }); + }) + .addAttribute('scopeType', { + type: 'string', + validate: (value) => !value || Object.values(Opportunity.SCOPE_TYPES).includes(value), + }) + .addAttribute('scopeId', { + type: 'string', + validate: (value) => !value || isValidUUID(value), + }) + .addIndex({ composite: ['scopeId'] }, { composite: ['scopeType'] }); export default schema.build(); diff --git a/packages/spacecat-shared-data-access/test/it/opportunity/opportunity.test.js b/packages/spacecat-shared-data-access/test/it/opportunity/opportunity.test.js index 2acdbdf6a..d918e71b7 100644 --- a/packages/spacecat-shared-data-access/test/it/opportunity/opportunity.test.js +++ b/packages/spacecat-shared-data-access/test/it/opportunity/opportunity.test.js @@ -383,6 +383,74 @@ describe('Opportunity IT', async () => { expect(record2).to.eql(data[1]); }); + describe('allByScope', () => { + const BRAND_A_ID = 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa'; + const BRAND_B_ID = 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbbbb'; + + const scopedOpportunityBase = { + siteId, + title: 'Scoped Opportunity', + description: 'Test for allByScope', + type: 'prerender', + origin: 'AI', + status: 'NEW', + updatedBy: 'system', + }; + + let oppA; + let oppB; + + after(async () => { + await Promise.all([oppA?.remove(), oppB?.remove()].filter(Boolean)); + }); + + it('returns only opportunities matching the given scope', async () => { + // Create one opp for brand-a and one for brand-b on the same site + oppA = await Opportunity.create({ + ...scopedOpportunityBase, + title: 'Brand A Opportunity', + scopeType: 'brand', + scopeId: BRAND_A_ID, + }); + oppB = await Opportunity.create({ + ...scopedOpportunityBase, + title: 'Brand B Opportunity', + scopeType: 'brand', + scopeId: BRAND_B_ID, + }); + + const resultA = await Opportunity.allByScope('brand', BRAND_A_ID); + const resultB = await Opportunity.allByScope('brand', BRAND_B_ID); + + const idSetA = resultA.map((o) => o.getId()); + const idSetB = resultB.map((o) => o.getId()); + + expect(idSetA).to.include(oppA.getId()); + expect(idSetA).to.not.include(oppB.getId()); + + expect(idSetB).to.include(oppB.getId()); + expect(idSetB).to.not.include(oppA.getId()); + }); + + it('returns an empty array when no opportunities match the scopeId', async () => { + const UNKNOWN_BRAND_ID = 'cccccccc-cccc-4ccc-cccc-cccccccccccc'; + const result = await Opportunity.allByScope('brand', UNKNOWN_BRAND_ID); + expect(result).to.be.an('array').with.length(0); + }); + + it('rejects co-presence violation: scopeType set without scopeId', async () => { + await expect( + Opportunity.create({ ...scopedOpportunityBase, scopeType: 'brand' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('rejects co-presence violation: scopeId set without scopeType', async () => { + await expect( + Opportunity.create({ ...scopedOpportunityBase, scopeId: BRAND_A_ID }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + }); + describe('addFixEntities', () => { it('creates fix entities with valid suggestions', async () => { const opportunity = await Opportunity.findById(sampleData.opportunities[2].getId()); diff --git a/packages/spacecat-shared-data-access/test/it/postgrest/docker-compose.yml b/packages/spacecat-shared-data-access/test/it/postgrest/docker-compose.yml index 777bf436b..a03341f9e 100644 --- a/packages/spacecat-shared-data-access/test/it/postgrest/docker-compose.yml +++ b/packages/spacecat-shared-data-access/test/it/postgrest/docker-compose.yml @@ -17,7 +17,20 @@ services: data-service: platform: ${MYSTICAT_DATA_SERVICE_PLATFORM:-linux/amd64} - image: ${MYSTICAT_DATA_SERVICE_REPOSITORY:-682033462621.dkr.ecr.us-east-1.amazonaws.com/mysticat-data-service}:${MYSTICAT_DATA_SERVICE_TAG:-v5.1.1} + # Default tag is bumped here whenever a new schema migration requires a matching data-service + # image. The tag is supplemented by an immutable digest so CI cannot silently consume a + # re-pushed tag — if ECR re-publishes v5.1.1 with a different binary, the digest mismatch + # causes a hard pull failure instead of silent drift. + # + # To obtain or refresh the digest: + # aws ecr describe-images \ + # --profile --region us-east-1 \ + # --repository-name mysticat-data-service \ + # --image-ids imageTag=v5.1.1 \ + # --query 'imageDetails[0].imageDigest' --output text + # Then export MYSTICAT_DATA_SERVICE_DIGEST=sha256: before npm run test:it, + # or set it in the .env file consumed by docker compose. Leave unset for local dev. + image: ${MYSTICAT_DATA_SERVICE_REPOSITORY:-682033462621.dkr.ecr.us-east-1.amazonaws.com/mysticat-data-service}:${MYSTICAT_DATA_SERVICE_TAG:-v5.1.1}${MYSTICAT_DATA_SERVICE_DIGEST:+@${MYSTICAT_DATA_SERVICE_DIGEST}} depends_on: db: condition: service_healthy diff --git a/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.collection.test.js b/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.collection.test.js index 7db3f1e65..afa2e731f 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.collection.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.collection.test.js @@ -12,6 +12,7 @@ import { expect, use as chaiUse } from 'chai'; import chaiAsPromised from 'chai-as-promised'; +import { stub } from 'sinon'; import sinonChai from 'sinon-chai'; import Opportunity from '../../../../src/models/opportunity/opportunity.model.js'; @@ -70,4 +71,199 @@ describe('OpportunityCollection', () => { expect(model).to.be.an('object'); }); }); + + describe('create', () => { + it('throws ValidationError when scopeType is set but scopeId is absent', async () => { + await expect( + instance.create({ type: 'content', scopeType: 'brand' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('throws ValidationError when scopeId is set but scopeType is absent', async () => { + await expect( + instance.create({ type: 'content', scopeId: '11111111-1111-1111-1111-111111111111' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('passes co-presence check when both scopeType and scopeId are set', async () => { + // Co-presence check passes — mock create succeeds without complaining. + await expect( + instance.create({ scopeType: 'brand', scopeId: '11111111-1111-1111-1111-111111111111' }), + ).to.be.fulfilled; + }); + + it('passes co-presence check when neither scopeType nor scopeId is set', async () => { + // Neither present — co-presence check passes and mock create succeeds. + await expect( + instance.create({ type: 'content' }), + ).to.be.fulfilled; + }); + + it('handles null item gracefully by delegating to super.create', async () => { + // null item: co-presence check skips (both undefined → equal), super.create handles it. + await expect(instance.create(null)).to.be.rejectedWith(/Failed to create/); + }); + }); + + describe('createMany (scope co-presence)', () => { + let superCreateManyStub; + + beforeEach(() => { + // Stub the inherited createMany so the ElectroDB batch chain is not invoked. + // The per-item co-presence guard runs before super.createMany, so stubbing + // the super lets us isolate the validation logic. + superCreateManyStub = stub(Object.getPrototypeOf(Object.getPrototypeOf(instance)), 'createMany').resolves({ createdItems: [] }); + }); + + afterEach(() => { + superCreateManyStub.restore(); + }); + + it('rejects when any item has scopeType set without scopeId', async () => { + await expect( + instance.createMany([ + { type: 'content', scopeType: 'brand', scopeId: '11111111-1111-1111-1111-111111111111' }, + { type: 'content', scopeType: 'brand' }, + ]), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + expect(superCreateManyStub).to.not.have.been.called; + }); + + it('rejects when any item has scopeId set without scopeType', async () => { + await expect( + instance.createMany([ + { type: 'content', scopeId: '11111111-1111-1111-1111-111111111111' }, + ]), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + expect(superCreateManyStub).to.not.have.been.called; + }); + + it('delegates to super.createMany when all items pass co-presence', async () => { + const items = [ + { type: 'content' }, + { type: 'content', scopeType: 'brand', scopeId: '11111111-1111-1111-1111-111111111111' }, + ]; + await expect(instance.createMany(items)).to.be.fulfilled; + expect(superCreateManyStub).to.have.been.calledOnce; + expect(superCreateManyStub.firstCall.args[0]).to.equal(items); + }); + + it('delegates to super.createMany when items is not an array', async () => { + // Non-array input bypasses per-item validation and is handled by the super class. + await expect(instance.createMany(null)).to.be.fulfilled; + expect(superCreateManyStub).to.have.been.calledOnceWith(null); + }); + }); + + describe('updateByKeys', () => { + const KEYS = { opportunityId: 'op12345', siteId: 'site67890' }; + let superUpdateByKeysStub; + + beforeEach(() => { + // Stub the inherited updateByKeys so the ElectroDB patch chain is not invoked. + // The co-presence guard runs before super.updateByKeys, so stubbing the super + // lets us isolate the validation logic without needing a full ElectroDB mock. + superUpdateByKeysStub = stub(Object.getPrototypeOf(Object.getPrototypeOf(instance)), 'updateByKeys').resolves(); + }); + + afterEach(() => { + superUpdateByKeysStub.restore(); + }); + + it('throws ValidationError when updating scopeType without scopeId', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: 'brand' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('throws ValidationError when updating scopeId without scopeType', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeId: '11111111-1111-1111-1111-111111111111' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('throws ValidationError when clearing only one scope field', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: null }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('throws ValidationError when both keys are present but scopeId value is null', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: 'brand', scopeId: null }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('throws ValidationError when both keys are present but scopeType value is null', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: null, scopeId: '11111111-1111-1111-1111-111111111111' }), + ).to.be.rejectedWith('scopeType and scopeId must both be set or both be absent'); + }); + + it('passes co-presence check when setting both scopeType and scopeId', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: 'brand', scopeId: '11111111-1111-1111-1111-111111111111' }), + ).to.be.fulfilled; + }); + + it('passes co-presence check when clearing both scopeType and scopeId', async () => { + await expect( + instance.updateByKeys(KEYS, { scopeType: null, scopeId: null }), + ).to.be.fulfilled; + }); + + it('passes co-presence check when update does not touch scope fields', async () => { + await expect( + instance.updateByKeys(KEYS, { title: 'Updated title' }), + ).to.be.fulfilled; + }); + }); + + describe('schema validate guards', () => { + it('rejects scopeType values not in SCOPE_TYPES', () => { + const { scopeType } = schema.getAttributes(); + expect(scopeType.validate('page')).to.be.false; + expect(scopeType.validate('brand')).to.be.true; + expect(scopeType.validate(null)).to.be.true; + }); + + it('rejects scopeId values that are not valid UUIDs', () => { + const { scopeId } = schema.getAttributes(); + expect(scopeId.validate('not-a-uuid')).to.be.false; + expect(scopeId.validate('11111111-1111-1111-1111-111111111111')).to.be.true; + expect(scopeId.validate(null)).to.be.true; + }); + }); + + describe('allByScope', () => { + it('throws an error if scopeType is not provided', async () => { + await expect(instance.allByScope()).to.be.rejectedWith('allByScope: scopeType is required'); + }); + + it('throws an error if scopeId is not provided', async () => { + await expect(instance.allByScope('brand')).to.be.rejectedWith('allByScope: scopeId is required'); + }); + + it('delegates to allByIndexKeys with the correct arguments', async () => { + const mockOpportunity = { getOpportunityId: () => 'op-111' }; + instance.allByIndexKeys = stub().resolves([mockOpportunity]); + + const BRAND_UUID = 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa'; + const result = await instance.allByScope('brand', BRAND_UUID); + + expect(instance.allByIndexKeys).to.have.been.calledOnceWith({ + scopeType: 'brand', + scopeId: BRAND_UUID, + }); + expect(result).to.deep.equal([mockOpportunity]); + }); + + it('returns an empty array when no opportunities match the scope', async () => { + instance.allByIndexKeys = stub().resolves([]); + + const result = await instance.allByScope('brand', 'cccccccc-cccc-4ccc-cccc-cccccccccccc'); + + expect(result).to.be.an('array').with.lengthOf(0); + }); + }); }); diff --git a/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.model.test.js b/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.model.test.js index 2db66a24a..eb9322f6d 100755 --- a/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.model.test.js +++ b/packages/spacecat-shared-data-access/test/unit/models/opportunity/opportunity.model.test.js @@ -384,4 +384,94 @@ describe('OpportunityModel', () => { expect(instance.record.data).to.deep.equal({ newInfo: 'updatedInfo' }); }); }); + + describe('getScopeType and setScopeType', () => { + it('returns undefined when scopeType is not set', () => { + expect(instance.getScopeType()).to.be.undefined; + }); + + it('returns the scopeType value', () => { + mockRecord.scopeType = 'brand'; + expect(instance.getScopeType()).to.equal('brand'); + }); + + it('sets the scopeType value', () => { + instance.setScopeType('brand'); + expect(instance.record.scopeType).to.equal('brand'); + }); + }); + + describe('getScopeId and setScopeId', () => { + it('returns undefined when scopeId is not set', () => { + expect(instance.getScopeId()).to.be.undefined; + }); + + it('returns the scopeId value', () => { + mockRecord.scopeId = 'brand-uuid-123'; + expect(instance.getScopeId()).to.equal('brand-uuid-123'); + }); + + it('sets the scopeId value', () => { + instance.setScopeId('brand-uuid-456'); + expect(instance.record.scopeId).to.equal('brand-uuid-456'); + }); + }); + + describe('SCOPE_TYPES', () => { + it('defines expected scope type constants', () => { + expect(Opportunity.SCOPE_TYPES.BRAND).to.equal('brand'); + expect(Opportunity.SCOPE_TYPES.SITE).to.be.undefined; + }); + }); + + describe('save (scope co-presence)', () => { + it('throws ValidationError when scopeType is set but scopeId is absent', async () => { + instance.setScopeType('brand'); + await expect(instance.save()).to.be.rejectedWith( + 'scopeType and scopeId must both be set or both be absent', + ); + }); + + it('throws ValidationError when scopeId is set but scopeType is absent', async () => { + instance.setScopeId('11111111-1111-1111-1111-111111111111'); + await expect(instance.save()).to.be.rejectedWith( + 'scopeType and scopeId must both be set or both be absent', + ); + }); + + it('delegates to super.save() when both scopeType and scopeId are set', async () => { + instance.setScopeType('brand'); + instance.setScopeId('11111111-1111-1111-1111-111111111111'); + const patcherSaveStub = stub(instance.patcher, 'save').resolves(); + await expect(instance.save()).to.be.fulfilled; + expect(patcherSaveStub.calledOnce).to.be.true; + }); + + it('delegates to super.save() when neither scopeType nor scopeId is set', async () => { + const patcherSaveStub = stub(instance.patcher, 'save').resolves(); + await expect(instance.save()).to.be.fulfilled; + expect(patcherSaveStub.calledOnce).to.be.true; + }); + + it('throws ValidationError when a fully-scoped record has scopeId cleared', async () => { + // Realistic regression scenario: a record loaded from DB has both scope fields set, + // and a caller clears only scopeId via the setter. The save() guard must catch the + // half-scoped state before the patch reaches the DB. + instance.record.scopeType = 'brand'; + instance.record.scopeId = '11111111-1111-1111-1111-111111111111'; + instance.setScopeId(null); + await expect(instance.save()).to.be.rejectedWith( + 'scopeType and scopeId must both be set or both be absent', + ); + }); + + it('throws ValidationError when a fully-scoped record has scopeType cleared', async () => { + instance.record.scopeType = 'brand'; + instance.record.scopeId = '11111111-1111-1111-1111-111111111111'; + instance.setScopeType(null); + await expect(instance.save()).to.be.rejectedWith( + 'scopeType and scopeId must both be set or both be absent', + ); + }); + }); });