`Development`: Introduce module-API for Atlas #9486

ole-ve · 2024-10-14T22:14:18Z

Checklist

General

I tested all changes and their related features with all corresponding user types on a test server.
I chose a title conforming to the naming conventions for pull requests.

Motivation and Context

As part of my master thesis, I am trying to create more well-defined boundaries between different modules. The main motivation is to improve the developer experience and test efficiency.

Description

Each module (subpackages of de.tum.cit.aet.artemis) will have its own api-subpackage. Classes from other modules are only allowed to access functionality from this api folder which acts as a facade to services (and repositories) within each module. Apart from the api-subpackage, other modules can access all classes/interfaces in domain and dto.

This is also enforced by Arch-Tests. Examples:

Exercise module -> atlas.api.CompetencyProgressApi ✅
Exercise module -> atlas.domain.Competency or atlas.dto.CompetencyDTO ✅
Exercise module -> atlas.service.CompetencyProgressService ❌

Out of scope

For now, the PROFILE_ATLAS has been added but is not used across the atlas module. Background being to keep this PR as small as possible.

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.

Summary by CodeRabbit

New Features
- Introduced CompetencyProgressApi for managing competency progress updates.
- Added new methods in CompetencyProgressApi for asynchronous and synchronous updates.
- Enhanced integration tests for various functionalities, including modeling and programming exercises.
Bug Fixes
- Updated verification calls in tests to use competencyProgressApi instead of competencyProgressService, ensuring accurate competency progress tracking.
Refactor
- Replaced CompetencyProgressService with CompetencyProgressApi across multiple services and tests for streamlined competency progress management.
- Simplified dependency injection in migration entries by consolidating competency progress updates into a single API call.
- Updated CourseService and CourseResource to utilize LearningPathApi for improved learning path management.

This reverts commit 8350776.

coderabbitai · 2024-10-14T22:45:17Z

Walkthrough

This pull request introduces several significant changes, including the addition of new abstract classes for module configuration and API handling, as well as the introduction of a new competency progress API. Key updates involve replacing the existing competency progress service with the new API across various service and test classes. Additionally, several tests are enhanced or modified to ensure they align with the new architecture, focusing on competency progress updates and integration tests for various educational functionalities.

Changes

File	Change Summary
`src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java`	Added `abstract class AbstractModuleConfig` with a constructor for module configuration.
`src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java`	Added `abstract class AbstractApi` with methods for checking active profiles and managing instances.
`src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java`	Added `abstract class AbstractModuleAccessArchitectureTest` for validating architectural constraints.
`src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadExerciseIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/ExerciseUnitIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/LectureIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/OnlineUnitIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java`	Renamed `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/lecture/VideoUnitIntegrationTest.java`	Renamed `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi` and enhanced test coverage.
`src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java`	Added `@SpyBean` for `CompetencyProgressApi` and updated reset method.
`src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java`	Updated verification calls from `competencyProgressService` to `competencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java`	Introduced `CompetencyProgressApi` class for managing competency progress updates.
`src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java`	Replaced `CompetencyRepository` and `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi` in service methods.

Possibly related PRs

Development: Use @EnforceAdmin annotation on class level #9422: The main PR introduces an abstract class for module-level access control, which aligns with the changes in the @EnforceAdmin annotations across various admin resources, ensuring consistent access control.
Development: Refactor tutorial group tests #9419: The refactoring of tutorial group tests to use an abstract base class enhances the organization of tests, similar to the modular approach in the main PR.
Development: Execute architecture tests on module basis #9409: The introduction of architecture tests on a module basis complements the main PR's focus on modularization and structured testing.
Development: Refactor lti tests #9375: The refactor of LTI tests into a modular structure aligns with the main PR's goal of improving the organization of tests across different modules.
Adaptive learning: Fix prerequisites not shown in course enrollment #9358: The changes to prerequisites visibility in course enrollment relate to the main PR's focus on enhancing user access and visibility within the system.
Development: Refactor atlas tests #9352: The removal of redundant architecture tests in the atlas module supports the main PR's aim to streamline and improve the testing framework.
Integrated code lifecycle: Improve access log handling #9425: The improvements in access log handling for version control actions enhance the logging capabilities, which is consistent with the main PR's focus on improving functionality and user experience.
Development: Fix issues with server test flakiness #9417: The fixes for test flakiness directly relate to the main PR's goal of ensuring reliable and consistent test outcomes across the project.

Suggested reviewers

JohannesStoehr
MaximilianAnzinger
SimonEntholzer
az108

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?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 57

🧹 Outside diff range comments (6)

src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseImportService.java (1)

Line range hint 24-76: Summary: Successful integration of CompetencyProgressApi

The changes in this file successfully replace the usage of CompetencyProgressService with CompetencyProgressApi. This includes:

Adding the correct import statement

Declaring a private final field for the API

Updating the constructor to inject the new dependency

Updating the method call to use the new API

These changes improve the modularity of the system and align with the PR objectives of establishing clearer boundaries between different modules. The implementation follows coding guidelines and best practices such as constructor injection and the principle of least access.

To further improve the modularity, consider defining an interface for the competency progress operations in your module. This would allow you to depend on an abstraction rather than a concrete implementation from another module, adhering to the Dependency Inversion Principle.
src/main/java/de/tum/cit/aet/artemis/atlas/service/LearningMetricsService.java (3)
Line range hint 66-106: Consider refactoring for improved readability and performance.

While the getStudentExerciseMetrics method effectively retrieves and processes exercise metrics, there are a few areas for potential improvement:

The method is quite long and complex. Consider breaking it down into smaller, more focused methods to improve readability and maintainability.

There are multiple database calls (repository method invocations). Consider optimizing these calls to reduce database round-trips, potentially by combining queries or using batch operations where possible.

The use of functional programming concepts is good, but some operations might be optimized for better performance, especially when dealing with large datasets.

Example refactoring (partial):
private Map<Long, ExerciseInformationDTO> getStartedExerciseInfoMap(long courseId) {
    return exerciseMetricsRepository.findAllExerciseInformationByCourseId(courseId).stream()
            .filter(e -> e.start() != null && e.start().isBefore(ZonedDateTime.now()))
            .collect(toMap(ExerciseInformationDTO::id, identity()));
}

private Map<Long, Set<String>> getCategoryMap(Set<Long> exerciseIds) {
    return exerciseMetricsRepository.findCategoriesByExerciseIds(exerciseIds).stream()
            .collect(groupingBy(Entry::getKey, mapping(Entry::getValue, toSet())));
}

// Similar methods for other data retrievals...

public ExerciseStudentMetricsDTO getStudentExerciseMetrics(long userId, long courseId) {
    final var exerciseInfoMap = getStartedExerciseInfoMap(courseId);
    final var exerciseIds = exerciseInfoMap.keySet();
    final var categoryMap = getCategoryMap(exerciseIds);
    // ... other data retrievals ...

    return new ExerciseStudentMetricsDTO(exerciseInfoMap, categoryMap, /* other parameters */);
}
This refactoring improves readability and makes the code more modular and easier to test.

Line range hint 123-152: Consider refactoring for improved readability and performance.

The getStudentCompetencyMetrics method, while functional, could benefit from similar improvements as suggested for the getStudentExerciseMetrics method:

Break down the method into smaller, more focused methods to improve readability and maintainability.

Optimize database calls to reduce round-trips, potentially by combining queries or using batch operations where possible.

Consider performance optimizations for stream operations, especially when dealing with large datasets.

Example refactoring (partial):
private Map<Long, CompetencyInformationDTO> getCompetencyInfoMap(long courseId) {
    return competencyMetricsRepository.findAllCompetencyInformationByCourseId(courseId).stream()
            .collect(toMap(CompetencyInformationDTO::id, identity()));
}

private Map<Long, Set<Long>> getCompetencyExerciseMap(Set<Long> competencyIds) {
    return competencyMetricsRepository.findAllExerciseIdsByCompetencyIds(competencyIds).stream()
            .collect(groupingBy(MapEntryLongLong::key, mapping(MapEntryLongLong::value, toSet())));
}

// Similar methods for other data retrievals...

public CompetencyStudentMetricsDTO getStudentCompetencyMetrics(long userId, long courseId) {
    final var competencyInfoMap = getCompetencyInfoMap(courseId);
    final var competencyIds = competencyInfoMap.keySet();
    final var exerciseMap = getCompetencyExerciseMap(competencyIds);
    // ... other data retrievals ...

    return new CompetencyStudentMetricsDTO(competencyInfoMap, exerciseMap, /* other parameters */);
}
This refactoring improves readability, modularity, and testability of the code.

Line range hint 1-153: Overall good implementation with room for improvement.

The LearningMetricsService class has been successfully moved to the new atlas package, aligning with the PR objectives for modularization. The overall structure of the class follows Spring best practices, using constructor injection and appropriate annotations.

Key points:

The package change is appropriate and supports the modularization effort.

The class structure and dependency injection are well-implemented.

Some methods, particularly getStudentExerciseMetrics and getStudentCompetencyMetrics, are quite complex and could benefit from refactoring for improved readability and maintainability.

There might be room for performance optimization, especially in reducing database round-trips and optimizing stream operations for large datasets.

Suggestions for improvement:

Break down complex methods into smaller, more focused methods.

Consider optimizing database calls by combining queries or using batch operations where possible.

Review and optimize stream operations for performance, especially when dealing with large datasets.

These improvements would enhance the overall quality, maintainability, and potentially the performance of the service.
src/main/java/de/tum/cit/aet/artemis/lecture/web/OnlineUnitResource.java (1)

Line range hint 1-220: Overall assessment: Changes successfully implement the new CompetencyProgressApi.

The modifications in this file effectively replace the usage of CompetencyProgressService with CompetencyProgressApi, aligning with the PR objective of introducing a module-API for Atlas. The changes are consistent throughout the file and adhere to the provided coding guidelines. The implementation maintains the existing functionality while updating the service calls to use the new API.

To further improve the code:

Consider adding unit tests to verify the behavior of the updated methods, especially updateOnlineUnit and createOnlineUnit.

Update the class-level documentation to reflect the usage of CompetencyProgressApi if necessary.

Would you like assistance in generating unit tests for the updated methods or updating the class-level documentation?

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)

Line range hint 193-213: Consider reducing the number of constructor parameters for better maintainability

The constructor of CourseResource now includes a large number of parameters, which can make the class harder to maintain and test. This violates the principles of small methods and single responsibility. Consider refactoring by grouping related services into composite objects or using a dependency injection framework to manage dependencies more efficiently.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between a5167be and 1a10cdc.

📒 Files selected for processing (43)

src/main/java/de/tum/cit/aet/artemis/assessment/service/ParticipantScoreScheduleService.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/AbstractAtlasApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningMetricsApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningPathApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/api/ScienceEventApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasConfig.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/service/LearningMetricsService.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/atlas/web/MetricsResource.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/core/exception/ModuleNotPresentException.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (8 hunks)
src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportScienceEventService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/core/service/user/UserService.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (6 hunks)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ParticipationService.java (5 hunks)
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadExerciseImportService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java (5 hunks)
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisPipelineService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/web/OnlineUnitResource.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/web/TextUnitResource.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/lecture/web/VideoUnitResource.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseImportService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java (6 hunks)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (6 hunks)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (5 hunks)
src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizExerciseImportService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (5 hunks)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseImportService.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (6 hunks)
src/test/java/de/tum/cit/aet/artemis/atlas/architecture/AtlasApiArchitectureTest.java (1 hunks)
src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java (1 hunks)

🧰 Additional context used

📓 Path-based instructions (43)

src/main/java/de/tum/cit/aet/artemis/assessment/service/ParticipantScoreScheduleService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/AbstractAtlasApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningMetricsApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningPathApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/api/ScienceEventApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasConfig.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/service/LearningMetricsService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/atlas/web/MetricsResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/exception/ModuleNotPresentException.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportScienceEventService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/service/user/UserService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/exercise/service/ParticipationService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadExerciseImportService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisPipelineService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/web/OnlineUnitResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/web/TextUnitResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/lecture/web/VideoUnitResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseImportService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizExerciseImportService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseImportService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/test/java/de/tum/cit/aet/artemis/atlas/architecture/AtlasApiArchitectureTest.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: true

src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.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: true

📓 Learnings (1)

src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureService.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.

🪛 GitHub Check: H2 Tests

src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java

