feat(clinsched): permission-gated scheduler views for own-schedule & rotation-specific users#241
Merged
Merged
Conversation
Add CanAccessRotationViewAsync, CanAccessClinicianViewAsync and GetClinicianViewLabelAsync so server-side callers can mirror the client's view-gating getters, plus a public HasEditOwnSchedulePermissionAsync used by rotation self-scheduling checks.
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Bundle ReportChanges will increase total bundle size by 799 bytes (0.04%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
Files in
Files in
Files in
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #241 +/- ##
==========================================
+ Coverage 44.58% 44.65% +0.06%
==========================================
Files 896 897 +1
Lines 51733 51890 +157
Branches 4834 4868 +34
==========================================
+ Hits 23063 23169 +106
- Misses 28098 28141 +43
- Partials 572 580 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Pull request overview
Adds permission-aware gating across the Clinical Scheduler so users only see (and can call) the scheduler views they’re authorized for, with explicit support for “EditOwnSchedule”-only users to self-schedule rotations without the rotation dropdown silently emptying.
Changes:
- Introduces server-side “can access rotation/clinician view” + dynamic clinician-view label checks in
SchedulePermissionService(with unit tests) and uses them to gate the left-nav scheduler subitems. - Updates rotations endpoints + Vue rotation fetches to support an own-schedule self-scheduling bypass (only when the caller supplies their own MothraId), covering both the plain and
with-scheduled-weekspaths with regression tests. - Improves the own-schedule UX by making the clinician selector effectively read-only when appropriate (including auto-select when only one clinician is returned) and updates home-card copy accordingly.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/Controllers/LayoutController.cs | Makes left-nav loading async and injects permission service to build permission-gated scheduler nav. |
| web/Areas/ClinicalScheduler/Services/SchedulePermissionService.cs | Adds view-access checks, clinician-view label helper, and an exposed EditOwnSchedule permission check. |
| web/Areas/ClinicalScheduler/Services/ISchedulePermissionService.cs | Extends the permission service contract with new view-access/label methods. |
| web/Areas/ClinicalScheduler/Services/ClinicalSchedulerNavMenu.cs | Gates scheduler sub-nav items based on server-side permission checks and dynamic label. |
| web/Areas/ClinicalScheduler/Controllers/RotationsController.cs | Adds self-scheduling bypass logic (based on matching mothraId + EditOwnSchedule) shared across both rotation endpoints. |
| VueApp/src/ClinicalScheduler/services/rotation-service.ts | Adds optional clinicianMothraId query parameter support for rotation endpoints. |
| VueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vue | Passes selected clinician mothraId down to the rotation selector so own-schedule users get the bypass behavior. |
| VueApp/src/ClinicalScheduler/pages/ClinicalSchedulerHome.vue | Updates clinician-card description copy to reflect own-schedule vs general clinician scheduling. |
| VueApp/src/ClinicalScheduler/components/RotationSelector.vue | Threads clinicianMothraId into rotation API calls for both rotations endpoints. |
| VueApp/src/ClinicalScheduler/components/ClinicianSelector.vue | Adds read-only behavior + auto-select when a single clinician is returned and adjusts UI affordances accordingly. |
| VueApp/src/ClinicalScheduler/tests/clinician-selector-permissions.test.ts | Adds coverage for read-only locking when only own-schedule permission + single clinician result. |
| VueApp/src/ClinicalScheduler/tests/clinician-selector-basic.test.ts | Adds coverage for auto-select behavior when only one clinician is returned and tightens fetch expectations. |
| VueApp/src/ClinicalScheduler/tests/clinical-scheduler-home-display.test.ts | Updates/extends assertions for the new clinician-card description behavior. |
| test/ClinicalScheduler/SchedulePermissionServiceTest.cs | Adds unit tests for new view-access checks and clinician-view label behavior. |
| test/ClinicalScheduler/RotationsControllerTest.cs | Adds regression tests ensuring own-schedule self-scheduling returns all rotations on both endpoints. |
Contributor
📝 WalkthroughWalkthroughAdds clinician-scoped rotation access, own-schedule view labeling, single-clinician selector behavior, and permission-based navigation and home text updates across the Clinical Scheduler flow. ChangesClinician rotation scheduling and permission gating
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant LayoutController
participant ClinicalSchedulerNavMenu
participant SchedulePermissionService
LayoutController->>ClinicalSchedulerNavMenu: Nav(HttpContext.RequestAborted)
ClinicalSchedulerNavMenu->>SchedulePermissionService: CanAccessRotationViewAsync()
ClinicalSchedulerNavMenu->>SchedulePermissionService: CanAccessClinicianViewAsync()
ClinicalSchedulerNavMenu->>SchedulePermissionService: GetClinicianViewLabelAsync()
ClinicalSchedulerNavMenu-->>LayoutController: NavMenu
sequenceDiagram
participant ClinicianScheduleView
participant RotationSelector
participant RotationService
participant RotationsController
ClinicianScheduleView->>RotationSelector: clinician-mothra-id = selected clinician
RotationSelector->>RotationService: loadRotations(clinicianMothraId)
RotationService->>RotationsController: GET rotations with clinicianMothraId
RotationsController-->>RotationService: filtered rotations
RotationService-->>RotationSelector: rotation data
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Only advertise the sub-views a user can actually open: the left-nav now lists Schedule by Rotation and/or the clinician view based on the new permission checks, and shows "Edit My Schedule" instead of "Schedule by Clinician" for own-schedule users. The home card copy matches. GetLeftNav becomes async to resolve these checks.
Own-schedule users have no service edit permissions, so the rotation list was filtered to empty and their "Add Rotation" dropdown showed nothing. The rotations endpoints now accept clinicianMothraId; when it matches the current user and they hold EditOwnSchedule, permission filtering is bypassed so they see every rotation for their own schedule. A shared helper applies this on both the plain and with-scheduled-weeks endpoints, each covered by tests so the empty-dropdown regression cannot return on either path.
When an own-schedule user's list resolves to a single clinician (themselves), the selector renders as a read-only display - person icon, no search, clear, or affiliates toggle - and auto-selects that clinician instead of an editable combobox. Also types the vitest mocks and de-nests a conditional expect in the selector tests so they satisfy the lint gate.
rlorenzo
force-pushed
the
patch/clinicial-rotations
branch
from
f55eaeb to
3620c8f
Compare
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/Controllers/LayoutController.cs (1)
82-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the async leftnav clinical-scheduler branch.
Codecov flagged uncovered lines in this file. Consider a controller test exercising the "clinicalscheduler" nav case now that it awaits
ISchedulePermissionService-gated results.🤖 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 `@web/Controllers/LayoutController.cs` around lines 82 - 97, Add controller test coverage for the async left-nav clinical-scheduler path in GetLeftNav by exercising the "clinicalscheduler" (and/or "viper-clinical-scheduler") switch case and verifying the awaited Nav(HttpContext.RequestAborted) result from ClinicalSchedulerNavMenu. Use LayoutController as the entry point and mock ISchedulePermissionService so the branch executes and is asserted, ensuring the uncovered async branch is hit.
🤖 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.
Inline comments:
In `@test/ClinicalScheduler/SchedulePermissionServiceTest.cs`:
- Around line 490-540: Add missing Admin-permission test coverage for the
clinician view paths in SchedulePermissionServiceTest. The current
CanAccessClinicianViewAsync and GetClinicianViewLabelAsync tests cover
Manage/EditClnSchedules but not the Admin branch used by
HasFullAccessPermission, so add cases using
SetupUserWithAdminPermission(TestUserMothraId) to verify
CanAccessClinicianViewAsync returns true and GetClinicianViewLabelAsync returns
the clinician schedule label. Keep the new tests alongside the existing
CanAccessClinicianViewAsync and GetClinicianViewLabelAsync cases so the
full-access OR paths are covered.
In
`@VueApp/src/ClinicalScheduler/__tests__/clinical-scheduler-home-display.test.ts`:
- Around line 58-71: Add a test case in ClinicalSchedulerHome’s display spec to
cover the hybrid hasClinicianViewReadOnly path used by isPersonalClinicianView.
Reuse the existing own-schedule assertion pattern in the ClinicalSchedulerHome
wrapper setup, but set hasClinicianViewReadOnly instead of
hasOnlyOwnSchedulePermission and verify the same “Schedule your own rotations”
copy and banner are shown for this branch.
In `@VueApp/src/ClinicalScheduler/components/RotationSelector.vue`:
- Around line 84-85: The RotationSelector component is missing reactivity for
the clinicianMothraId prop, so changes from the parent won’t trigger a refresh.
Add clinicianMothraId to the existing watcher setup in RotationSelector.vue
alongside serviceFilter, year, and onlyWithScheduledWeeks so loadRotations()
runs whenever the clinician selection changes. Make sure the update is wired in
the same place as the current watchers/getters that drive rotation loading.
In `@web/Areas/ClinicalScheduler/Controllers/RotationsController.cs`:
- Around line 362-386: `FilterRotationsByPermissionAsync` currently mixes
identity lookup with permission filtering by calling
`_userHelper.GetCurrentUser()` and comparing `currentUser.MothraId` to
`clinicianMothraId`. Move this self-identification logic into
`ISchedulePermissionService`, either by extending
`HasEditOwnSchedulePermissionAsync` or adding a dedicated
`IsSelfSchedulingAsync(clinicianMothraId)`-style method, and have
`RotationsController` rely on that service instead of direct user-helper access.
Keep the controller focused on filtering rotations and service ownership checks,
with the authenticated-user comparison encapsulated in the permission layer.
In `@web/Areas/ClinicalScheduler/Services/ClinicalSchedulerNavMenu.cs`:
- Around line 32-46: Add unit test coverage for ClinicalSchedulerNavMenu to lock
in the new permission-gated branching. Create tests around the
ClinicalSchedulerNavMenu build logic that exercise the
CanAccessRotationViewAsync, CanAccessClinicianViewAsync, and
GetClinicianViewLabelAsync paths, covering base-permission-denied,
rotation-only, clinician-only, own-schedule label, and full-access cases. Verify
the generated NavMenuItem entries and MenuItemText/MenuItemURL/IndentLevel
values for each scenario so the new nav behavior stays covered.
- Around line 32-46: The scheduler nav permission checks in
ClinicalSchedulerNavMenu are not protected against service failures, so a thrown
exception can break LayoutController.GetLeftNav() instead of simply omitting
those sub-items. Update the menu-building logic around
CanAccessRotationViewAsync, CanAccessClinicianViewAsync, and
GetClinicianViewLabelAsync to catch failures and fall back to adding no
scheduler-specific items, keeping the nav render resilient.
In `@web/Areas/ClinicalScheduler/Services/SchedulePermissionService.cs`:
- Around line 175-186: CanAccessRotationViewAsync currently triggers a full
Services load through GetUserEditableServicesAsync on every nav render for
non-full-access users, which is wasteful. Update the permission check to avoid
re-querying the entire table on each call, preferably by reusing a cheaper
access check or introducing per-request/short-TTL caching around the
editable-services lookup. Keep the behavior of HasFullAccessPermission and the
current access decision intact, but change the data access path used by
CanAccessRotationViewAsync.
- Around line 175-186: Wrap the remaining permission checks in the same
database-exception guard so these methods fail closed instead of throwing:
`CanAccessRotationViewAsync`, `CanAccessClinicianViewAsync`,
`GetClinicianViewLabelAsync`, and `HasEditOwnSchedulePermissionAsync` in
`SchedulePermissionService`. `HasFullAccessPermission()` and
`HasEditOwnSchedulePermission()` both call into `UserHelper.HasPermission()` via
RAPS-backed queries, so catch the same DB-related exceptions around those checks
and return false (or the existing safe fallback) when they occur. Keep the guard
logic consistent with the existing exception handling already used in the
service.
---
Outside diff comments:
In `@web/Controllers/LayoutController.cs`:
- Around line 82-97: Add controller test coverage for the async left-nav
clinical-scheduler path in GetLeftNav by exercising the "clinicalscheduler"
(and/or "viper-clinical-scheduler") switch case and verifying the awaited
Nav(HttpContext.RequestAborted) result from ClinicalSchedulerNavMenu. Use
LayoutController as the entry point and mock ISchedulePermissionService so the
branch executes and is asserted, ensuring the uncovered async branch is hit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b305a5ac-bc7d-43ed-a92a-94cfb330181e
📒 Files selected for processing (15)
VueApp/src/ClinicalScheduler/__tests__/clinical-scheduler-home-display.test.tsVueApp/src/ClinicalScheduler/__tests__/clinician-selector-basic.test.tsVueApp/src/ClinicalScheduler/__tests__/clinician-selector-permissions.test.tsVueApp/src/ClinicalScheduler/components/ClinicianSelector.vueVueApp/src/ClinicalScheduler/components/RotationSelector.vueVueApp/src/ClinicalScheduler/pages/ClinicalSchedulerHome.vueVueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vueVueApp/src/ClinicalScheduler/services/rotation-service.tstest/ClinicalScheduler/RotationsControllerTest.cstest/ClinicalScheduler/SchedulePermissionServiceTest.csweb/Areas/ClinicalScheduler/Controllers/RotationsController.csweb/Areas/ClinicalScheduler/Services/ClinicalSchedulerNavMenu.csweb/Areas/ClinicalScheduler/Services/ISchedulePermissionService.csweb/Areas/ClinicalScheduler/Services/SchedulePermissionService.csweb/Controllers/LayoutController.cs
The Add-Rotation picker used the with-scheduled-weeks endpoint, so it only listed rotations that already had instructor schedules in the selected year - showing progressively fewer (and eventually zero) rotations for future years. The legacy scheduler always listed every active rotation (getRotations active=1); switch the own-schedule picker to the all-active list to match. Also add a watcher on RotationSelector's clinicianMothraId prop so the list refetches when the target clinician changes (it previously reacted only to serviceFilter/year/onlyWithScheduledWeeks), with tests covering the prop forwarding and the refetch.
- Auto-select of a sole clinician now fires only in own-schedule contexts; the rotation view's add-clinician picker previously staged a clinician for scheduling without user action whenever the list had one entry, re-firing on every refetch - Emit update:modelValue/change at most once per clinician fetch; own-schedule loads previously emitted duplicates - Ignore stale out-of-order rotation-list responses in RotationSelector - Prove the clinicianMothraId bypass negatives: a mismatched ID or missing EditOwnSchedule keeps normal service filtering - Require IUserHelper injection in RotationsController; drop the dead new UserHelper() fallback
Contributor
Author
|
@bsedwards Ready for review/merging |
bsedwards
approved these changes
Jul 2, 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
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.
Summary
Permission-gates the Clinical Scheduler so the UI only surfaces the views a user can actually use, and lets
EditOwnScheduleusers manage their own rotations.Commits
add view-access and own-schedule permission checks- new permission-service methods + unit tests.gate nav and home cards by scheduler permissions- nav menu + home cards + async left-nav.let own-schedule users self-schedule all rotations- rotations API bypass for self-scheduling (fixes the empty "Add Rotation" dropdown). The bypass is DRY'd into one shared helper covering both the plain andwith-scheduled-weeksendpoints, each with a regression test so the empty-dropdown cannot return on either path.make clinician selector read-only in own-schedule mode- read-only single-clinician display + auto-select.Testing