Skip to content

fix: persist site config via setConfig after disableImport #240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
prithipalpatwal wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −0
Open

fix: persist site config via setConfig after disableImport #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/tasks/disable-import-audit-processor/handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down
37 changes: 37 additions & 0 deletions test/tasks/disable-import-audit-processor/disable-import-audit-processor.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

Expand All @@ -56,6 +68,7 @@ describe('Disable Import Audit Processor', () => {

mockSite = {
getConfig: sandbox.stub().returns(mockSiteConfig),
setConfig: sandbox.stub(),
save: sandbox.stub().resolves(),
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
});

Expand All @@ -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;
});
Expand Down Expand Up @@ -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;

Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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;
});
});
Expand Down
Loading