[failure] 19-19: de.tum.cit.aet.artemis.assessment.AssessmentTeamComplaintIntegrationTest ► initializationError
Failed test found in:
build/test-results/test/TEST-de.tum.cit.aet.artemis.assessment.AssessmentTeamComplaintIntegrationTest.xml
Error:
java.lang.IllegalStateException: Failed to load ApplicationContext for [WebMergedContextConfiguration@3789436b testClass = de.tum.cit.aet.artemis.assessment.AssessmentTeamComplaintIntegrationTest, locations = [], classes = [de.tum.cit.aet.artemis.ArtemisApp], contextInitializerClasses = [], activeProfiles = ["test", "artemis", "core", "scheduling", "athena", "apollon", "iris", "aeolus", "theia", "lti"], propertySourceDescriptors = [PropertySourceDescriptor[locations=[], ignoreResourceNotFound=false, name=null, propertySourceFactory=null, encoding=null]], propertySourceProperties = ["artemis.user-management.use-external=false", "spring.jpa.properties.hibernate.cache.hazelcast.instance_name=Artemis_independent", "org.springframework.boot.test.context.SpringBootTestContextBootstrapper=true"], contextCustomizers = [org.springframework.boot.test.autoconfigure.actuate.observability.ObservabilityContextCustomizerFactory$DisableObservabilityContextCustomizer@1f, org.springframework.boot.test.autoconfigure.properties.PropertyMappingContextCustomizer@4b3fa0b3, org.springframework.boot.test.autoconfigure.web.servlet.WebDriverContextCustomizer@75f60966, [ImportsContextCustomizer@65ec7719 key = [org.springframework.boot.test.autoconfigure.web.servlet.MockMvcWebDriverAutoConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcAutoConfiguration, org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration, de.tum.cit.aet.artemis.shared.TestRepositoryConfiguration, org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration, org.springframework.boot.autoconfigure.security.servlet.SecurityFilterAutoConfiguration, org.springframework.boot.autoconfigure.security.servlet.UserDetailsServiceAutoConfiguration, org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcSecurityConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcWebClientAutoConfiguration, org.springframework.boot.test.autoconfigure.web.reactive.WebTestClientAutoConfiguration]], org.springframework.boot.test.context.filter.ExcludeFilterContextCustomizer@9342d5, org.springframework.boot.test.json.DuplicateJsonObjectContextCustomizerFactory$DuplicateJsonObjectContextCustomizer@88318a9, org.springframework.boot.test.mock.mockito.MockitoContextCustomizer@78777796, org.springframework.boot.test.web.client.TestRestTemplateContextCustomizer@4fd11488, org.springframework.boot.test.web.reactor.netty.DisableReactorResourceFactoryGlobalResourcesContextCustomizerFactory$DisableReactorResourceFactoryGlobalResourcesContextCustomizerCustomizer@7ee57e07, io.zonky.test.db.EmbeddedDatabaseContextCustomizerFactory$EmbeddedDatabaseContextCustomizer@d96d647b, org.springframework.boot.test.context.SpringBootTestAnnotation@855b767], resourceBasePath = "src/main/webapp", contextLoader = org.springframework.boot.test.context.SpringBootContextLoader, parent = null]

[failure] 19-19: de.tum.cit.aet.artemis.assessment.ParticipantScoreIntegrationTest ► initializationError
Failed test found in:
build/test-results/test/TEST-de.tum.cit.aet.artemis.assessment.ParticipantScoreIntegrationTest.xml
Error:
java.lang.IllegalStateException: Failed to load ApplicationContext for [WebMergedContextConfiguration@5019a0c1 testClass = de.tum.cit.aet.artemis.assessment.ParticipantScoreIntegrationTest, locations = [], classes = [de.tum.cit.aet.artemis.ArtemisApp, de.tum.cit.aet.artemis.programming.icl.TestBuildAgentConfiguration], contextInitializerClasses = [], activeProfiles = ["test", "artemis", "buildagent", "core", "scheduling", "localci", "localvc", "ldap-only", "lti", "aeolus", "iris"], propertySourceDescriptors = [PropertySourceDescriptor[locations=[], ignoreResourceNotFound=false, name=null, propertySourceFactory=null, encoding=null]], propertySourceProperties = ["server.port=49152", "artemis.version-control.url=http://localhost:49152", "artemis.user-management.use-external=false", "artemis.version-control.local-vcs-repo-path=${java.io.tmpdir}", "artemis.build-logs-path=${java.io.tmpdir}/build-logs", "artemis.continuous-integration.specify-concurrent-builds=true", "artemis.continuous-integration.concurrent-build-size=1", "artemis.continuous-integration.asynchronous=false", "artemis.continuous-integration.build.images.java.default=dummy-docker-image", "artemis.continuous-integration.image-cleanup.enabled=true", "artemis.continuous-integration.image-cleanup.disk-space-threshold-mb=1000000000", "spring.liquibase.enabled=true", "artemis.iris.health-ttl=500", "artemis.version-control.ssh-private-key-folder-path=${java.io.tmpdir}", "artemis.version-control.build-agent-use-ssh=true", "info.contact=test@localhost", "artemis.version-control.ssh-template-clone-url=ssh://git@localhost:7921/", "spring.jpa.properties.hibernate.cache.hazelcast.instance_name=Artemis_localcilocalvc", "org.springframework.boot.test.context.SpringBootTestContextBootstrapper=true"], contextCustomizers = [org.springframework.boot.test.autoconfigure.actuate.observability.ObservabilityContextCustomizerFactory$DisableObservabilityContextCustomizer@1f, org.springframework.boot.test.autoconfigure.properties.PropertyMappingContextCustomizer@4b3fa0b3, org.springframework.boot.test.autoconfigure.web.servlet.WebDriverContextCustomizer@75f60966, [ImportsContextCustomizer@595bfce key = [org.springframework.boot.test.autoconfigure.web.servlet.MockMvcWebDriverAutoConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcAutoConfiguration, org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration, de.tum.cit.aet.artemis.shared.TestRepositoryConfiguration, org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration, org.springframework.boot.autoconfigure.security.servlet.SecurityFilterAutoConfiguration, org.springframework.boot.autoconfigure.security.servlet.UserDetailsServiceAutoConfiguration, org.springframework.boot.autoconfigure.security.oauth2.resource.servlet.OAuth2ResourceServerAutoConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcSecurityConfiguration, org.springframework.boot.test.autoconfigure.web.servlet.MockMvcWebClientAutoConfiguration, org.springframework.boot.test.autoconfigure.web.reactive.WebTestClientAutoConfiguration]], org.springframework.boot.test.context.filter.ExcludeFilterContextCustomizer@9342d5, org.springframework.boot.test.json.DuplicateJsonObjectContextCustomizerFactory$DuplicateJsonObjectContextCustomizer@88318a9, org.springframework.boot.test.mock.mockito.MockitoContextCustomizer@a3d6b71e, org.springframework.boot.test.web.client.TestRestTemplateContextCustomizer@4fd11488, org.springframework.boot.test.web.reactor.netty.DisableReactorResourceFactoryGlobalResourcesContextCustomizerFactory$DisableReactorResourceFactoryGlobalResourcesContextCustomizerCustomizer@7ee57e07, io.zonky.test.db.EmbeddedDatabaseContextCustomizerFactory$EmbeddedDatabaseContextCustomizer@d96d647b, org.springframework.boot.test.context.SpringBootTestAnnotation@bfcfb386], resourceBasePath = "src/main/webapp", contextLoader = org.springframework.boot.test.context.SpringBootContextLoader, parent = null]

🔇 Additional comments (123)

src/test/java/de/tum/cit/aet/artemis/atlas/architecture/AtlasApiArchitectureTest.java (4)

1-1: LGTM: Package declaration is correct and follows conventions.

The package declaration de.tum.cit.aet.artemis.atlas.architecture is appropriate for an architecture test of the Atlas module and follows Java naming conventions.

3-3: LGTM: Import statement is correct and necessary.

The import of AbstractModuleAccessArchitectureTest is appropriate as it's the base class for this test, providing the necessary framework for module access testing.

5-5: LGTM: Class declaration is well-structured and follows best practices.

The class AtlasApiArchitectureTest is appropriately named, public, and extends AbstractModuleAccessArchitectureTest, which is suitable for module-specific architecture testing.

7-10: LGTM: getModulePackage() method is correctly implemented.

The getModulePackage() method:

Properly uses the @Override annotation.

Correctly concatenates ARTEMIS_PACKAGE with ".atlas" to specify the Atlas module package.

Follows the expected implementation pattern for module-specific architecture tests.

src/main/java/de/tum/cit/aet/artemis/atlas/api/AbstractAtlasApi.java (3)

1-7: LGTM: Package declaration and imports are well-structured.

The package declaration follows the project structure, and imports are specific without using star imports. The static import for PROFILE_ATLAS is appropriate for a constant.

9-9: LGTM: Class declaration follows best practices.

The AbstractAtlasApi class is well-defined:

Follows CamelCase naming convention.

Abstract class promotes code reuse by extending AbstractApi.

Adheres to the single responsibility principle by providing a base for Atlas-specific APIs.

1-14: Overall, excellent implementation of the AbstractAtlasApi class.

This new abstract class aligns well with the PR objectives:

Establishes a clear boundary for Atlas-specific APIs.

Promotes modularity and code reuse by extending the core AbstractApi.

Follows coding guidelines and best practices.

The implementation will contribute to improved developer experience and maintainability of the Atlas module.

src/main/java/de/tum/cit/aet/artemis/core/exception/ModuleNotPresentException.java (2)

1-1: LGTM: Package declaration is correct and follows conventions.

The package name de.tum.cit.aet.artemis.core.exception is appropriate for a core exception class and follows the standard Java naming conventions.

1-12: Summary: Well-implemented exception class aligning with PR objectives.

The ModuleNotPresentException class is a well-structured addition that aligns with the PR's goal of establishing clearer boundaries between modules. It provides a specific exception for cases where a required module is not present, which can significantly improve error handling and debugging in the context of the new modular architecture.

The implementation follows Java best practices and the project's coding guidelines. The minor suggestions provided (adding @since tag and extracting the error message format) are optional improvements that could enhance maintainability and documentation.

Overall, this addition contributes positively to the modular structure being implemented in the Artemis project.

src/main/java/de/tum/cit/aet/artemis/atlas/api/ScienceEventApi.java (3)

1-11: LGTM: Package declaration and imports are well-organized.

The package name follows the project structure, and imports are correctly organized without using wildcard imports, adhering to the coding guidelines.

12-13: LGTM: Class declaration and annotations are appropriate.

The class name ScienceEventApi follows CamelCase convention, and the @Controller annotation is correctly used. Extending AbstractAtlasApi suggests good code reuse and adherence to the single responsibility principle.

15-20: LGTM: Constructor and field declarations follow best practices.

The use of constructor injection and Optional<ScienceEventRepository> demonstrates good dependency management. The private final field adheres to the least access principle. The constructor properly initializes the superclass and the field.

src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningPathApi.java (5)

1-13: LGTM: Import statements are appropriate and concise.

The import statements are well-organized and include only the necessary classes. Good job avoiding star imports as per the coding guidelines.

14-15: LGTM: Class declaration follows best practices.

The LearningPathApi class is correctly annotated as a @Controller and extends AbstractAtlasApi. The naming convention (CamelCase) is followed as per the coding guidelines.

17-17: LGTM: Field declaration follows best practices.

The optionalLearningPathService field is correctly declared as private and final, adhering to the principle of least access. The use of Optional is appropriate for a potentially absent service.

19-22: LGTM: Constructor uses dependency injection correctly.

The constructor follows best practices by using constructor injection for dependencies. It properly initializes the superclass and assigns the optionalLearningPathService to the field.

1-31: Great job implementing the module-API for learning paths!

This new LearningPathApi class successfully introduces the module-API as described in the PR objectives. It adheres to Spring best practices, follows the coding guidelines, and maintains a clear separation of concerns. The use of Optional<LearningPathService> allows for flexibility in service availability.

Some minor suggestions for improvement:

Consider adding error handling or logging when the LearningPathService is not present.

You might want to add some Javadoc comments to describe the purpose of the class and its methods.

Overall, this is a solid implementation that contributes to the goal of establishing clearer boundaries between modules.

src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.java (1)

15-20: LGTM: Improved readability and consistent formatting

The changes to the @EnableJpaRepositories annotation improve readability by splitting the long list of packages across multiple lines. The formatting is consistent and adheres to the project's coding guidelines.
src/main/java/de/tum/cit/aet/artemis/atlas/web/MetricsResource.java (2)
Line range hint 1-48: LGTM! Well-structured and follows best practices.

The MetricsResource class is well-implemented and adheres to best practices:

Proper use of REST controller annotations and request mapping.

Constructor injection for dependencies, following the coding guideline for dependency injection.

Appropriate security measures with @EnforceAtLeastStudentInCourse.

Good use of logging for debugging purposes.

Clear and concise method implementation.

The class structure and implementation align well with the provided coding guidelines and general best practices for Spring Boot REST controllers.

15-15: LGTM! Verify consistency across the codebase.

The import change from de.tum.cit.aet.artemis.exercise.service.LearningMetricsService to de.tum.cit.aet.artemis.atlas.service.LearningMetricsService aligns with the PR objectives of establishing clearer boundaries between modules. This change contributes to the modularization effort described in the PR summary.

To ensure consistency, please run the following script to check if similar changes have been made across the codebase:

This will help verify that the modularization has been consistently applied throughout the project.

✅ Verification successful

Consistency Verified Across the Codebase.

The import of LearningMetricsService has been successfully updated to de.tum.cit.aet.artemis.atlas.service.LearningMetricsService in all relevant files. No residual imports from the old package were found, ensuring the modularization objectives are met.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for any remaining imports of LearningMetricsService from the old package

# Test: Search for old import statements. Expect: No results
rg --type java 'import de\.tum\.cit\.aet\.artemis\.exercise\.service\.LearningMetricsService;'

# Test: Search for new import statements. Expect: Multiple results
rg --type java 'import de\.tum\.cit\.aet\.artemis\.atlas\.service\.LearningMetricsService;'
Length of output: 465
src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadExerciseImportService.java (4)

18-18: LGTM: Import statement for CompetencyProgressApi added.

The import statement for CompetencyProgressApi aligns with the PR objective of introducing module-API and follows the Java convention of avoiding star imports.

35-35: LGTM: Field declaration for competencyProgressApi added.

The field declaration follows best practices:

It's declared as private and final, adhering to the principle of least access.

It uses constructor injection, which is the preferred method for dependency injection.

The naming convention (camelCase) is followed correctly.

39-43: LGTM: Constructor parameter and initialization for competencyProgressApi added.

The changes in the constructor are well-implemented:

