Development: Remove Course Management/LTI modules
#10184
Conversation
github-actions
bot
added
the
client
tobias-lippert
changed the title
Development: Remove Course Management client modulesDevelopment: Remove Course Management/LTI modules
github-actions
bot
added
the
database
# Conflicts: # src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.module.ts # src/main/webapp/app/course/tutorial-groups/tutorial-groups-management/tutorial-groups-management.module.ts # src/main/webapp/app/exam/participate/summary/exam-result-summary.module.ts # src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.module.ts # src/main/webapp/app/exercises/shared/participation/participation.module.ts # src/main/webapp/app/grading-system/grading-system.module.ts
tobias-lippert
force-pushed
the
chore/remove-course-management-lti-modules
branch
from
cd1f610 to
a4598d7
Compare
github-actions
bot
removed
the
database
# Conflicts: # src/main/webapp/app/exam/manage/exam-management.module.ts # src/main/webapp/app/exam/participate/summary/exam-result-summary.module.ts
WalkthroughThis pull request applies extensive refactoring across multiple Angular modules and routing configurations. Key changes include replacing module imports with direct component imports (e.g., replacing Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.ts (1)
58-67: Consider implementing memory management.The component subscribes to multiple observables but doesn't implement proper cleanup. Consider implementing OnDestroy and using a Subject to unsubscribe from all subscriptions when the component is destroyed.
-export class AssessmentDashboardComponent implements OnInit { +export class AssessmentDashboardComponent implements OnInit, OnDestroy { + private destroy$ = new Subject<void>(); + private courseService = inject(CourseManagementService); private exerciseService = inject(ExerciseService); private examManagementService = inject(ExamManagementService); private alertService = inject(AlertService); private accountService = inject(AccountService); private route = inject(ActivatedRoute); private guidedTourService = inject(GuidedTourService); private sortService = inject(SortService); + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + }Then update the subscriptions to use takeUntil:
- this.examManagementService.getExamWithInterestingExercisesForAssessmentDashboard(...).subscribe( + this.examManagementService.getExamWithInterestingExercisesForAssessmentDashboard(...) + .pipe(takeUntil(this.destroy$)) + .subscribe(src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts (1)
340-341: Prevent potential memory leaks in renderFeedbackWidgets.The setTimeout callback could execute after component destruction, potentially causing memory leaks or errors.
Store and clear the timeout to prevent memory leaks:
+private feedbackRenderTimeout?: number; protected renderFeedbackWidgets(lineOfWidgetToFocus?: number) { this.changeDetectorRef.detectChanges(); - setTimeout(() => { + if (this.feedbackRenderTimeout) { + clearTimeout(this.feedbackRenderTimeout); + } + this.feedbackRenderTimeout = window.setTimeout(() => { this.editor().disposeWidgets(); // ... rest of the code }, 0); } +ngOnDestroy() { + if (this.feedbackRenderTimeout) { + clearTimeout(this.feedbackRenderTimeout); + } +}src/main/webapp/app/exam/exam-scores/exam-scores.component.ts (2)
810-850: Enhance error handling in score comparison logic.The error handling in
compareNewExamScoresCalculationWithOldCalculationcould be improved to provide more context and better error recovery:
- The method silently continues if either
studentResultsorexamScoreDTOsis falsy- Error messages could be more structured for better debugging
Consider these improvements:
private compareNewExamScoresCalculationWithOldCalculation(examScoreDTOs: ScoresDTO[]) { if (!this.studentResults || !examScoreDTOs) { + this.logErrorOnSentry('Cannot compare exam scores: Missing required data'); return; } try { for (const examScoreDTO of examScoreDTOs) { this.studentIdToExamScoreDTOs.set(examScoreDTO.studentId!, examScoreDTO); } for (const studentResult of this.studentResults) { const overAllPoints = roundValueSpecifiedByCourseSettings(studentResult.overallPointsAchieved, this.course); const overallScore = roundValueSpecifiedByCourseSettings(studentResult.overallScoreAchieved, this.course); const regularCalculation = { scoreAchieved: overallScore, pointsAchieved: overAllPoints, userId: studentResult.userId, userLogin: studentResult.login, }; const examScoreDTO = this.studentIdToExamScoreDTOs.get(studentResult.userId); if (!examScoreDTO) { - const errorMessage = `Exam scores not included in new calculation: ${JSON.stringify(regularCalculation)}`; + const errorMessage = { + message: 'Exam scores not included in new calculation', + context: { + calculation: regularCalculation, + studentId: studentResult.userId + } + }; this.logErrorOnSentry(errorMessage); continue; } examScoreDTO.scoreAchieved = roundValueSpecifiedByCourseSettings(examScoreDTO.scoreAchieved, this.course); examScoreDTO.pointsAchieved = roundValueSpecifiedByCourseSettings(examScoreDTO.pointsAchieved, this.course); if (Math.abs(examScoreDTO.pointsAchieved - regularCalculation.pointsAchieved) > 0.1) { - const errorMessage = `Different exam points in new calculation. Regular Calculation: ${JSON.stringify(regularCalculation)}. New Calculation: ${JSON.stringify(examScoreDTO)}`; + const errorMessage = { + message: 'Different exam points in new calculation', + context: { + regularCalculation, + newCalculation: examScoreDTO, + difference: Math.abs(examScoreDTO.pointsAchieved - regularCalculation.pointsAchieved) + } + }; this.logErrorOnSentry(errorMessage); } if (Math.abs(examScoreDTO.scoreAchieved - regularCalculation.scoreAchieved) > 0.1) { - const errorMessage = `Different exam score in new calculation. Regular Calculation: ${JSON.stringify(regularCalculation)}. New Calculation : ${JSON.stringify(examScoreDTO)}`; + const errorMessage = { + message: 'Different exam score in new calculation', + context: { + regularCalculation, + newCalculation: examScoreDTO, + difference: Math.abs(examScoreDTO.scoreAchieved - regularCalculation.scoreAchieved) + } + }; this.logErrorOnSentry(errorMessage); } } + } catch (error) { + this.logErrorOnSentry({ + message: 'Failed to compare exam scores', + error, + context: { + examScoreDTOsCount: examScoreDTOs.length, + studentResultsCount: this.studentResults.length + } + }); + } }
633-645: Enhance type safety and error handling in export functionality.The export functionality could be improved with better type safety and error handling:
- No validation of export data before processing
- No error handling for failed exports
- No progress indication for large datasets
Consider these improvements:
+ interface ExportResult { + success: boolean; + error?: Error; + fileName?: string; + } - exportExamResults(customCsvOptions?: CsvExportOptions) { + async exportExamResults(customCsvOptions?: CsvExportOptions): Promise<ExportResult> { + try { + if (!this.studentResults?.length) { + throw new Error('No exam results available for export'); + } const headers = this.generateExportColumnNames(); const rows = this.studentResults.map((studentResult) => { return this.convertToExportRow(studentResult, customCsvOptions); }); + // Validate data before export + if (!this.validateExportData(headers, rows)) { + throw new Error('Invalid export data structure'); + } if (customCsvOptions) { - this.exportAsCsv(headers, rows, customCsvOptions); + return await this.exportAsCsv(headers, rows, customCsvOptions); } else { - this.exportAsExcel(headers, rows); + return await this.exportAsExcel(headers, rows); } + } catch (error) { + this.alertService.error('Failed to export exam results: ' + error.message); + return { success: false, error }; + } } + private validateExportData(headers: string[], rows: ExportRow[]): boolean { + if (!headers?.length || !rows?.length) { + return false; + } + // Validate that all rows have values for all headers + return rows.every(row => + headers.every(header => Object.prototype.hasOwnProperty.call(row, header)) + ); + }
🧹 Nitpick comments (13)
src/main/webapp/app/course/plagiarism-cases/shared/plagiarism-cases-shared.module.ts (1)
9-10: LGTM! Consider improving code formatting.The migration to standalone components looks good. However, consider breaking the long imports/exports arrays into multiple lines for better readability.
- imports: [ArtemisSharedModule, ArtemisSharedComponentModule, ArtemisPlagiarismModule, PlagiarismCaseVerdictComponent, PlagiarismCaseReviewComponent], - exports: [PlagiarismCaseVerdictComponent, PlagiarismCaseReviewComponent, ArtemisSharedModule, ArtemisSharedComponentModule, ArtemisPlagiarismModule], + imports: [ + ArtemisSharedModule, + ArtemisSharedComponentModule, + ArtemisPlagiarismModule, + PlagiarismCaseVerdictComponent, + PlagiarismCaseReviewComponent, + ], + exports: [ + PlagiarismCaseVerdictComponent, + PlagiarismCaseReviewComponent, + ArtemisSharedModule, + ArtemisSharedComponentModule, + ArtemisPlagiarismModule, + ],src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts (3)
52-58: LGTM! Good transition to standalone components.The changes align well with Angular's modern best practices by removing the module-based approach in favor of standalone components. This improves maintainability and reduces bundle size by enabling better tree-shaking.
Consider grouping related imports together for better organization:
imports: [ ArtemisSharedModule, MonacoEditorComponent, CodeEditorHeaderComponent, - CodeEditorTutorAssessmentInlineFeedbackSuggestionComponent, - CodeEditorTutorAssessmentInlineFeedbackComponent, + // Assessment components + CodeEditorTutorAssessmentInlineFeedbackComponent, + CodeEditorTutorAssessmentInlineFeedbackSuggestionComponent, ],
260-261: Address TODO comments regarding feedback handling.There are two related TODOs that should be addressed to improve the robustness of feedback handling:
- Support for multiple feedback items per line
- Unique ID generation for feedback nodes
These limitations could lead to issues when multiple feedback items need to be displayed on the same line.
Would you like me to help implement a solution that:
- Supports multiple feedback items per line using a container element
- Generates unique IDs using a combination of timestamp and line number?
Also applies to: 384-385
178-179: Improve responsive layout handling.The editor layout is only updated on changes, but it should also respond to window resize events for better responsiveness.
Add window resize handling:
+private resizeSubscription?: Subscription; constructor() { // ... existing code ... + this.resizeSubscription = fromEvent(window, 'resize') + .pipe(debounceTime(100)) + .subscribe(() => { + this.editor()?.layout(); + }); } +ngOnDestroy() { + // ... existing cleanup code ... + this.resizeSubscription?.unsubscribe(); }Don't forget to import:
import { fromEvent, Subscription } from 'rxjs'; import { debounceTime } from 'rxjs/operators';src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts (1)
26-62: Consider extracting cleanup operations configuration.The hardcoded cleanup operations array could be moved to a configuration file or service for better maintainability.
+// cleanup-operations.config.ts +export const CLEANUP_OPERATIONS: Partial<CleanupOperation>[] = [ + { + name: 'deleteOrphans', + deleteFrom: dayjs().subtract(12, 'months'), + deleteTo: dayjs().subtract(6, 'months'), + }, + // ... other operations +]; -cleanupOperations: CleanupOperation[] = [ - { - name: 'deleteOrphans', - deleteFrom: dayjs().subtract(12, 'months'), - // ... - }, - // ... -]; +cleanupOperations: CleanupOperation[] = CLEANUP_OPERATIONS.map(op => ({ + ...op, + lastExecuted: undefined, + datesValid: signal(true), +}));src/main/webapp/app/exam/exam-scores/exam-scores.component.ts (3)
73-86: LGTM! Component decorator changes align with standalone components migration.The changes successfully transition from module-based imports to standalone component imports, which aligns with modern Angular best practices.
Consider adding
standalone: trueto the component decorator to make it explicit that this is a standalone component:@Component({ selector: 'jhi-exam-scores', templateUrl: './exam-scores.component.html', changeDetection: ChangeDetectionStrategy.OnPush, styleUrls: ['./exam-scores.component.scss', '../../shared/chart/vertical-bar-chart.scss'], + standalone: true, imports: [ RouterLink, ArtemisSharedComponentModule, ArtemisSharedCommonModule, ExamScoresAverageScoresGraphComponent, ParticipantScoresDistributionComponent, ExportButtonComponent, ], })
312-376: Optimize performance of statistical calculations.The
calculateFilterDependentStatisticsmethod performs multiple calculations that could be optimized:
- Multiple iterations over the same data
- Repeated calculations that could be memoized
- Potential for parallel processing of independent calculations
Consider these performance improvements:
+ private memoizedResults = new Map<string, any>(); private calculateFilterDependentStatistics() { + const cacheKey = `${this.filterForSubmittedExams}_${this.filterForNonEmptySubmissions}`; + if (this.memoizedResults.has(cacheKey)) { + const cached = this.memoizedResults.get(cacheKey); + this.noOfExamsFiltered = cached.noOfExamsFiltered; + this.scores = [...cached.scores]; + this.gradesWithBonus = [...cached.gradesWithBonus]; + this.aggregatedExerciseGroupResults = cached.aggregatedExerciseGroupResults; + return; + } // ... existing initialization code ... + // Process all students in a single pass + const processedData = this.studentResults.reduce((acc, studentResult) => { + if (!studentResult.submitted && this.filterForSubmittedExams) { + return acc; + } + + acc.scores.push(studentResult.overallScoreAchieved ?? 0); + if (this.hasBonus) { + acc.gradesWithBonus.push(studentResult.gradeWithBonus?.finalGrade?.toString() ?? ''); + } + acc.noOfExamsFiltered++; + + // Process exercise groups + if (studentResult.exerciseGroupIdToExerciseResult) { + this.processExerciseGroups(studentResult, acc.groupResults); + } + + return acc; + }, { + scores: [] as number[], + gradesWithBonus: [] as string[], + noOfExamsFiltered: 0, + groupResults: new Map(Array.from(groupIdToGroupResults)) + }); + // Update component state + this.noOfExamsFiltered = processedData.noOfExamsFiltered; + this.scores = processedData.scores; + this.gradesWithBonus = processedData.gradesWithBonus; + this.calculateExerciseGroupStatistics(Array.from(processedData.groupResults.values())); + // Cache results + this.memoizedResults.set(cacheKey, { + noOfExamsFiltered: this.noOfExamsFiltered, + scores: [...this.scores], + gradesWithBonus: [...this.gradesWithBonus], + aggregatedExerciseGroupResults: this.aggregatedExerciseGroupResults + }); } + private processExerciseGroups(studentResult: StudentResult, groupResults: Map<number, AggregatedExerciseGroupResult>) { + const entries = Object.entries(studentResult.exerciseGroupIdToExerciseResult); + for (const [exGroupId, studentExerciseResult] of entries) { + if (!studentExerciseResult.hasNonEmptySubmission && this.filterForNonEmptySubmissions) { + continue; + } + const exGroupResult = groupResults.get(Number(exGroupId)); + if (!exGroupResult) { + throw new Error(`ExerciseGroup with id ${exGroupId} does not exist in this exam!`); + } + this.updateExerciseGroupStats(exGroupResult, studentExerciseResult); + } + }
862-865: Enhance accessibility for data visualization.The data label formatting could be improved to better support screen readers and keyboard navigation:
Consider these accessibility improvements:
formatDataLabel(submissionCount: number): string { - const percentage = this.noOfExamsFiltered && this.noOfExamsFiltered > 0 ? this.roundAndPerformLocalConversion((submissionCount * 100) / this.noOfExamsFiltered) : 0; - return submissionCount + ' (' + percentage + '%)'; + const percentage = this.noOfExamsFiltered && this.noOfExamsFiltered > 0 + ? this.roundAndPerformLocalConversion((submissionCount * 100) / this.noOfExamsFiltered) + : 0; + const label = `${submissionCount} (${percentage}%)`; + + // Add ARIA label for screen readers + return `<span aria-label="Submission count: ${submissionCount}, Percentage: ${percentage}%">${label}</span>`; }Additionally, consider adding these accessibility attributes to your chart components:
<ngx-charts-bar-vertical role="img" aria-label="Exam scores distribution chart" tabindex="0" [results]="chartData" [dataLabelFormatting]="formatDataLabel" ... > </ngx-charts-bar-vertical>src/main/webapp/app/exercises/shared/participation/participation.route.ts (1)
7-17: Consider adding route resolver for exerciseId.While the route configuration is correct, consider adding a resolver for the exerciseId parameter to ensure data is available before component activation.
import { inject } from '@angular/core'; import { ExerciseService } from 'app/exercises/shared/exercise.service'; export const exerciseResolver = (route: ActivatedRouteSnapshot) => { const exerciseService = inject(ExerciseService); return exerciseService.find(route.params['exerciseId']); }; export const routes: Routes = exerciseTypes.map((exerciseType) => ({ path: exerciseType + '-exercises/:exerciseId/participations', component: ParticipationComponent, resolve: { exercise: exerciseResolver }, data: { authorities: [Authority.TA, Authority.EDITOR, Authority.INSTRUCTOR, Authority.ADMIN], pageTitle: 'artemisApp.participation.home.title', }, canActivate: [UserRouteAccessService], }));src/main/webapp/app/exercises/programming/manage/programming-exercise-resolve.service.ts (1)
10-21: Consider adding error handling and type safety improvements.While the resolver implementation is good, consider these improvements:
- Add error handling for the HTTP request
- Validate the non-null assertion
- Add type guard for the response body
@Injectable({ providedIn: 'root' }) export class ProgrammingExerciseResolve implements Resolve<ProgrammingExercise> { private service = inject(ProgrammingExerciseService); resolve(route: ActivatedRouteSnapshot) { const exerciseId = route.params['exerciseId'] ? route.params['exerciseId'] : undefined; if (exerciseId) { - return this.service.find(exerciseId, true).pipe(map((programmingExercise: HttpResponse<ProgrammingExercise>) => programmingExercise.body!)); + return this.service.find(exerciseId, true).pipe( + map((response: HttpResponse<ProgrammingExercise>) => { + if (!response.body) { + throw new Error(`Exercise with id ${exerciseId} not found`); + } + return response.body; + }), + catchError((error) => { + console.error('Error loading programming exercise:', error); + return of(new ProgrammingExercise(undefined, undefined)); + }) + ); } return of(new ProgrammingExercise(undefined, undefined)); } }src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-resolver.service.ts (1)
24-44: Consider breaking down the complex resolution logic.The
resolvemethod contains nested conditional logic that could be simplified for better maintainability.Consider extracting the resolution logic into separate private methods:
-resolve(route: ActivatedRouteSnapshot) { +private resolveExistingExercise(exerciseId: number) { + return this.textExerciseService.find(exerciseId, true).pipe( + filter((res) => !!res.body), + map((textExercise: HttpResponse<TextExercise>) => textExercise.body!), + ); +} + +private resolveExerciseFromGroup(courseId: number, examId: number, exerciseGroupId: number) { + return this.exerciseGroupService.find(courseId, examId, exerciseGroupId).pipe( + filter((res) => !!res.body), + map((exerciseGroup: HttpResponse<ExerciseGroup>) => new TextExercise(undefined, exerciseGroup.body || undefined)), + ); +} + +private resolveExerciseFromCourse(courseId: number) { + return this.courseService.find(courseId).pipe( + filter((res) => !!res.body), + map((course: HttpResponse<Course>) => new TextExercise(course.body || undefined, undefined)), + ); +} + +resolve(route: ActivatedRouteSnapshot) { + const exerciseId = route.params['exerciseId']; + if (exerciseId) { + return this.resolveExistingExercise(exerciseId); + } + + const courseId = route.params['courseId']; + if (courseId) { + const examId = route.params['examId']; + const exerciseGroupId = route.params['exerciseGroupId']; + if (examId && exerciseGroupId) { + return this.resolveExerciseFromGroup(courseId, examId, exerciseGroupId); + } + return this.resolveExerciseFromCourse(courseId); + } + + return of(new TextExercise(undefined, undefined)); +}src/main/webapp/app/exercises/text/assess/text-submission-assessment-resolve.service.ts (1)
19-29: Improve type safety in the NewStudentParticipationResolver.The type casting in the map operator could be made more type-safe.
Consider this safer approach:
- .pipe(map((submission?: TextSubmission) => <StudentParticipation | undefined>submission?.participation)) + .pipe( + map((submission?: TextSubmission) => { + if (!submission?.participation) { + return undefined; + } + return submission.participation as StudentParticipation; + }) + )src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-management.route.ts (1)
8-71: LGTM! Route configurations are well-structured and secure.The route configurations maintain proper security through UserRouteAccessService and appropriate authority checks. The lazy loading implementation follows Angular best practices.
Consider adding a resolver for the new submissions route to ensure data is available before component activation:
{ path: 'file-upload-exercises/:exerciseId/submissions/:submissionId', loadChildren: () => import('app/exercises/file-upload/assess/file-upload-assessment.route').then((m) => m.routes), + resolve: { + fileUploadExercise: FileUploadExerciseManagementResolve, + }, },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts(1 hunks)src/main/webapp/app/app.config.ts(2 hunks)src/main/webapp/app/app.routes.ts(4 hunks)src/main/webapp/app/assessment/assessment-instructions/assessment-instructions.module.ts(0 hunks)src/main/webapp/app/assessment/assessment-locks/assessment-locks.route.ts(2 hunks)src/main/webapp/app/assessment/assessment-shared.module.ts(0 hunks)src/main/webapp/app/complaints/list-of-complaints/list-of-complaints.module.ts(0 hunks)src/main/webapp/app/complaints/list-of-complaints/list-of-complaints.route.ts(5 hunks)src/main/webapp/app/course/competencies/competency.module.ts(0 hunks)src/main/webapp/app/course/competencies/components/import-course-competencies-settings/import-course-competencies-settings.component.ts(1 hunks)src/main/webapp/app/course/competencies/forms/common-course-competency-form.component.ts(2 hunks)src/main/webapp/app/course/course-scores/course-scores-routing.module.ts(0 hunks)src/main/webapp/app/course/course-scores/course-scores.module.ts(0 hunks)src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.ts(2 hunks)src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.module.ts(0 hunks)src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.route.ts(0 hunks)src/main/webapp/app/course/learning-paths/components/learning-path-exercise/learning-path-exercise.component.ts(0 hunks)src/main/webapp/app/course/manage/course-management.module.ts(0 hunks)src/main/webapp/app/course/manage/course-management.route.ts(3 hunks)src/main/webapp/app/course/plagiarism-cases/instructor-view/plagiarism-instructor-view.route.ts(2 hunks)src/main/webapp/app/course/plagiarism-cases/shared/plagiarism-cases-shared.module.ts(1 hunks)src/main/webapp/app/course/tutorial-groups/shared/tutorial-groups-shared.module.ts(0 hunks)src/main/webapp/app/course/tutorial-groups/tutorial-groups-management/tutorial-groups-management.module.ts(0 hunks)src/main/webapp/app/detail-overview-list/detail.module.ts(0 hunks)src/main/webapp/app/exam/exam-scores/exam-scores.component.ts(2 hunks)src/main/webapp/app/exam/manage/exam-management.route.ts(3 hunks)src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.module.ts(0 hunks)src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.route.ts(2 hunks)src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-management.module.ts(0 hunks)src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-management.route.ts(6 hunks)src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.module.ts(0 hunks)src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.route.ts(2 hunks)src/main/webapp/app/exercises/modeling/assess/modeling-assessment.module.ts(0 hunks)src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.module.ts(0 hunks)src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.route.ts(0 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise-management.module.ts(0 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts(0 hunks)src/main/webapp/app/exercises/modeling/manage/modeling-exercise.route.ts(9 hunks)src/main/webapp/app/exercises/modeling/participate/modeling-participation.module.ts(0 hunks)src/main/webapp/app/exercises/programming/assess/programming-assessment.module.ts(0 hunks)src/main/webapp/app/exercises/programming/assess/programming-assessment.route.ts(2 hunks)src/main/webapp/app/exercises/programming/assess/programming-manual-assessment.module.ts(0 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routes.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management.module.ts(0 hunks)src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-instructions-editor.module.ts(0 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts(0 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-management.route.ts(20 hunks)src/main/webapp/app/exercises/programming/manage/programming-exercise-resolve.service.ts(1 hunks)src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.module.ts(0 hunks)src/main/webapp/app/exercises/programming/manage/update/update-components/difficulty/programming-exercise-difficulty.component.ts(1 hunks)src/main/webapp/app/exercises/programming/participate/programming-participation.module.ts(0 hunks)src/main/webapp/app/exercises/programming/participate/programming-repository.module.ts(0 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/code-editor.module.ts(0 hunks)src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts(1 hunks)src/main/webapp/app/exercises/programming/shared/programming-exercise.module.ts(0 hunks)src/main/webapp/app/exercises/quiz/manage/quiz-management.module.ts(0 hunks)src/main/webapp/app/exercises/quiz/manage/quiz-management.route.ts(8 hunks)src/main/webapp/app/exercises/shared/assessment-progress-label/assessment-progress-label.module.ts(0 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.module.ts(0 hunks)src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.route.ts(0 hunks)src/main/webapp/app/exercises/shared/difficulty-picker/difficulty-picker.module.ts(0 hunks)src/main/webapp/app/exercises/shared/example-submission/example-submissions.module.ts(0 hunks)src/main/webapp/app/exercises/shared/exercise-detail-common-actions/non-programming-exercise-detail-common-actions.module.ts(0 hunks)src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.module.ts(0 hunks)src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.route.ts(1 hunks)src/main/webapp/app/exercises/shared/exercise-title-channel-name/exercise-title-channel-name.module.ts(0 hunks)src/main/webapp/app/exercises/shared/manage/exercise-create-buttons.module.ts(0 hunks)src/main/webapp/app/exercises/shared/participation/participation.module.ts(0 hunks)src/main/webapp/app/exercises/shared/participation/participation.route.ts(1 hunks)src/main/webapp/app/exercises/shared/plagiarism/exercise-update-plagiarism/exercise-update-plagiarism.module.ts(0 hunks)src/main/webapp/app/exercises/shared/presentation-score/presentation-score.module.ts(0 hunks)src/main/webapp/app/exercises/shared/team/team.module.ts(0 hunks)src/main/webapp/app/exercises/text/assess/text-submission-assessment-resolve.service.ts(1 hunks)src/main/webapp/app/exercises/text/assess/text-submission-assessment.module.ts(0 hunks)src/main/webapp/app/exercises/text/assess/text-submission-assessment.route.ts(1 hunks)src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.module.ts(0 hunks)src/main/webapp/app/exercises/text/manage/text-exercise-management.module.ts(0 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-resolver.service.ts(1 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts(0 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts(8 hunks)src/main/webapp/app/feature-overview/feature-overview.module.ts(0 hunks)src/main/webapp/app/grading-system/grading-system.module.ts(0 hunks)
⛔ Files not processed due to max files limit (26)
- src/main/webapp/app/iris/settings/iris-course-settings-update/iris-course-settings-update.route.ts
- src/main/webapp/app/lecture/lecture-resolve.service.ts
- src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/lecture-unit-management-resolve.service.ts
- src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/lecture-unit-management.module.ts
- src/main/webapp/app/lecture/lecture-unit/lecture-unit-management/lecture-unit-management.route.ts
- src/main/webapp/app/lecture/lecture.module.ts
- src/main/webapp/app/lecture/lecture.route.ts
- src/main/webapp/app/lti/lti.module.ts
- src/main/webapp/app/overview/exercise-details/course-exercise-details.module.ts
- src/main/webapp/app/shared/course-group/course-group.module.ts
- src/main/webapp/app/shared/dashboards/dashboards.module.ts
- src/main/webapp/app/shared/dashboards/tutor-leaderboard/tutor-leaderboard.module.ts
- src/main/webapp/app/shared/date-time-picker/date-time-picker.module.ts
- src/main/webapp/app/shared/export/export.module.ts
- src/main/webapp/app/shared/grading-instruction-link-icon/grading-instruction-link-icon.module.ts
- src/main/webapp/app/shared/participant-scores/participant-scores.module.ts
- src/main/webapp/app/shared/type-ahead-search-field/type-ahead-user-search-field.module.ts
- src/main/webapp/app/shared/user-import/user-import.module.ts
- src/main/webapp/app/shared/user-settings/user-settings.module.ts
- src/test/javascript/spec/component/competencies/competency-form.component.spec.ts
- src/test/javascript/spec/component/competencies/edit-competency.component.spec.ts
- src/test/javascript/spec/component/competencies/edit-prerequisite.component.spec.ts
- src/test/javascript/spec/component/competencies/prerequisite-form.component.spec.ts
- src/test/javascript/spec/component/example-modeling/example-modeling-submission.component.spec.ts
- src/test/javascript/spec/component/programming-assessment/programming-assessment-repo-export-dialog.component.spec.ts
- src/test/javascript/spec/service/text-assessment.service.spec.ts
💤 Files with no reviewable changes (52)
- src/main/webapp/app/exercises/programming/participate/programming-participation.module.ts
- src/main/webapp/app/exercises/modeling/assess/modeling-assessment.module.ts
- src/main/webapp/app/exercises/modeling/participate/modeling-participation.module.ts
- src/main/webapp/app/feature-overview/feature-overview.module.ts
- src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.module.ts
- src/main/webapp/app/exercises/shared/difficulty-picker/difficulty-picker.module.ts
- src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.route.ts
- src/main/webapp/app/course/tutorial-groups/shared/tutorial-groups-shared.module.ts
- src/main/webapp/app/exercises/shared/manage/exercise-create-buttons.module.ts
- src/main/webapp/app/detail-overview-list/detail.module.ts
- src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management.module.ts
- src/main/webapp/app/exercises/shared/exercise-title-channel-name/exercise-title-channel-name.module.ts
- src/main/webapp/app/exercises/shared/presentation-score/presentation-score.module.ts
- src/main/webapp/app/exercises/shared/plagiarism/exercise-update-plagiarism/exercise-update-plagiarism.module.ts
- src/main/webapp/app/exercises/modeling/manage/modeling-exercise.module.ts
- src/main/webapp/app/exercises/shared/participation/participation.module.ts
- src/main/webapp/app/complaints/list-of-complaints/list-of-complaints.module.ts
- src/main/webapp/app/exercises/programming/manage/update/programming-exercise-update.module.ts
- src/main/webapp/app/grading-system/grading-system.module.ts
- src/main/webapp/app/exercises/shared/example-submission/example-submissions.module.ts
- src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.module.ts
- src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.route.ts
- src/main/webapp/app/course/learning-paths/components/learning-path-exercise/learning-path-exercise.component.ts
- src/main/webapp/app/exercises/programming/participate/programming-repository.module.ts
- src/main/webapp/app/exercises/shared/assessment-progress-label/assessment-progress-label.module.ts
- src/main/webapp/app/exercises/programming/shared/programming-exercise.module.ts
- src/main/webapp/app/exercises/text/manage/example-text-submission/example-text-submission.module.ts
- src/main/webapp/app/exercises/shared/team/team.module.ts
- src/main/webapp/app/exercises/modeling/manage/example-modeling/example-modeling-submission.module.ts
- src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.route.ts
- src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.module.ts
- src/main/webapp/app/exercises/shared/exercise-detail-common-actions/non-programming-exercise-detail-common-actions.module.ts
- src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-management.module.ts
- src/main/webapp/app/course/manage/course-management.module.ts
- src/main/webapp/app/assessment/assessment-shared.module.ts
- src/main/webapp/app/exercises/modeling/manage/modeling-exercise-management.module.ts
- src/main/webapp/app/exercises/programming/assess/programming-assessment.module.ts
- src/main/webapp/app/exercises/programming/manage/programming-exercise-management.module.ts
- src/main/webapp/app/exercises/programming/manage/instructions-editor/programming-exercise-instructions-editor.module.ts
- src/main/webapp/app/course/course-scores/course-scores.module.ts
- src/main/webapp/app/course/tutorial-groups/tutorial-groups-management/tutorial-groups-management.module.ts
- src/main/webapp/app/exercises/programming/shared/code-editor/code-editor.module.ts
- src/main/webapp/app/course/course-scores/course-scores-routing.module.ts
- src/main/webapp/app/exercises/text/assess/text-submission-assessment.module.ts
- src/main/webapp/app/exercises/text/manage/text-exercise-management.module.ts
- src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.module.ts
- src/main/webapp/app/assessment/assessment-instructions/assessment-instructions.module.ts
- src/main/webapp/app/exercises/quiz/manage/quiz-management.module.ts
- src/main/webapp/app/course/competencies/competency.module.ts
- src/main/webapp/app/exercises/shared/dashboards/tutor/exercise-assessment-dashboard.module.ts
- src/main/webapp/app/exercises/programming/assess/programming-manual-assessment.module.ts
- src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.module.ts
🧰 Additional context used
📓 Path-based instructions (30)
src/main/webapp/app/app.config.ts (1)
src/main/webapp/app/course/competencies/forms/common-course-competency-form.component.ts (1)
src/main/webapp/app/course/plagiarism-cases/shared/plagiarism-cases-shared.module.ts (1)
src/main/webapp/app/exercises/programming/assess/programming-assessment.route.ts (1)
src/main/webapp/app/exercises/programming/manage/update/update-components/difficulty/programming-exercise-difficulty.component.ts (1)
src/main/webapp/app/course/competencies/components/import-course-competencies-settings/import-course-competencies-settings.component.ts (1)
src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.route.ts (1)
src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.route.ts (1)
src/main/webapp/app/assessment/assessment-locks/assessment-locks.route.ts (1)
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routes.ts (1)
src/main/webapp/app/complaints/list-of-complaints/list-of-complaints.route.ts (1)
src/main/webapp/app/exercises/shared/participation/participation.route.ts (1)
src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.route.ts (1)
src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts (1)
src/main/webapp/app/exercises/file-upload/manage/file-upload-exercise-management.route.ts (1)
src/main/webapp/app/exam/exam-scores/exam-scores.component.ts (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-resolver.service.ts (1)
src/main/webapp/app/exercises/text/assess/text-submission-assessment.route.ts (1)
src/main/webapp/app/exercises/text/assess/text-submission-assessment-resolve.service.ts (1)
src/main/webapp/app/course/plagiarism-cases/instructor-view/plagiarism-instructor-view.route.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-resolve.service.ts (1)
src/main/webapp/app/exercises/programming/manage/programming-exercise-management.route.ts (1)
src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts (1)
src/main/webapp/app/exercises/quiz/manage/quiz-management.route.ts (1)
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.ts (1)
src/main/webapp/app/course/manage/course-management.route.ts (1)
src/main/webapp/app/exercises/modeling/manage/modeling-exercise.route.ts (1)
src/main/webapp/app/exam/manage/exam-management.route.ts (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts (1)
src/main/webapp/app/app.routes.ts (1)
📓 Learnings (4)
src/main/webapp/app/exercises/programming/manage/update/update-components/difficulty/programming-exercise-difficulty.component.ts (1)
Learnt from: florian-glombik
PR: ls1intum/Artemis#8451
File: src/main/webapp/app/exercises/programming/shared/lifecycle/programming-exercise-lifecycle.component.html:187-187
Timestamp: 2024-11-12T12:51:58.050Z
Learning: Use `artemisTranslate` for handling translations in Angular templates within the Artemis platform, as per the project-specific requirements.
src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts (1)
Learnt from: undernagruzez
PR: ls1intum/Artemis#9296
File: src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts:19-44
Timestamp: 2024-11-12T12:51:46.554Z
Learning: In `cleanup-service.component.ts`, the date range for cleanup operations is configurable through user input via the date time picker.
src/main/webapp/app/exercises/programming/manage/programming-exercise-management.route.ts (1)
Learnt from: SimonEntholzer
PR: ls1intum/Artemis#10228
File: src/main/webapp/app/exercises/programming/participate/programming-repository-routing.module.ts:9-9
Timestamp: 2025-01-29T08:21:07.083Z
Learning: The programming repository routing module uses `:repositoryId` parameter which is correctly handled by the localvc components (commit-details-view, commit-history, and repository-view components).
src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.ts (1)
Learnt from: pzdr7
PR: ls1intum/Artemis#9505
File: src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html:9-9
Timestamp: 2024-11-12T12:51:58.050Z
Learning: In Angular templates within the Artemis project (e.g., `src/main/webapp/app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component.html`), properties like `selectedFile()`, `readOnlyManualFeedback()`, `highlightDifferences()`, and `course()` are signals. It is appropriate to call these signals directly in the template.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: client-tests-selected
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Analyse
🔇 Additional comments (75)
src/main/webapp/app/course/dashboards/assessment-dashboard/assessment-dashboard.component.ts (2)
33-35: LGTM! Good migration to standalone components.The change from module imports to standalone component imports aligns with modern Angular best practices.
53-55: LGTM! Components properly imported.The standalone components are correctly added to the imports array of the @component decorator.
src/main/webapp/app/exercises/programming/manage/update/update-components/difficulty/programming-exercise-difficulty.component.ts (1)
1-14: LGTM! Well-structured component following modern Angular practices.The changes successfully:
- Replace module import with standalone component
- Use modern Angular input.required syntax
- Follow Angular style guide recommendations
src/main/webapp/app/course/competencies/components/import-course-competencies-settings/import-course-competencies-settings.component.ts (1)
17-22: LGTM! Excellent use of modern Angular patterns.The component demonstrates:
- Proper use of signals and computed properties
- Clean form state management
- Good separation of concerns
src/main/webapp/app/admin/cleanup-service/cleanup-service.component.ts (1)
12-17: LGTM! Successfully migrated to standalone component.The changes align with the module removal objective while maintaining existing functionality.
src/main/webapp/app/course/competencies/forms/common-course-competency-form.component.ts (1)
15-32: LGTM! Clean migration to standalone component.The changes successfully:
- Replace module import with standalone component
- Maintain type safety with DateTimePickerType enum
- Follow Angular component composition best practices
src/main/webapp/app/app.config.ts (2)
44-54: Consider deferring module import.The TODO comment indicates a plan to exclude modules in the future, and this PR aims to remove modules in favor of standalone components. Consider if this module can be imported directly in components that need it, rather than globally.
✅ Verification successful
Global Import of OwlNativeDateTimeModule Is Intentional
The shell scripts confirm that the date/time picker functionality is used broadly across the codebase. Since several components (and their tests) depend on it, deferring its import to individual components isn’t currently feasible without significant refactoring. The existing global import in the app configuration remains the most pragmatic solution until a dedicated migration to standalone components is planned.
- The OwlNativeDateTimeModule is referenced in multiple components (and tests) across the repository.
- Removing the global import now would likely necessitate changes in numerous files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing usage of the datetime picker to determine if global import is necessary # Search for potential usages of datetime picker components echo "Searching for datetime picker usage..." rg -l "datetime-picker|OwlDateTime" --type ts # Check if the module is imported elsewhere echo "Checking for other module imports..." rg -l "OwlNativeDateTimeModule" --type tsLength of output: 9465
35-35: Verify package dependency.Please ensure that
@danielmoncada/angular-datetime-pickeris properly listed inpackage.jsonwith a fixed version.✅ Verification successful
Dependency declaration verified.
The package
@danielmoncada/angular-datetime-pickeris correctly listed in the project'spackage.jsonwith version"19.1.0". No version conflicts were evident from the review of the dependency declaration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the package is listed in package.json and get its version # Check if package.json exists and contains the dependency if [ -f "package.json" ]; then echo "Checking package.json for @danielmoncada/angular-datetime-picker..." jq '.dependencies["@danielmoncada/angular-datetime-picker"]' package.json else echo "package.json not found" fi # Check for any potential version conflicts npm ls @danielmoncada/angular-datetime-pickerLength of output: 349
src/main/webapp/app/exercises/quiz/manage/quiz-management.route.ts (3)
1-8: LGTM! Well-organized imports and route configuration setup.The imports are properly organized and follow Angular's style guide. The route configuration is correctly exported as a constant.
10-85: LGTM! Well-implemented lazy loading and security measures.The route configurations demonstrate good practices:
- Proper lazy loading implementation using dynamic imports
- Consistent authority checks and guards
- Appropriate use of PendingChangesGuard for form routes
- Localized page titles
10-10: Verify the handling of course context after removing :courseId parameter.The removal of the
:courseIdparameter from routes aligns with the PR objectives but requires verification of how course context is now managed.Let's check how course context is handled in the components:
Also applies to: 20-20, 29-29, 38-38, 47-47, 57-57, 67-67, 77-77
✅ Verification successful
Verification of Course Context Handling in Quiz Components
The shell script outputs show that many quiz-related components still extract the course ID (via statements like
Number(this.route.snapshot.paramMap.get('courseId'))or via parent route fallbacks) even though the local route for “new quiz exercises” (in quiz-management.route.ts) no longer contains the:courseIdparameter. This indicates that the course context is expected to be provided by a parent route (e.g. under/course-management/:courseId) rather than locally in these new routes.Key points:
- Components such as quiz-exercise-detail, quiz-exercise-export, quiz-participation, etc., continue to retrieve the course ID from the route. In at least one case (quiz-participation), a fallback to parent route parameters is used.
- This behavior implies that the removal of
:courseIdin the quiz-management route is intentional and relies on a parent route to supply the course context.- The error from ast-grep was linked to an unrelated encoded file, so it does not impact this analysis.
Based on the collected information, the course context is still handled appropriately throughout the quiz components after the removal of the
:courseIdparameter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify how course context is managed after courseId removal # Check for courseId usage in quiz components echo "Checking quiz components for courseId usage..." rg -A 5 "courseId" "src/main/webapp/app/exercises/quiz" # Check for course service injection in quiz components echo "Checking for course service usage..." ast-grep --pattern 'class $_ { constructor($$$, private courseService: $_) { $$$ } }'Length of output: 55602
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts (11)
1-1: Consistent import usage.
No issues observed with the import statement forRoutes.
4-4: Resolver import looks good.
ImportingTextExerciseResolverhere is consistent with the Angular style guide.
8-8: Removed:courseIdparameter – verify references.
The route for creating new text exercises no longer includes:courseId. Confirm that all links or navigation paths referencing the old parameterized path have been updated.
20-20: Removed:courseIdparameter – verify references.
The detail route now omits:courseId. Ensure that any UI links (buttons or anchor tags) pointing to:courseId/text-exercises/:exerciseIdare updated accordingly.
32-33: Edit route – no issues.
The edit route is straightforward and follows Angular best practices.
45-45: Import route looks correct.
No issues with removing:courseId. Validate if external references were updated.
57-57: Plagiarism route – no concerns.
Removing the:courseIdsegment is consistent with the new routing approach.
69-69: Example submissions route update.
Ensure all references to the old:courseId-based path are updated across the codebase.
81-81: Iris settings route updated.
Omission of the:courseIdparameter aligns with simplifying route definitions for text exercises.
86-86: Exercise statistics path simplified.
Looks good. Keep an eye out for any references to the retired:courseIdparameter.
98-98: Tutor effort statistics route.
No specific issues here; consistent with the new approach.src/main/webapp/app/exercises/programming/manage/programming-exercise-management.route.ts (24)
1-1: Consistent route import.
No issues noted with theRoutesimport.
7-7: Resolver import is correct.
ProgrammingExerciseResolveusage aligns well with best practices for route data pre-fetch.
11-11: New programming exercise creation route.
Ensure references to the old:courseId/programming-exercises/newpath are updated.
23-23: Edit path simplified.
Changed from:courseId/programming-exercises/:exerciseId/edit. This is consistent with the goal of removing thecourseIdparam.
35-35: Import route includes:exerciseIdparameter.
The route is nowprogramming-exercises/import/:exerciseId; verify external references for correctness.
47-47: Addedimport-from-fileroute.
Straightforward addition. Good approach for file-based import handling.
59-59: View details without:courseId.
Confirm that navigation to the exercise detail page has been fully updated throughout the codebase.
71-71: Plagiarism route.
No immediate issues; consistent with the general removal ofcourseId.
83-83: Grading sub-route with:tab.
Be sure to test eachtabscenario for correct loading and external references.
93-93: Exercise statistics route.
This is consistent with the newly simplified approach.
105-105: Iris settings sub-route.
Looks good; confirm no leftover references to:courseId/iris-settings.
110-110: Build plan editor path.
Works as intended. No immediate concerns.
122-122: Repository base route.
Make sure the application’s links or navigation for repository access were updated accordingly.
131-131: Repository path with:repositoryId.
Well-structured route for clarifying the repository type and ID.
140-140: Commit history path.
Maintains clarity and consistency with the new route structure.
149-149: Commit history with:repositoryId.
No issues; route semantics are consistent.
158-158: VCS access log path.
No immediate concerns; straightforward.
167-167: Repo VCS access log path with:repositoryId.
Continue verifying that external references to old routes are updated.
176-176: Commit details path.
No issues, consistent approach.
185-185: Participation-based repository route.
Ensure that references or direct navigations have been updated.
194-194: Commit history under participations.
No concerns.
203-203: VCS access log for participations.
Confirm references for old routes are removed.
212-212: Commit-specific details for participations.
Well-defined path.
220-223: New route for submission assessment.
Addingsubmissions/:submissionIdsub-route is a good way to keep assessment functionality organized. Test thoroughly.src/main/webapp/app/course/manage/course-management.route.ts (12)
10-12: New tutor dashboards and Orion check.
Dynamically loading the Orion variant (OrionExerciseAssessmentDashboardComponent) or default (ExerciseAssessmentDashboardComponent) is creative. Verify usage in server-side or universal environments, as dynamic local references might not always be SSR-friendly.
57-57: Grading system route loading updated.
Confirm that the newly referencedgradingSystem.routeis functioning correctly; some older references might still point to the previous module.
61-62: Iris course settings route.
No major issues; route-based dynamic loading is consistent.
63-65: Lectures route.
Route for lectures is now lazy-loaded. Looks good.
72-73: Tutorial groups route.
Ensure that references to the old module-based import have been removed or updated.
74-82: Assessment dashboard for a specific exercise.
UsingisOrionat runtime is fine but test thoroughly to ensure Orion-based route resolution is correct.
83-94: Assessment dashboard route without exercise ID.
Good approach for showing a high-level assessment dashboard.
95-106: Course scores route.
Looks consistent with new patterns.
231-233: Complaints route defined at an empty path.
Multiple child routes at''can lead to confusion or overshadow other nested modules if not carefully ordered.
234-236: Assessment locks route.
Likewise, define explicit sub-paths if overshadowing arises.
239-259: Redirecting old exercise paths.
These redirects unify all exercise-related routes under/exercises. Ensure the UI transitions seamlessly.
260-286: Consolidating remaining exercise routes.
Loading them all at empty paths could be okay, but watch out for collisions if multiple modules define the same child paths.src/main/webapp/app/exercises/shared/participation/participation.route.ts (1)
1-1: LGTM! Route configuration aligns with modern Angular practices.The changes correctly transition from NgModule-based routing to a simpler exported routes configuration, aligning with modern Angular best practices.
Also applies to: 7-7
src/main/webapp/app/exercises/shared/exercise-scores/exercise-scores.route.ts (1)
1-1: LGTM! Route configuration follows modern Angular practices.The changes correctly transition from NgModule-based routing to a simpler exported routes configuration, aligning with modern Angular best practices. The spread operator with exerciseTypes.map is used effectively to generate routes dynamically.
Also applies to: 7-19
src/main/webapp/app/exercises/programming/assess/programming-assessment.route.ts (1)
1-1: Verify the impact of simplified path on navigation.While the route configuration follows modern Angular practices, the significant simplification of the path from a dynamic route to just 'assessment' might affect navigation. Please ensure that:
- All navigation links to this route have been updated
- The required parameters are passed correctly through the state or query parameters
Also applies to: 8-18
src/main/webapp/app/exercises/file-upload/assess/file-upload-assessment.route.ts (1)
1-25: LGTM! Clean route configuration with proper lazy loading.The route configuration follows Angular best practices with:
- Proper implementation of standalone component lazy loading
- Correct access control with UserRouteAccessService
- Clean route paths aligned with the PR objective
src/main/webapp/app/exercises/modeling/assess/modeling-assessment-editor/modeling-assessment-editor.route.ts (1)
12-13: Verify the authority difference between routes.The routes have different authority levels:
- 'assessment' allows ADMIN, INSTRUCTOR, EDITOR, and TA
- 'assessments/:resultId' only allows ADMIN and INSTRUCTOR
Please confirm if this authority difference is intentional.
Also applies to: 22-23
src/main/webapp/app/course/plagiarism-cases/instructor-view/plagiarism-instructor-view.route.ts (1)
1-30: LGTM! Well-structured route configuration.The route configuration is clean and follows best practices:
- Proper lazy loading of standalone components
- Consistent access control
- Clear route organization with named exports
src/main/webapp/app/assessment/assessment-locks/assessment-locks.route.ts (1)
11-13: Verify CourseManagementResolve compatibility with simplified routes.The routes use CourseManagementResolve but no longer include the courseId parameter. Please verify:
- How CourseManagementResolve obtains the course information without courseId
- If any updates to the resolver are needed
Run this script to check the resolver implementation:
Also applies to: 23-25
✅ Verification successful
CourseManagementResolve handles missing courseId without issues.
- The resolver checks for a
courseIdinroute.params.- If
courseIdis provided, it fetches the course; if not, it returns a newCourse.- This fallback behavior is consistent with the simplified routes in assessment-locks, so no changes to the resolver appear necessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check CourseManagementResolve implementation ast-grep --pattern 'class CourseManagementResolve { $$$ resolve($_, $_) { $$$ } $$$ }'Length of output: 224
Script:
#!/bin/bash # Step 1: Find all TypeScript files that contain the CourseManagementResolve class declaration echo "Searching for CourseManagementResolve class declarations in TS files:" rg --type ts "class CourseManagementResolve" -n # Step 2: For each identified file, display context around the resolve method to inspect how course information is retrieved. echo -e "\nDisplaying context for the resolve method in CourseManagementResolve files:" for file in $(rg --type ts -l "class CourseManagementResolve"); do echo -e "\n--- File: $file ---" rg --max-columns=200 -C 15 "resolve\s*\(" "$file" doneLength of output: 2010
src/main/webapp/app/exercises/text/assess/text-submission-assessment.route.ts (2)
1-1: LGTM! Clean import statements following Angular style guide.The imports are well-organized and follow Angular's style guide recommendations.
Also applies to: 4-4
7-47: LGTM! Well-structured route definitions with proper lazy loading.The route configuration demonstrates good practices:
- Proper use of lazy loading with
loadComponent- Clear authority checks
- Consistent route naming
- Appropriate resolver integration
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise-resolver.service.ts (1)
14-19: LGTM! Modern dependency injection pattern.Good use of the new
inject()function for dependency injection, following Angular's latest best practices.src/main/webapp/app/exercises/text/assess/text-submission-assessment-resolve.service.ts (1)
40-51: LGTM! Well-structured error handling.The resolver properly handles all edge cases with appropriate error handling and fallbacks.
src/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routes.ts (1)
6-63: LGTM! Well-structured route configuration with proper security.The routes are well-organized with:
- Consistent lazy loading pattern
- Proper authority checks
- Clear route parameter handling
However, verify that the removal of the NgModule doesn't affect route registration.
✅ Verification successful
Code Editor Routes: NgModule Removal Verified as Harmless
The lazy-loaded route configuration in both
app.routes.tsandexam-management.route.tscorrectly imports the routes fromcode-editor-management-routes.ts. The dynamic import of the exportedroutesconstant confirms that the removal of the NgModule does not adversely affect route registration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the routes are properly registered in the main routing module rg -A 5 "routes.*from.*code-editor-management" src/main/webapp/app/Length of output: 69
Script:
#!/bin/bash # Search for any reference to the code-editor-management-routes.ts file to check if the routes are imported and used. rg -R "code-editor-management-routes\\.ts" .Length of output: 450
Script:
#!/bin/bash # Search for any reference to "code-editor-management-routes" in the repository rg "code-editor-management-routes" .Length of output: 403
src/main/webapp/app/complaints/list-of-complaints/list-of-complaints.route.ts (1)
10-74: Verify CourseManagementResolve compatibility with new route structure.The CourseManagementResolve is still being used in routes where courseId has been removed from the path. Ensure the resolver can still resolve course data correctly with the new route structure.
✅ Verification successful
CourseManagementResolve Behavior Matches the New Route Structure
The resolver checks for a
courseIdin the route parameters and, if absent, returns a new Course instance. This behavior appears to be intentional for the routes without acourseIdparameter, so no adjustment is needed unless you require different behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check CourseManagementResolve implementation ast-grep --pattern 'class CourseManagementResolve { $$$ resolve($_, $_) { $$$ } }'Length of output: 218
Script:
#!/bin/bash # Locate the CourseManagementResolve definition in TypeScript files rg "class\s+CourseManagementResolve" -n --type ts # If CourseManagementResolve is found, inspect its resolve() method for usage of courseId from route parameters echo "Searching for courseId-related access in CourseManagementResolve..." rg "route\.snapshot\.params\[['\"]courseId['\"]\]" -n --type tsLength of output: 405
Script:
#!/bin/bash # Check if CourseManagementResolve uses alternative methods like paramMap for "courseId" rg "paramMap.get\(['\"]courseId['\"]\)" src/main/webapp/app/course/manage/course-management-resolve.service.ts -nLength of output: 119
Script:
#!/bin/bash echo "Printing first 200 lines of CourseManagementResolve for context..." sed -n '1,200p' src/main/webapp/app/course/manage/course-management-resolve.service.ts echo "" echo "Searching for any route parameter usage (courseId, examId, etc.)..." rg "route\.snapshot.params" src/main/webapp/app/course/manage/course-management-resolve.service.ts -n rg "route\.snapshot.paramMap" src/main/webapp/app/course/manage/course-management-resolve.service.ts -nLength of output: 1737
src/main/webapp/app/exercises/modeling/manage/modeling-exercise.route.ts (1)
8-103: LGTM! Route configurations follow consistent patterns.The route configurations maintain proper security through UserRouteAccessService, implement lazy loading correctly, and follow consistent patterns for path structures.
src/main/webapp/app/app.routes.ts (1)
66-67: LGTM! Route imports updated consistently.The changes from module imports to route imports are consistent and maintain proper lazy loading. The security configurations are preserved.
Verify that all imported route files exist:
Also applies to: 70-71, 133-134, 137-138, 152-153, 178-179, 186-187, 190-191
✅ Verification successful
Route Import Files Verified
All the referenced route files exist in the codebase as expected, and the changes from module imports to route imports are consistent.
src/main/webapp/app/exercises/shared/team/team.route.tssrc/main/webapp/app/course/manage/course-management.route.tssrc/main/webapp/app/exercises/programming/manage/code-editor/code-editor-management-routes.tssrc/main/webapp/app/grading-system/grading-system.route.tssrc/main/webapp/app/feature-overview/feature-overview.route.tssrc/main/webapp/app/lti/lti.route.ts🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check existence of route files files=( "./exercises/shared/team/team.route.ts" "./course/manage/course-management.route.ts" "./exercises/programming/manage/code-editor/code-editor-management-routes.ts" "./grading-system/grading-system.route.ts" "./feature-overview/feature-overview.route.ts" "./lti/lti.route.ts" ) for file in "${files[@]}"; do if [ ! -f "src/main/webapp/app/$file" ]; then echo "Missing route file: $file" fi doneLength of output: 935
src/main/webapp/app/exam/manage/exam-management.route.ts (2)
25-26: LGTM! Import paths updated to use dedicated resolver services.The changes align with Angular's best practices by importing resolvers from dedicated service files instead of module files.
917-917: LGTM! Route configurations updated to use modern lazy loading approach.The changes correctly implement Angular's recommended approach by:
- Replacing module imports with direct route imports
- Using the modern lazy loading syntax
- Moving towards standalone components
Also applies to: 921-921, 925-925
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts (1)
9-18: Consider standardizing module loading approach.The routes mix two different loading approaches:
loadComponentfor standalone componentsloadChildrenfor feature modulesWhile this works, consider standardizing to standalone components throughout for better consistency with Angular's modern practices.
Example standardization:
- loadChildren: () => import('../tutor-effort/tutor-effort-statistics.module').then((m) => m.ArtemisTutorEffortStatisticsModule), + loadComponent: () => import('../tutor-effort/tutor-effort-statistics.component').then((m) => m.TutorEffortStatisticsComponent),Also applies to: 21-27, 29-31, 34-43, 46-55, 58-67, 70-79, 82-84, 87-96, 99-100, 102-104
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/webapp/app/exercises/text/assess/text-submission-assessment.route.ts(3 hunks)src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/app/exercises/text/assess/text-submission-assessment.route.ts
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: server-tests
- GitHub Check: client-tests-selected
- GitHub Check: client-tests
- GitHub Check: Analyse
🔇 Additional comments (1)
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts (1)
1-6: LGTM! Clean imports following Angular style guide.The imports are well-organized and follow Angular best practices.
src/main/webapp/app/exercises/text/manage/text-exercise/text-exercise.route.ts
Show resolved
Hide resolved
|
github-actions
bot
added
the
deployment-error
tobias-lippert
added
deploy:artemis-test3
and removed
deployment-error
Checklist
General
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
We want to remove all ngModules step by step to completely follow modern Angular best practices without modules and standalone components instead. This ensures the long-term maintainability of the Artemis client application.
Description
Removed all modules related to course management and LTI and adapted routes as necessary.
Steps for Testing
Prerequisites:
LTI Testing
Access to Moodle
Testserver 1
Make sure you are logged in to Artemis with a test user (artemis_test_user_{1-5})
Navigate to Moodle and login with the same test user (artemis_test_user_{1-5}, same PW as for the Artemis Test Server)
Go to My Courses and open the course TS1 - Artemis Feature Demo Course
Click on one of the exercise
Artemis should open in an iFrame and display the exercise
(If Moodle throws an error in the iFrame, just refresh the page)
Exam Mode Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
unchanged, the changes only affect routes and module files
Summary by CodeRabbit
New Features
Refactor
Chores