[CSM Portal][FE] feat(csm-timecards): core utility functions and tests (2/6) - #994
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds three new utility modules for the CSM timecards feature: ChangesTimecard utility modules
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 1, 2026
Merged
Hesara2003
added
Type/New Feature
Hesara2003
force-pushed
the
feature/timecards-p2-api
branch
3 times, most recently
from
July 1, 2026 06:13
299067b to
b64ac54
Compare
Hesara2003
force-pushed
the
feature/timecards-p2-api
branch
2 times, most recently
from
July 2, 2026 08:58
684e5bf to
d06df55
Compare
cloby99
previously approved these changes
Jul 2, 2026
Hesara2003
dismissed
cloby99’s stale review
July 2, 2026 17:07
The merge-base changed after approval.
cloby99
previously approved these changes
Jul 2, 2026
Hesara2003
dismissed
cloby99’s stale review
July 2, 2026 17:16
The merge-base changed after approval.
rksk
previously approved these changes
Jul 2, 2026
rksk
left a comment
rksk
left a comment
Contributor
There was a problem hiding this comment.
Approving to land the time-cards stack on v2. Post-merge hardening tracked separately (search truncation, self-approval, hour rounding, identity gate).
rksk
previously approved these changes
Jul 2, 2026
rksk
previously approved these changes
Jul 2, 2026
Adds the three utilities the API store depends on (timeCardTotals, timeSheetWeek, timeSheetState) together with their unit tests so downstream branches can compile cleanly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rksk
force-pushed
the
feature/timecards-p2-api
branch
from
July 2, 2026 17:55
d06df55 to
47c6177
Compare
rksk
approved these changes
Jul 2, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
apps/csm-portal/webapp/src/features/csm-timecards/utils/timeCardTotals.ts (1)
22-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
emptyBreakdownfromACTIVITY_KEYSto avoid duplication.The keys are hardcoded here in addition to
ACTIVITY_KEYSexport const ACTIVITY_KEYS = [ "analysisDebugging", "reproduce", "settingUp", "providingSolution", "answering", ] as const;. TypeScript catches drift via theRecordtype, but keeping a single source of truth is cleaner.♻️ Proposed refactor
export function emptyBreakdown(): ActivityBreakdown { - return { - analysisDebugging: 0, - reproduce: 0, - settingUp: 0, - providingSolution: 0, - answering: 0, - }; + return ACTIVITY_KEYS.reduce( + (acc, key) => ({ ...acc, [key]: 0 }), + {} as ActivityBreakdown, + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeCardTotals.ts` around lines 22 - 31, `emptyBreakdown` is duplicating the activity names that already live in `ACTIVITY_KEYS`. Refactor `emptyBreakdown` in `timeCardTotals.ts` to derive the returned `ActivityBreakdown` from `ACTIVITY_KEYS` so there is a single source of truth, while still preserving the same zeroed values for each key. Keep the change localized around `ACTIVITY_KEYS`, `ActivityBreakdown`, and `emptyBreakdown` so future activity additions only need one update.apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetWeek.ts (2)
45-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding test coverage for
weekKeyandlocalTodayIso.These are exported and used elsewhere in the stack (per PR objectives), but the test file only covers
weekStartOf,weekEndOf, andweekLabel.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetWeek.ts` around lines 45 - 58, Add test coverage for the exported helpers weekKey and localTodayIso in the existing timeSheetWeek test suite. Verify that weekKey delegates to weekStartOf and returns the expected week-start ISO string, and add a deterministic test for localTodayIso that asserts it produces the local calendar date (not UTC) by controlling the system date/time or timezone. Use the function names weekKey, localTodayIso, and weekStartOf to locate the relevant assertions.
60-72: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
weekLabelyear is taken only from the week end, which can mislabel year-spanning weeks.For a week like Dec 29, 2026 – Jan 4, 2027, the label would render as
"Dec 29 – Jan 4, 2027", implying the start date is also in 2027. Consider showing both years when they differ.♻️ Suggested fix
const year = end.getUTCFullYear(); + const startYear = start.getUTCFullYear(); if (mon(start) === mon(end)) { return `${mon(start)} ${day(start)} – ${day(end)}, ${year}`; } - return `${mon(start)} ${day(start)} – ${mon(end)} ${day(end)}, ${year}`; + return startYear === year + ? `${mon(start)} ${day(start)} – ${mon(end)} ${day(end)}, ${year}` + : `${mon(start)} ${day(start)}, ${startYear} – ${mon(end)} ${day(end)}, ${year}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetWeek.ts` around lines 60 - 72, The weekLabel helper currently always uses the end date’s year, which mislabels weeks that span two years. Update weekLabel in timeSheetWeek.ts so it compares the start and end years (using toUtcDate, weekEndOf, and the existing mon/day helpers) and includes both years in the returned label when they differ, while keeping the current format for same-year weeks.apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetState.ts (2)
54-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test coverage for this layer's RBAC logic.
Unlike the other two utility modules in this PR (
timeCardTotals,timeSheetWeek), this file has no accompanying__tests__suite despite encoding non-trivial, security-relevant branching (cardActions,sheetStatus,sheetActions). Given this drives which actions render/are permitted per role and state, unit tests covering each state × role combination would materially reduce regression risk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetState.ts` around lines 54 - 115, The RBAC branching in cardActions, sheetStatus, and sheetActions is not covered by tests, so add a new __tests__ suite for timeSheetState that exercises the state × role combinations for each helper. Verify the returned actions for cardActions across owner/admin/approver/non-privileged roles, confirm sheetStatus precedence for rejected/recalled/submitted/approved/open, and cover sheetActions for editable, submitted, and approved card mixes so regressions in permission logic are caught.
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EDITABLE_STATESis exported as a mutable array.It's exported and consumed by RBAC logic in
sheetActions(Line 104). Since it's a mutableTimeCardState[], any accidental external mutation (push/splice) would silently change permission checks everywhere it's used.♻️ Proposed fix
-export const EDITABLE_STATES: TimeCardState[] = ["pending", "rejected", "recalled"]; +export const EDITABLE_STATES: readonly TimeCardState[] = ["pending", "rejected", "recalled"] as const;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetState.ts` at line 46, `EDITABLE_STATES` is exported as a mutable array, so external callers like the RBAC checks in `sheetActions` can accidentally mutate shared permission state. Make `EDITABLE_STATES` immutable in `timeSheetState` by switching it to a readonly/frozen constant, and ensure any consumers only read from it rather than modifying it. Keep the symbol name `EDITABLE_STATES` so existing imports continue to work, but prevent `push`/`splice`-style changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeCardTotals.ts`:
- Around line 22-31: `emptyBreakdown` is duplicating the activity names that
already live in `ACTIVITY_KEYS`. Refactor `emptyBreakdown` in
`timeCardTotals.ts` to derive the returned `ActivityBreakdown` from
`ACTIVITY_KEYS` so there is a single source of truth, while still preserving the
same zeroed values for each key. Keep the change localized around
`ACTIVITY_KEYS`, `ActivityBreakdown`, and `emptyBreakdown` so future activity
additions only need one update.
In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetState.ts`:
- Around line 54-115: The RBAC branching in cardActions, sheetStatus, and
sheetActions is not covered by tests, so add a new __tests__ suite for
timeSheetState that exercises the state × role combinations for each helper.
Verify the returned actions for cardActions across
owner/admin/approver/non-privileged roles, confirm sheetStatus precedence for
rejected/recalled/submitted/approved/open, and cover sheetActions for editable,
submitted, and approved card mixes so regressions in permission logic are
caught.
- Line 46: `EDITABLE_STATES` is exported as a mutable array, so external callers
like the RBAC checks in `sheetActions` can accidentally mutate shared permission
state. Make `EDITABLE_STATES` immutable in `timeSheetState` by switching it to a
readonly/frozen constant, and ensure any consumers only read from it rather than
modifying it. Keep the symbol name `EDITABLE_STATES` so existing imports
continue to work, but prevent `push`/`splice`-style changes.
In `@apps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetWeek.ts`:
- Around line 45-58: Add test coverage for the exported helpers weekKey and
localTodayIso in the existing timeSheetWeek test suite. Verify that weekKey
delegates to weekStartOf and returns the expected week-start ISO string, and add
a deterministic test for localTodayIso that asserts it produces the local
calendar date (not UTC) by controlling the system date/time or timezone. Use the
function names weekKey, localTodayIso, and weekStartOf to locate the relevant
assertions.
- Around line 60-72: The weekLabel helper currently always uses the end date’s
year, which mislabels weeks that span two years. Update weekLabel in
timeSheetWeek.ts so it compares the start and end years (using toUtcDate,
weekEndOf, and the existing mon/day helpers) and includes both years in the
returned label when they differ, while keeping the current format for same-year
weeks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42147022-2727-43f2-800e-c0dc59023fd2
📒 Files selected for processing (5)
apps/csm-portal/webapp/src/features/csm-timecards/utils/__tests__/timeCardTotals.test.tsapps/csm-portal/webapp/src/features/csm-timecards/utils/__tests__/timeSheetWeek.test.tsapps/csm-portal/webapp/src/features/csm-timecards/utils/timeCardTotals.tsapps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetState.tsapps/csm-portal/webapp/src/features/csm-timecards/utils/timeSheetWeek.ts
This was referenced Jul 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Adds the three pure utility modules the API store depends on (
timeCardTotals,timeSheetWeek,timeSheetState) together with their unit tests. Requires PR 1/6 (#992) to be merged first.Goals
timeCardTotals.ts— compute total hours and per-activity summariestimeSheetWeek.ts— ISO week boundary helperstimeSheetState.ts—cardActions/sheetActionsRBAC logic,sheetStatusrollup,TimecardAction/SheetActiontypesApproach
Pure functions with no side-effects placed before the API layer so
timeCardStore.ts(PR 3/6) can import them without forward-reference errors.User stories
ISSU-009 — Time Management (Epic: Issue Management, Priority: Must have)
Release note
Internal utility functions for time-cards. No user-visible change in this PR.
Documentation
N/A
Training
N/A
Certification
N/A
Marketing
N/A
Automation tests
timeCardTotals.test.ts,timeSheetWeek.test.ts— totals calculation and week-boundary edge casesSecurity checks
Samples
N/A
Related PRs
6-PR series for ISSU-009 — merge into
v2in order:Migrations
N/A
Test environment
macOS 14, Node 20, Chrome 126
Learning
N/A
Summary by CodeRabbit
New Features
Bug Fixes