The constructor parameter aligns with the field declaration.

The initialization follows the principle of constructor injection.

The change is consistent with the PR objective of introducing module-API.

Line range hint 1-87: Overall assessment: Changes successfully implement the module-API for CompetencyProgress.

The modifications in this file effectively replace CompetencyProgressService with CompetencyProgressApi, aligning with the PR objective of introducing module-API for Atlas. The changes contribute to establishing clearer boundaries between modules and enhance the modularity of the Artemis project.

Key points:

The import statement, field declaration, constructor injection, and method call have been updated consistently.

The changes adhere to Java coding conventions and best practices.

The implementation maintains the existing functionality while introducing the new API structure.

These changes support the goals outlined in the PR objectives and contribute to improving the developer experience and testing efficiency.

src/main/java/de/tum/cit/aet/artemis/lecture/web/TextUnitResource.java (5)

21-21: LGTM: Import statement for CompetencyProgressApi added correctly.

The new import statement for CompetencyProgressApi is correctly added and aligns with the PR objective of introducing a module-API for Atlas.

47-54: LGTM: Constructor and field updated to use CompetencyProgressApi.

The changes in the constructor and field declaration are consistent with the PR objective. The use of constructor injection and final field declaration follows good practices and the coding guidelines.

106-106: LGTM: Progress update delegated to CompetencyProgressApi.

The change in the updateTextUnit method correctly delegates the progress update to the CompetencyProgressApi. The use of an asynchronous method (updateProgressForUpdatedLearningObjectAsync) is a good practice for performance. This change aligns with the single responsibility principle and the PR objectives.

142-142: LGTM: Progress update delegated to CompetencyProgressApi in createTextUnit method.

The change in the createTextUnit method correctly delegates the progress update to the CompetencyProgressApi. The use of an asynchronous method (updateProgressByLearningObjectAsync) is a good practice for performance. This change aligns with the single responsibility principle and the PR objectives.

Line range hint 1-146: Overall: Excellent refactoring to use CompetencyProgressApi.

The changes in this file consistently replace CompetencyProgressService with CompetencyProgressApi, aligning perfectly with the PR objectives. The refactoring maintains good coding practices, including:

Proper use of dependency injection

Maintaining immutability with final fields

Following the single responsibility principle

Using asynchronous methods for potentially time-consuming operations

These changes contribute to establishing clearer boundaries between modules and should improve the overall architecture of the system.

src/main/java/de/tum/cit/aet/artemis/lecture/web/VideoUnitResource.java (3)

23-23: LGTM: Import statement added correctly.

The import statement for CompetencyProgressApi has been added correctly. It follows Java naming conventions and avoids star imports, which is in line with best practices.

49-49: LGTM: Field and constructor updated correctly.

The changes to the field declaration and constructor are consistent with the new CompetencyProgressApi usage. The code maintains good practices such as:

Using private and final for the field (principle of least access)

Employing constructor injection for dependency management

Preserving the single responsibility principle of the class

These modifications align well with the coding guidelines and best practices.

Also applies to: 54-54, 58-58

102-102: Verify the impact of asynchronous operations.

The changes to use competencyProgressApi with asynchronous methods (updateProgressForUpdatedLearningObjectAsync and updateProgressByLearningObjectAsync) are consistent with the new API. However, the shift to asynchronous operations may impact the overall behavior and performance of the application.

Please ensure that:

The asynchronous nature of these calls doesn't negatively affect the response time or data consistency.

Error handling for these asynchronous operations is properly implemented.

The client-side code is updated to handle potential delays in competency progress updates.

To verify the usage and impact, you can run the following script:

This script will help identify potential issues with error handling, consistency in API usage, and any remaining synchronous calls that might have been missed during the refactoring.

Also applies to: 140-140

src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureService.java (4)

16-16: LGTM: Import statement for CompetencyProgressApi added.

The import statement for CompetencyProgressApi has been correctly added, following the CamelCase naming convention. This change aligns with the PR objective of introducing a module-API for Atlas.

48-48: LGTM: Class field for CompetencyProgressApi added.

The competencyProgressApi field has been correctly declared as private and final, following the principle of least access. It uses constructor injection, which is the preferred method for dependency injection in Spring. The naming convention (camelCase) is also followed correctly.

51-57: LGTM: Constructor updated with CompetencyProgressApi parameter.

The constructor has been correctly updated to include the CompetencyProgressApi parameter. The new parameter follows the camelCase naming convention and is properly assigned to the class field in the constructor body. This change maintains the single responsibility principle for the constructor and aligns with the PR objective of introducing the module-API for Atlas.

Line range hint 1-185: Overall assessment: Changes successfully introduce CompetencyProgressApi.

The modifications in this file successfully introduce the CompetencyProgressApi, aligning with the PR objective of establishing clearer boundaries between different modules. The changes are consistent throughout the file, maintaining existing functionality while improving the modularity of the codebase. The new API is correctly integrated into the LectureService class, following coding guidelines and best practices such as constructor injection and the principle of least access.

These changes contribute to the goal of enhancing the developer experience and improving testing efficiency by enforcing modularity and encapsulation.

src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseImportService.java (3)

24-24: LGTM: Import statement correctly added.

The import statement for CompetencyProgressApi is correctly added and follows the coding guideline of avoiding star imports.

44-44: LGTM: Field declaration correctly added.

The field competencyProgressApi is correctly declared as private and final, following the principle of least access. The naming convention (camelCase) is followed, and the type aligns with the import statement.

48-52: LGTM: Constructor parameter and initialization correctly added.

The constructor parameter for CompetencyProgressApi is correctly added and the field is properly initialized in the constructor. This change follows the principle of constructor injection and maintains the single responsibility principle for the class.

src/main/java/de/tum/cit/aet/artemis/atlas/service/LearningMetricsService.java (4)

1-1: Package change aligns with modularization objectives.

The package change from de.tum.cit.aet.artemis.exercise.service to de.tum.cit.aet.artemis.atlas.service is in line with the PR objectives to establish clearer boundaries between different modules. This change appropriately places the LearningMetricsService in the 'atlas' module.

Line range hint 39-53: Class structure follows good practices.

The LearningMetricsService class structure adheres to good practices:

It uses constructor injection for dependency injection.

It's properly annotated as a Spring service.

It follows the Single Responsibility Principle by focusing on learning metrics.

The @Profile(PROFILE_CORE) annotation appropriately restricts the service to the core profile.

Line range hint 55-64: Method follows good design principles.

The getStudentCourseMetrics method is well-designed:

It's concise and follows the Single Responsibility Principle.

It promotes code reuse by delegating specific metric retrievals to other methods.

The method signature clearly indicates its purpose and return type.

Line range hint 108-121: Well-structured and concise method.

The getStudentLectureUnitMetrics method is well-implemented:

It's concise and easy to understand.

It follows a consistent pattern with other methods in the class.

The use of streams for data processing is appropriate and efficient.

src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java (4)

18-18: LGTM: Import statement added for CompetencyProgressApi

The addition of this import aligns with the PR objective of introducing the module-API for Atlas. It also adheres to the Java coding guideline of avoiding star imports.

51-51: LGTM: Field declaration for CompetencyProgressApi

The field declaration adheres to the following guidelines:

It's declared as private and final, following the principle of least access.

It uses constructor injection, which is the recommended DI approach.

The naming convention follows camelCase as specified in the coding guidelines.

55-63: LGTM: Constructor updated to use CompetencyProgressApi

The constructor has been correctly updated to:

Accept CompetencyProgressApi as a parameter.

Assign the new parameter to the corresponding field.

This change aligns with the PR objective of replacing the CompetencyProgressService with the CompetencyProgressApi.

Line range hint 1-215: Overall assessment: Changes successfully implement the module-API for Atlas

The modifications in this file effectively replace the CompetencyProgressService with CompetencyProgressApi, aligning with the PR objectives. The changes adhere to the provided coding guidelines, including naming conventions, dependency injection practices, and access modifiers.

Key points:

Import statements have been updated correctly.

Field declaration follows best practices.

Constructor injection is properly implemented.

The updateAttachmentUnit method now uses the new API.

These changes contribute to establishing clearer boundaries between different modules within the Artemis project, as intended by the PR.
src/main/java/de/tum/cit/aet/artemis/lecture/web/OnlineUnitResource.java (4)
31-31: LGTM: Import statement for CompetencyProgressApi added.

The addition of this import aligns with the PR objective of introducing a module-API for Atlas and follows the Java coding guideline of avoiding star imports.

59-59: LGTM: Field type updated to CompetencyProgressApi.

The change from CompetencyProgressService to CompetencyProgressApi aligns with the PR objective of using the new API interface. The field declaration follows Java coding guidelines by using the least access modifier (private) and adhering to camelCase naming convention.

64-68: LGTM: Constructor updated to use CompetencyProgressApi.

The changes in the constructor align with the PR objective of using the new API interface. The code follows Java coding guidelines by using constructor injection for dependency injection and adhering to camelCase naming convention for the parameter.

113-113: LGTM: Method calls updated to use CompetencyProgressApi.

The changes in the updateOnlineUnit and createOnlineUnit methods align with the PR objective of using the new API interface. The method names follow camelCase convention, and the existing logic is maintained while updating the service calls to use the new API.

To ensure that the API changes are consistent across the codebase, please run the following verification script:

This script will help identify any inconsistencies in the API usage across the codebase.

Also applies to: 150-150

✅ Verification successful

: All usages of CompetencyProgressService in the main codebase have been successfully replaced with CompetencyProgressApi. The remaining instances in test files do not affect the production code. The API refactoring aligns with the PR objectives and maintains code consistency.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Verify that CompetencyProgressService is no longer used and CompetencyProgressApi is used consistently.

# Test 1: Check for any remaining usage of CompetencyProgressService
echo "Checking for any remaining usage of CompetencyProgressService:"
rg --type java "CompetencyProgressService"

# Test 2: Verify consistent usage of CompetencyProgressApi
echo "Verifying consistent usage of CompetencyProgressApi:"
rg --type java "CompetencyProgressApi"

# Test 3: Check for any mismatched method calls
echo "Checking for any mismatched method calls:"
rg --type java "competencyProgressApi\.(updateProgressForUpdatedLearningObjectAsync|updateProgressByLearningObjectAsync)"
Length of output: 22766
src/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.java (3)

25-25: LGTM: Import statement correctly updated

The import statement for CompetencyProgressApi has been correctly added to replace the removed CompetencyProgressService. This change aligns with the coding guidelines, using CamelCase naming and avoiding star imports.

65-74: LGTM: Field and constructor correctly updated

The changes to the class field and constructor are consistent with the switch from CompetencyProgressService to CompetencyProgressApi. The use of constructor injection aligns with the dependency injection guideline, and the changes maintain the single responsibility principle. The naming conventions are also followed correctly.

Line range hint 1-214: Summary: Changes align well with PR objectives

The modifications in this file successfully implement the transition from CompetencyProgressService to CompetencyProgressApi. These changes contribute to the PR's objective of establishing clearer boundaries between different modules within the Artemis project, specifically under the package de.tum.cit.aet.artemis.

The refactoring:

Introduces the use of the api subpackage (via CompetencyProgressApi)

Maintains the existing functionality while improving modularity

Adheres to the coding guidelines and best practices

These changes support the goal of enhancing the developer experience and improving testing efficiency by enforcing modularity and encapsulation.

src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisPipelineService.java (5)

20-20: LGTM: Import change aligns with module-API introduction.

The addition of import de.tum.cit.aet.artemis.atlas.api.LearningMetricsApi; is in line with the PR objective of introducing module-API for Atlas. It also adheres to the coding guideline of avoiding star imports.

64-64: LGTM: Field declaration updated to use LearningMetricsApi.

The change from LearningMetricsService to LearningMetricsApi aligns with the PR objective of introducing module-API. It maintains the use of constructor injection for dependency injection, adhering to the coding guidelines.

70-77: LGTM: Constructor updated to use LearningMetricsApi.

The constructor parameter change from LearningMetricsService to LearningMetricsApi is consistent with the field declaration change. It maintains the use of constructor injection for dependency injection, adhering to the coding guidelines and PR objectives.

189-189: LGTM: Method call updated to use LearningMetricsApi.

The change from learningMetricsService.getStudentCourseMetrics to learningMetricsApi.getStudentCourseMetrics is consistent with the previous changes and aligns with the PR objective of introducing module-API. The method signature and parameters remain unchanged, ensuring backward compatibility.

Line range hint 1-233: Summary: Successful introduction of LearningMetricsApi in PyrisPipelineService

The changes in this file successfully introduce the LearningMetricsApi as part of the module-API for Atlas. The modifications are consistent across imports, field declaration, constructor, and method implementation. These changes align well with the PR objectives and adhere to the provided coding guidelines.

Key points:

Proper import of the new API interface

Consistent use of LearningMetricsApi throughout the class

Maintained use of constructor injection for dependency injection

Preserved method signatures for backward compatibility

The introduction of the module-API enhances the modularity and encapsulation of the Artemis project, as intended in the PR objectives.

src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (4)

18-18: LGTM: Import statement added correctly.

The import for CompetencyProgressApi is added correctly and follows the project's naming conventions. Good job on using a specific import rather than a star import.

79-79: LGTM: Constructor changes implemented correctly.

The changes to replace CompetencyProgressService with CompetencyProgressApi are implemented correctly:

The field competencyProgressApi is added with the correct naming convention.

The constructor parameter is added appropriately.

The field is properly initialized in the constructor.

These changes align with the coding guidelines, particularly the use of constructor injection.

Also applies to: 85-86, 100-100

