Programming exercises: Improve template/solution comparison visibility
#8745
Conversation
…prove-template-solution-diff-visibility
WalkthroughThe updates primarily enhance the user interface for the diff report feature, adding new translations, tooltips, and button functionalities. They also introduce a comparison feature between template and solution repositories, providing clearer guidance and improved interaction for users managing programming exercises. Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional comments not posted (2)
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
florian-glombik
added
client
…prove-template-solution-diff-visibility
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range comments (3)
src/main/webapp/app/shared/components/button.component.ts (1)
Line range hint
1-4: Optimize imports by using TypeScript'simport type.To ensure that these imports are only used for type checking and do not contribute to the runtime bundle size, you can modify the import statements as follows:
- import { IconProp } from '@fortawesome/fontawesome-svg-core'; - import { faCircleNotch } from '@fortawesome/free-solid-svg-icons'; - import { FeatureToggle } from 'app/shared/feature-toggle/feature-toggle.service'; + import type { IconProp } from '@fortawesome/fontawesome-svg-core'; + import type { faCircleNotch } from '@fortawesome/free-solid-svg-icons'; + import type { FeatureToggle } from 'app/shared/feature-toggle/feature-toggle.service';src/test/javascript/spec/component/detail-overview-list.component.spec.ts (1)
Line range hint
79-79: Avoid usinganytype to ensure type safety.Using
anycan disable TypeScript's static type checking, which can lead to runtime errors. It's better to specify a more precise type:- null as any as Detail, + null as Detail, // or the appropriate typesrc/main/webapp/app/exam/manage/student-exams/student-exam-timeline/programming-exam-diff/programming-exercise-exam-diff.component.ts (1)
Line range hint
60-60: Avoid non-null assertions to ensure robust error handling.Using non-null assertions (
!) bypasses TypeScript's strict null checks, which can lead to runtime errors if the values are actually null. Consider handling potential null values explicitly:- this.previousSubmission.id!, + this.previousSubmission?.id,Also applies to: 76-76, 79-79, 112-112, 118-118, 144-144
Tools
Biome
[error] 10-11: All these imports are only used as types. (lint/style/useImportType)
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.
[error] 11-12: All these imports are only used as types. (lint/style/useImportType)
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.
[error] 13-14: All these imports are only used as types. (lint/style/useImportType)
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.
[error] 15-16: Some named imports are only used as types. (lint/style/useImportType)
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.
src/main/webapp/app/detail-overview-list/detail-overview-list.component.ts
Show resolved
Hide resolved
src/main/webapp/app/detail-overview-list/detail-overview-list.component.ts
Show resolved
Hide resolved
| @@ -570,7 +572,6 @@ | |||
| }, | |||
| "errorWhileFetchingRepos": "Fehler beim Abrufen der Repositories. Bitte überprüfe deine Internetverbindung und öffne das Popup erneut.", | |||
| "404": "Template-Lösungs-Diff wurde noch nicht generiert. Bitte pusche etwas in die Template- oder Lösungs-Repositories, um ihn zu erstellen.", | |||
| "lineStatLabel": "Zeilen, die zwischen Vorlage und Lösung hinzugefügt/entfernt wurden", | |||
There was a problem hiding this comment.
The translation is not used anymore, see src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range comments (4)
src/main/webapp/app/exercises/programming/manage/programming-exercise-detail.component.ts (4)
Line range hint
162-162: Replace non-null assertions with optional chaining.Using non-null assertions can lead to runtime errors if the object is actually null. It's safer to use optional chaining (
?.) which includes runtime checks.- programmingExercise.id! + programmingExercise.id?Please apply this change to all similar instances in the file.
Also applies to: 164-164, 183-183, 185-185, 223-223, 234-234, 249-249, 431-431, 443-443, 548-548
Line range hint
235-235: Avoid assignments within expressions.Assignments within expressions can lead to confusing code that is hard to read and maintain. Consider refactoring this to separate the assignment and the expression.
Line range hint
470-470: Use template literals instead of string concatenation.Template literals provide a clearer and more concise way to handle strings that involve variables. They also improve readability.
- 'string' + variable + `string ${variable}`Please apply this change to all similar instances in the file.
Also applies to: 481-481, 547-549
Line range hint
517-517: Use strict equality for comparisons.Using
!=can lead to unexpected type coercion. It's safer and more predictable to use!==for comparisons, ensuring that both the type and value match.- if (variable != null) + if (variable !== null)
src/main/webapp/app/detail-overview-list/detail-overview-list.component.ts
Show resolved
Hide resolved
Hialus
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
florian-glombik
added
ready to merge
and removed
lock:artemis-test2
ready for review
labels
krusche
deleted the
chore/programming-exercises/improve-template-solution-diff-visibility
branch
Checklist
General
Client
Motivation and Context
The template/solution comparison is inconspicuously represented on the programming exercise details page and shall grab instructors' attention to raise awareness of the feature's existence.
Description
Steps for Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Screenshots