Skip to content

[CSM Portal] Scope time-card approver search to the approver role - #1491

Merged
rksk merged 1 commit into
wso2-open-operations:mainfrom
Hesara2003:fix/csm-timecard-approver-role
Aug 18, 2026
Merged

rksk merged 1 commit into
wso2-open-operations:mainfrom
Hesara2003:fix/csm-timecard-approver-role

Conversation

@Hesara2003

@Hesara2003 Hesara2003 commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

The Log Time dialog's "Approver (team lead)" search let a submitter pick literally anyone at WSO2 as their approver — including people with no CS involvement at all — because it filtered by INTERNAL_USER_ROLES (any internal/agent/admin account), not by anything approver-specific.

Goals

Only accounts that actually hold the timecard-approver role should show up as approver candidates.

Approach

Swapped the search filter from roleIds: INTERNAL_USER_ROLES to roleIds: [TIMECARD_APPROVER_GROUP] in LogTimeCardDialog.tsx — the same role key useTimecardRole.ts already uses elsewhere to identify eligible approvers, so this reuses an existing, correct concept rather than inventing a new one.

User stories

  • As an engineer logging time, the approver search only shows people who can actually approve time cards, not every WSO2 employee.

Release note

The time-card "Approver (team lead)" search now only returns accounts with the approver role, instead of any internal WSO2 account.

Documentation

No API contract change — this only narrows an existing roleIds search filter value on the client.

Automation tests

Full csm-timecards suite (11 files / 86 tests) passes; tsc -b --noEmit clean on the touched file.

Security checks

No new data exposure — this narrows, rather than widens, what the search returns.

Test environment

Verified via tsc/vitest locally. Live verification (searching for a non-approver by name and confirming they no longer appear) pending.

Summary by CodeRabbit

  • Bug Fixes
    • Timecard approver searches now show only users with the designated timecard approver role, improving accuracy and reducing irrelevant results.

The Log Time dialog's "Approver (team lead)" search matched any
internal WSO2 account (INTERNAL_USER_ROLES), letting a submitter pick
literally anyone at the company, including people with no CS
involvement at all. Filters by TIMECARD_APPROVER_GROUP instead, the
role already used elsewhere to identify actual eligible approvers.
@coderabbitai

coderabbitai Bot commented Aug 18, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e5433ced-fb3e-48cb-8cf1-e502316209e4

📥 Commits

Reviewing files that changed from the base of the PR and between 346538d and b7fab2f.

📒 Files selected for processing (1)
  • apps/csm-portal/webapp/src/features/csm-timecards/components/LogTimeCardDialog.tsx

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The timecard dialog now searches for approvers by the dedicated TIMECARD_APPROVER_GROUP role instead of the broader internal-user role.

Changes

Timecard approver filtering

Layer / File(s) Summary
Use the dedicated approver role
apps/csm-portal/webapp/src/features/csm-timecards/components/LogTimeCardDialog.tsx
The dialog imports NormalizedUser as a standalone type and uses TIMECARD_APPROVER_GROUP when searching for approvers.

Estimated code review effort: 2 (Simple) | ~5 minutes

Merge Risk: ⚪ Minimal · up to b7fab

The change narrowly limits approver search results to accounts with the time-card approver role, with no actionable merge-blocking risk remaining after normal checks and review.

Possibly related PRs

Suggested labels: Type/Bug

Suggested reviewers: rksk, rashmika998

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: restricting time-card approver searches to the approver role.
Description check ✅ Passed The description covers the main purpose, solution, user story, release note, testing, security impact, and test environment.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Hesara2003
Hesara2003 requested a review from rksk August 18, 2026 11:26
@rksk
rksk merged commit f3e52ce into wso2-open-operations:main Aug 18, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants