-
Notifications
You must be signed in to change notification settings - Fork 291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adaptive learning
: Redesign edit competency relation section
#9447
base: develop
Are you sure you want to change the base?
Adaptive learning
: Redesign edit competency relation section
#9447
Conversation
Adaptive Learning
: Redesign edit competency relation sectionAdaptive learning
: Redesign edit competency relation section
WalkthroughThis pull request introduces updates to course competency management, including a new Data Transfer Object (DTO) for updating course competency relations, modifications to service constructors, and the addition of methods for handling competency relations. Several components have been added or removed, and user interface changes have been made, along with updates to internationalization files. New test suites have been created for the introduced components, ensuring functionality is covered in testing. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (2)
- src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (6 hunks)
- src/main/webapp/i18n/en/competency.json (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (1)
🔇 Additional comments (4)
src/main/webapp/i18n/en/competency.json (1)
175-176
: LGTM: New management buttons added correctly.The new keys
importAllButton
andeditRelationsButton
have been added correctly to thecourseCompetency.manage
section. The translations are clear, concise, and accurately describe the actions they represent.src/main/webapp/app/course/competencies/competency-management/competency-management.component.ts (3)
29-30
: Approved: Transition to standalone component with proper importsDeclaring the component as standalone and providing necessary imports enhances modularity and aligns with Angular's best practices.
59-65
: Approved: Usinginject()
for dependency injectionUtilizing the
inject()
function for dependency injection in standalone components is appropriate and follows Angular's recommendations.
117-125
: Verify the necessity of wrapping properties withsignal()
before passing to the modalIn the
openCourseCompetenciesRelationModal
method,courseId
andcourseCompetencies
are wrapped withsignal()
before assigning them to the modal instance.Ensure that the modal component expects these properties as signals. If not, consider passing them directly without wrapping.
...main/webapp/app/course/competencies/competency-management/competency-management.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS4, selecting individual relations is possible like described 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tetsted on TS4, works as described. I can select and update the relations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores