fix(people): address fourth round of cubic review findings

[Marfuen]

Summary

Fixes remaining Cubic findings from #2878 (fourth review pass):

P1:

• Fix test spec: add findFirst mock to offboardingAccessRevocation and fix AccessRevocationService instantiation
• Prefix checklist evidence filenames with attachment ID to prevent ZIP collisions

P2:

• Wrap syncAccessRevocationCompletion in try/catch so revocation stands even if sync fails
• Prevent non-expandable checklist rows from toggling open (empty expanded panel)
• Normalize preset date ranges to start of day (prevents excluding early-day records)
• Hide pending invitations when date filters are active (they have no dates)
• Use controlled tab value in ToDoOverview so async offboarding data activates the tab
• Revalidate from server on settings error instead of restoring stale snapshot

Test plan

• Click a non-expandable checklist item — should not expand
• Apply "Last 7 days" preset — records from start of that day should be included
• Apply date filter — pending invitations should be hidden
• Load overview with pending offboardings — offboarding tab should auto-select
• Toggle a setting, simulate error — should revalidate from server, not restore stale data

🤖 Generated with Claude Code

---

Summary by cubic

Fixes offboarding and people filtering issues from the Cubic review to make revocations reliable, evidence exports collision-free, and overview tabs select correctly. Supports CS-312 by making employment event filters accurate and evidence collection smoother.

• Bug Fixes
  • Access revocation: keep completion even if sync fails (wrap sync in try/catch) and log failures.
  • Evidence export: prefix checklist filenames with attachment ID to avoid ZIP collisions.
  • People: block opening non-expandable checklist rows; date presets start at beginning of day; hide pending invitations when any date filter is set; ToDo Overview switches to Offboarding via a controlled tab when data loads.
  • Settings: on save error, revalidate from the server instead of restoring stale state.
  • Tests: add findFirst mock and fix AccessRevocationService instantiation for undoVendorRevocation.

^{Written for commit e4c8388. Summary will update on new commits. Review in cubic}

[linear]

CS-312

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
comp-framework-editor	Ready	Preview, Comment	May 21, 2026 3:08pm

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
app	Skipped		May 21, 2026 3:08pm
portal	Skipped		May 21, 2026 3:08pm

[cubic-dev-ai]

cubic analysis

1 issue found across 7 files

Confidence score: 4/5

• This PR appears safe to merge with minimal risk, but there is a meaningful observability gap in apps/api/src/offboarding-checklist/access-revocation.service.ts.
• The catch currently swallows sync failures silently; if revocation sync fails, checklist state drift may go unnoticed and be harder to debug later.
• Severity is moderate (5/10) with high confidence (9/10), so this is worth fixing soon, but it is not clearly merge-blocking on its own.
• Pay close attention to apps/api/src/offboarding-checklist/access-revocation.service.ts - silent error handling can hide sync failures and mask checklist drift.

Linked issue analysis

Linked issue: CS-312: [Feature] Add personnel Employment events date tracking + supporting evidence upload section

Status	Acceptance criteria	Notes
✅	Click a non-expandable checklist item — it should not expand	The Collapsible now only opens/accepts open-change when the item is expandable, preventing non-expandable items from toggling open.
✅	Apply "Last 7 days" preset — records from the start of that day should be included	Preset range computation is normalized to the start of the day so early-day records are included.
✅	Apply date filter — pending invitations should be hidden	When any date filter is active, non-member items (e.g., pending invitations that have no dates) are excluded from dateFilteredItems.
✅	Load overview with pending offboardings — offboarding tab should auto-select	ToDoOverview now uses a controlled tab value and a useEffect that selects the offboarding tab when pending offboardings are present.
✅	Toggle a setting, simulate error — should revalidate from server, not restore stale data	On settings update error the code now calls mutate() to revalidate from the server instead of restoring a previous snapshot locally.

<sub>Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic</sub>

[Marfuen]

@cubic-dev-ai review this

[cubic-dev-ai]

@cubic-dev-ai review this

@Marfuen I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

cubic analysis

No issues found across 7 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

Linked issue analysis

Linked issue: CS-312: [Feature] Add personnel Employment events date tracking + supporting evidence upload section

Status	Acceptance criteria	Notes
✅	Date presets normalize to start-of-day so preset ranges (e.g. "Last 7 days") include records from the start of that day.	getPresetRange now calls from.setHours(0,0,0,0), ensuring the preset includes early-day records.
✅	Pending invitations are hidden when any date filter is active (they have no dates).	Introduced hasAnyDateFilter and updated dateFilteredItems to exclude non-member items when a date filter is set.
✅	Checklist evidence filenames in exported ZIPs are prefixed with attachment ID to prevent name collisions.	Archive entries now include file.id in the filename prefix to avoid collisions.
✅	Non-expandable checklist rows do not toggle open (prevent empty expanded panels).	Collapsible is now controlled so non-expandable items cannot open.
✅	ToDo overview auto-selects the Offboarding tab when pending offboarding data loads (uses a controlled tab value).	Added activeTab state and useEffect to set it to 'offboarding' when pending offboardings load; Tabs now use controlled value.
✅	Access revocation completion persists even if syncing the revocation to downstream systems fails.	syncAccessRevocationCompletion call is wrapped in try/catch so revocation still returns on sync errors.
✅	On checklist settings save error, the client revalidates from server instead of restoring a stale snapshot.	Replaced manual restore-with-previous with a simple mutate() call to trigger revalidation on error.

_{Re-trigger cubic}