Line range hint 1-241: Summary: Successful integration of CompetencyProgressApi

The changes in this file successfully replace CompetencyProgressService with CompetencyProgressApi. This modification aligns with the PR objectives of establishing clearer boundaries between different modules and enhancing modularity.

Key points:

Import statements are updated correctly.

Constructor injection is properly implemented for the new API.

The method call is updated to use the new asynchronous API method.

These changes contribute to improved modularity and potentially better performance due to the asynchronous nature of the new API method. However, ensure that the asynchronous behavior doesn't introduce any unexpected side effects in the delete method's overall functionality.

Overall, the changes adhere to the provided coding guidelines and seem to be a step in the right direction for the project's architecture.

207-207: Verify the impact of changing to an asynchronous method.

The change from competencyProgressService.updateProgressByCompetency(competency) to competencyProgressApi::updateProgressByCompetencyAsync looks good and is consistent with the API changes. The use of a method reference is a nice touch for improved readability.

However, please verify that:

The asynchronous nature of the new method doesn't affect the overall behavior of the delete method.

There's no need to wait for these operations to complete before the method returns.

Any potential exceptions from these async operations are properly handled.

To help verify the impact, you can run the following script:

src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseImportService.java (4)

30-30: LGTM: Import statement follows guidelines.

The import statement for CompetencyProgressApi is correctly added and follows the naming convention (CamelCase). It's also a specific import, which aligns with the coding guidelines to avoid star imports.

59-59: LGTM: Field declaration follows best practices.

The field declaration for CompetencyProgressApi follows best practices:

It uses camelCase naming convention.

It's declared as private and final, adhering to the principle of least access.

Constructor injection is used, which is the recommended approach for dependency injection.

Line range hint 30-98: Summary: Changes align with PR objectives and improve modularity.

The modifications to TextExerciseImportService successfully replace CompetencyProgressService with CompetencyProgressApi. This change:

Establishes clearer boundaries between modules, aligning with the PR's primary goal.

Enhances modularity by restricting access to the API subpackage.

Maintains consistency across the class, updating both the field declaration and method call.

These changes contribute to a more modular and maintainable codebase, supporting the objectives outlined in the PR summary.

98-98: LGTM: Method call updated correctly. Consider error handling.

The method call has been correctly updated to use competencyProgressApi instead of competencyProgressService, maintaining consistency with the field declaration change.

However, since this is an asynchronous call, it's important to ensure proper error handling. Consider adding error handling to manage potential exceptions in the asynchronous operation. You can verify the current error handling implementation with the following script:

If the script doesn't return any results, it might indicate a lack of error handling for these asynchronous calls.

src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizExerciseImportService.java (4)

27-27: LGTM: Import statement correctly added

The import statement for CompetencyProgressApi is correctly added and follows the coding guideline of avoiding star imports.

59-59: LGTM: Field declaration correctly added

The field declaration for CompetencyProgressApi is correctly added. It follows the principle of least access (private), uses constructor injection, and adheres to the camelCase naming convention.

Line range hint 1-293: Summary: Changes align with PR objectives and coding guidelines

The modifications to QuizExerciseImportService successfully introduce the CompetencyProgressApi, replacing the previous CompetencyProgressService. These changes align with the PR objectives of establishing clearer boundaries between modules and enhancing modularity.

The code adheres to the provided coding guidelines, including naming conventions, dependency injection practices, and access modifiers. The changes maintain the existing functionality while improving the system's structure.

Consider implementing error handling for the asynchronous updateProgressByLearningObjectAsync call to enhance robustness.

Overall, the changes contribute positively to the codebase's modularity and maintainability.

63-68: LGTM: Constructor and method changes look good, consider error handling

The changes to the constructor and the importQuizExercise method are correct. The CompetencyProgressApi is properly injected and used.

However, since updateProgressByLearningObjectAsync is an asynchronous call, consider adding error handling to manage potential exceptions that might occur during the asynchronous operation.

To ensure proper error handling is in place, you can run the following command to check for try-catch blocks or error handling mechanisms around asynchronous calls:

This will show the context around asynchronous method calls, allowing us to verify if appropriate error handling is implemented.

Also applies to: 93-93

src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)

307-310: LGTM: New constant PROFILE_ATLAS added correctly.

The new constant PROFILE_ATLAS is well-defined and properly documented. It follows the established naming conventions for constants and is appropriately placed among other Spring profile constants. The Javadoc comment clearly explains its purpose.

src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java (3)

32-32: LGTM: Import statement updated correctly.

The import statement for CompetencyProgressApi has been added, which aligns with the PR objective of introducing module-API for clearer boundaries between modules.

72-72: LGTM: Field declaration updated correctly.

The field declaration has been updated from CompetencyProgressService to CompetencyProgressApi, which is consistent with the PR objective of introducing module-API. The field is correctly declared as private and final, adhering to best practices.

80-87: LGTM: Constructor updated correctly.

The constructor has been updated to use CompetencyProgressApi instead of CompetencyProgressService. This change is consistent with the field declaration update and adheres to the constructor injection principle for dependency injection.

src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java (5)

31-31: LGTM: Import statement correctly added

The import for CompetencyProgressApi is correctly added to replace the previous CompetencyProgressService. This change aligns with the PR objective of introducing a module-API for Atlas.

110-110: LGTM: Field and constructor correctly updated

The CompetencyProgressApi field is correctly declared as private and final, adhering to the least access principle. Constructor injection is properly used for dependency injection. These changes align with the PR objective and follow the coding guidelines.

Also applies to: 117-133

163-163: LGTM: API call correctly updated

The competencyProgressApi.updateProgressByLearningObjectAsync(result) call correctly replaces the previous service method. The async nature of the call is maintained, which is good for performance. This change aligns with the PR objective of introducing the module-API for Atlas.

290-290: LGTM: API call correctly updated with proper use of Optional

The competencyProgressApi.updateProgressForUpdatedLearningObjectAsync() call correctly replaces the previous service method. The async nature of the call is maintained, which is good for performance. The use of Optional.of(fileUploadExercise) is appropriate here. This change aligns with the PR objective and follows good practices.

Line range hint 1-433: Overall assessment: Changes successfully implement the module-API for Atlas

The changes in this file successfully replace the CompetencyProgressService with CompetencyProgressApi. All modifications are consistent with the PR objective and adhere to the coding guidelines. The implementation maintains the async nature of the calls, which is good for performance. No issues were found in the changes.

src/main/java/de/tum/cit/aet/artemis/assessment/service/ParticipantScoreScheduleService.java (4)

40-40: Import statement updated correctly.

The import statement for CompetencyProgressApi has been added, which is consistent with the change from CompetencyProgressService to CompetencyProgressApi. This follows the coding guideline of avoiding star imports.

77-77: Field declaration updated correctly.

The field declaration has been updated from CompetencyProgressService to CompetencyProgressApi. This change is consistent with the new API usage and follows the coding guidelines for naming (CamelCase) and the principle of least access (private and final).

99-103: Constructor updated correctly.

The constructor has been updated to use CompetencyProgressApi instead of CompetencyProgressService. This change is consistent with the field declaration update and follows the coding guideline for constructor injection. The parameter name matches the field name, maintaining good naming conventions.

Line range hint 1-479: Summary of changes: Replacement of CompetencyProgressService with CompetencyProgressApi

The changes in this file are part of the larger effort to introduce a module-API for Atlas, as mentioned in the PR objectives. The modifications are minimal and focused:

Import statement updated

Field declaration changed

Constructor parameter updated

Method call adjusted

These changes consistently replace CompetencyProgressService with CompetencyProgressApi throughout the file. The existing logic and structure of the ParticipantScoreScheduleService remain intact, which is good for maintaining the current functionality.

To ensure a smooth transition, please verify that:

The CompetencyProgressApi interface is fully compatible with the previous CompetencyProgressService.

All other occurrences of CompetencyProgressService in the codebase have been updated similarly.

The new API implementation provides the same functionality as the previous service.

Please run the verification script provided in the previous comment to check the API interface compatibility and usage across the codebase.

src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java (4)

32-32: Import change aligns with PR objectives

The import statement has been updated to use CompetencyProgressApi instead of CompetencyProgressService. This change is in line with the PR objectives of introducing module-API for Atlas.

84-84: Field declaration updated correctly

The field declaration has been updated to use CompetencyProgressApi instead of CompetencyProgressService. The field is correctly declared as private and final, following best practices.

129-129: Constructor parameter updated consistently

The constructor parameter has been updated to use CompetencyProgressApi, which is consistent with the field declaration update. This change aligns with the PR objectives of introducing module-API for Atlas.

32-32: Summary: Changes align well with PR objectives

The changes in this file successfully implement the transition from CompetencyProgressService to CompetencyProgressApi, aligning with the PR objective of introducing module-API for Atlas. The modifications are consistent throughout the file, including import statements, field declarations, constructor parameters, and method calls. The changes are focused and don't introduce any apparent unintended side effects or issues.

Also applies to: 84-84, 129-129, 185-185, 255-255

src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java (4)

45-45: LGTM: Import statement updated.

The import statement has been correctly updated to use the new CompetencyProgressApi.

98-98: LGTM: Field declaration updated.

The field declaration has been correctly updated to use CompetencyProgressApi instead of CompetencyProgressService.

139-139: LGTM: Constructor parameter and assignment updated.

The constructor parameter and field assignment have been correctly updated to use CompetencyProgressApi instead of CompetencyProgressService.

Also applies to: 155-155

262-262: Verify the impact of the API change.

The method call has been updated to use the new CompetencyProgressApi. However, we should verify that this change doesn't affect the behavior of the importProgrammingExercise method.

Let's check if there are any other usages of CompetencyProgressService in the codebase that might need to be updated:
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (2)
155-155: Approve the change to CompetencyProgressApi

The change from CompetencyProgressService to CompetencyProgressApi has been correctly implemented in the class member declaration and constructor. This change aligns with the PR objective of establishing clearer boundaries between different modules.

Also applies to: 164-164, 189-189

Line range hint 1-638: Verify complete replacement of CompetencyProgressService

The changes to replace CompetencyProgressService with CompetencyProgressApi appear to be correctly implemented in the modified sections. However, it's important to ensure that all instances of CompetencyProgressService have been replaced throughout the file.

Please run the following script to check for any remaining usages of CompetencyProgressService:

✅ Verification successful

CompetencyProgressService Replacement Confirmed

All instances of CompetencyProgressService have been successfully replaced with CompetencyProgressApi in TextExerciseResource.java.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for any remaining usages of CompetencyProgressService in the file

# Test: Search for CompetencyProgressService in the file
rg --type java 'CompetencyProgressService' src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java

# If no results are found, the replacement is complete
if [ $? -ne 0 ]; then
    echo "All instances of CompetencyProgressService have been replaced successfully."
else
    echo "Warning: Some instances of CompetencyProgressService may still exist in the file."
fi
Length of output: 293
src/main/java/de/tum/cit/aet/artemis/exercise/service/ParticipationService.java (3)

27-27: LGTM: Import and field declaration for CompetencyProgressApi.

The changes to import and declare CompetencyProgressApi are correct and align with the goal of replacing CompetencyProgressService.

Also applies to: 112-112

121-121: LGTM: Constructor updated for CompetencyProgressApi.

The constructor has been correctly updated to use CompetencyProgressApi instead of CompetencyProgressService. The new parameter is properly assigned to the class field.

Also applies to: 142-142

932-932: Verify asynchronous behavior of updateProgressByLearningObjectAsync.

The change from updateCompetencyProgressForExercise to updateProgressByLearningObjectAsync is correct. However, the new method name implies asynchronous behavior. Please ensure that this change doesn't affect the overall flow of the deleteAllByExercise method, especially if any subsequent operations depend on the completion of this update.

To verify the impact of this change, you can run the following script:

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (3)

47-47: Import statement looks good.

The addition of the CompetencyProgressApi import from the Atlas module is appropriate and follows Java conventions.

185-185: Field declaration is appropriate.

The private final CompetencyProgressApi field is correctly declared, maintaining good encapsulation and immutability practices.

201-201: Constructor injection implemented correctly.

The CompetencyProgressApi parameter is appropriately added to the constructor, and the field is correctly initialized. This change adheres to the principle of constructor injection for effective dependency management.

Also applies to: 234-234
src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java (1)
7-8: 🧹 Nitpick (assertive)

Avoid unnecessary imports by importing specific classes

Instead of importing the entire org.springframework.core.env package, consider importing only the necessary classes to adhere to the coding guideline of avoiding star imports.

Apply this diff to specify the imports:
-import org.springframework.core.env.Environment;
-import org.springframework.core.env.Profiles;
+import org.springframework.core.env.*;
Likely invalid or redundant comment.
src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (3)

12-14: Fields are appropriately encapsulated

The fields environment and profileName are declared as private final, adhering to best practices for encapsulation and immutability.

16-19: Constructor properly initializes fields

The constructor correctly assigns the provided environment and profileName to the class fields, ensuring the object is properly initialized.

21-23: isActive method correctly checks profile activation

The isActive method accurately determines if the specified profile is active in the current environment using environment.acceptsProfiles.

src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasConfig.java (4)

24-24: Ensure type compatibility with Set<Profiles>

The declaration of requiredProfiles uses Set.of(Profiles.of(PROFILE_CORE)). Confirm that Profiles.of(PROFILE_CORE) returns an object of type Profiles. If Profiles.of returns a different type, this may lead to type incompatibility issues.

26-28: Good use of constructor injection for dependency management

The constructor properly uses dependency injection by accepting an Environment parameter and passing it to the superclass. This aligns with the best practice of constructor injection.

