Conversation
- Add percent rollover with preview, SSE progress, and audit-aware skipping of manually edited records - Add clinical import from Clinical Scheduler with preview and SSE - Add TermValidationHelper for rollover/import eligibility checks - Add Vue dialogs (PercentRolloverDialog, ClinicalImportDialog) and supporting table components - Add post-deployment task to clean up duplicate percentages - Refine HarvestDialog layout and add comprehensive tests
- Remove GuestAccountPhase, guest constants, IsGuest DTO property, and guest UI tab/email guards/dashboard filtering - Add post-deploy task 16 to purge legacy guest persons/records with tightened predicate and transactional delete - Add IsNew flag and status badges to harvest preview tables - Exclude RESID courses from removed-courses detection - Fix clinical week counting to use distinct WeekId - Fix clinical course priority fallback (0 → int.MaxValue) - Strengthen origin validation to check scheme and port - Fix percent rollover duplicate detection to match on EndDate - Extract shared format utils, remove unused mobile grid views - Rewrite guest-related tests as removed-items and IsNew tests
There was a problem hiding this comment.
Pull request overview
This PR implements percent assignment rollover functionality for Fall academic terms, enabling automatic extension of percentage assignments from one academic year to the next. Additionally, it removes legacy guest account functionality and adds clinical import capabilities.
Changes:
- Adds percent assignment rollover service with idempotency checks and audit trail exclusion for manually-edited assignments
- Removes guest account import phase and related constants (APCGUEST, PHRGUEST, etc.)
- Adds clinical import service with multiple import modes (AddNewOnly, ClearReplace, Sync)
- Introduces TermValidationHelper for centralized term eligibility checks
- Updates frontend with new dialogs for rollover and clinical import operations
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/Areas/Effort/Services/PercentRolloverService.cs | New service implementing percent assignment rollover logic for Fall terms |
| web/Areas/Effort/Services/ClinicalImportService.cs | New service interface for clinical data import (implementation not shown) |
| web/Areas/Effort/Services/HarvestService.cs | Integrates rollover into harvest flow; removes guest account phase |
| web/Areas/Effort/Services/Harvest/GuestAccountPhase.cs | Deleted - removes guest account functionality |
| web/Areas/Effort/Services/VerificationService.cs | Removes guest instructor email validation checks |
| web/Areas/Effort/Helpers/TermValidationHelper.cs | New helper for centralized term eligibility validation |
| web/Areas/Effort/Controllers/PercentRolloverController.cs | New API controller for rollover with SSE progress streaming |
| web/Areas/Effort/Controllers/ClinicalImportController.cs | New API controller for clinical import with SSE progress |
| web/Areas/Effort/Scripts/EffortPostDeployment.cs | Adds cleanup tasks for duplicate percentages and guest accounts |
| VueApp/src/Effort/components/PercentRolloverDialog.vue | New Vue component for rollover preview and execution |
| VueApp/src/Effort/components/ClinicalImportDialog.vue | New Vue component for clinical import with mode selection |
| VueApp/src/Effort/pages/TermManagement.vue | Adds rollover and clinical import buttons to term management |
| test/Effort/PercentRolloverServiceTests.cs | Comprehensive unit tests for rollover service |
| test/Effort/TermValidationHelperTests.cs | Unit tests for term validation helper |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…diff - Add BatchResolveDepartmentsAsync using full resolution chain (override → jobs → employee fields → fallback) across all three harvest phases, replacing DeptSimpleNameLookup - Validate clinical-only instructors against AAUD/title codes - Show old → new weeks in clinical preview for updated records - Exclude R-course placeholders from dashboard zero-hours query - Refactor percent rollover to LINQ Select + AddRange
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Sanitize requestHost via LogSanitizer to prevent log injection in BaseEffortController cross-origin warning - Use double literal (7.0) in AddDays to avoid possible integer overflow in ClinicalImportServiceTests week seeding
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add .github/copilot-instructions.md with C#, DB, and security conventions for Copilot
- Percent rollover is now a standalone operation by academic year, no longer tied to Fall harvest; auto-detects year from June/July boundary with unusual-timing warnings - ClinicalImportService is the single code path for all clinical records (standalone and harvest), with AAUD validation and auto-creation of missing EffortPerson/Course records - Fix harvest preview showing existing clinical records as "New"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 63 out of 63 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix RolloverResult TS type to match backend DTO field name (count → assignmentsCreated) - Sanitize year param in PercentRolloverController log calls - Use AddRange instead of per-item Add in rollover progress loop
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 63 out of 63 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Wrap await importTask/rolloverTask so client disconnect doesn't surface as unhandled exception; remove ct from Task.Run - Push EndDate filter into DB query for rollover idempotency check - Qualify both columns in guest-cleanup SQL join predicate
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 63 out of 63 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…a lookups - Replace manual joins/subqueries with ViperPerson navigations, removing VIPERContext from rollover service - Centralize percentage display/storage conversion in EffortConstants helpers - Skip loading edit metadata for view-only users in InstructorEdit
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 74 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Backend now returns display-ready values (e.g., 12.5 not 0.125) so frontend no longer multiplies by 100 - Use getStatus helper for consistent row key generation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 74 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
web/Areas/Effort/Services/VerificationService.cs:342
- Guest-email suppression was removed here. If any legacy guest placeholder rows still exist (and the post-deploy cleanup hasn’t run yet), they can now receive verification emails. Consider keeping a defensive server-side block for
FirstName == "GUEST"until the cleanup task is guaranteed to have been executed.
web/Areas/Effort/Services/Harvest/HarvestPhaseBase.cs:154 - Removing the “skip when Hours and Weeks are both 0” guard means harvest can now insert placeholder records with no time allocation (e.g., Non-CREST Director records use
Hours = 0andWeeks = null). Those records will be treated as “zero hour” data-quality issues elsewhere (e.g.,HasZeroHoursuses(Hours ?? 0) == 0 && (Weeks ?? 0) == 0). If these records are intentional, consider adjusting the downstream zero-hour detection/alerts; otherwise, restore a skip/validation so empty records aren’t imported.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Overage warning used F0 format, hiding values like 100.4% as "100%"; now uses F1 to match PercentDisplayDecimals - Fix IsFallTerm doc that falsely claimed single-char code support
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 74 out of 74 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rollover now targets Fall Semester only (code *09), dropping unnecessary Fall Quarter fallback - Remove unused GetPercentagesForPersonByDateRangeAsync and IsFallTerm/IsFallTermByCode helpers - Remove build script timeout that killed long-running builds
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
web/Areas/Effort/Services/VerificationService.cs:342
- Guest placeholder instructors may still exist in the database until cleanup runs, but the guest check was removed from single-recipient email sending. This can lead to attempts to send verification emails to legacy guest placeholders. Consider retaining an explicit guest exclusion (or ensuring guest placeholders cannot appear in the instructor list) until the cleanup step is complete.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2 participants
Also includes: VPR-68 - Feedback: Do not import guest users