From 4c7ddaa52649ad9d7e9e624b87726e390183e36e Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Wed, 18 Mar 2026 15:22:11 -0500 Subject: [PATCH 1/3] Add disclaimer about onboard status --- .../opportunity-status-processor/handler.js | 79 ++++++++++++++++++- 1 file changed, 75 insertions(+), 4 deletions(-) diff --git a/src/tasks/opportunity-status-processor/handler.js b/src/tasks/opportunity-status-processor/handler.js index 659958da..9597b12b 100644 --- a/src/tasks/opportunity-status-processor/handler.js +++ b/src/tasks/opportunity-status-processor/handler.js @@ -241,11 +241,51 @@ async function isScrapingAvailable(baseUrl, context, onboardStartTime) { } /** - * Checks scrape results for bot protection blocking - * @param {Array} scrapeResults - Array of scrape URL results - * @param {object} context - The context object with log - * @returns {object|null} Bot protection details if detected, null otherwise + * Checks which audit types have completed since onboardStartTime by querying the database. + * An audit is considered completed if a record exists with auditedAt >= onboardStartTime. + * Falls back conservatively (all pending) if the DB query fails. + * + * @param {string} siteId - The site ID + * @param {Array} auditTypes - Audit types expected for this onboard session + * @param {number} onboardStartTime - Onboarding start timestamp in ms + * @param {object} dataAccess - Data access object + * @param {object} log - Logger + * @returns {Promise<{pendingAuditTypes: Array, completedAuditTypes: Array}>} */ +async function checkAuditCompletionFromDB(siteId, auditTypes, onboardStartTime, dataAccess, log) { + const pendingAuditTypes = []; + const completedAuditTypes = []; + try { + const { Audit } = dataAccess; + const latestAudits = await Audit.allLatestForSite(siteId); + const auditsByType = {}; + if (latestAudits) { + for (const audit of latestAudits) { + auditsByType[audit.getAuditType()] = audit; + } + } + for (const auditType of auditTypes) { + const audit = auditsByType[auditType]; + if (!audit) { + pendingAuditTypes.push(auditType); + } else { + const auditedAt = new Date(audit.getAuditedAt()).getTime(); + if (onboardStartTime && auditedAt < onboardStartTime) { + // Record exists but predates this onboard session — treat as pending + pendingAuditTypes.push(auditType); + } else { + completedAuditTypes.push(auditType); + } + } + } + } catch (error) { + log.warn(`Could not check audit completion from DB for site ${siteId}: ${error.message}`); + // Conservative fallback: mark all as pending so disclaimer is always shown on error + pendingAuditTypes.push(...auditTypes.filter((t) => !completedAuditTypes.includes(t))); + } + return { pendingAuditTypes, completedAuditTypes }; +} + /** * Analyzes missing opportunities and determines the root cause * @param {Array} missingOpportunities - Array of missing opportunity types @@ -680,6 +720,37 @@ export async function runOpportunityStatusProcessor(message, context) { } else { await say(env, log, slackContext, 'No audit errors found'); } + + // Audit completion disclaimer — check DB for pending audits and warn if any are still running + if (auditTypes.length > 0) { + const { pendingAuditTypes } = await checkAuditCompletionFromDB( + siteId, + auditTypes, + onboardStartTime, + dataAccess, + log, + ); + const isRecheck = taskContext?.isRecheck === true; + if (pendingAuditTypes.length > 0) { + const pendingList = pendingAuditTypes.map(getOpportunityTitle).join(', '); + await say( + env, + log, + slackContext, + `:warning: *Heads-up:* The following audit${pendingAuditTypes.length > 1 ? 's' : ''} ` + + `may still be in progress: *${pendingList}*.\n` + + 'The statuses above reflect data available at this moment and may be incomplete. ' + + `Run \`onboard status ${siteUrl}\` to re-check once all audits have completed.`, + ); + } else if (isRecheck) { + await say( + env, + log, + slackContext, + ':white_check_mark: All audits have completed. The statuses above are up to date.', + ); + } + } } log.info(`Processed ${opportunities.length} opportunities for site ${siteId}`); From e53ccd5646189d6a655f3cfccf99d3df7e3843e3 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Wed, 18 Mar 2026 15:49:55 -0500 Subject: [PATCH 2/3] tests --- .../opportunity-status-processor.test.js | 204 ++++++++++++++++++ 1 file changed, 204 insertions(+) diff --git a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js index 8434c86a..1619086a 100644 --- a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js +++ b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js @@ -4122,4 +4122,208 @@ describe('Opportunity Status Processor', () => { ); }); }); + + describe('Audit Completion Disclaimer', () => { + let sayStub; + let disclaimerHandler; + + beforeEach(async () => { + sayStub = sinon.stub().resolves(); + const esmockLocal = (await import('esmock')).default; + disclaimerHandler = await esmockLocal('../../../src/tasks/opportunity-status-processor/handler.js', { + '../../../src/utils/slack-utils.js': { say: sayStub }, + '@adobe/spacecat-shared-utils': { + resolveCanonicalUrl: sinon.stub().callsFake(async (url) => url), + }, + '@adobe/spacecat-shared-rum-api-client': { + default: { + createFrom: sinon.stub().returns({ retrieveDomainkey: sinon.stub().rejects() }), + }, + }, + '@adobe/spacecat-shared-google-client': { + default: { createFrom: sinon.stub().rejects() }, + }, + '@adobe/spacecat-shared-scrape-client': { + ScrapeClient: { + createFrom: sinon.stub().returns({ getScrapeJobsByBaseURL: sinon.stub().resolves([]) }), + }, + }, + '../../../src/utils/cloudwatch-utils.js': { + getAuditStatus: sinon.stub().resolves({ executed: true, failureReason: null }), + }, + '../../../src/utils/bot-detection.js': { + checkAndAlertBotProtection: sinon.stub().resolves(null), + }, + }); + }); + + function makeDisclaimerMessage(onboardStartTime, auditTypes, isRecheck = false) { + return { + siteId: 'test-site-id', + siteUrl: 'https://example.com', + organizationId: 'test-org-id', + taskContext: { + auditTypes, + slackContext: { channelId: 'test-channel', threadTs: 'test-thread' }, + onboardStartTime, + isRecheck, + }, + }; + } + + function makeDisclaimerContext(auditRecords) { + return { + ...context, + dataAccess: { + Site: { + findById: sinon.stub().resolves({ + getOpportunities: sinon.stub().resolves([]), + getBaseURL: sinon.stub().returns('https://example.com'), + }), + }, + SiteTopPage: { + allBySiteIdAndSourceAndGeo: sinon.stub().resolves([]), + }, + Audit: { + allLatestForSite: sinon.stub().resolves(auditRecords), + }, + }, + }; + } + + it('sends pending audit warning when audit record predates onboardStartTime', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv']); + const staleAudit = { + getAuditType: () => 'cwv', + getAuditedAt: () => new Date(onboardStartTime - 1000).toISOString(), + }; + const testContext = makeDisclaimerContext([staleAudit]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + sinon.match(/may still be in progress.*Core Web Vitals/), + ); + }); + + it('sends pending warning when no audit record exists for an expected type', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv']); + // No audit records at all → cwv is pending + const testContext = makeDisclaimerContext([]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + sinon.match(/may still be in progress/), + ); + }); + + it('sends "all complete" confirmation when isRecheck=true and all audits have run', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv'], true); + const freshAudit = { + getAuditType: () => 'cwv', + getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString(), + }; + const testContext = makeDisclaimerContext([freshAudit]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + ':white_check_mark: All audits have completed. The statuses above are up to date.', + ); + }); + + it('sends no disclaimer when all audits complete and isRecheck=false', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv'], false); + const freshAudit = { + getAuditType: () => 'cwv', + getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString(), + }; + const testContext = makeDisclaimerContext([freshAudit]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + const disclaimerCalls = sayStub.args.map((a) => a[3]).filter(Boolean); + expect(disclaimerCalls.some((m) => m.includes('may still be in progress'))).to.be.false; + expect(disclaimerCalls.some((m) => m.includes('All audits have completed'))).to.be.false; + }); + + it('skips disclaimer check when auditTypes is empty', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, []); + const testContext = makeDisclaimerContext([]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + // Audit.allLatestForSite should not be called for disclaimer (auditTypes empty) + expect(testContext.dataAccess.Audit.allLatestForSite).to.not.have.been.called; + }); + + it('falls back conservatively when Audit.allLatestForSite throws in disclaimer check', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv']); + const testContext = { + ...context, + dataAccess: { + Site: { + findById: sinon.stub().resolves({ + getOpportunities: sinon.stub().resolves([]), + getBaseURL: sinon.stub().returns('https://example.com'), + }), + }, + SiteTopPage: { + allBySiteIdAndSourceAndGeo: sinon.stub().resolves([]), + }, + Audit: { + allLatestForSite: sinon.stub().rejects(new Error('DB unavailable')), + }, + }, + }; + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + expect(testContext.log.warn).to.have.been.calledWith( + sinon.match(/Could not check audit completion from DB for site test-site-id: DB unavailable/), + ); + // Conservative fallback: pending warning sent + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + sinon.match(/may still be in progress/), + ); + }); + + it('includes siteUrl in the "run onboard status" hint within pending warning', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['broken-backlinks']); + const staleAudit = { + getAuditType: () => 'broken-backlinks', + getAuditedAt: () => new Date(onboardStartTime - 500).toISOString(), + }; + const testContext = makeDisclaimerContext([staleAudit]); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + sinon.match(/onboard status https:\/\/example\.com/), + ); + }); + }); }); From 329d0de5cdac085687d28a544e01ab758f7f0937 Mon Sep 17 00:00:00 2001 From: Tej Kotthakota Date: Wed, 18 Mar 2026 17:20:20 -0500 Subject: [PATCH 3/3] fix disclaimer message and statues --- .../opportunity-status-processor/handler.js | 69 +++++++++------- .../opportunity-status-processor.test.js | 80 ++++++++++++++++++- 2 files changed, 118 insertions(+), 31 deletions(-) diff --git a/src/tasks/opportunity-status-processor/handler.js b/src/tasks/opportunity-status-processor/handler.js index 9597b12b..efbe5530 100644 --- a/src/tasks/opportunity-status-processor/handler.js +++ b/src/tasks/opportunity-status-processor/handler.js @@ -18,7 +18,7 @@ import { resolveCanonicalUrl } from '@adobe/spacecat-shared-utils'; import { getAuditStatus } from '../../utils/cloudwatch-utils.js'; import { checkAndAlertBotProtection } from '../../utils/bot-detection.js'; import { say } from '../../utils/slack-utils.js'; -import { getOpportunitiesForAudit } from './audit-opportunity-map.js'; +import { getOpportunitiesForAudit, getAuditsForOpportunity } from './audit-opportunity-map.js'; import { OPPORTUNITY_DEPENDENCY_MAP } from './opportunity-dependency-map.js'; const TASK_TYPE = 'opportunity-status-processor'; @@ -598,6 +598,15 @@ export async function runOpportunityStatusProcessor(message, context) { statusMessages.push(`GSC ${gscStatus}`); statusMessages.push(`Scraping ${scrapingStatus}`); + // Determine which audits are still pending so opportunity statuses can reflect + // in-progress state (⏳) rather than showing stale data as ✅/❌. + // Only meaningful when we have an onboardStartTime anchor to compare against. + let pendingAuditTypes = []; + if (auditTypes && auditTypes.length > 0 && onboardStartTime) { + // eslint-disable-next-line max-len + ({ pendingAuditTypes } = await checkAuditCompletionFromDB(siteId, auditTypes, onboardStartTime, dataAccess, log)); + } + // Process opportunities by type to avoid duplicates // Only process opportunities that are expected based on the profile's audit types const processedTypes = new Set(); @@ -626,23 +635,28 @@ export async function runOpportunityStatusProcessor(message, context) { } processedTypes.add(opportunityType); - // eslint-disable-next-line no-await-in-loop - const suggestions = await opportunity.getSuggestions(); - const opportunityTitle = getOpportunityTitle(opportunityType); - const hasSuggestions = suggestions && suggestions.length > 0; - const status = hasSuggestions ? ':white_check_mark:' : ':x:'; - statusMessages.push(`${opportunityTitle} ${status}`); - - // Track failed opportunities (no suggestions) - if (!hasSuggestions) { - // Use informational message for opportunities with zero suggestions - const reason = 'Audit executed successfully, opportunity added, but found no suggestions'; - - failedOpportunities.push({ - title: opportunityTitle, - reason, - }); + + // If the source audit is still running, show ⏳ instead of stale ✅/❌ + const sourceAuditIsPending = getAuditsForOpportunity(opportunityType) + .some((auditType) => pendingAuditTypes.includes(auditType)); + + if (sourceAuditIsPending) { + statusMessages.push(`${opportunityTitle} :hourglass_flowing_sand:`); + } else { + // eslint-disable-next-line no-await-in-loop + const suggestions = await opportunity.getSuggestions(); + const hasSuggestions = suggestions && suggestions.length > 0; + const status = hasSuggestions ? ':white_check_mark:' : ':x:'; + statusMessages.push(`${opportunityTitle} ${status}`); + + // Track failed opportunities (no suggestions) + if (!hasSuggestions) { + failedOpportunities.push({ + title: opportunityTitle, + reason: 'Audit executed successfully, opportunity added, but found no suggestions', + }); + } } } @@ -721,23 +735,22 @@ export async function runOpportunityStatusProcessor(message, context) { await say(env, log, slackContext, 'No audit errors found'); } - // Audit completion disclaimer — check DB for pending audits and warn if any are still running + // Audit completion disclaimer — reuse pendingAuditTypes already computed above. + // Only list audit types that have known opportunity mappings; infrastructure audits + // (auto-suggest, auto-fix, scrape, etc.) are not shown since they don't affect + // the displayed opportunity statuses. if (auditTypes.length > 0) { - const { pendingAuditTypes } = await checkAuditCompletionFromDB( - siteId, - auditTypes, - onboardStartTime, - dataAccess, - log, - ); const isRecheck = taskContext?.isRecheck === true; - if (pendingAuditTypes.length > 0) { - const pendingList = pendingAuditTypes.map(getOpportunityTitle).join(', '); + const relevantPendingTypes = pendingAuditTypes.filter( + (t) => getOpportunitiesForAudit(t).length > 0, + ); + if (relevantPendingTypes.length > 0) { + const pendingList = relevantPendingTypes.map(getOpportunityTitle).join(', '); await say( env, log, slackContext, - `:warning: *Heads-up:* The following audit${pendingAuditTypes.length > 1 ? 's' : ''} ` + `:warning: *Heads-up:* The following audit${relevantPendingTypes.length > 1 ? 's' : ''} ` + `may still be in progress: *${pendingList}*.\n` + 'The statuses above reflect data available at this moment and may be incomplete. ' + `Run \`onboard status ${siteUrl}\` to re-check once all audits have completed.`, diff --git a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js index 1619086a..f25f7903 100644 --- a/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js +++ b/test/tasks/opportunity-status-processor/opportunity-status-processor.test.js @@ -2491,13 +2491,22 @@ describe('Opportunity Status Processor', () => { message.siteUrl = 'https://example.com'; message.taskContext.auditTypes = ['cwv', 'broken-backlinks']; - message.taskContext.onboardStartTime = Date.now() - 3600000; + const onboardStartTime = Date.now() - 3600000; + message.taskContext.onboardStartTime = onboardStartTime; message.taskContext.slackContext = { channelId: 'test-channel', threadTs: 'test-thread', }; context.env.AWS_REGION = 'us-east-1'; + // Provide fresh audit records so audits are "complete" (not pending) + context.dataAccess.Audit = { + allLatestForSite: sinon.stub().resolves([ + { getAuditType: () => 'cwv', getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString() }, + { getAuditType: () => 'broken-backlinks', getAuditedAt: () => new Date(onboardStartTime + 1000).toISOString() }, + ]), + }; + // Mock TWO opportunities with no suggestions to ensure loop executes multiple times const mockOpportunity1 = { getType: () => 'cwv', @@ -4261,15 +4270,58 @@ describe('Opportunity Status Processor', () => { expect(disclaimerCalls.some((m) => m.includes('All audits have completed'))).to.be.false; }); - it('skips disclaimer check when auditTypes is empty', async () => { + it('skips disclaimer and pending check when auditTypes is empty', async () => { const onboardStartTime = Date.now() - 3600000; const testMessage = makeDisclaimerMessage(onboardStartTime, []); const testContext = makeDisclaimerContext([]); await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); - // Audit.allLatestForSite should not be called for disclaimer (auditTypes empty) + // No audit completion DB call when auditTypes is empty expect(testContext.dataAccess.Audit.allLatestForSite).to.not.have.been.called; + const disclaimerCalls = sayStub.args.map((a) => a[3]).filter(Boolean); + expect(disclaimerCalls.some((m) => m.includes('may still be in progress'))).to.be.false; + }); + + it('shows hourglass in opportunity status when source audit is pending', async () => { + const onboardStartTime = Date.now() - 3600000; + const testMessage = makeDisclaimerMessage(onboardStartTime, ['cwv']); + const staleAudit = { + getAuditType: () => 'cwv', + getAuditedAt: () => new Date(onboardStartTime - 1000).toISOString(), + }; + const cwvOpp = { + getType: sinon.stub().returns('cwv'), + getSuggestions: sinon.stub().resolves([{ id: 'sug-1' }]), + }; + const testContext = { + ...context, + dataAccess: { + Site: { + findById: sinon.stub().resolves({ + getOpportunities: sinon.stub().resolves([cwvOpp]), + getBaseURL: sinon.stub().returns('https://example.com'), + }), + }, + SiteTopPage: { + allBySiteIdAndSourceAndGeo: sinon.stub().resolves([]), + }, + Audit: { + allLatestForSite: sinon.stub().resolves([staleAudit]), + }, + }, + }; + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + // cwv audit is pending → opportunity shows ⏳, getSuggestions is NOT called + expect(cwvOpp.getSuggestions).to.not.have.been.called; + expect(sayStub).to.have.been.calledWith( + sinon.match.any, + sinon.match.any, + sinon.match.any, + sinon.match(/Core Web Vitals :hourglass_flowing_sand:/), + ); }); it('falls back conservatively when Audit.allLatestForSite throws in disclaimer check', async () => { @@ -4325,5 +4377,27 @@ describe('Opportunity Status Processor', () => { sinon.match(/onboard status https:\/\/example\.com/), ); }); + + it('excludes infrastructure audit types not in AUDIT_OPPORTUNITY_MAP from disclaimer', async () => { + const onboardStartTime = Date.now() - 3600000; + // cwv is in the map; scrape-top-pages is not + const testMessage = makeDisclaimerMessage( + onboardStartTime, + ['cwv', 'scrape-top-pages'], + ); + const staleAudits = [ + { getAuditType: () => 'cwv', getAuditedAt: () => new Date(onboardStartTime - 500).toISOString() }, + { getAuditType: () => 'scrape-top-pages', getAuditedAt: () => new Date(onboardStartTime - 500).toISOString() }, + ]; + const testContext = makeDisclaimerContext(staleAudits); + + await disclaimerHandler.runOpportunityStatusProcessor(testMessage, testContext); + + const calls = sayStub.args.map((a) => a[3]).filter(Boolean); + const disclaimer = calls.find((m) => m.includes('may still be in progress')); + expect(disclaimer).to.exist; + expect(disclaimer).to.include('Core Web Vitals'); + expect(disclaimer).to.not.include('Scrape Top Pages'); + }); }); });