Old code clean up - #392
nadjajovancevic wants to merge 37 commits into
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… elevated permissions
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate issues affect compilation, routing, permissions, grading, filesystem operations, and UI behavior.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 7
Open (12)
Enable manual grading switch creation · New Invoke elevated permission check · New Use a supported endpoint for grading scores · New Use a registered submissions endpoint · New Await directory creation in path order · New Target the registered submission feedback endpoint · New Retain ES2019 library support for array methods · New Prevent malformed deadline text for zero-valued units · New Await submission status update during save · New Remove duplicate slash from remote status URL · New Build selected-files query parameters correctly · New Initialize theme before first render · New
What changed in this PR
Refactors the labextension’s legacy services and UI, adds grading/submission/local-file functionality, and updates routing, styling, TypeScript, and dependencies.
Changes:
- Reorganized services, widgets, commands, and notebook components.
- Added grading, submission, Git, filesystem, and manual-grading functionality.
- Updated UI styles, compiler settings, and dependency metadata.
| File | Summary |
|---|---|
packages/labextension/yarn.lock |
Updates dependency resolutions. Critical (2): lockfile does not match the manifest and may fail immutable installs. |
packages/labextension/tsconfig.json |
Updates TypeScript compiler settings. Critical (1): removing ES2019 breaks flat/flatMap declarations. |
packages/labextension/style/css/output.css |
Regenerates utility styles. |
packages/labextension/src/widget.tsx |
Updates the main widget. Moderate (4): initial theme is not set before the first render. |
packages/labextension/src/services/user.service.ts |
Moves HTTP method imports. |
packages/labextension/src/services/submissions.service.ts |
Adds submission APIs. Critical (3): feedback targets an unregistered endpoint; Moderate (1): response types inaccurately declare Submission results in two methods. |
packages/labextension/src/services/storage.service.ts |
Simplifies storage helpers. |
packages/labextension/src/services/request.service.ts |
Refactors request handling. |
packages/labextension/src/services/queries/lectures.queries.ts |
Uses renamed lecture APIs. |
packages/labextension/src/services/permission.service.ts |
Extracts permission enums and helpers. |
packages/labextension/src/services/local-file.service.ts |
Adds local file operations. Critical (4): asynchronous directory creation is not awaited or ordered. |
packages/labextension/src/services/lectures.service.ts |
Refactors lecture APIs. |
packages/labextension/src/services/grading.service.ts |
Adds grading APIs. Critical (3): unsupported student-name score routes; Critical (2): missing user-submissions handler; Moderate (1): feedback creation uses GET instead of POST. |
packages/labextension/src/services/gradebook.service.ts |
Adds gradebook operations. |
packages/labextension/src/services/git.service.ts |
Adds repository operations. Moderate (4): selected-file query parameters are malformed when no commit message is supplied. |
packages/labextension/src/services/file.service.ts |
Adds remote file APIs. Moderate (4): generated URL contains a double slash and does not match the handler. |
packages/labextension/src/services/file-browser.service.ts |
Removes the replaced file-browser helper. |
packages/labextension/src/services/enums/permissions-scope.enum.ts |
Adds the permission scope enum. |
packages/labextension/src/services/enums/http-methods.enum.ts |
Adds the HTTP method enum. |
packages/labextension/src/services/assignments.service.ts |
Refactors assignment URLs and APIs. |
packages/labextension/src/menu.ts |
Updates Grader Service navigation. Moderate (1): instructor lectures are omitted; Moderate (1): navigation uses a relative path. |
packages/labextension/src/index.ts |
Rewires commands, widgets, and permissions. Critical (4): permission function is tested without invocation, exposing instructor tooling to all users. |
packages/labextension/src/components/notebook/create-assignment/creation-component.tsx |
Removes the old assignment-creation component location. |
packages/labextension/src/app/shadcn-components/ui/tooltip.tsx |
Adjusts tooltip sizing. |
packages/labextension/src/app/shadcn-components/ui/switch.tsx |
Updates switch styling. |
packages/labextension/src/app/shadcn-components/ui/field.tsx |
Adds minimum-width handling. |
packages/labextension/src/app/shadcn-components/ui/dialog.tsx |
Adds dialog sizing. |
packages/labextension/src/app/shadcn-components/ui/date-time-picker.tsx |
Adds deadline difference handling. |
packages/labextension/src/app/pages/instructor-view/lecture.tsx |
Improves lecture title layout. |
packages/labextension/src/app/pages/instructor-view/dashboard.tsx |
Updates lecture APIs and pluralization helpers. |
packages/labextension/src/app/components/utils/utils.tsx |
Renames helpers and expands date differences. |
packages/labextension/src/app/components/utils/repo-type.tsx |
Adds repository types. |
packages/labextension/src/app/components/utils/deadline.tsx |
Fixes the component import path. |
packages/labextension/src/app/components/ui/global-sidebar.tsx |
Gates view switching by permissions. |
packages/labextension/src/app/components/notebook/switch-factory.tsx |
Updates notebook permissions and path imports. |
packages/labextension/src/app/components/notebook/student-plugin/hint-widget.tsx |
Adds hint widget support. |
packages/labextension/src/app/components/notebook/student-plugin/hint-component.tsx |
Adds hint rendering. |
packages/labextension/src/app/components/notebook/student-plugin/deadline-wrapper.tsx |
Updates deadline service usage. |
packages/labextension/src/app/components/notebook/student-plugin/deadline-widget.tsx |
Adds deadline widget support. |
packages/labextension/src/app/components/notebook/slider.tsx |
Adds notebook mode switching. |
packages/labextension/src/app/components/notebook/model.ts |
Handles missing metadata values. |
packages/labextension/src/app/components/notebook/manual-grading/grading-switch.tsx |
Adds grading mode controls. Critical (2): component is not instantiated; Moderate (1): dirty state is cleared before a potentially failing request; Moderate (2): status update is not awaited. |
packages/labextension/src/app/components/notebook/manual-grading/grade-widget/points-component.tsx |
Adds score and extra-credit inputs. |
packages/labextension/src/app/components/notebook/manual-grading/grade-widget/grade-widget.tsx |
Adds the grade widget. |
packages/labextension/src/app/components/notebook/manual-grading/grade-widget/grade-component.tsx |
Adds grade controls. |
packages/labextension/src/app/components/notebook/manual-grading/grade-widget/comment-component.tsx |
Adds grading comments. |
packages/labextension/src/app/components/notebook/manual-grading/data-widget/data-widget.tsx |
Adds the grading data widget. |
packages/labextension/src/app/components/notebook/manual-grading/data-widget/data-component.tsx |
Displays cell grading data. |
packages/labextension/src/app/components/notebook/create-assignment/validator.tsx |
Updates validation layout. |
packages/labextension/src/app/components/notebook/create-assignment/error-widget.tsx |
Adds the error widget. |
packages/labextension/src/app/components/notebook/create-assignment/error-component.tsx |
Adjusts error styling. |
packages/labextension/src/app/components/notebook/create-assignment/creation-widget.tsx |
Adds the creation widget. |
packages/labextension/src/app/components/notebook/create-assignment/creation-switch.tsx |
Updates creation-switch imports. |
packages/labextension/src/app/components/notebook/create-assignment/creation-component.tsx |
Rebuilds assignment creation UI. Moderate (1): missing metadata represented by null is treated as enabled. |
packages/labextension/src/app/components/grader-service/lectures/lecture-row.tsx |
Uses the renamed user API. |
packages/labextension/src/app/components/grader-service/lectures/lecture-card.tsx |
Updates lecture metadata and styling. |
packages/labextension/src/app/components/grader-service/lectures/edit-lecture-dialog.tsx |
Updates edit icon styling. |
packages/labextension/src/app/components/grader-service/assignments/export-grades-dialog.tsx |
Uses updated export and file APIs. |
packages/labextension/src/app/components/grader-service/assignments/assignment-create-edit-dialog.tsx |
Adds deadline difference display. Moderate (3): deadline text can render malformed output for missing or zero units. |
packages/labextension/src/app/components/grader-service/assignments/assignment-card.tsx |
Adds dark-mode styling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import { ManualStatus } from '../../../../model/manualStatus'; | ||
| import { Switch } from '../../../shadcn-components/ui/switch'; | ||
|
|
||
| export class GradingModeSwitch extends React.Component<IModeSwitchProps> { |
| console.log( | ||
| 'Non-student permissions found! Adding coursemanage launcher and connecting creation mode' | ||
| ); | ||
| if (UserPermissions.hasElevatedPermissions) { |
| return request<any>( | ||
| HTTPMethod.GET, | ||
| `${baseUrl({ lectureId, assignmentId })}grading/${student.name}/score`, | ||
| null |
| let url = `/api/users/${username}/submissions`; | ||
| const params = new URLSearchParams({ | ||
| format: format | ||
| }); | ||
| url += '?' + params.toString(); |
| export const makeDirs = async (path: string, names: string[]) => { | ||
| let p = path; | ||
| names.map(async name => { | ||
| p = await makeDir(p, name); | ||
| }); | ||
| return p; | ||
| }; |
| let timeDiffText = ''; | ||
| if (daysDiff > 0) { | ||
| timeDiffText += `${daysDiff} ${pluralize({ | ||
| text: 'day', | ||
| data: daysDiff | ||
| })}, `; | ||
| } | ||
| if (hoursDiff > 0) { | ||
| timeDiffText += `${hoursDiff} ${pluralize({ | ||
| text: 'hour', | ||
| data: hoursDiff | ||
| })} and `; | ||
| } | ||
| if (minutesDiff > 0) { | ||
| timeDiffText += `${minutesDiff} ${pluralize({ | ||
| text: 'minute', | ||
| data: minutesDiff | ||
| })}`; | ||
| } | ||
| return timeDiffText; |
| updateSubmission( | ||
| this.lecture.id, | ||
| this.assignment.id, | ||
| this.subID, | ||
| submission | ||
| ); |
| const url = `${baseUrl({ | ||
| lectureId: lecture.id, | ||
| assignmentId: assignment.id | ||
| })}/remote-file-status/${repo}/?file=${encodeURIComponent(filePath)}`; |
| let url = `${baseUrl({ lectureId, assignmentId })}push/${repoType}`; | ||
| if (commitMessage) { | ||
| const searchParams = new URLSearchParams({ | ||
| 'commit-message': commitMessage | ||
| }); | ||
| url += '?' + searchParams; | ||
| } | ||
|
|
||
| if (selectedFiles && selectedFiles.length > 0) { | ||
| selectedFiles.forEach(file => { | ||
| url += `&selected-files=${encodeURIComponent(file)}`; | ||
| }); | ||
| } |
| themeManager.themeChanged.connect(() => { | ||
| this.theme = themeManager.isLight(themeManager.theme ?? 'light') | ||
| ? 'light' | ||
| : 'dark'; | ||
| this.update(); // <- tells Lumino to re-invoke render() | ||
| this.update(); // tells Lumino to re-invoke render() |
| if (extraCredit) { | ||
| return extraCredit; | ||
| } else { | ||
| return 0.0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Wouldn't it be equivalent to return extraCredit || 0.0? Or is there some edge case that I'm missing?
| let sum = 0; | ||
| const grades_dict = this.properties['notebooks'][notebook]['grades_dict']; | ||
| for (const cellId of Object.keys(grades_dict)) { | ||
| sum += this.getGradeScore(notebook, cellId); | ||
| } | ||
| return sum; |
There was a problem hiding this comment.
It looks like map and reduce would be a perfect combo to replace the for loop and the variable ;)
const grades_dict = this.properties['notebooks'][notebook]['grades_dict'];
return Object.keys(grades_dict)
.map(cellId => this.getGradeScore(notebook, cellId))
.reduce((sum, grade) => sum + grade, 0);
| let sum = 0; | ||
| for (const notebook of Object.keys(this.properties['notebooks'])) { | ||
| sum += this.getNotebookPoints(notebook); | ||
| } | ||
| return sum; |
There was a problem hiding this comment.
Here also the functional approach would be neater, I suppose :)
There are a few other places where a for-loop is used to sum some numbers, and those could be replaced with reduce.


Refactored old code and added missing endpoints from the old code.
Small changes in the new code, including a few fixes and refactoring.