30-33: Verify the necessity of the static modifier on the inner class

The inner class AtlasDatabaseConfig is declared as static. In nested classes, the static modifier is typically used when the inner class does not need access to the instance variables of the outer class. Confirm that making this class static is appropriate and that it doesn't require any instance variables from AtlasConfig.

1-34: Overall code structure is clean and adheres to guidelines

The class AtlasConfig is well-structured, following Java naming conventions and best practices. It extends AbstractModuleConfig appropriately and sets up the necessary configurations for the Atlas module.

src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (4)

9-9: Appropriate use of module API import

The import of CompetencyProgressApi from the atlas.api package adheres to the new modular structure and promotes encapsulation as outlined in the PR objectives.

20-20: Field declaration aligns with dependency injection practices

Declaring competencyProgressApi as a private final field supports constructor-based dependency injection, complying with the project's coding guidelines.

22-24: Constructor correctly injects dependencies

The constructor properly injects CourseRepository and CompetencyProgressApi, following best practices for dependency injection.

33-33: Ensure exceptions are properly handled in the execute method

Consider handling potential exceptions from competencyProgressApi.updateProgressForCourses(activeCourses) to ensure that any issues during the migration are appropriately logged and do not interrupt the migration process.

Run the following script to check if updateProgressForCourses method declares any exceptions:

src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportScienceEventService.java (1)

31-32: Constructor Injection Applied Correctly

Using constructor injection for ScienceEventApi aligns with dependency injection best practices, promoting better testability and modularity.

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1)

58-62: 🧹 Nitpick (assertive)

Handle exceptions during asynchronous updates.

When performing asynchronous updates in competencies.forEach(competencyProgressService::updateProgressByCompetencyAsync);, any exceptions thrown might not be appropriately handled.

Ensure that exceptions in the asynchronous processes are caught and handled correctly to prevent silent failures. Consider adding error handling or logging within the asynchronous methods.

src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java (3)

24-25: Imports updated to use new API interfaces

The imports have been correctly updated to use CompetencyProgressApi and CourseCompetencyApi, aligning with the shift towards a more API-driven architecture.

62-62: Field declarations updated to use new API interfaces

The fields competencyProgressApi and courseCompetencyApi have been appropriately declared using the new API interfaces. This promotes better modularity and encapsulation within the application.

Also applies to: 64-64

68-68: Constructor updated for dependency injection of new APIs

The constructor has been updated to include CompetencyProgressApi and CourseCompetencyApi, following best practices for dependency injection. The assignments are correctly implemented, ensuring that the new dependencies are properly injected.

Also applies to: 76-77

src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java (1)

151-151: Proper implementation of constructor injection for CompetencyProgressApi

The CompetencyProgressApi is correctly injected via constructor injection, adhering to dependency injection best practices. This ensures better testability and maintainability.

Also applies to: 159-159, 182-182

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (5)

56-57: New API interfaces are correctly imported

The imports for CourseCompetencyApi and LearningPathApi are appropriately added.

131-133: Dependency injection of API interfaces is correctly implemented

The CourseCompetencyApi and LearningPathApi are properly declared as final fields and will be injected via the constructor.

257-258: Assignment of API interfaces in constructor is correct

The courseCompetencyApi and learningPathApi are properly assigned in the constructor.

346-346: Ensure exception handling in findAndSetCompetenciesForCourse

Verify that courseCompetencyApi.findAndSetCompetenciesForCourse(course) handles exceptions properly to prevent any runtime issues during course retrieval.

487-487: Ensure exception handling in deleteCompetenciesOfCourse

Verify that courseCompetencyApi.deleteCompetenciesOfCourse(course) handles exceptions appropriately to ensure a complete and error-free course deletion process.

src/test/java/de/tum/cit/aet/artemis/atlas/architecture/AtlasApiArchitectureTest.java

src/main/java/de/tum/cit/aet/artemis/atlas/api/AbstractAtlasApi.java

src/main/java/de/tum/cit/aet/artemis/core/exception/ModuleNotPresentException.java

src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.java

src/main/java/de/tum/cit/aet/artemis/core/service/user/UserService.java

src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java

coderabbitai · 2024-10-14T22:52:03Z

Walkthrough

The pull request introduces significant changes to the codebase, primarily focusing on the integration of CompetencyProgressApi as a replacement for CompetencyProgressService. This change is reflected across multiple classes, including service and controller layers. Additionally, several new classes and methods are introduced, particularly in the Atlas module, enhancing its functionality. The overall structure is updated to support these changes, including adjustments to configuration and architectural tests.

Changes

