-
Notifications
You must be signed in to change notification settings - Fork 0
VPR-17 - Create Term Management capability #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- VPR-12: Create Effort .NET area structure (Controllers, Services, Constants, DbContext) and Vue SPA (router, pages, services, types) - VPR-13: Implement EffortDbContext with 14 entity models and Fluent API mappings for the effort schema tables - VPR-14: Add 4 response DTOs (Term, Person, Course, Record) with mapping extensions; only Term API is implemented in this commit
- Add TermManagement and TermSelection pages with lifecycle controls - Create EffortLayout with permission-aware navigation - Add APIs for term creation, open/close/reopen actions
- Add AuditList.vue with responsive desktop/tablet/mobile layouts - Create AuditController with server-side pagination and filtering - Implement bidirectional course/subject filter options - Support URL-based filter persistence for bookmarkable searches - Extend Audit entity with instructor, course, and term relationships
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements Phase 2 of the Effort system modernization, adding term management capabilities, audit trail functionality, and Vue.js frontend components.
Key Changes:
- Added comprehensive term management API with workflow status tracking (Created → Harvested → Opened → Closed)
- Implemented audit trail system with granular action tracking and filtering capabilities
- Created Vue.js frontend with term selection, management, and audit viewing interfaces
- Refined database schema to use granular audit actions instead of generic INSERT/UPDATE/DELETE
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/Program.cs | Registered Effort DbContext and services; added "Effort" to VueAppNames array; improved Vite fallback logging |
| web/Models/VIPER/Term.cs | Added FacilityScheduleTermCode constant to filter out facility schedule term |
| web/Areas/Effort/docs/*.md | Updated documentation to reflect TermCode column usage and removed LegacyTermCode references |
| web/Areas/Effort/docs/Effort_Database_Schema.sql | Changed Audits.Action column to varchar(50) for granular actions; moved TermCode out of legacy columns |
| web/Areas/Effort/Services/* | Implemented TermService, EffortPermissionService, and EffortAuditService with comprehensive business logic |
| web/Areas/Effort/Controllers/* | Added API controllers for terms and audit trail with permission-based access control |
| web/Areas/Effort/Models/Entities/* | Created entity classes for all Effort database tables |
| web/Areas/Effort/EffortDbContext.cs | Implemented EF Core DbContext with complete entity configurations |
| web/Areas/Effort/Constants/* | Defined permission and audit action constants |
| VueApp/src/Effort/* | Created complete Vue.js frontend with term management, audit viewing, and routing |
| test/Effort/TermServiceTests.cs | Added comprehensive unit tests for TermService operations |
Comments suppressed due to low confidence (3)
web/Areas/Effort/docs/MIGRATION_GUIDE.md:1
- Large sections of migration documentation were removed (sprint timeline, risk assessment, pre-production checklist). If this content is still relevant for the migration process, consider preserving it in a separate document or ensuring it's documented elsewhere.
web/Areas/Effort/Scripts/README.md:1 - The entire README.md file for migration scripts was deleted. This appears to contain important documentation about the migration process, execution order, and verification steps. Consider whether this documentation should be preserved or moved to another location.
web/Areas/Effort/Services/TermService.cs:1 - The exclusion of FacilityScheduleTermCode is hardcoded in this query. Consider making this filter reusable through a helper method or extension method to ensure consistency across the codebase when filtering terms.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix redundant null casts in ApplyTermNameSorting SortKey calculation - Simplify else-if to else in SerializeChanges since oldValues is guaranteed non-null at that point
- Reorder nullable casts in ternary expressions to eliminate constant condition warnings about expressions being "never null" or "always null" - Cast non-nullable int values to int? explicitly instead of casting the null literal
- Reorder null-coalescing chain in ApplyTermNameSorting so each fallback can legitimately be null, eliminating constant-condition warnings
- Refactor ApplyTermNameSorting to use let clauses with default(int?) to eliminate constant-condition warnings while preserving behavior
- Fix usp_delCourse FK constraint error by deleting course relationships before the course (modern schema has FK constraints legacy lacked) - Continue SP verification even when view verification has failures, allowing all issues to be seen at once rather than blocking - Add tblAudit to expected row differences (unmapped ChangedBy skipped) - Remove early exit that skipped all SP tests on view failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 68 out of 68 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.
- Extract EffortLeftNav component from EffortLayout using ViperLayout slot - Replace manual ToDto() extensions with AutoMapperProfileEffort - Add ManageAccess and Reports permission constants - Fix external link security (rel="noopener noreferrer")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 70 out of 70 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.
bsedwards
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still seeing highlighted top nav = Effort instead of Faculty but otherwise approved
No description provided.