From 876f29dc1241010c76c89e8ea110492f24c5e4e6 Mon Sep 17 00:00:00 2001 From: ppatwal Date: Tue, 31 Mar 2026 16:01:29 +0530 Subject: [PATCH 1/2] fix(disable-import-audit): persist site config via setConfig after disableImport Site save tracks updates through setters; mutating getConfig() alone did not reliably persist import changes. Mirror brand-profile by calling setConfig(Config.toDynamoItem(siteConfig)) before save. Tests mock Config.toDynamoItem and assert setConfig on success paths, and that setConfig is skipped when the site is not found or lookup fails. Made-with: Cursor --- .../disable-import-audit-processor/handler.js | 4 ++ .../disable-import-audit-processor.test.js | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/tasks/disable-import-audit-processor/handler.js b/src/tasks/disable-import-audit-processor/handler.js index 87564eb..9e5f82e 100644 --- a/src/tasks/disable-import-audit-processor/handler.js +++ b/src/tasks/disable-import-audit-processor/handler.js @@ -11,6 +11,8 @@ */ import { ok } from '@adobe/spacecat-shared-http-utils'; +import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js'; + import { say } from '../../utils/slack-utils.js'; const TASK_TYPE = 'disable-import-audit-processor'; @@ -55,6 +57,8 @@ export async function runDisableImportAuditProcessor(message, context) { siteConfig.disableImport(importType); } + site.setConfig(Config.toDynamoItem(siteConfig)); + const configuration = await Configuration.findLatest(); for (const auditType of auditTypes) { configuration.disableHandlerForSite(auditType, site); diff --git a/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js b/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js index 18ad6a6..71de32e 100644 --- a/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js +++ b/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js @@ -30,6 +30,13 @@ describe('Disable Import Audit Processor', () => { let mockSite; let mockSiteConfig; let mockConfiguration; + let toDynamoItemStub; + + const serializedConfigFixture = { + slack: {}, + handlers: {}, + imports: [], + }; beforeEach(async () => { // Reset all stubs @@ -41,11 +48,16 @@ describe('Disable Import Audit Processor', () => { // Mock the say function mockSay = sandbox.stub().resolves(); + toDynamoItemStub = sandbox.stub().returns(serializedConfigFixture); + // Dynamic import with mocked dependencies const handlerModule = await esmock('../../../src/tasks/disable-import-audit-processor/handler.js', { '../../../src/utils/slack-utils.js': { say: mockSay, }, + '@adobe/spacecat-shared-data-access/src/models/site/config.js': { + Config: { toDynamoItem: toDynamoItemStub }, + }, }); runDisableImportAuditProcessor = handlerModule.runDisableImportAuditProcessor; @@ -56,6 +68,7 @@ describe('Disable Import Audit Processor', () => { mockSite = { getConfig: sandbox.stub().returns(mockSiteConfig), + setConfig: sandbox.stub(), save: sandbox.stub().resolves(), }; @@ -114,6 +127,9 @@ describe('Disable Import Audit Processor', () => { expect(mockSiteConfig.disableImport).to.have.been.calledWith('screaming-frog'); expect(mockSiteConfig.disableImport).to.have.callCount(2); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); + // Verify audit types were disabled expect(context.dataAccess.Configuration.findLatest).to.have.been.calledOnce; expect(mockConfiguration.disableHandlerForSite).to.have.been.calledWith('cwv', mockSite); @@ -160,6 +176,9 @@ describe('Disable Import Audit Processor', () => { expect(mockSite.save).to.have.been.calledOnce; expect(mockConfiguration.save).to.have.been.calledOnce; + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); + // Verify Slack messages with "None" text expect(mockSay.firstCall).to.have.been.calledWith( context.env, @@ -190,6 +209,9 @@ describe('Disable Import Audit Processor', () => { ':broom: *For site: https://example.com: Disabled imports*: None *and audits*: None', ); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); + expect(result).to.exist; }); @@ -200,6 +222,8 @@ describe('Disable Import Audit Processor', () => { // Should complete without error expect(context.log.info).to.have.been.calledWith('For site: https://example.com: Disabled imports and audits'); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); expect(mockSay).to.have.been.calledTwice; expect(result).to.exist; }); @@ -275,6 +299,7 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); expect(context.log.error).to.have.been.calledWith('Error in disable import and audit processor:', sinon.match.instanceOf(Error)); + expect(mockSite.setConfig).to.not.have.been.called; expect(mockSite.save).to.not.have.been.called; expect(mockConfiguration.save).to.not.have.been.called; @@ -296,6 +321,8 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); expect(context.log.error).to.have.been.calledWith('Error in disable import and audit processor:', saveError); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); expect(mockSay).to.have.been.calledWith( context.env, context.log, @@ -313,6 +340,8 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); expect(context.log.error).to.have.been.calledWith('Error in disable import and audit processor:', saveError); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); expect(mockSay).to.have.been.calledWith( context.env, context.log, @@ -330,6 +359,7 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); expect(context.log.error).to.have.been.calledWith('Error in disable import and audit processor:', lookupError); + expect(mockSite.setConfig).to.not.have.been.called; expect(mockSay).to.have.been.calledWith( context.env, context.log, @@ -347,6 +377,8 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); expect(context.log.error).to.have.been.calledWith('Error in disable import and audit processor:', configError); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); expect(mockSay).to.have.been.calledWith( context.env, context.log, @@ -365,6 +397,8 @@ describe('Disable Import Audit Processor', () => { const result = await runDisableImportAuditProcessor(message, context); // Should still complete successfully despite Slack error + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); expect(mockSite.save).to.have.been.calledOnce; expect(mockConfiguration.save).to.have.been.calledOnce; expect(result).to.exist; @@ -421,6 +455,9 @@ describe('Disable Import Audit Processor', () => { ':broom: *For site: https://example.com: Disabled imports*: single-import *and audits*: single-audit', ); + expect(toDynamoItemStub).to.have.been.calledOnceWith(mockSiteConfig); + expect(mockSite.setConfig).to.have.been.calledOnceWith(serializedConfigFixture); + expect(result).to.exist; }); }); From cebcf9e4ae2465d5e5469af041d3c05ee83a98b1 Mon Sep 17 00:00:00 2001 From: ppatwal Date: Tue, 31 Mar 2026 21:53:21 +0530 Subject: [PATCH 2/2] fix: import Config from spacecat-shared-data-access public entry Use the package root instead of an internal src/models path so semver covers export stability. Align esmock with the new import specifier. Made-with: Cursor --- src/tasks/disable-import-audit-processor/handler.js | 2 +- .../disable-import-audit-processor.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tasks/disable-import-audit-processor/handler.js b/src/tasks/disable-import-audit-processor/handler.js index 9e5f82e..983030e 100644 --- a/src/tasks/disable-import-audit-processor/handler.js +++ b/src/tasks/disable-import-audit-processor/handler.js @@ -11,7 +11,7 @@ */ import { ok } from '@adobe/spacecat-shared-http-utils'; -import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js'; +import { Config } from '@adobe/spacecat-shared-data-access'; import { say } from '../../utils/slack-utils.js'; diff --git a/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js b/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js index 71de32e..5969f9b 100644 --- a/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js +++ b/test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js @@ -55,7 +55,7 @@ describe('Disable Import Audit Processor', () => { '../../../src/utils/slack-utils.js': { say: mockSay, }, - '@adobe/spacecat-shared-data-access/src/models/site/config.js': { + '@adobe/spacecat-shared-data-access': { Config: { toDynamoItem: toDynamoItemStub }, }, });