-
Notifications
You must be signed in to change notification settings - Fork 0
VPR-19 - Update course relationships (cross list and section) #88
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
base: VPR-18-course-crud
Are you sure you want to change the base?
Conversation
- Support CrossList and Section parent-child relationships - Add CourseLinkDialog and CourseDetail page with relationship display - Create CourseRelationshipsController and service with validation - Prevent cycles and multiple-parent relationships
…ondition - Hide link button when course already has parent (enforces flat hierarchy) - Add ParentCourseId to CourseDto to track child status - Fix async race condition when navigating between courses quickly
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 course relationship management for cross-listing and sectioning courses in the Effort system. The implementation enforces a flat hierarchy where courses can either be parents with multiple children, or children with a single parent, but not both simultaneously.
Key Changes:
- Database schema updated to support course relationships with unique constraint ensuring each child has only one parent
- Backend service and API endpoints for creating, viewing, and deleting course relationships
- Frontend UI components for linking/unlinking courses with permission-based access control
- Comprehensive test coverage for both backend services and controllers
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| web/Program.cs | Registers CourseRelationshipService in DI container |
| web/Areas/Effort/docs/Effort_Database_Schema.sql | Adds CourseRelationships table with constraint updates and unique index |
| web/Areas/Effort/Scripts/CreateEffortDatabase.cs | Database creation script with CourseRelationships table |
| web/Areas/Effort/Services/ICourseRelationshipService.cs | Service interface defining relationship operations |
| web/Areas/Effort/Services/CourseRelationshipService.cs | Service implementation with validation and hierarchy enforcement |
| web/Areas/Effort/Services/CourseService.cs | Updated to populate ParentCourseId in GetCoursesAsync |
| web/Areas/Effort/Controllers/CourseRelationshipsController.cs | API endpoints with permission and authorization checks |
| web/Areas/Effort/Models/DTOs/* | Request/Response DTOs for relationship operations |
| web/Areas/Effort/EffortDbContext.cs | EF Core configuration for CourseRelationships entity |
| VueApp/src/Effort/types/index.ts | TypeScript type definitions for relationships |
| VueApp/src/Effort/services/effort-service.ts | Frontend service methods for relationship API calls |
| VueApp/src/Effort/router/routes.ts | New CourseDetail route definition |
| VueApp/src/Effort/pages/CourseList.vue | Added link button and CourseLinkDialog integration |
| VueApp/src/Effort/pages/CourseDetail.vue | New page displaying course relationships |
| VueApp/src/Effort/components/CourseLinkDialog.vue | Dialog component for managing course links |
| VueApp/src/Effort/composables/use-effort-permissions.ts | Added hasLinkCourses permission check |
| test/Effort/CourseRelationshipsControllerTests.cs | Unit tests for API controller |
| test/Effort/CourseRelationshipServiceTests.cs | Unit tests for service layer |
| VueApp/src/Effort/tests/course-link-dialog.test.ts | Frontend component tests |
Comments suppressed due to low confidence (1)
web/Areas/Effort/Services/CourseService.cs:85
- The GetCourseAsync method does not populate the ParentCourseId field, while GetCoursesAsync does. This inconsistency means that when a single course is fetched, the frontend won't know if it's a child course and can't properly hide the link button. The method should be updated to query and populate the ParentCourseId similar to GetCoursesAsync.
public async Task<CourseDto?> GetCourseAsync(int courseId, CancellationToken ct = default)
{
var course = await _context.Courses
.AsNoTracking()
.FirstOrDefaultAsync(c => c.Id == courseId, ct);
return course == null ? null : ToDto(course);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
web/Areas/Effort/Models/DTOs/Requests/CreateCourseRelationshipRequest.cs
Outdated
Show resolved
Hide resolved
web/Areas/Effort/Models/DTOs/Requests/CreateCourseRelationshipRequest.cs
Show resolved
Hide resolved
- Populate ParentCourseId in GetCourseAsync for link button visibility - Reload course list when relationships change - Add LinkCourses permission to CourseDetail route - Add Range validation for ChildCourseId - Optimize relationship query to filter by term
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 21 out of 21 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.