Adaptive learning: Redesign edit competency relation section
#9447
Conversation
github-actions
bot
added
tests
client
github-actions
bot
added
server
MaximilianAnzinger
changed the title
Adaptive Learning: Redesign edit competency relation sectionAdaptive learning: Redesign edit competency relation section
JohannesWt
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
JohannesWt
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
WalkthroughThis pull request introduces significant modifications across several Java and TypeScript files related to competency management. Key changes include updates to service constructors, the addition of new methods for managing course competency relations, and the introduction of new components for handling competency relations in the frontend. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
JohannesWt
dismissed stale reviews from az108, coderabbitai[bot], pzdr7, and JohannesStoehr
via
462c38a
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/i18n/de/competency.json (1)
246-275: New "relations" section added successfully.The new "relations" section has been added with appropriate translations for managing relationships between course competencies. The translations are consistent and use the informal "du" form as required. However, there's a minor suggestion for improvement:
Consider adding a brief explanation for each relation type in the "relationTypes" object. This could help users understand the differences between "ASSUMES", "EXTENDS", and "MATCHES" more easily. For example:
"relationTypes": { - "ASSUMES": "Setzt voraus", - "EXTENDS": "Erweitert", - "MATCHES": "Stimmt überein mit" + "ASSUMES": "Setzt voraus (Eine Kompetenz ist Voraussetzung für eine andere)", + "EXTENDS": "Erweitert (Eine Kompetenz baut auf einer anderen auf)", + "MATCHES": "Stimmt überein mit (Zwei Kompetenzen sind gleichwertig)" },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- src/main/webapp/app/course/competencies/competency-management/competency-management.component.html (1 hunks)
- src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (6 hunks)
- src/main/webapp/i18n/de/competency.json (2 hunks)
- src/main/webapp/i18n/en/competency.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/webapp/app/course/competencies/competency-management/competency-management.component.html
- src/main/webapp/i18n/en/competency.json
🧰 Additional context used
📓 Path-based instructions (2)
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (1)
src/main/webapp/i18n/de/competency.json (1)
Pattern
src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
🪛 GitHub Check: client-tests-selected
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts
[failure] 47-47:
Cannot find name 'faEdit'. Did you mean the instance member 'this.faEdit'?
🪛 GitHub Check: client-tests
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts
[failure] 47-47:
Cannot find name 'faEdit'. Did you mean the instance member 'this.faEdit'?
🪛 GitHub Check: client-compilation
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts
[failure] 47-47:
Cannot find name 'faEdit'. Did you mean the instance member 'this.faEdit'?
🔇 Additional comments (2)
src/main/webapp/i18n/de/competency.json (2)
175-177: LGTM! New buttons added correctly.The new buttons "Alles eines Kurses importieren", "Beziehungen bearbeiten", and "Hilfe" have been added correctly to the "manage" section. The translations are appropriate and consistent with the existing style.
Line range hint
1-424: Overall assessment: Changes look good!The new translations added to this file are consistent with the existing style and correctly use the informal "du" form as required by the coding guidelines. The additions provide necessary support for the competency relations feature and are well-integrated into the existing structure. No major issues were found, and only a minor suggestion for improvement was made regarding the explanation of relation types.
...main/webapp/app/course/competencies/competency-management/competency-management.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (1)
124-132: Consider changing the access modifier to 'private'The method
openCourseCompetenciesRelationModalis declared asprotected. Unless this method needs to be accessed by subclasses, consider marking it asprivateto encapsulate it within the component and maintain consistency in access modifiers.Apply this diff to adjust the access modifier:
- protected openCourseCompetenciesRelationModal(): void { + private openCourseCompetenciesRelationModal(): void {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (1)
🔇 Additional comments (1)
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (1)
130-131: Verify the necessity of wrapping data withsignalwhen passing to the modalIn lines 130-131, you are wrapping
this.courseIdandthis.courseCompetencieswith Angular signals before assigning them to the modal's component instance. Ensure that the modal component is designed to accept signals and that this approach is intentional. If the modal does not specifically require signals, consider passing the raw data directly to avoid unnecessary complexity.
JohannesWt
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Server
Client
Motivation and Context
Description
This PR introduces a redesigned section for creating/deleting and updating a relation between course competencies. Therefore a new button has been added to the competency management page which opens an own modal to create relations between competencies. The new graph in the modal distinguishes between Prerequisites and Competencies to make to give the Instructor a hint what to connect.
This PR is the first part of a makeover to convert the competency management page to use signals and make all involved components standalone or replace them completely.
Steps for 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
Code Review
Manual Tests
Test Coverage
Client
Server
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests