fix(people): address fourth round of cubic review findings#2892
Conversation
- add findFirst mock to test spec for undoVendorRevocation - prefix checklist evidence filenames with ID to prevent ZIP collisions - wrap sync call in try/catch so revocation stands on sync failure - prevent non-expandable checklist rows from toggling open - normalize preset date ranges to start of day - hide pending invitations when date filters are active - use controlled tab value in ToDoOverview for async data - revalidate from server on error instead of restoring stale snapshot Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
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
catchcurrently 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. |
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
|
@cubic-dev-ai review this |
@Marfuen I have started the AI code review. It will take a few minutes to complete. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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. |
# [3.60.0](v3.59.2...v3.60.0) (2026-05-21) ### Bug Fixes * **cloud-tests:** show meaningful Auto-Remediate diff for configure-only plans ([90c95f6](90c95f6)) * **evidence-export:** load automations one at a time to prevent OOM ([07f02e4](07f02e4)) * **people:** address cubic review findings for offboarding feature ([#2884](#2884)) ([9d43a6b](9d43a6b)) * **people:** address fifth round of cubic review findings ([#2893](#2893)) ([dbc364c](dbc364c)) * **people:** address fourth round of cubic review findings ([#2892](#2892)) ([ca9d9a5](ca9d9a5)) * **people:** address remaining cubic review findings for offboarding ([#2890](#2890)) ([8026352](8026352)) * **people:** address third round of cubic review findings ([#2891](#2891)) ([8ec5214](8ec5214)) * **people:** ds component compatibility fixes for offboarding UI ([200b112](200b112)) * **ui:** close MultipleSelector dropdown on blur so it stops blocking sibling form controls ([b9d08c8](b9d08c8)) ### Features * **api:** unblock cloud-tests mutations for API key + service token callers ([26e53da](26e53da)) * **cloud-tests:** add deterministic AWS plan normalizer for SLR params ([e0ec0f7](e0ec0f7)) * **cloud-tests:** fail fast on missing required AWS command params ([5f2d342](5f2d342)) * **cloud-tests:** universal AI step-repair on AWS validation errors ([8adf505](8adf505)) * **frameworks:** show controls as default tab with requirement column ([e41365d](e41365d)) * **people:** add employment events tracking and offboarding checklist ([5e15a73](5e15a73))
|
🎉 This PR is included in version 3.60.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Fixes remaining Cubic findings from #2878 (fourth review pass):
P1:
findFirstmock tooffboardingAccessRevocationand fixAccessRevocationServiceinstantiationP2:
syncAccessRevocationCompletionin try/catch so revocation stands even if sync failsTest plan
🤖 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.
findFirstmock and fixAccessRevocationServiceinstantiation forundoVendorRevocation.Written for commit e4c8388. Summary will update on new commits. Review in cubic