-
Notifications
You must be signed in to change notification settings - Fork 0
Student photo gallery #57
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
- Photo gallery UI with grid, list, and print sheet views - Multi-level filtering: class year, eighths, twentieths, teams (V3), streams (V3) - Ross student support with special designation badge in UI and exports - Export to Word (4-column table) and PDF (QuestPDF) with photo rosters - Updated lint-staged to continue linting with errors - Team and stream data restricted to V3 students only
- Fixing API path
- Fixing API path for backend
- Fix 403 errors on TEST server by removing [Area] attribute and using ApiController base class
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.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
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
Implements a new Student Photo Gallery feature end-to-end, including backend APIs/services for photo retrieval and export (Word/PDF), SIS data access, and a full Vue-based UI with grid/list/printable sheet views.
- Adds SIS context and StudentDesignation model to fetch Ross student designations
- Introduces Photo, Student Group, and Export services with API endpoints; integrates QuestPDF and OpenXML for exports
- Adds Vue pages, store, services, and components for browsing, exporting, and printing student photos
Reviewed Changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/appsettings.json | Adds PhotoGallery configuration keys (paths, default file, cache hours) to support photo lookup and caching. |
| web/Viper.csproj | Adds DocumentFormat.OpenXml and QuestPDF dependencies for export features. |
| web/Program.cs | Registers SISContext and new Students services in DI. |
| web/Models/SIS/StudentDesignation.cs | New EF model mapping for Ross student designations. |
| web/Classes/SQLContext/SISContext.cs | New DbContext for SIS, wiring up StudentDesignations and connection string. |
| web/Areas/Students/Services/StudentGroupService.cs | Core logic to fetch students by class/various groupings; handles Ross students; provides grouping metadata. |
| web/Areas/Students/Services/PhotoService.cs | Reads student photos from configured path with caching, safe path validation, and default fallback. |
| web/Areas/Students/Services/PhotoExportService.cs | Generates Word/PDF exports of the gallery with centralized title logic. |
| web/Areas/Students/Models/PhotoGalleryViewModel.cs | Shared view models for gallery data and export contracts. |
| web/Areas/Students/Controller/PhotoGalleryController.cs | API endpoints for gallery data, photos (student/default), exports, and metadata. |
| scripts/verify-build.js | Adjusts test build path used by CI verification. |
| package.json | Updates lint-staged and changes precommit lint behavior; bumps lint-staged version. |
| VueApp/src/layouts/ViperLayout.vue | Minor prop defaults; adds no-print classes for print layout. |
| VueApp/src/layouts/LeftNav.vue | Minor prop defaults/cleanup and no-print. |
| VueApp/src/composables/validation-error.ts | Extracted typed ValidationError class. |
| VueApp/src/composables/auth-error.ts | Extracted typed AuthError class. |
| VueApp/src/composables/ViperFetch.ts | Refactors fetch wrapper, typed helpers, adds postForBlob; re-exports as named. |
| VueApp/src/Students/stores/photo-gallery-store.ts | Pinia store managing gallery state, fetching, grouping counts, and exports. |
| VueApp/src/Students/services/photo-gallery-service.ts | Client service for gallery API and downloads. |
| VueApp/src/Students/router/routes.ts | Students area routes incl. PhotoGallery; switches to named export. |
| VueApp/src/Students/router/index.ts | Imports named routes; keeps auth guard. |
| VueApp/src/Students/pages/PhotoGallery.vue | Main gallery page with filters, views, exports, and print handling. |
| VueApp/src/Students/composables/use-photo-url.ts | Helper to construct photo URLs. |
| VueApp/src/Students/components/PhotoGallery/* | UI components for grid/list/sheet views and photo dialog. |
| .oxlintrc.json | Rule override for ViperFetch file naming. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Looks like a good start. Obviously will need to sanitize/validate things like class level and group. I appreciate the examples of PDFs and Word document generation - the CF equivalent is simple but much less flexible. |
- Add responsive design to student modal with mobile/desktop layouts - Add input validation to all photo gallery endpoints - Optimize photo export with parallel loading - Fix EF query translation issue in Ross student lookup - Fix TypeScript validation error types
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 29 out of 31 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Added tabbed interface with Photo Gallery and Student List views - Implemented URL-based navigation supporting direct links to specific students with browser history integration - Created new student list API endpoint with printable roster view - Display prior class year information on cards and in detail dialog
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 33 out of 35 changed files in this pull request and generated 39 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 33 out of 35 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Reduced logging - Using LogSanitizer from main branch
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 33 out of 35 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add course filtering: view students by term/CRN - Add Ross students toggle only for V3/V4 - Add comprehensive test coverage: 5 new test suites, 130+ tests - Sanitizing log entries
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 62 out of 65 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- VueApp/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@bsedwards The course group filters have been added, and the Word/PDF generation has been improved. Done general code cleanup as well, including moving the files from Students/Controller to Students/Controllers to match .NET conventions. |
- Resolves all 43 compiler and SonarLint warnings in Students area
- Add 3×3 large photo layout for Eighths/Twentieths/Teams exports - Display names as "FirstName LastName" in group exports - Hide title/date headers and group info for group exports - Add repeating group headers with ranges (e.g., "1A1 (1-9)") - Added descriptive export filenames (e.g., "Student Groups V1 eighths.docx")
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 66 out of 69 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- VueApp/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fixed filename for exports for all export types - Made PDF exports match Word exports
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 66 out of 69 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- VueApp/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@bsedwards I improved the Word/PDF exports to better match the legacy system. Please review on TEST. |
- Add specific exception handlers for ArgumentException, InvalidOperationException, and DbUpdateException in filename generation to improve diagnostic logging - Simplify redundant ternary operator in large layout chunk where useLargeLayout is guaranteed true
|
@bsedwards Made one last change. But how do we get this linked to in VIPER2? Seems that the link to the gallery in the sidenav is controlled by a CMS value? |
On TEST: https://secure-test.vetmed.ucdavis.edu/2/Students/PhotoGallery
Still TODO:
But wanted to get some initial feedback.