Feat/coord interface/integration #526
Conversation
* initial table * table css work * table css work * front end table searching and select data logic * front end table searching and select data logic * search filter works * im done fr now fixed select glitch and styled input and buttons slightly * finished front end table * functions and dropdown basics * touchups for front end filtering integration and css * copy button works and table header structure * fixed filtering bugs and css for action buttons * fixed weird double copy bug * fixed filter copy bug * finished coord interface --------- Co-authored-by: Heidi Chan <[email protected]>
…eley/csm_web into feat/coord-interface/integration
This reverts commit 5bc7521.
csm_web
|
||||||||||||||||||||||||||||
| Project |
csm_web
|
| Branch Review |
feat/coord-interface/integration
|
| Run status |
|
| Run duration | 01m 47s |
| Commit |
|
| Committer | gabrielhan23 |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
42
|
| View all changes introduced in this branch ↗︎ | |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
gabrielhan23
requested review from
edwardneo,
smartspot2,
ElaineShu0312,
isabellaalps and
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR implements a coordinator interface integration that allows coordinators to view and manage students and mentors in their courses. The implementation includes backend API endpoints for coordinator data access, frontend React components for displaying tabular data with filtering capabilities, and routing updates to support the new coordinator views.
- Adds backend API endpoints for coordinator authentication and data retrieval
- Implements React-based coordinator interface with search and filtering functionality
- Integrates coordinator view access through existing course interface
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| csm_web/scheduler/views/coord.py | New coordinator API views with authentication and data serialization |
| csm_web/scheduler/serializers.py | Added coordinator-specific serializers and docstring updates |
| csm_web/scheduler/models.py | Added mentor family field, section day_time property, and pylint disable comments |
| csm_web/frontend/src/components/coord_interface/*.tsx | React components for coordinator table interface with filtering and search |
| csm_web/frontend/src/utils/queries/coord.tsx | API query utilities for coordinator data |
| csm_web/scheduler/urls.py | URL routing for coordinator endpoints |
| csm_web/frontend/src/components/App.tsx | Added coordinator routes to main app |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| export const getCoordData = async (courseId: number, isStudents: boolean) => { | ||
| // query disabled when id undefined | ||
| if (isNaN(courseId!)) { |
There was a problem hiding this comment.
The non-null assertion operator ! is used on courseId which is already typed as number. This assertion is unnecessary and could mask potential runtime errors if courseId is actually undefined.
| if (isNaN(courseId!)) { | |
| if (isNaN(courseId)) { |
Copilot uses AI. Check for mistakes.
| setSearch([]); | ||
| setSelected([]); | ||
| setAllSelected(false); | ||
| const checkbox = document.getElementById("checkcheck") as HTMLInputElement; |
There was a problem hiding this comment.
The element ID 'checkcheck' doesn't match the actual checkbox ID which should be 'checkcheck' (from CheckBox component). This should be consistent with the CheckBox component's ID generation pattern.
Copilot uses AI. Check for mistakes.
| if (field in row) { | ||
| const value = row[field as keyof typeof row]; | ||
| if (filter.includes("+")) { | ||
| return value != null ? value.toString() >= filter.slice(0, -1) : false; | ||
| } | ||
| return value != null ? value.toString().includes(filter) : false; | ||
| } |
There was a problem hiding this comment.
String comparison using >= for filtering (line 131) will perform lexicographic comparison rather than numeric comparison. For numeric fields like numUnexcused or numStudents, this should use Number(value) >= Number(filter.slice(0, -1)).
Copilot uses AI. Check for mistakes.
| </tr> | ||
| </thead> | ||
| <tbody> | ||
| {searchData.length === 0 ? <div className="no-data">No data found...</div> : null} |
There was a problem hiding this comment.
A <div> element cannot be a direct child of <tbody>. This should be wrapped in a <tr><td colSpan={columnCount}> structure to be valid HTML.
| {searchData.length === 0 ? <div className="no-data">No data found...</div> : null} | |
| {searchData.length === 0 ? ( | |
| <tr> | |
| <td className="no-data" colSpan={6}>No data found...</td> | |
| </tr> | |
| ) : null} |
Copilot uses AI. Check for mistakes.
| ) | ||
| if not is_coord: | ||
| raise PermissionDenied( | ||
| "You do not have permission to view the coordinator view." |
There was a problem hiding this comment.
The error message references 'view the coordinator view' which is redundant and unclear. It should be more specific, such as 'You do not have permission to delete sections for this course.'
| "You do not have permission to view the coordinator view." | |
| "You do not have permission to delete sections for this course." |
Copilot uses AI. Check for mistakes.
No description provided.