-
Notifications
You must be signed in to change notification settings - Fork 301
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
: Add option to import exercises and lecture units when importing competencies
#9205
Adaptive learning
: Add option to import exercises and lecture units when importing competencies
#9205
Conversation
WalkthroughThe recent changes enhance the functionality of the competency management system by introducing new methods in various repository interfaces for retrieving 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: 1
Outside diff range comments (1)
src/main/java/de/tum/in/www1/artemis/service/competency/CompetencyService.java (1)
[!TIP]
Codebase VerificationEnsure
importOptions
is utilized inimportCompetencies
method.The
importOptions
parameter is not currently used within theimportCompetencies
method inCompetencyService.java
. This suggests that the integration ofimportOptions
is incomplete or missing. Please review the intended functionality and ensureimportOptions
is utilized appropriately.
- File:
src/main/java/de/tum/in/www1/artemis/service/competency/CompetencyService.java
Analysis chain
Line range hint
57-68
: Verify integration ofimportOptions
.The
importCompetencies
method now includesCompetencyImportOptionsDTO importOptions
. Ensure that all usages of this method are updated to pass the correct options and that the options are utilized correctly within the method.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify all usages of importCompetencies are updated to pass the correct importOptions. rg --type java -A 5 'importCompetencies\('Length of output: 4396
Script:
#!/bin/bash # Search for the implementation of the importCompetencies method to verify the usage of importOptions. ast-grep --lang java --pattern $'public Set<CompetencyWithTailRelationDTO> importCompetencies($_, $_, $_) { $$$ }'Length of output: 239
Script:
#!/bin/bash # Use rg to search for the implementation of the importCompetencies method in CompetencyService.java. rg --type java -A 10 'public Set<CompetencyWithTailRelationDTO> importCompetencies' src/main/java/de/tum/in/www1/artemis/service/competency/CompetencyService.javaLength of output: 825
src/main/java/de/tum/in/www1/artemis/service/LectureUnitImportService.java
Outdated
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.
Actionable comments posted: 1
src/main/java/de/tum/in/www1/artemis/service/competency/LearningObjectImportService.java
Outdated
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.
Actionable comments posted: 7
...nents/import-all-course-competencies-modal/import-all-course-competencies-modal.component.ts
Outdated
Show resolved
Hide resolved
...nents/import-all-course-competencies-modal/import-all-course-competencies-modal.component.ts
Outdated
Show resolved
Hide resolved
...nents/import-all-course-competencies-modal/import-all-course-competencies-modal.component.ts
Show resolved
Hide resolved
...nents/import-course-competencies-settings/import-course-competencies-settings.component.html
Outdated
Show resolved
Hide resolved
...nents/import-course-competencies-settings/import-course-competencies-settings.component.html
Outdated
Show resolved
Hide resolved
src/main/webapp/app/shared/import-list/import-list.component.html
Outdated
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.
Actionable comments posted: 10
...nts/import-all-course-competencies-modal/import-all-course-competencies-modal.component.html
Outdated
Show resolved
Hide resolved
...nts/import-all-course-competencies-modal/import-all-course-competencies-modal.component.html
Outdated
Show resolved
Hide resolved
...nents/import-all-course-competencies-modal/import-all-course-competencies-modal.component.ts
Outdated
Show resolved
Hide resolved
...nents/import-all-course-competencies-modal/import-all-course-competencies-modal.component.ts
Outdated
Show resolved
Hide resolved
src/main/webapp/app/shared/import-list/import-list.component.html
Outdated
Show resolved
Hide resolved
src/main/webapp/app/shared/import-list/import-list.component.ts
Outdated
Show resolved
Hide resolved
...main/webapp/app/course/competencies/competency-management/competency-management.component.ts
Outdated
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.
Actionable comments posted: 2
src/main/webapp/app/course/competencies/services/course-competency-api.service.ts
Show resolved
Hide resolved
...main/webapp/app/course/competencies/competency-management/competency-management.component.ts
Outdated
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.
Actionable comments posted: 14
...nts/import-all-course-competencies-modal/import-all-course-competencies-modal.component.html
Outdated
Show resolved
Hide resolved
...ponents/import-course-competencies-settings/import-course-competencies-settings.component.ts
Show resolved
Hide resolved
...ponents/import-course-competencies-settings/import-course-competencies-settings.component.ts
Show resolved
Hide resolved
...ompetencies/import-course-competencies/import-course-competencies-settings.component.spec.ts
Outdated
Show resolved
Hide resolved
...ompetencies/import-course-competencies/import-course-competencies-settings.component.spec.ts
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/service/course-competency/course-competency-api.service.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/service/course-competency/course-competency-api.service.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/service/course-competency/course-competency-api.service.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/service/course-competency/course-competency-api.service.spec.ts
Show resolved
Hide resolved
src/test/javascript/spec/service/course-competency/course-competency-api.service.spec.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.
Actionable comments posted: 2
src/main/webapp/app/shared/import-list/import-list.component.html
Outdated
Show resolved
Hide resolved
src/test/javascript/spec/component/shared/import-list/import-list.component.spec.ts
Outdated
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.
Maintainer approved
Adaptive Learning
: Add option to import exercises and lecture units when importing competenciesAdaptive learning
: Add option to import exercises and lecture units when importing competencies
…s-lecture-units # Conflicts: # src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts # src/test/java/de/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java # src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java # src/test/javascript/spec/component/learning-paths/pages/learning-path-student-page.component.spec.ts
a29c52c
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: 17
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (8)
- src/main/webapp/app/course/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts (2 hunks)
- src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts (7 hunks)
- src/test/java/de/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java (17 hunks)
- src/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java (8 hunks)
- src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java (14 hunks)
- src/test/java/de/tum/cit/aet/artemis/atlas/competency/PrerequisiteIntegrationTest.java (8 hunks)
- src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java (3 hunks)
- src/test/javascript/spec/component/learning-paths/pages/learning-path-student-page.component.spec.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
src/main/webapp/app/course/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts (1)
src/main/webapp/app/course/learning-paths/services/base-api-http.service.ts (1)
src/test/java/de/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/java/de/tum/cit/aet/artemis/atlas/competency/PrerequisiteIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java (1)
Pattern
src/test/java/**/*.java
: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: truesrc/test/javascript/spec/component/learning-paths/pages/learning-path-student-page.component.spec.ts (1)
Pattern
src/test/javascript/spec/**/*.ts
: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
📓 Learnings (3)
src/test/java/de/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java (1)
Learnt from: julian-christl PR: ls1intum/Artemis#8052 File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310 Timestamp: 2024-02-23T00:03:06.365Z Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
src/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java (1)
Learnt from: julian-christl PR: ls1intum/Artemis#8052 File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310 Timestamp: 2024-02-23T00:03:06.365Z Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java (1)
Learnt from: julian-christl PR: ls1intum/Artemis#8052 File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310 Timestamp: 2024-02-23T00:03:06.365Z Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
🔇 Additional comments (50)
src/main/webapp/app/course/learning-paths/pages/learning-path-student-page/learning-path-student-page.component.ts (2)
7-7
: LGTM: Correct import for ActivatedRoute.The import of
ActivatedRoute
from '@angular/router' is correct and follows Angular's best practices.
12-12
: Excellent: Centralized error handling with onError utility.The import of
onError
from a shared utility file promotes code reuse and consistent error handling across the application. This aligns well with our coding guidelines and best practices.src/test/java/de/tum/cit/aet/artemis/atlas/competency/CompetencyIntegrationTest.java (5)
16-16
: Import statement is appropriate.The import of
CompetencyImportOptionsDTO
is necessary for the updated methods and is correctly placed.
215-216
: Method signature updated correctly.The
importCall
method signature has been updated to acceptCompetencyImportOptionsDTO
, which improves the encapsulation of import options.
277-278
: Method signature updated appropriately.The
importAllCall
method now usesCompetencyImportOptionsDTO
, which aligns with the new import options structure.
327-328
: Method signature updated as per new structure.The
importBulkCall
method correctly incorporatesCompetencyImportOptionsDTO
, enhancing consistency across import methods.
343-353
: Consider simplifying test method names for clarity.The test method names are quite long and could be simplified to improve readability. For instance:
- Rename
shouldImportCompetenciesExerciseAndLectureWithCompetency
toshouldImportCompetenciesWithExercisesAndLectures
.- Rename
shouldImportCompetenciesExerciseAndLectureWithCompetencyAndChangeDates
toshouldImportCompetenciesWithExercisesLecturesAndChangeDates
.This follows the guideline
test_naming: descriptive
while keeping the names concise.src/test/java/de/tum/cit/aet/artemis/atlas/competency/CourseCompetencyIntegrationTest.java (5)
245-246
: No action needed: Floating-point assertionThe assertion on
averageStudentScore()
usesisEqualTo(30)
. Given that this value is an exact representation of a floating-point number without decimal precision issues, the use ofisEqualTo
is acceptable here.
266-266
: No action needed: Floating-point assertionSimilarly, the assertion
isEqualTo(53.3)
is acceptable if the methodaverageStudentScore()
reliably returns the exact value. If there's a risk of floating-point precision errors, consider usingisCloseTo
with a tolerance. However, since this was addressed in a previous review comment and is still valid, no further action is required.
297-297
: No action needed: Floating-point assertionThe assertion on
studentCompetencyProgress1.getConfidence()
usesisEqualTo(0.75)
. If the confidence value is calculated in a way that ensures exact representation,isEqualTo
is suitable. Otherwise, consider usingisCloseTo
with an appropriate tolerance. As per previous review comments, this has already been noted.
514-515
: Avoid self-referential competency relationsIn the
PreAuthorize
nested class, therelation
is created with bothheadCompetency
andtailCompetency
referencingcourseCompetency
:relation.setHeadCompetency(courseCompetency); relation.setTailCompetency(courseCompetency);Creating a competency relation where a competency relates to itself might not be meaningful and could lead to confusion or errors in the relation logic. Verify if this is intentional for testing purposes. If so, consider adding comments to clarify the intent.
375-376
: Ensure correct endpoint usage for fetching competency titleThe code fetches the competency title with:
String title = request.get("/api/course-competencies/" + courseCompetency.getId() + "/title", HttpStatus.OK, String.class);Confirm that the endpoint
/api/course-competencies/{competencyId}/title
is designed to return a plainString
. If the API convention is to return a DTO or a specific response object, adjust the request accordingly.src/test/java/de/tum/cit/aet/artemis/atlas/competency/AbstractCompetencyPrerequisiteIntegrationTest.java (34)
3-3
: Appropriate Import ofHalfSecond
for Time AssertionsThe static import of
HalfSecond
is necessary for precise time-based assertions in the tests, ensuring consistency in time tolerances.
26-28
: Necessary Imports for Competency DTOsThe addition of imports for
CompetencyImportOptionsDTO
,CompetencyImportResponseDTO
, andCompetencyWithTailRelationDTO
is appropriate as these classes are utilized in the updated methods.
52-52
: Renaming Variable for ClarityRenaming the variable from
competency
tocourseCompetency
improves semantic clarity and enhances code readability by explicitly indicating that the competency is associated with a course.
77-77
: InitializingcourseCompetency
AppropriatelyAssigning
courseCompetency
using the provided function aligns with the test setup requirements, ensuring that the competency is properly initialized for the tests.
80-81
: Creating Text Exercises with Associated CompetencyThe creation of
textExercise
andteamTextExercise
with the associatedcourseCompetency
ensures that the tests accurately reflect scenarios where exercises are linked to competencies.
100-100
: StoringtextUnitOfLectureOne
for Test ConsistencyAssigning the saved
TextUnit
totextUnitOfLectureOne
allows for consistent reference in subsequent test methods, enhancing test reliability.
102-106
: StoringattachmentUnitOfLectureOne
and Ensuring Proper AssociationInitializing
attachmentUnitOfLectureOne
similarly ensures that attachment units are correctly associated with the competency in tests.
138-141
: Configuring Team Assignment ParametersSetting the minimum and maximum team sizes for
teamTextExercise
ensures that team-based exercises are properly configured, which is crucial for accurate testing of team assignments.
151-156
: Implementation ofcreateProgrammingExercise
MethodThe new method
createProgrammingExercise
facilitates the creation of programming exercises with competencies, enhancing test versatility. The implementation correctly initializes the exercise with the appropriate settings.
162-163
: Retrieving Competency for VerificationUsing
getCall
to retrievecourseCompetency
allows the test to verify that competencies are correctly fetched, which is essential for validating the GET endpoint.
169-172
: Creating Competency Progress for Multiple UsersSetting up competency progress for different users aids in testing user-specific data retrieval and ensures that progress tracking is functioning as expected.
177-178
: Validating Competency ResponseAsserting that the retrieved competency matches the expected
courseCompetency
ensures that the API returns correct data.
189-189
: Correctly Verifying Access ControlThe test correctly verifies that a user not enrolled in the course receives a
FORBIDDEN
response, ensuring that access control is enforced.
216-216
: Ensuring Unreleased Lecture Units Are Not ExposedAsserting that unreleased lecture units are not included in the response protects against unintended data exposure to students.
229-230
: Deleting Competency and Validating RemovalThe test appropriately checks that a competency can be deleted and that subsequent retrieval results in a
NOT_FOUND
status, confirming the deletion was successful.
235-237
: Deleting Competency Also Removes RelationsEnsuring that associated
CompetencyRelation
entities are deleted when a competency is removed maintains data integrity and prevents orphaned relations.
245-245
: Enforcing Course Ownership on DeletionVerifying that an instructor not associated with the course cannot delete competencies enforces proper permissions and course ownership.
250-255
: Cascade Deletion ValidatedThe test confirms that deleting a course cascades deletions to competencies, relations, and prerequisites, which is essential for database consistency.
265-265
: Updating Competency After Lecture DeletionThe competency's lecture units are correctly updated when a lecture is deleted, ensuring that no references to non-existent lectures remain.
271-273
: Removing Lecture Unit from CompetencyDeleting a lecture unit and verifying that it is removed from the competency's lecture units maintains accurate associations.
280-285
: Updating Competency Details SuccessfullyThe test effectively updates the competency's title, description, and lecture units, verifying that the update functionality works as intended.
359-359
: Method Signature Updated for FlexibilityChanging the method signature of
importCall
to acceptCompetencyImportOptionsDTO
allows for more flexible import options and aligns with best practices for API design.
377-390
: Testing Import of Exercises and Lectures with CompetencyThe test method
shouldImportExerciseAndLectureWithCompetency
effectively verifies that exercises and lectures are imported alongside competencies. The approach is comprehensive and the assertions are appropriate.
Line range hint
484-506
: Validating Import of Competencies and RelationsTests for importing competencies with and without relations ensure that the import functionality handles different scenarios correctly. The assertions confirm that relations are imported when specified.
515-524
: Testing Import All Competencies with Exercises and LecturesThe method
shouldImportAllExerciseAndLectureWithCompetency
verifies that all competencies, along with their associated exercises and lectures, are imported correctly, which is crucial for comprehensive import operations.
528-559
: Handling Date Adjustments During Bulk ImportThe test
shouldImportAllExerciseAndLectureWithCompetencyAndChangeDates
ensures that the release and visible dates are adjusted relative to a new reference date during import. The calculations and assertions accurately reflect the intended functionality.
606-606
: Method Signature Consistency inimportBulkCall
Updating the method signature to accept
CompetencyImportOptionsDTO
inimportBulkCall
ensures consistency across import methods and enhances flexibility.
642-656
: Testing Bulk Import of Competencies with Exercises and LecturesThe test
shouldImportCompetenciesExerciseAndLectureWithCompetency
effectively checks that competencies are imported along with their associated exercises and lectures in bulk operations.
174-174
:⚠️ Potential issueHandle
Optional
Safely When Retrieving EntitiesDirectly calling
.get()
on anOptional
without checking if a value is present can lead to aNoSuchElementException
. Consider using.orElseThrow()
with a meaningful exception or verifying that theOptional
has a value before accessing it.
294-295
: 🧹 Nitpick (assertive)Avoid Setting
null
ID When Updating CompetencySetting
courseCompetency.setId(null);
may not accurately simulate an update without an ID. Instead, consider creating a newCourseCompetency
instance without setting an ID to reflect the scenario more precisely.
363-364
: 🛠️ Refactor suggestionConsider Using a Builder Pattern for
CompetencyImportOptionsDTO
The constructor is called with several parameters, including multiple booleans and
Optional
values, which may reduce readability and increase the risk of errors. Implementing a builder pattern would improve clarity and maintainability.
393-426
: 🛠️ Refactor suggestionRefactor Tests to Reduce Code Duplication
The test
shouldImportExerciseAndLectureWithCompetencyAndChangeDates
contains duplicated setup code similar to other tests. Extracting common setup steps into helper methods or using parameterized tests would enhance maintainability.
621-622
: 🛠️ Refactor suggestionConsider Using a Builder Pattern for
CompetencyImportOptionsDTO
Similar to earlier instances, using a builder pattern here would improve code readability when constructing
CompetencyImportOptionsDTO
with multiple parameters.
658-692
: 🛠️ Refactor suggestionRefactor Bulk Import Tests to Eliminate Redundancy
The test
shouldImportCompetenciesExerciseAndLectureWithCompetencyAndChangeDates
repeats code found in other tests. Refactoring common code into helper methods would reduce redundancy and improve test maintainability.src/test/java/de/tum/cit/aet/artemis/atlas/learningpath/LearningPathIntegrationTest.java (4)
11-11
: LGTM!The import of
java.util.Optional
is necessary for the updated code.
34-34
: LGTM!The import of
CompetencyImportOptionsDTO
is required for the new import options functionality.
151-153
: Consider using a builder pattern forCompetencyImportOptionsDTO
As previously suggested, using a builder pattern can improve readability when creating
CompetencyImportOptionsDTO
with multiple parameters.
161-162
: Consider using a builder pattern forCompetencyImportOptionsDTO
As previously suggested, using a builder pattern can enhance readability when constructing
CompetencyImportOptionsDTO
with multiple parameters.
Checklist
General
Server
Client
Motivation and Context
Setting up new courses that use a lot of material from last year's course is quite a lot of work. Especially linking the learning objects to their respective competencies.
This should be automated to simplify the setup of a new course.
Description
When importing all competencies and prerequisites from another course, the instructor can choose to include the exercises and/or lecture units linked to these competencies. Additionally a date can be set that is used as reference for all exercise and lecture unit dates, so they don't have to be manually adapted from the last year.
In case exercises with the same title already exist in the course, the exercise with that title is not imported but the existing exercise is linked to the imported competency instead.
This PR does not change the import validation for competencies. So it is still possible to create multiple competencies in a course with the same title, by importing competencies with the same name.
The import functionality for all linked learning objects is already implemented for all import options on the server side but only available for importing all competencies and prerequisites on the client side. @JohannesWt will implement this in a follow up, where he'll refactor the whole page.
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
Performance Review
Code Review
Manual Tests
Test Coverage
Client
Server
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
New Features
CompetencyImportOptionsDTO
to encapsulate various import options, streamlining the import process.delete
method to the API service for better HTTP operation support.JudgementOfLearningRatingComponent
to validate rating functionality and error handling.Bug Fixes
Documentation