File	Change Summary
`src/main/java/de/tum/cit/aet/artemis/assessment/service/ParticipantScoreScheduleService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi` in field and constructor. Updated `executeTask` method to use new API.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/AbstractAtlasApi.java`	Added abstract class `AbstractAtlasApi` with a constructor accepting `Environment`.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java`	Introduced `CompetencyProgressApi` class with methods for competency progress updates.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java`	Added `CourseCompetencyApi` controller with methods for managing course competencies.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningMetricsApi.java`	Added `LearningMetricsApi` controller for retrieving student course metrics.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/LearningPathApi.java`	Added `LearningPathApi` controller for generating learning paths.
`src/main/java/de/tum/cit/aet/artemis/atlas/api/ScienceEventApi.java`	Added `ScienceEventApi` controller for managing science events.
`src/main/java/de/tum/cit/aet/artemis/atlas/config/AtlasConfig.java`	Introduced `AtlasConfig` configuration class for the Atlas module.
`src/main/java/de/tum/cit/aet/artemis/atlas/service/LearningMetricsService.java`	Changed package name from `exercise.service` to `atlas.service`.
`src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java`	Added `AbstractApi` class for managing API functionalities.
`src/main/java/de/tum/cit/aet/artemis/core/config/AbstractModuleConfig.java`	Introduced `AbstractModuleConfig` for module configuration.
`src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java`	Added constant `PROFILE_ATLAS` for Atlas profile.
`src/main/java/de/tum/cit/aet/artemis/core/config/DatabaseConfiguration.java`	Updated `@EnableJpaRepositories` to remove `atlas.repository` package.
`src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java`	Updated constructor to accept `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/core/exception/ModuleNotPresentException.java`	Introduced `ModuleNotPresentException` for signaling missing modules.
`src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java`	Replaced repository dependencies with API dependencies for competencies and learning paths.
`src/main/java/de/tum/cit/aet/artemis/core/service/export/DataExportScienceEventService.java`	Replaced `ScienceEventRepository` with `ScienceEventApi`.
`src/main/java/de/tum/cit/aet/artemis/core/service/user/UserService.java`	Replaced `ScienceEventRepository` with `ScienceEventApi`.
`src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java`	Replaced `LearningPathService` with `LearningPathApi`.
`src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/exercise/service/ParticipationService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/fileupload/service/FileUploadExerciseImportService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/fileupload/web/FileUploadExerciseResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisPipelineService.java`	Replaced `LearningMetricsService` with `LearningMetricsApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/service/AttachmentUnitService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureUnitService.java`	Replaced `CompetencyProgressService` and `CourseCompetencyRepository` with `CompetencyProgressApi` and `CourseCompetencyApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/web/AttachmentUnitResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/web/OnlineUnitResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/web/TextUnitResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/lecture/web/VideoUnitResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/modeling/service/ModelingExerciseImportService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/modeling/web/ModelingExerciseResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseExportImportResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/quiz/service/QuizExerciseImportService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/quiz/web/QuizExerciseResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseImportService.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java`	Replaced `CompetencyProgressService` with `CompetencyProgressApi`.
`src/test/java/de/tum/cit/aet/artemis/atlas/architecture/AtlasApiArchitectureTest.java`	Introduced `AtlasApiArchitectureTest` for validating architectural constraints.
`src/test/java/de/tum/cit/aet/artemis/shared/architecture/module/AbstractModuleAccessArchitectureTest.java`	Introduced `AbstractModuleAccessArchitectureTest` for module API architectural validation.

Possibly related PRs

Adaptive learning: Add endpoint to retrieve student course metrics #8508: This PR introduces a new endpoint to retrieve student course metrics, which is directly related to the changes in the ParticipantScoreScheduleService class that involve updating how scores are managed and retrieved.
Development: Add endpoint for learning path competency graph #8649: This PR adds a new endpoint for the Learning Path Competency Graph, which may relate to the competency progress updates in the main PR, as both involve managing competencies.
Development: Add science events to user data export #8861: This PR adds a service for exporting science events, which could connect to the competency progress updates if competencies are tied to science events in the application.
Adaptive learning: Fix course deletion when competency relations exist #8969: This PR addresses course deletion issues related to competency relations, which may intersect with the competency progress updates in the main PR.
Adaptive learning: Fix competency progress update for exercise and lecture unit operations #8976: This PR fixes competency progress updates for various operations, directly relating to the changes made in the ParticipantScoreScheduleService class.
Development: Use @EnforceAdmin annotation on class level #9422: This PR implements class-level @EnforceAdmin annotations, which may affect access to the metrics and progress updates in the main PR.
Development: Remove redundant architecture test in atlas module #9429: This PR removes a redundant architecture test in the atlas module, which may indirectly relate to the overall structure and testing of the competency progress updates.
Development: Fix architecture tests for exam deletion summary #9458: This PR fixes issues with server test flakiness, which could impact the reliability of tests related to the competency progress updates in the main PR.

Suggested labels

ready for review, lti, communication

Suggested reviewers

JohannesStoehr
MaximilianAnzinger
SimonEntholzer
pzdr7

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?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between a8f0cc0 and bf2dd86.

📒 Files selected for processing (1)

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

🔇 Additional comments (1)

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1)

28-35: Avoid using Optional as a parameter type

Using Optional as a method parameter is generally discouraged in Java. This issue was previously raised and is still applicable.

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java

coderabbitai

Actionable comments posted: 17

🧹 Outside diff range comments (2)

src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java (1)

Line range hint 1-174: Overall, the changes align well with PR objectives and coding guidelines.

The modifications in this file consistently replace competencyProgressService with competencyProgressApi, which aligns perfectly with the PR objective of introducing a module-API for Atlas. The test methods adhere to the coding guidelines, particularly in terms of:

Descriptive test naming (with room for minor improvements)

Small and specific tests

Proper use of static mocks

Appropriate handling of asynchronous operations

The changes maintain the existing test structure while updating the API calls, which is a good practice for ensuring backward compatibility and maintaining test coverage during refactoring.

To further improve the modularity and encapsulation as mentioned in the PR objectives, consider:

Ensuring that all access to the competency progress functionality goes through the new API in other parts of the codebase.

Updating the relevant documentation to reflect the new API usage.

If not already done, consider adding integration tests that specifically verify the boundaries between modules, ensuring that access is restricted to the api subpackage as intended.

src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java (1)

Line range hint 201-309: Overall LGTM! Consider reviewing for consistency across test methods.

The changes consistently replace competencyProgressService with competencyProgressApi, which aligns well with the PR objectives of establishing clearer module boundaries. This is a positive improvement that should enhance the modularity of the codebase.

Consider reviewing the CompetencyProgressApi interface for consistency:

Some methods use updateProgressByLearningObjectAsync while others use updateProgressForUpdatedLearningObjectAsync or updateProgressByCompetencyAsync.

Parameters vary between methods (e.g., createdExercise, Optional.of(programmingExercise), competency).

Verification approaches differ (some use timeout and times(1), others don't).

Harmonizing these aspects could improve the overall consistency and maintainability of the test suite and the API itself. If the differences are intentional due to specific requirements, consider adding comments to explain the reasoning behind these variations.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between bf2dd86 and 755f3be.

📒 Files selected for processing (11)

src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadExerciseIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/ExerciseUnitIntegrationTest.java (1 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/LectureIntegrationTest.java (1 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/OnlineUnitIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/lecture/VideoUnitIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java (4 hunks)
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (3 hunks)
src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java (3 hunks)

🧰 Additional context used

📓 Path-based instructions (11)

src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadExerciseIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/ExerciseUnitIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/LectureIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/OnlineUnitIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/lecture/VideoUnitIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.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: true

src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.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: true

📓 Learnings (6)

src/test/java/de/tum/cit/aet/artemis/lecture/ExerciseUnitIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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/lecture/LectureIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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/lecture/OnlineUnitIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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/lecture/TextUnitIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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/lecture/VideoUnitIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java (2)

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.

Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
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 (14)

src/test/java/de/tum/cit/aet/artemis/lecture/VideoUnitIntegrationTest.java (1)

Line range hint 1-228: Overall, the changes in this file are well-implemented and align with the PR objectives.

The consistent replacement of competencyProgressService with competencyProgressApi and the shift to asynchronous methods (updateProgressByLearningObjectAsync and updateProgressForUpdatedLearningObjectAsync) demonstrate a clear implementation of the module-API for Atlas. These changes should improve modularity and potentially enhance performance.

The tests maintain their descriptive naming convention, remain small and specific, and continue to use static mocks as per the guidelines. The addition of timeouts for asynchronous verifications is a good practice.

To further improve the tests, consider updating the test method names as suggested in the individual comments to better reflect the asynchronous nature of the competency progress updates.

src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractArtemisIntegrationTest.java (1)

174-176: LGTM: Addition of CompetencyProgressApi SpyBean

The addition of the CompetencyProgressApi SpyBean is consistent with the existing pattern in the class and aligns with the PR objectives of introducing a module-API for Atlas. This will allow for effective testing and mocking of the CompetencyProgressApi in integration tests.
src/test/java/de/tum/cit/aet/artemis/lecture/OnlineUnitIntegrationTest.java (4)
Line range hint 1-270: Summary: Changes align with PR objectives and improve modularity.

The modifications in this file consistently replace competencyProgressService with competencyProgressApi and introduce async methods. These changes align well with the PR objectives of establishing clearer boundaries between modules and potentially improving performance. The test cases have been updated appropriately to reflect these changes.

150-150: LGTM! Verify the implementation of the new async method.

The change from competencyProgressService to competencyProgressApi and the update to an async method (updateProgressForUpdatedLearningObjectAsync) align with the PR objectives. This modification enhances modularity and potentially improves performance through asynchronous processing.

To ensure the correct implementation of the new async method, run the following script:
#!/bin/bash
# Description: Check the implementation of updateProgressForUpdatedLearningObjectAsync
rg --type java -A 10 'updateProgressForUpdatedLearningObjectAsync' src/
115-115: LGTM! Verify consistent usage across the codebase.

The change from competencyProgressService to competencyProgressApi aligns with the PR objective of introducing a module-API for Atlas. This modification enhances modularity and encapsulation as intended.

To ensure consistency, run the following script to check for any remaining usages of competencyProgressService:

✅ Verification successful

Verification Successful!

All instances of competencyProgressService have been replaced with competencyProgressApi across the codebase, ensuring consistent usage as intended.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for remaining usages of competencyProgressService
rg --type java 'competencyProgressService' src/
Length of output: 36439
267-267: LGTM! Verify consistent async method usage.

The change from competencyProgressService to competencyProgressApi and the update to an async method (updateProgressForUpdatedLearningObjectAsync) are consistent with the previous modifications and align with the PR objectives.

To ensure consistent usage of async methods across similar operations, run the following script:

✅ Verification successful

Verified! Consistent async method usage confirmed across the codebase.
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Description: Check for consistent usage of async methods in competencyProgressApi
rg --type java 'competencyProgressApi.*Async' src/
Length of output: 36439
src/test/java/de/tum/cit/aet/artemis/programming/icl/ProgrammingExerciseLocalVCLocalCIIntegrationTest.java (2)

226-226: LGTM! Please clarify the method name change and additional parameter.

The change from competencyProgressService to competencyProgressApi is consistent with the module boundary improvements. The addition of a timeout and verification of exactly one call enhances the test's robustness.

Could you please clarify:

Why was the method name changed from updateProgressByLearningObjectAsync to updateProgressForUpdatedLearningObjectAsync?

What's the purpose of the additional Optional.of(programmingExercise) parameter?

These changes seem to indicate a modification in the underlying API. It would be helpful to understand the reasoning behind these changes to ensure they align with the intended functionality.

259-259: LGTM! Please explain the changes in method name and parameter.

The switch from competencyProgressService to competencyProgressApi is consistent with the module boundary improvements.

Could you please provide more information on:

Why was the method name changed from updateProgressByLearningObjectAsync to updateProgressByCompetencyAsync?

Why is the parameter now competency instead of the exercise?

These changes suggest a shift in how competency progress is updated when an exercise is deleted. It would be beneficial to understand the reasoning behind these changes to ensure they align with the expected behavior when deleting a programming exercise.

src/test/java/de/tum/cit/aet/artemis/lecture/LectureIntegrationTest.java (1)

368-368: Verify the impact of switching from competencyProgressService to competencyProgressApi.

The change from competencyProgressService to competencyProgressApi appears to be part of a larger refactoring effort to use an API instead of directly accessing the service. This aligns with the PR objectives of establishing clearer boundaries between modules.

However, we should ensure that this change doesn't affect the test's behavior or expectations.

To confirm that this change doesn't introduce any unintended side effects, please run the following verification script:

This script will help us verify that:

All instances of competencyProgressService have been replaced.

The competencyProgressApi is being used consistently across the codebase.

The method signature of updateProgressForUpdatedLearningObjectAsync remains the same, ensuring that this test's expectations are still valid.

src/test/java/de/tum/cit/aet/artemis/fileupload/FileUploadExerciseIntegrationTest.java (2)

334-334: LGTM: Change aligns with PR objectives.

The update from competencyProgressService to competencyProgressApi is in line with the PR's goal of establishing clearer boundaries between modules. This change likely contributes to the new API structure mentioned in the PR description.

693-693: LGTM: Consistent change aligning with PR objectives.

This update from competencyProgressService to competencyProgressApi is consistent with the previous change and continues to align with the PR's goal of establishing clearer boundaries between modules. It further implements the new API structure mentioned in the PR description.

src/test/java/de/tum/cit/aet/artemis/lecture/AttachmentUnitIntegrationTest.java (2)

279-279: Verification of competencyProgressApi usage is appropriate

The update to use competencyProgressApi in the verify statement aligns with the new modular API structure. The use of timeout(1000).times(1) ensures that the method is called once within an acceptable timeframe.

355-355: Verification of asynchronous update with competencyProgressApi is correct

The change to use competencyProgressApi for verifying the asynchronous update reflects the intended architectural changes. The use of timeout(1000).times(1) is appropriate to ensure the method is invoked as expected.

src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java (1)

243-243: Verification with timeout is appropriate.

The use of verify with timeout(1000).times(1) correctly ensures that updateProgressForUpdatedLearningObjectAsync is called once within the specified time frame.

coderabbitai · 2024-10-15T14:43:02Z

src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java

@@ -89,7 +89,7 @@ void createTextUnit_asEditor_shouldCreateTextUnitUnit() throws Exception {
        var persistedTextUnit = request.postWithResponseBody("/api/lectures/" + this.lecture.getId() + "/text-units", textUnit, TextUnit.class, HttpStatus.CREATED);
        assertThat(persistedTextUnit.getId()).isNotNull();
        assertThat(persistedTextUnit.getName()).isEqualTo("LoremIpsum");
-        verify(competencyProgressService).updateProgressByLearningObjectAsync(eq(persistedTextUnit));
+        verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(persistedTextUnit));


🧹 Nitpick (assertive)

LGTM! Consider enhancing the test method name.

The change from competencyProgressService to competencyProgressApi aligns well with the PR objectives of introducing a module-API for Atlas. The use of verify on a mock object is consistent with the static mocks strategy outlined in the coding guidelines.

Consider updating the test method name to reflect the verification of the competency progress update:

-void createTextUnit_asEditor_shouldCreateTextUnitUnit() throws Exception { +void createTextUnit_asEditor_shouldCreateTextUnitAndUpdateCompetencyProgress() throws Exception {

This change would make the test's purpose more explicit and align better with the "descriptive" test naming guideline.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(persistedTextUnit));

void createTextUnit_asEditor_shouldCreateTextUnitAndUpdateCompetencyProgress() throws Exception {

coderabbitai · 2024-10-15T14:43:02Z

src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java

@@ -161,7 +161,7 @@
        assertThat(this.textUnit.getId()).isNotNull();
        request.delete("/api/lectures/" + lecture.getId() + "/lecture-units/" + this.textUnit.getId(), HttpStatus.OK);
        request.get("/api/lectures/" + lecture.getId() + "/text-units/" + this.textUnit.getId(), HttpStatus.NOT_FOUND, TextUnit.class);
-        verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.empty()));
+        verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.empty()));


🧹 Nitpick (assertive)

LGTM! Consider enhancing the test method name.

The change from competencyProgressService to competencyProgressApi is in line with the PR objectives. The use of verify with a timeout for the asynchronous operation and the argument matching with eq are appropriate. The verification of an empty Optional for the deleted text unit is correct.

Consider updating the test method name to reflect the verification of the competency progress update:

-void deleteTextUnit_correctId_shouldDeleteTextUnit() throws Exception { +void deleteTextUnit_correctId_shouldDeleteTextUnitAndUpdateCompetencyProgress() throws Exception {

This change would make the test's purpose more explicit and align better with the "descriptive" test naming guideline.

coderabbitai · 2024-10-15T14:43:02Z

src/test/java/de/tum/cit/aet/artemis/lecture/TextUnitIntegrationTest.java

@@ -112,7 +112,7 @@
        textUnitFromRequest.setContent("Changed");
        TextUnit updatedTextUnit = request.putWithResponseBody("/api/lectures/" + lecture.getId() + "/text-units", textUnitFromRequest, TextUnit.class, HttpStatus.OK);
        assertThat(updatedTextUnit.getContent()).isEqualTo("Changed");
-        verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.of(textUnit)));
+        verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.of(textUnit)));


🧹 Nitpick (assertive)

LGTM! Consider enhancing the test method name.

The change from competencyProgressService to competencyProgressApi is consistent with the PR objectives. The use of verify with a timeout is appropriate for testing asynchronous operations, and the argument matching with eq improves test readability.

Consider updating the test method name to reflect the verification of the competency progress update:

-void updateTextUnit_asEditor_shouldUpdateTextUnit() throws Exception { +void updateTextUnit_asEditor_shouldUpdateTextUnitAndUpdateCompetencyProgress() throws Exception {

This change would make the test's purpose more explicit and align better with the "descriptive" test naming guideline.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.of(textUnit)));

void updateTextUnit_asEditor_shouldUpdateTextUnitAndUpdateCompetencyProgress() throws Exception {

coderabbitai · 2024-10-15T14:43:02Z

src/test/java/de/tum/cit/aet/artemis/lecture/ExerciseUnitIntegrationTest.java

@@ -214,7 +214,7 @@ void deleteExerciseUnit_exerciseConnectedWithExerciseUnit_shouldNOTDeleteExercis
            request.get("/api/exercises/" + exercise.getId(), HttpStatus.OK, Exercise.class);
        }

-        verify(competencyProgressService, never()).updateProgressForUpdatedLearningObjectAsync(any(), any());
+        verify(competencyProgressApi, never()).updateProgressForUpdatedLearningObjectAsync(any(), any());


🧹 Nitpick (assertive)

LGTM! Consider adding a descriptive comment for clarity.

The change from competencyProgressService to competencyProgressApi aligns with the PR objectives of introducing a module-API for Atlas. This modification maintains the test's purpose while adhering to the new architectural boundaries.

To enhance test clarity, consider adding a brief comment explaining why this verification is important:

// Verify that competency progress is not updated when deleting exercise units verify(competencyProgressApi, never()).updateProgressForUpdatedLearningObjectAsync(any(), any());

coderabbitai · 2024-10-15T14:43:02Z

src/test/java/de/tum/cit/aet/artemis/lecture/VideoUnitIntegrationTest.java

@@ -204,7 +204,7 @@ void deleteVideoUnit_correctId_shouldDeleteVideoUnit() throws Exception {
        assertThat(this.videoUnit.getId()).isNotNull();
        request.delete("/api/lectures/" + lecture1.getId() + "/lecture-units/" + this.videoUnit.getId(), HttpStatus.OK);
        request.get("/api/lectures/" + lecture1.getId() + "/video-units/" + this.videoUnit.getId(), HttpStatus.NOT_FOUND, VideoUnit.class);
-        verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(videoUnit), eq(Optional.empty()));
+        verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(videoUnit), eq(Optional.empty()));


🧹 Nitpick (assertive)

LGTM! Consider updating the test method name.

The change from competencyProgressService to competencyProgressApi and the shift to an asynchronous method (updateProgressForUpdatedLearningObjectAsync) align well with the PR objectives. The addition of a timeout in the verify call is a good practice for asynchronous operations.

Consider updating the test method name to reflect the asynchronous nature of the competency progress update:

-void deleteVideoUnit_correctId_shouldDeleteVideoUnit() throws Exception { +void deleteVideoUnit_correctId_shouldDeleteVideoUnitAndUpdateCompetencyProgressAsync() throws Exception {

This change would make the test name more descriptive and align better with the actual behavior being tested.

Committable suggestion was skipped due to low confidence.

coderabbitai · 2024-10-15T14:43:04Z

src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java

@@ -412,7 +411,7 @@ void testDeleteModelingExerciseWithCompetency() throws Exception {

        request.delete("/api/modeling-exercises/" + classExercise.getId(), HttpStatus.OK);

-        verify(competencyProgressService).updateProgressByCompetencyAsync(eq(competency));
+        verify(competencyProgressApi).updateProgressByCompetencyAsync(eq(competency));


🛠️ Refactor suggestion

Add timeout to the verify call for consistent test behavior.

Consider adding a timeout to the verify call to prevent the test from hanging indefinitely if the method updateProgressByCompetencyAsync is not called, and to maintain consistency with other tests.

Apply this diff to add a timeout to the verify call:

- verify(competencyProgressApi).updateProgressByCompetencyAsync(eq(competency)); + verify(competencyProgressApi, timeout(1000).times(1)).updateProgressByCompetencyAsync(eq(competency));

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

verify(competencyProgressApi).updateProgressByCompetencyAsync(eq(competency));

verify(competencyProgressApi, timeout(1000).times(1)).updateProgressByCompetencyAsync(eq(competency));

coderabbitai · 2024-10-15T14:43:04Z

src/test/java/de/tum/cit/aet/artemis/modeling/ModelingExerciseIntegrationTest.java

@@ -482,7 +481,7 @@
        Channel channelFromDB = channelRepository.findChannelByExerciseId(importedExercise.getId());
        assertThat(channelFromDB).isNotNull();
        assertThat(channelFromDB.getName()).isEqualTo(uniqueChannelName);
-        verify(competencyProgressService).updateProgressByLearningObjectAsync(eq(importedExercise));
+        verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(importedExercise));


🛠️ Refactor suggestion

Add timeout to the verify call for consistent test behavior.

Consider adding a timeout to the verify call to prevent potential indefinite waits if updateProgressByLearningObjectAsync is not invoked, and to align with the convention used in other tests.

Apply this diff to add a timeout to the verify call:

- verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(importedExercise)); + verify(competencyProgressApi, timeout(1000).times(1)).updateProgressByLearningObjectAsync(eq(importedExercise));

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(importedExercise));

verify(competencyProgressApi, timeout(1000).times(1)).updateProgressByLearningObjectAsync(eq(importedExercise));

coderabbitai · 2024-10-15T14:43:04Z

src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java

@@ -429,7 +429,7 @@ void updateTextExercise() throws Exception {
        assertThat(updatedTextExercise.getCourseViaExerciseGroupOrCourseMember().getId()).as("courseId was not updated").isEqualTo(course.getId());
        verify(examLiveEventsService, never()).createAndSendProblemStatementUpdateEvent(any(), any(), any());
        verify(groupNotificationScheduleService, times(1)).checkAndCreateAppropriateNotificationsWhenUpdatingExercise(any(), any(), any());
-        verify(competencyProgressService, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textExercise), eq(Optional.of(textExercise)));
+        verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textExercise), eq(Optional.of(textExercise)));


⚠️ Potential issue

Verify that competencyProgressApi is correctly mocked in this test

Since competencyProgressApi is being used in the verification step, confirm that it is properly mocked within the TextExerciseIntegrationTest class. This will ensure that the test functions correctly and that the interaction with the mock is accurately captured.

coderabbitai · 2024-10-15T14:43:04Z

src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java

@@ -592,7 +592,7 @@
        var newTextExercise = request.postWithResponseBody("/api/text-exercises/import/" + textExercise.getId(), textExercise, TextExercise.class, HttpStatus.CREATED);
        Channel channel = channelRepository.findChannelByExerciseId(newTextExercise.getId());
        assertThat(channel).isNotNull();
-        verify(competencyProgressService).updateProgressByLearningObjectAsync(eq(newTextExercise));
+        verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(newTextExercise));


⚠️ Potential issue

Ensure proper mocking of competencyProgressApi for interaction verification

In this test, you're verifying an interaction with competencyProgressApi. Make sure that competencyProgressApi is properly mocked and that any necessary setup is performed. This will prevent potential test failures due to missing mocks or incorrect configurations.

coderabbitai · 2024-10-15T14:43:04Z

src/test/java/de/tum/cit/aet/artemis/text/TextExerciseIntegrationTest.java

@@ -199,7 +199,7 @@

        request.delete("/api/text-exercises/" + textExercise.getId(), HttpStatus.OK);

-        verify(competencyProgressService).updateProgressByCompetencyAsync(eq(competency));
+        verify(competencyProgressApi).updateProgressByCompetencyAsync(eq(competency));


⚠️ Potential issue

Ensure competencyProgressApi is properly mocked

The test now verifies interactions with competencyProgressApi. Please ensure that competencyProgressApi is correctly mocked and injected into the test class. Without proper mocking, the test may fail due to a NullPointerException or may not accurately verify the expected behavior.

cremertim

refactoring lgtm

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java

coderabbitai

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)

src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (1)
34-35: Consider logging an error before throwing the exception

Adding an error-level log before throwing ModuleNotPresentException can provide more context and aid in debugging when the exception occurs.

Suggested change:
 if (!competencyProgressApi.isActive()) {
+    log.error("CompetencyProgressApi is not active; cannot update competency progress.");
     throw new ModuleNotPresentException("CompetencyProgressApi is not active");
 }
src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1)

38-56: Add JavaDoc comments to public methods for better documentation

Currently, only the updateProgressForCourses method has a JavaDoc comment. Adding JavaDoc comments to the other public methods will enhance code readability and maintainability.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 755f3be and 763afc1.

📒 Files selected for processing (4)

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (3 hunks)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (6 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java

🧰 Additional context used

📓 Path-based instructions (3)

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

🔇 Additional comments (10)

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (5)

47-47: LGTM: Import statement follows module-API pattern.

The import statement correctly references the new API package, aligning with the PR's objective of establishing clearer module boundaries.

185-185: LGTM: Field declaration follows best practices.

The field is correctly declared as private and final, following constructor injection pattern and Java naming conventions.

201-201: LGTM: Constructor injection properly implemented.

The constructor parameter and field assignment follow dependency injection best practices.

Also applies to: 234-234

339-340: Error handling still missing for async operation.

620-621: Error handling still missing for async operation.

src/main/java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java (4)

9-9: Specific import statement aligns with guidelines

The import of CompetencyProgressApi is specific and avoids star imports, adhering to the coding guideline java:avoid_star_imports.

12-12: Imported exception class correctly

The ModuleNotPresentException is correctly imported for exception handling.

21-21: Field declaration follows best practices

The field competencyProgressApi is declared private final, promoting immutability and encapsulation.

23-25: Constructor updated with dependency injection

The constructor now properly injects CompetencyProgressApi, following the dependency injection principle specified in the coding guidelines (di:constructor_injection).

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java (1)

1-77: Implementation aligns with coding guidelines

The class provides the intended API for managing competency progress, and the code adheres to the specified coding standards.

...java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java

JohannesStoehr

Code

cremertim

Reapprove after latest changes

krusche

I do not understand why certain repositories are declared as optional in the new APIs.
I am not sure if the annotation @Controller is the correct one in this case

ole-ve · 2024-10-29T00:27:47Z

I do not understand why certain repositories are declared as optional in the new APIs. I am not sure if the annotation @Controller is the correct one in this case

Actually, all atlas services/repositories are optionally autowired. Reasoning being to ensure that these components could be profile-annotatated and then not be autowired as part of Artemis without Atlas (for instance). Otherwise, the code might get cluttered up with ifPresents checking for the existence of the APIs.
Also, this make testing easier as an exercise-dev does not need to care about the atlas-internals, but can just verify the call to the API. this would not be possible if we conditionally invoke the API.

We did the decision on @Controller together a few weeks ago as it's some kind of interface between modules. What would you suggest, otherwise?

coderabbitai

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (1)

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)

342-342: Consider making learning path generation asynchronous

The learning path generation operations could potentially be time-consuming. Consider making these operations asynchronous to improve response times.

You could:

Use Spring's @Async annotation with a dedicated thread pool
Return a response immediately and handle the generation in the background
Implement a status endpoint to check generation progress

Would you like me to provide a detailed implementation for this architectural improvement?

Also applies to: 1277-1277

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2f3c744 and 90711b6.

📒 Files selected for processing (6)

src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1 hunks)
src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (8 hunks)
src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (6 hunks)
src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (4 hunks)
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (6 hunks)
src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (6 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (1)

Pattern src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports

🔇 Additional comments (17)

src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseDeletionService.java (4)

19-19: LGTM: Import statement follows conventions.

The addition of the CompetencyProgressApi import aligns with the PR's objective of establishing module boundaries through API facades.

81-81: LGTM: Field declaration follows best practices.

The field declaration follows Java best practices:

Uses constructor injection (field is final)
Follows proper encapsulation (private)
Uses correct naming convention (camelCase)

89-90: LGTM: Constructor changes maintain dependency injection pattern.

The constructor changes properly integrate the new API dependency while maintaining clean code principles:

Uses constructor injection
Maintains logical parameter ordering
Properly initializes the field

Also applies to: 105-105

217-217: Consider adding error handling for async operations.

While the async update is good for performance, consider adding error handling to catch and log any failures in competency progress updates. This is especially important since this occurs during exercise deletion, where we want to ensure all cleanup operations are properly tracked.

Consider wrapping the async call with error handling:

-        competencies.forEach(competencyProgressApi::updateProgressByCompetencyAsync);
+        competencies.forEach(competency -> {
+            try {
+                competencyProgressApi.updateProgressByCompetencyAsync(competency)
+                    .exceptionally(throwable -> {
+                        log.error("Failed to update progress for competency {}: {}", competency.getId(), throwable.getMessage());
+                        return null;
+                    });
+            } catch (Exception e) {
+                log.error("Failed to initiate progress update for competency {}: {}", competency.getId(), e.getMessage());
+            }
+        });

src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)

310-314: LGTM! Verify profile usage across the codebase.

The new PROFILE_ATLAS constant is well-implemented with proper documentation and consistent naming. Let's verify its usage across the codebase to ensure proper module boundaries.

src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java (3)

44-44: LGTM: Import statement follows the new module-API structure.

The import statement correctly references the new API package structure, aligning with the PR's objective of establishing clearer module boundaries.

157-157: LGTM: Field declaration follows best practices.

The field is properly declared as private final and follows Java naming conventions.

235-235: Previous review comment about error handling is still applicable.

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (2)

48-48: LGTM: Clean integration of CompetencyProgressApi.

The import statement and field declaration follow Java best practices with proper encapsulation and immutability.

Also applies to: 186-186

202-202: LGTM: Proper constructor injection implementation.

The CompetencyProgressApi is correctly injected through the constructor and initialized, following the established pattern in the class.

Also applies to: 235-235

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java (4)

70-70: LGTM!

The import statement for LearningPathApi is correctly placed with other imports.

181-181: LGTM!

The field declaration follows the established pattern and uses proper access modifiers.

194-194: LGTM!

Constructor injection and field assignment are properly implemented following Spring best practices.

Also applies to: 214-214

342-342: ⚠️ Potential issue

Add error handling for learning path generation

The call to learningPathApi.generateLearningPaths() should include error handling to ensure system stability.

Apply this change:

- learningPathApi.generateLearningPaths(courseWithCompetencies);
+ try {
+     learningPathApi.generateLearningPaths(courseWithCompetencies);
+ } catch (Exception e) {
+     log.error("Failed to generate learning paths for course {}: {}", courseWithCompetencies.getId(), e.getMessage());
+     // Consider whether to rethrow or handle the error differently
+     throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error generating learning paths", e);
+ }

Likely invalid or redundant comment.

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java (3)

56-57: Imports of CourseCompetencyApi and LearningPathApi look appropriate

The added imports for CourseCompetencyApi and LearningPathApi are correctly placed and follow the project's import conventions.

132-134: Dependency declarations for APIs are correctly implemented

The declarations of courseCompetencyApi and learningPathApi as private final fields are appropriate, ensuring immutability and proper encapsulation.

258-259: Assignment of API dependencies in the constructor

The assignments of this.courseCompetencyApi and this.learningPathApi in the constructor correctly initialize the API dependencies.

coderabbitai · 2024-10-29T08:40:05Z

src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java

@@ -166,7 +166,7 @@ public TextExerciseResource(TextExerciseRepository textExerciseRepository, TextE
            GradingCriterionRepository gradingCriterionRepository, TextBlockRepository textBlockRepository, GroupNotificationScheduleService groupNotificationScheduleService,
            InstanceMessageSendService instanceMessageSendService, PlagiarismDetectionService plagiarismDetectionService, CourseRepository courseRepository,
            ChannelService channelService, ChannelRepository channelRepository, Optional<AthenaModuleService> athenaModuleService,
-            CompetencyProgressService competencyProgressService, Optional<IrisSettingsService> irisSettingsService) {
+            CompetencyProgressApi competencyProgressApi, Optional<IrisSettingsService> irisSettingsService) {


🛠️ Refactor suggestion

Consider refactoring to reduce constructor parameters.

While the new parameter and field assignment are correctly implemented, the constructor has a high number of dependencies (25+ parameters). This could make the class harder to maintain and test.

Consider using the Builder pattern or breaking down this class into smaller, more focused components. For example:

Create separate handlers for different operations (CRUD, export, plagiarism)

Use a configuration object to group related dependencies

Consider facade services to aggregate related functionalities

Example configuration object approach:

public class TextExerciseConfig { private final FeedbackRepository feedbackRepository; private final TextBlockRepository textBlockRepository; private final TextExerciseService textExerciseService; // ... group related dependencies } public class TextExerciseResource { public TextExerciseResource( TextExerciseConfig config, CompetencyProgressApi competencyProgressApi, // ... reduced number of direct dependencies ) { // ... } }

Also applies to: 194-194

coderabbitai · 2024-10-29T08:40:05Z

src/main/java/de/tum/cit/aet/artemis/text/web/TextExerciseResource.java

@@ -293,7 +293,7 @@
        exerciseService.checkExampleSubmissions(updatedTextExercise);
        exerciseService.notifyAboutExerciseChanges(textExerciseBeforeUpdate, updatedTextExercise, notificationText);

-        competencyProgressService.updateProgressForUpdatedLearningObjectAsync(textExerciseBeforeUpdate, Optional.of(textExercise));
+        competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(textExerciseBeforeUpdate, Optional.of(textExercise));


⚠️ Potential issue

Add error handling for the async operation.

The asynchronous call should be wrapped in a try-catch block to handle potential exceptions gracefully.

- competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(textExerciseBeforeUpdate, Optional.of(textExercise)); + try { + competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(textExerciseBeforeUpdate, Optional.of(textExercise)); + } catch (Exception e) { + log.error("Failed to update competency progress for updated TextExercise {}: {}", textExercise.getId(), e.getMessage()); + }

Committable suggestion was skipped due to low confidence.

coderabbitai · 2024-10-29T08:40:05Z

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java

@@ -337,7 +337,7 @@ public ProgrammingExercise createProgrammingExercise(ProgrammingExercise program
        // Step 12c: Check notifications for new exercise
        groupNotificationScheduleService.checkNotificationsForNewExerciseAsync(savedProgrammingExercise);
        // Step 12d: Update student competency progress
-        competencyProgressService.updateProgressByLearningObjectAsync(savedProgrammingExercise);
+        competencyProgressApi.updateProgressByLearningObjectAsync(savedProgrammingExercise);


⚠️ Potential issue

Add error handling for competency progress update.

The asynchronous call to update competency progress should include error handling to prevent silent failures and ensure proper logging of any issues.

Apply this diff to add error handling:

- competencyProgressApi.updateProgressByLearningObjectAsync(savedProgrammingExercise); + try { + competencyProgressApi.updateProgressByLearningObjectAsync(savedProgrammingExercise); + } catch (Exception e) { + log.error("Failed to update competency progress for new exercise {}", savedProgrammingExercise.getId(), e); + // The error is logged but we don't rethrow as this is a non-critical operation + }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

competencyProgressApi.updateProgressByLearningObjectAsync(savedProgrammingExercise);

try {

competencyProgressApi.updateProgressByLearningObjectAsync(savedProgrammingExercise);

} catch (Exception e) {

log.error("Failed to update competency progress for new exercise {}", savedProgrammingExercise.getId(), e);

// The error is logged but we don't rethrow as this is a non-critical operation

}

coderabbitai · 2024-10-29T08:40:05Z

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java

@@ -623,7 +623,7 @@

        exerciseService.notifyAboutExerciseChanges(programmingExerciseBeforeUpdate, updatedProgrammingExercise, notificationText);

-        competencyProgressService.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise));
+        competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise));


⚠️ Potential issue

Add error handling for competency progress update on exercise changes.

The asynchronous call to update competency progress for exercise changes should include error handling to prevent silent failures and ensure proper logging of any issues.

Apply this diff to add error handling:

- competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise)); + try { + competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise)); + } catch (Exception e) { + log.error("Failed to update competency progress for updated exercise {}", updatedProgrammingExercise.getId(), e); + // The error is logged but we don't rethrow as this is a non-critical operation + }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise));

try {

competencyProgressApi.updateProgressForUpdatedLearningObjectAsync(programmingExerciseBeforeUpdate, Optional.of(updatedProgrammingExercise));

} catch (Exception e) {

log.error("Failed to update competency progress for updated exercise {}", updatedProgrammingExercise.getId(), e);

// The error is logged but we don't rethrow as this is a non-critical operation

}

coderabbitai · 2024-10-29T08:40:05Z

src/main/java/de/tum/cit/aet/artemis/core/web/CourseResource.java

@@ -1274,7 +1274,7 @@ public ResponseEntity<Void> addUserToCourseGroup(String userLogin, User instruct
            courseService.addUserToGroup(userToAddToGroup.get(), group);
            if (role == Role.STUDENT && course.getLearningPathsEnabled()) {
                Course courseWithCompetencies = courseRepository.findWithEagerCompetenciesAndPrerequisitesByIdElseThrow(course.getId());
-                learningPathService.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get());
+                learningPathApi.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get());


⚠️ Potential issue

Add error handling for user learning path generation

The call to learningPathApi.generateLearningPathForUser() should include error handling to ensure system stability.

Apply this change:

- learningPathApi.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get()); + try { + learningPathApi.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get()); + } catch (Exception e) { + log.error("Failed to generate learning path for user {} in course {}: {}", userToAddToGroup.get().getLogin(), courseWithCompetencies.getId(), e.getMessage()); + // Consider whether to rethrow or handle the error differently + throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error generating learning path for user", e); + }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

learningPathApi.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get());

try {

learningPathApi.generateLearningPathForUser(courseWithCompetencies, userToAddToGroup.get());

} catch (Exception e) {

log.error("Failed to generate learning path for user {} in course {}: {}", userToAddToGroup.get().getLogin(), courseWithCompetencies.getId(), e.getMessage());

// Consider whether to rethrow or handle the error differently

throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Error generating learning path for user", e);

}

coderabbitai · 2024-10-29T08:40:06Z

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java

-        course.setNumberOfCompetencies(competencyRepository.countByCourse(course));
-        // NOTE: in this call we only want to know if prerequisites exist in the course, we will load them when the user navigates into them
-        course.setNumberOfPrerequisites(prerequisiteRepository.countByCourse(course));
+        courseCompetencyApi.findAndSetCompetenciesForCourse(course);


⚠️ Potential issue

Add exception handling for courseCompetencyApi.findAndSetCompetenciesForCourse

Consider handling potential exceptions that could arise from the call to courseCompetencyApi.findAndSetCompetenciesForCourse(course); to enhance robustness in case of API failures or unexpected behavior. This will prevent the entire method from failing if the API call encounters an issue.

coderabbitai · 2024-10-29T08:40:06Z

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java

@@ -496,7 +485,7 @@

        deleteExercisesOfCourse(course);
        deleteLecturesOfCourse(course);
-        deleteCompetenciesOfCourse(course);
+        courseCompetencyApi.deleteCompetenciesOfCourse(course);


⚠️ Potential issue

Handle exceptions from courseCompetencyApi.deleteCompetenciesOfCourse

When invoking courseCompetencyApi.deleteCompetenciesOfCourse(course);, it's advisable to wrap the call in a try-catch block to handle any possible exceptions. This ensures that the deletion process of the course remains stable even if the competencies deletion encounters problems.

coderabbitai · 2024-10-29T08:40:06Z

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java

+        // ToDo: Should the check for getLearningPathsEnabled() be rather in the API
        if (course.getLearningPathsEnabled()) {
-            learningPathService.generateLearningPathForUser(course, user);
+            learningPathApi.generateLearningPathForUser(course, user);


⚠️ Potential issue

Ensure exception handling for learningPathApi.generateLearningPathForUser

While generating the learning path with learningPathApi.generateLearningPathForUser(course, user);, consider wrapping this call in a try-catch block to manage any exceptions effectively. This will maintain the stability of the enrollment process and provide a better user experience in case of API errors.

coderabbitai · 2024-10-29T08:40:06Z

src/main/java/de/tum/cit/aet/artemis/core/service/CourseService.java

+                // ToDo: Should the check for getLearningPathsEnabled() be rather in the API
+                learningPathApi.generateLearningPathForUser(course, optionalStudent.get());


💡 Codebase verification

Based on the evidence gathered, I can now provide the final response:

The check for getLearningPathsEnabled() should be added before the API call

The LearningPathApi and LearningPathService do not internally check if learning paths are enabled for a course. This check is consistently performed in other parts of the codebase (e.g., CourseCompetencyService) before making learning path-related operations. For consistency and to prevent unnecessary API calls, add the check before line 635:

if (course.getLearningPathsEnabled()) { learningPathApi.generateLearningPathForUser(course, optionalStudent.get()); }

🔗 Analysis chain

Verify necessity of learningPathsEnabled check before API call

At line 635, learningPathApi.generateLearningPathForUser(course, optionalStudent.get()); is invoked without checking course.getLearningPathsEnabled(), unlike in a similar block at lines 599-601. Verify whether the check is intentionally omitted or if it should be included here for consistency and to prevent unintended API calls when learning paths are disabled.

To confirm if any calls to learningPathApi.generateLearningPathForUser are made when learning paths are disabled, you can run the following script:

This script searches for calls to generateLearningPathForUser that are not immediately preceded by a check for course.getLearningPathsEnabled(). Ensure that all such calls are intended.

Let me generate additional scripts to verify the implementation of learningPathApi.generateLearningPathForUser to see if it has internal checks.

Let me try a different approach to find the implementation using ripgrep.

Let me check if the LearningPathApi has any internal checks for learningPathsEnabled.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash # Description: Find invocations of generateLearningPathForUser without a preceding check for getLearningPathsEnabled() # Search for lines where generateLearningPathForUser is called rg 'learningPathApi\.generateLearningPathForUser$\s*course\s*,.*$' -A 5 -B 5 | rg -v 'if\s*$\s*course\.getLearningPathsEnabled\(\s*$\s*\)'

Length of output: 2724

Script:

#!/bin/bash # Search for the implementation of generateLearningPathForUser ast-grep --pattern 'generateLearningPathForUser($$$) { $$$ }'

Length of output: 65

Script:

#!/bin/bash # Search for the interface or implementation of LearningPathApi rg -l "class.*LearningPathApi|interface.*LearningPathApi" # Search for the method signature and implementation rg "generateLearningPathForUser.*$.*Course.*,.*User.*$" -A 10

Length of output: 2832

Script:

#!/bin/bash # Check if LearningPathApi or LearningPathService has any checks for learningPathsEnabled rg -B 5 -A 5 "getLearningPathsEnabled|learningPathsEnabled" src/main/java/de/tum/cit/aet/artemis/atlas/

Length of output: 8976

github-actions · 2024-11-05T12:05:51Z

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

ole-ve · 2024-11-12T10:17:45Z

Closed in favor of #9752.

ole-ve added 6 commits October 14, 2024 23:47

Add common AbstractApi

d3bb5e5

Add PROFILE_ATLAS constant

152af19

Move atlas db config to atlas module

dc1dbab

Add atlas API classes and use it in every other module

76c4b38

Add exemplary arch tests for api access, inheritance, and annotation

1db7e34

Typo

ce7853d

ole-ve self-assigned this Oct 14, 2024

ole-ve requested a review from a team as a code owner October 14, 2024 22:14

ole-ve requested review from krusche and MaximilianAnzinger October 14, 2024 22:14

ole-ve added 3 commits October 15, 2024 00:22

Thanks to the ArchTests

8350776

Add missing classes

786324d

Revert "Thanks to the ArchTests"

1a10cdc

This reverts commit 8350776.

ls1intum deleted a comment from coderabbitai bot Oct 14, 2024

coderabbitai bot requested changes Oct 14, 2024

View reviewed changes

coderabbitai bot requested changes Oct 15, 2024

View reviewed changes

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java Outdated Show resolved Hide resolved

src/main/java/de/tum/cit/aet/artemis/core/api/AbstractApi.java Show resolved Hide resolved

coderabbitai bot previously approved these changes Oct 15, 2024

View reviewed changes

Use competencyProgressApi in spy-verification (in non-atlas tests)

755f3be

ole-ve dismissed coderabbitai[bot]’s stale review via 755f3be October 15, 2024 14:30

coderabbitai bot approved these changes Oct 15, 2024

View reviewed changes

coderabbitai bot requested changes Oct 15, 2024

View reviewed changes

ole-ve added the ready for review label Oct 16, 2024

cremertim previously approved these changes Oct 19, 2024

View reviewed changes

Merge branch 'develop' into chore/atlas-java-api

23f7a0d

JohannesStoehr reviewed Oct 22, 2024

View reviewed changes

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java Show resolved Hide resolved

src/main/java/de/tum/cit/aet/artemis/atlas/api/CompetencyProgressApi.java Show resolved Hide resolved

Make updateProgressForCourses ignore module not present

763afc1

ole-ve dismissed cremertim’s stale review via 763afc1 October 22, 2024 19:01

coderabbitai bot requested changes Oct 22, 2024

View reviewed changes

...java/de/tum/cit/aet/artemis/core/config/migration/entries/MigrationEntry20240614_140000.java Show resolved Hide resolved

Merge branch 'develop' into chore/atlas-java-api

2f3c744

JohannesStoehr previously approved these changes Oct 23, 2024

View reviewed changes

cremertim previously approved these changes Oct 23, 2024

View reviewed changes

ole-ve added ready to merge and removed ready for review labels Oct 25, 2024

krusche reviewed Oct 27, 2024

View reviewed changes

Merge branch 'develop' into chore/atlas-java-api

90711b6

ole-ve dismissed stale reviews from cremertim and JohannesStoehr via 90711b6 October 29, 2024 08:25

coderabbitai bot requested changes Oct 29, 2024

View reviewed changes

github-actions bot added stale and removed ready to merge labels Nov 5, 2024

ole-ve closed this Nov 12, 2024

ole-ve deleted the chore/atlas-java-api branch November 12, 2024 10:17

	verify(competencyProgressApi).updateProgressByLearningObjectAsync(eq(persistedTextUnit));
	void createTextUnit_asEditor_shouldCreateTextUnitAndUpdateCompetencyProgress() throws Exception {

	verify(competencyProgressApi, timeout(1000).times(1)).updateProgressForUpdatedLearningObjectAsync(eq(textUnit), eq(Optional.of(textUnit)));
	void updateTextUnit_asEditor_shouldUpdateTextUnitAndUpdateCompetencyProgress() throws Exception {

		// ToDo: Should the check for getLearningPathsEnabled() be rather in the API
		learningPathApi.generateLearningPathForUser(course, optionalStudent.get());

Development: Introduce module-API for Atlas #9486

Development: Introduce module-API for Atlas #9486

Conversation

ole-ve commented Oct 14, 2024 • edited by coderabbitai bot Loading

Checklist

General

Motivation and Context

Description

Out of scope

Testserver States

Summary by CodeRabbit

coderabbitai bot commented Oct 14, 2024 • edited Loading

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot commented Oct 14, 2024

Walkthrough

Changes

Possibly related PRs

Suggested labels

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 15, 2024

Choose a reason for hiding this comment

cremertim left a comment

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

JohannesStoehr left a comment

Choose a reason for hiding this comment

cremertim left a comment

Choose a reason for hiding this comment

krusche left a comment

Choose a reason for hiding this comment

ole-ve commented Oct 29, 2024 • edited Loading

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

coderabbitai bot Oct 29, 2024

`Development`: Introduce module-API for Atlas #9486

`Development`: Introduce module-API for Atlas #9486

ole-ve commented Oct 14, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 14, 2024 •

edited

Loading

ole-ve commented Oct 29, 2024 •

edited

Loading