feat(scheduler): Clinical Scheduler audit trail + inline per-week history#236
Open
rlorenzo wants to merge 9 commits into
Open
feat(scheduler): Clinical Scheduler audit trail + inline per-week history#236rlorenzo wants to merge 9 commits into
rlorenzo wants to merge 9 commits into
Conversation
Bundle ReportChanges will increase total bundle size by 22.53kB (1.06%) ⬆️. 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
Files in
Files in
Files in
Files in
Files in
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
rlorenzo
force-pushed
the
feature/clinical-scheduler-audit-log
branch
3 times, most recently
from
d936116 to
3bc2ddf
Compare
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
📝 WalkthroughWalkthroughAdds an end-to-end audit trail feature to the Clinical Scheduler: new C# DTOs, ChangesClinical Scheduler Audit Trail
Effort AuditChangeDetail Refactor
Sequence DiagramsequenceDiagram
participant Browser
participant AuditLogPage
participant AuditLogService
participant ScheduleAuditController
participant ScheduleAuditService
Browser->>AuditLogPage: mount
AuditLogPage->>ScheduleAuditController: GET /audit (year, filters)
ScheduleAuditController->>ScheduleAuditService: GetAuditLogAsync(gradYear, filters)
ScheduleAuditService-->>ScheduleAuditController: List<AuditLogEntryDto>
ScheduleAuditController-->>AuditLogPage: 200 OK entries
AuditLogPage-->>Browser: render responsive table
Browser->>AuditLogPage: click WeekHistoryButton in WeekCell
AuditLogPage->>AuditLogService: getRotationWeekHistory(rotationId, weekId)
AuditLogService->>ScheduleAuditController: GET /audit/rotation-week
ScheduleAuditController->>ScheduleAuditService: GetRotationWeekAuditAsync
ScheduleAuditService-->>ScheduleAuditController: List<AuditLogEntryDto>
ScheduleAuditController-->>AuditLogService: 200 OK entries
AuditLogService-->>AuditLogPage: ApiResult
AuditLogPage-->>Browser: render WeekHistoryContent popover/dialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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: 12
🤖 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 `@VueApp/src/ClinicalScheduler/components/WeekHistoryButton.vue`:
- Around line 97-114: Refactor loadHistory in WeekHistoryButton.vue to satisfy
the quality gate by breaking the logic into small helpers instead of keeping the
fetch selection and result/error handling inline. Extract one helper to choose
the correct AuditLogService call based on props.viewMode and another to
normalize successful and failed responses/errors into entries.value and
error.value. Keep loadHistory focused on toggling isLoading and orchestrating
these helpers so the method stays simple and readable.
In `@VueApp/src/ClinicalScheduler/pages/AuditLogPage.vue`:
- Around line 54-60: The AuditLogPage.vue filters expansion item is using an
inline desktop visibility style, which violates the Vue styling rules in this
repo. Update the q-expansion-item in AuditLogPage to remove the :header-style
binding and use Quasar utility classes or separate mobile/desktop filter header
containers instead, keeping the same behavior while avoiding inline styles.
- Around line 570-578: The clearFilters function in AuditLogPage.vue does not
fully reset the page because currentYear is left unchanged, so the year-scoped
data and term list remain filtered. Update clearFilters to also reset
currentYear and reuse the same refresh path as onYearChange() so the
year-specific term list and audit log state are restored consistently when
clearing filters.
- Around line 484-510: loadAuditTrail() in AuditLogPage.vue is too complex and
needs to be split up to pass the Fallow gate. Extract the
AuditLogService.getAuditLog filter construction into a helper and move the
success/error response handling into a separate helper so loadAuditTrail() stays
a simple linear fetch flow. Keep the existing behavior intact while reducing the
branching inside loadAuditTrail(), and use the current symbols like currentYear,
selectedTermCode, selectedRotationId, selectedArea, selectedModifier,
selectedPerson, fromDate, toDate, and AuditLogService.getAuditLog to organize
the refactor.
- Around line 484-515: `loadAuditTrail()` can resolve out of order and overwrite
newer filter results in `AuditLogPage.vue`. Add a request guard in
`loadAuditTrail` (for example, a monotonically increasing request id or
cancellation token) so only the latest invocation is allowed to set
`entries.value`, `error.value`, and `isLoading.value`. Make sure the debounced
loader and any other callers still invoke `loadAuditTrail`, but stale responses
are ignored before committing state.
In `@VueApp/src/Effort/components/AuditChangeDetail.vue`:
- Line 3: Remove the narration comments from AuditChangeDetail’s template
branches; they just restate the conditional rendering and add noise. Update the
Vue template around the inline reference-value and changed-value sections by
deleting the explanatory comments, while keeping the existing branch logic and
structure in place.
In `@VueApp/src/Effort/pages/AuditList.vue`:
- Around line 152-167: The search input in AuditList.vue currently relies only
on placeholder text, so it loses its visible name after typing. Update the
q-input used for filter.searchText to include a persistent label while keeping
the existing placeholder as hint text, using the existing search field markup
and v-model binding to locate it.
In `@web/Areas/ClinicalScheduler/Controllers/ScheduleAuditController.cs`:
- Around line 141-187: The week-history endpoints in ScheduleAuditController
return List<AuditLogEntryDto> even though
ScheduleAuditService.BuildEnrichedAuditQuery(...) does not populate required
fields such as WeekNum and Term. Update the projection used by
GetRotationWeekHistory and GetClinicianWeekHistory to fully populate
AuditLogEntryDto, or switch those actions to a narrower DTO that only includes
the fields actually returned. Ensure the response contract matches the frontend
AuditLogEntry expectations.
- Around line 15-18: The ScheduleAuditController route is using a hardcoded
nonstandard path, so update the route attribute on ScheduleAuditController to
follow the repo’s controller-based API convention. Replace the current fixed
audit route with the standard absolute /api/{area}/{controller} pattern so the
controller resolves through its name instead of a custom endpoint.
- Around line 31-41: The XML documentation for ScheduleAuditController’s
GetAuditLogAsync contract is misleading about the person filter. Update the
<param name="person"> comment so it clearly states this parameter is a MothraID
exact match, not a substring search on display name. Keep the docs aligned with
the behavior in IScheduleAuditService.GetAuditLogAsync and
ScheduleAuditService.GetAuditLogAsync, and adjust any related wording in the
controller summary if needed.
In `@web/Areas/ClinicalScheduler/Services/ScheduleAuditService.cs`:
- Around line 388-401: The week-history projection in ScheduleAuditService’s
audit query is returning AuditLogEntryDto without setting WeekNum, TermCode, or
Term, which leaves those members at default values and makes the shared DTO
inconsistent across endpoints. Update the select new AuditLogEntryDto in the
GetRotationWeekAuditAsync/GetClinicianWeekAuditAsync query to populate those
fields from the available week-related data, or introduce a separate DTO for
week history and use it there instead of reusing AuditLogEntryDto.
- Around line 237-240: Replace the filtered catch in ScheduleAuditService with
explicit typed handlers so each failure type is handled separately. Update the
exception handling around the audit retrieval logic in the relevant service
method to use distinct catch blocks for DbUpdateException, SqlException, and
InvalidOperationException instead of a single `catch (Exception) when (...)`,
and keep the existing logging/throw behavior in each branch using the same
logger message pattern and the method context around `gradYear`.
🪄 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: a18a5a59-93d9-4808-80e7-38949f8f10db
📒 Files selected for processing (24)
VueApp/src/ClinicalScheduler/__tests__/audit-log-page-access.test.tsVueApp/src/ClinicalScheduler/components/ScheduleLegend.vueVueApp/src/ClinicalScheduler/components/ScheduleView.vueVueApp/src/ClinicalScheduler/components/SchedulerNavigation.vueVueApp/src/ClinicalScheduler/components/WeekCell.vueVueApp/src/ClinicalScheduler/components/WeekHistoryButton.vueVueApp/src/ClinicalScheduler/components/WeekHistoryContent.vueVueApp/src/ClinicalScheduler/constants/permission-messages.tsVueApp/src/ClinicalScheduler/pages/AuditLogPage.vueVueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vueVueApp/src/ClinicalScheduler/pages/RotationScheduleView.vueVueApp/src/ClinicalScheduler/router/routes.tsVueApp/src/ClinicalScheduler/services/audit-log-service.tsVueApp/src/ClinicalScheduler/types/audit-types.tsVueApp/src/ClinicalScheduler/utils/audit-actions.tsVueApp/src/Effort/components/AuditChangeDetail.vueVueApp/src/Effort/pages/AuditList.vueweb/Areas/ClinicalScheduler/Controllers/ScheduleAuditController.csweb/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditLogEntryDto.csweb/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditModifierDto.csweb/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditTermDto.csweb/Areas/ClinicalScheduler/Services/ClinicalSchedulerNavMenu.csweb/Areas/ClinicalScheduler/Services/IScheduleAuditService.csweb/Areas/ClinicalScheduler/Services/ScheduleAuditService.cs
rlorenzo
force-pushed
the
feature/clinical-scheduler-audit-log
branch
from
3bc2ddf to
b8e1cd4
Compare
There was a problem hiding this comment.
Pull request overview
Surfaces and productizes existing Clinical Scheduler audit data by adding a manager-gated audit-log API + UI (full page and inline per-week history), replacing the legacy ColdFusion “Schedule Changes Audit log”.
Changes:
- Added new Clinical Scheduler audit endpoints (
/api/clinicalscheduler/audit/*) and service methods returning enriched, filterable audit DTOs. - Added new Vue Audit Trail page + routing/navigation, plus inline per-week history popover/dialog for managers.
- Refactored Effort audit list change-detail rendering into a shared
AuditChangeDetailcomponent and adjusted filter UX defaults.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/Areas/ClinicalScheduler/Services/ScheduleAuditService.cs | Adds enriched audit-log query methods (filters, term mapping, per-week history projections). |
| web/Areas/ClinicalScheduler/Services/IScheduleAuditService.cs | Extends the service contract to include audit-log, term, modifier/person, and per-week history methods. |
| web/Areas/ClinicalScheduler/Services/ClinicalSchedulerNavMenu.cs | Adds “Audit Trail” nav item for users with Manage permission. |
| web/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditTermDto.cs | New DTO for term filter options scoped to grad year. |
| web/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditModifierDto.cs | New DTO for “Modified By” / “Person” filter options. |
| web/Areas/ClinicalScheduler/Models/DTOs/Responses/AuditLogEntryDto.cs | New enriched audit row DTO used by page + inline week history. |
| web/Areas/ClinicalScheduler/Controllers/ScheduleAuditController.cs | New API controller exposing audit-log/filter endpoints and per-week history endpoints (Manage-gated). |
| VueApp/src/Effort/pages/AuditList.vue | Improves filter defaults/UX and uses shared change-detail component. |
| VueApp/src/Effort/components/AuditChangeDetail.vue | New shared renderer for change-detail old/new values. |
| VueApp/src/ClinicalScheduler/utils/audit-actions.ts | Shared mapping from schedule-audit action → Quasar color. |
| VueApp/src/ClinicalScheduler/types/audit-types.ts | New TS types for schedule audit rows and filter option DTOs. |
| VueApp/src/ClinicalScheduler/services/audit-log-service.ts | New frontend service for Clinical Scheduler audit endpoints. |
| VueApp/src/ClinicalScheduler/router/routes.ts | Adds route for the new Audit Trail page. |
| VueApp/src/ClinicalScheduler/pages/RotationScheduleView.vue | Plumbs “can view history” + context info into the schedule grid. |
| VueApp/src/ClinicalScheduler/pages/ClinicianScheduleView.vue | Plumbs “can view history” + context info into the schedule grid. |
| VueApp/src/ClinicalScheduler/pages/AuditLogPage.vue | New Audit Trail page with responsive results display and filter controls. |
| VueApp/src/ClinicalScheduler/constants/permission-messages.ts | Adds access-denied messaging for the new audit trail feature. |
| VueApp/src/ClinicalScheduler/composables/use-audit-entries.ts | Shared loader/state composable for audit entries (page + inline history). |
| VueApp/src/ClinicalScheduler/components/WeekHistoryContent.vue | New reusable content component for per-week audit history UI. |
| VueApp/src/ClinicalScheduler/components/WeekHistoryButton.vue | New trigger button that shows per-week history (popover on desktop, dialog on mobile). |
| VueApp/src/ClinicalScheduler/components/WeekCell.vue | Adds per-week history icon and prevents cell actions when tapping embedded controls. |
| VueApp/src/ClinicalScheduler/components/ScheduleView.vue | Plumbs history props through to week cells and legend; minor export cleanup + mobile layout tweak. |
| VueApp/src/ClinicalScheduler/components/SchedulerNavigation.vue | Adds “Audit Trail” tab for managers. |
| VueApp/src/ClinicalScheduler/components/ScheduleLegend.vue | Adds manager-only legend row describing the history icon. |
| VueApp/src/ClinicalScheduler/tests/audit-log-page-access.test.ts | Adds access-control tests for the audit-log page behavior. |
rlorenzo
force-pushed
the
feature/clinical-scheduler-audit-log
branch
from
b8e1cd4 to
4f6cd6d
Compare
Replace the legacy ColdFusion "Schedule Changes Audit log" with a filterable audit trail (year, rotation, area, modified-by, person, date range) and colored action badges, matching the Effort audit trail UI. Reachable from a left-nav link and in-page tab gated on SVMSecure.ClnSched.Manage. The ScheduleAudit table was already written on every add/remove/primary-evaluator change but had no reader; this surfaces that data, so the audit read paths are no longer dead code.
Surfaces the schedule-change audit data in more places and rounds out the audit trail page: - Inline per-week change-history popover (centered dialog on mobile) on each week in Schedule-by-Rotation / Schedule-by-Clinician, gated on SVMSecure.ClnSched.Manage, backed by new per-week audit endpoints. - Audit trail: Term column and grad-year-scoped Term filter, "Year" relabeled to "Grad Year", and an explicit "All ..." default on every filter. - Responsive audit table: full table on desktop, a stacked table on tablet/iPad, and cards on phones, matching the Effort audit trail. - Fix a mobile bug where tapping an in-cell control (history, remove, primary toggle) also fired the cell's tap-to-schedule, and validate the per-week endpoint parameters.
- Default every filter to an explicit "All ..." state and move the free-text search to a bar on top of the results table. - Extract a shared AuditChangeDetail component, removing the duplicated old-to-new diff markup in the desktop and mobile views.
Replace .HasValue/.Value with "is T x" pattern matching on the audit-log filter params (rotation, term, from/to dates) so the code-quality analyzer stops flagging the guarded .Value access as a possible null dereference. No behavior change.
The per-week change-history clock icon now has a key in the schedule "Quick Guide" legend, shown only when the user can view history (SVMSecure.ClnSched.Manage), so the icon is explained for the people who can use it.
rlorenzo
force-pushed
the
feature/clinical-scheduler-audit-log
branch
from
4f6cd6d to
15d53b4
Compare
- main's fallow 2.101 bump tightened the template complexity gate, failing CI on the audit page and two uncovered new components - extract the responsive results tables into AuditLogResultsTable and compute responsive classes once so templates stay branch-free - cover WeekHistoryContent, AuditChangeDetail, and the new table with component tests
Contributor
Author
|
@bsedwards Should be on TEST now. Adds audit trail inline on the weeks display as well as a dedicated page |
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
Surfaces the clinical schedule-change audit data VIPER already records, replacing the legacy ColdFusion "Schedule Changes Audit log". Everything is gated on
SVMSecure.ClnSched.Manage, the same permission the legacy page used, so no new permission is introduced.ScheduleAudittable (grad year, term, rotation, area, modified-by, person, date range) with colored action badges.AuditChangeDetailcomponent (removes duplicated diff markup).Activates previously-dead code (flagged by fallow)
This branch is mostly surfacing audit code that already existed but was dead, not new plumbing:
ScheduleAudittable has been written on every add/remove/primary-evaluator change, but had no reader. At this branch's base the read methods (GetInstructorScheduleAuditHistoryAsync,GetRotationWeekAuditHistoryAsync) had zero callers, i.e. dead code.