fix: Add disclaimer about onboard status

[tkotthakota-adobe]

https://jira.corp.adobe.com/browse/SITES-41933

• add disclaimer about onboard status
  Test:
  https://cq-dev.slack.com/archives/C060T2PPF8V/p1773880250583619

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[github-actions]

This PR will trigger no release when merged.

[tkotthakota-adobe]

@rpapani
Can you get full mapping for these?

https://github.com/adobe/spacecat-task-processor/blob/main/src/tasks/opportunity-status-processor/audit-opportunity-map.js

https://github.com/adobe/spacecat-task-processor/blob/main/src/tasks/opportunity-status-processor/opportunity-dependency-map.js

[solaris007]

Hey @tkotthakota-adobe,

Strengths

• Conservative fallback on DB errors: catch block marks all remaining audit types as pending, preventing false-positive green statuses (handler.js:281-284)
• Hourglass gating skips unnecessary getSuggestions() calls when audit is pending - avoids stale data and reduces DB load (handler.js:645-647)
• Smart filtering of infrastructure audit types from disclaimer via getOpportunitiesForAudit(t).length > 0 (handler.js:744-746)
• isRecheck strict comparison (=== true) prevents truthy coercion from unexpected SQS payloads (handler.js:747)
• Thorough test coverage: 9 new test cases plus correctly updated existing tests

Issues

Important (Should Fix)

1. getOpportunityTitle receives audit types instead of opportunity types in disclaimer - handler.js:748

relevantPendingTypes contains audit type strings, but getOpportunityTitle is designed for opportunity types. For experimentation-opportunities, the disclaimer shows "Experimentation Opportunities" (kebab fallback) instead of the actual opportunity name. Multiple reviewers flagged this independently.

Fix: Map audit types to opportunity types first:

const pendingOpportunityNames = relevantPendingTypes
  .flatMap((t) => getOpportunitiesForAudit(t))
  .map(getOpportunityTitle);
const pendingList = [...new Set(pendingOpportunityNames)].join(', ');

2. auditedAt NaN silently treated as completed - handler.js:273-274

If audit.getAuditedAt() returns null or an unparseable string, new Date(value).getTime() returns NaN. NaN < onboardStartTime is false, so the audit lands in completedAuditTypes - the opposite of the conservative intent.

Fix: Add a NaN guard:

const auditedAt = new Date(audit.getAuditedAt()).getTime();
if (!onboardStartTime || Number.isNaN(auditedAt) || auditedAt < onboardStartTime) {
  pendingAuditTypes.push(auditType);
} else {
  completedAuditTypes.push(auditType);
}

3. Duplicate entries in pendingAuditTypes on mid-loop error - handler.js:281-284

If processing throws after some types are already pushed to pendingAuditTypes, the catch block's auditTypes.filter((t) => !completedAuditTypes.includes(t)) re-adds them, creating duplicates. The disclaimer would show repeated audit names.

Fix: Clear pendingAuditTypes before the fallback push:

pendingAuditTypes.length = 0;
pendingAuditTypes.push(...auditTypes.filter((t) => !completedAuditTypes.includes(t)));

4. Cross-repo duplication with companion PR #1986 - Both PRs implement audit completion checking with different anchor timestamps (onboardStartTime here vs lastAuditRunTime in api-service) and different data access patterns (Audit.allLatestForSite vs LatestAudit.allBySiteId). These can disagree about pending status as the system evolves.

Fix: Track via follow-up Jira ticket; extract shared logic to a shared package.

Minor (Nice to Have)

5. Redundant onboardStartTime guard - handler.js:273 - The caller already ensures onboardStartTime is truthy. The inner check is harmless but adds confusion about the function's contract.

6. auditedAt === onboardStartTime boundary - handler.js:273 - When timestamps match exactly, the audit is classified as "completed." Worth documenting the intended behavior with a test case.

7. False "All audits completed" when onboardStartTime is falsy + isRecheck is true - If onboardStartTime is falsy but auditTypes.length > 0 and isRecheck is true, the DB check is skipped, pendingAuditTypes stays empty, and the "All audits completed" message fires even though no check was performed.

Recommendations

• Prioritize fixes #1-#3 - they are correctness bugs that surface under plausible conditions.
• Consider adding a timeout/expiry to the "pending" state. If an audit fails silently (never writes a DB record), the hourglass shows indefinitely.
• Add a test for the onboardStartTime = undefined + isRecheck = true path to lock down the expected behavior.
• Create a follow-up Jira ticket to extract shared audit-opportunity mapping, title functions, and pending-audit logic into a shared package.

Assessment

Ready to merge? With fixes

The core logic is sound and well-tested. Issues 1 (audit-type naming mismatch), 2 (NaN-as-completed), and 3 (duplicate entries) are correctness bugs that should be fixed before merge. Issue 4 (cross-repo duplication) can be a tracked follow-up.

Next Steps

1. Fix the getOpportunityTitle audit-type vs opportunity-type mismatch (#1).
2. Add NaN guard in checkAuditCompletionFromDB (#2).
3. Fix duplicate entries in catch fallback (#3).
4. Create a follow-up Jira ticket for shared package extraction (#4).
5. Minor items are optional.