Programming exercises: Add logging of failed authorization attempts to the VCS access log
#10369
Conversation
github-actions
bot
added
server
WalkthroughThis pull request introduces modifications across authentication and version control access handling. The changes extend the enumeration of authentication mechanisms by adding a new constant Changes
Suggested labels
Possibly related PRs
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java
Outdated
Show resolved
Hide resolved
helios-aet
bot
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
SimonEntholzer
dismissed stale reviews from cremertim, coolchock, ole-ve, and coderabbitai[bot]
via
121de51
src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java
Outdated
Show resolved
Hide resolved
SimonEntholzer
dismissed stale reviews from ole-ve and coderabbitai[bot]
via
f659e98
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/AuthenticationContext.java (3)
7-26: Add JavaDoc documentation to improve code maintainability.The new
AuthenticationContextinterface design is clean and follows good principles, but would benefit from proper documentation explaining:
- The purpose of this interface
- How it relates to logging failed authorization attempts
- When to use each implementation
Add documentation like:
/** * Provides a common interface for accessing authentication context information * from different sources (SSH sessions and HTTP requests). * Used primarily for logging failed authorization attempts to the VCS access log. */ public sealed interface AuthenticationContext { /** * Implementation for SSH-based authentication. */ record Session(ServerSession session) implements AuthenticationContext { // ... } /** * Implementation for HTTP-based authentication. */ record Request(HttpServletRequest request) implements AuthenticationContext { // ... } /** * Returns the IP address of the client. * @return the IP address as a string */ String getIpAddress(); }
12-14: Consider adding null checks for defensive programming.The implementation assumes that
sessionwill never be null. If there's a possibility of null values, consider adding defensive checks.@Override public String getIpAddress() { - return session.getClientAddress().toString(); + return session != null && session.getClientAddress() != null + ? session.getClientAddress().toString() + : "unknown"; }
20-22: Consider adding null checks for defensive programming.Similar to the Session implementation, consider adding checks for null request.
@Override public String getIpAddress() { - return request.getRemoteAddr(); + return request != null ? request.getRemoteAddr() : "unknown"; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/AuthenticationContext.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/SshGitLocationResolverService.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/localvc/AuthenticationContext.java
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java
Outdated
Show resolved
Hide resolved
SimonEntholzer
dismissed stale reviews from coderabbitai[bot] and magaupp
via
53ef0ff
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java (3)
315-347: Authentication mechanism resolution could be more robust.The method determines the authentication mechanism based on context, but the exception handling in the Request case silently catches all exceptions and defaults to AUTH_HEADER_MISSING.
Consider catching only specific exceptions you expect (like
LocalVCAuthException) and logging unexpected exceptions:try { return resolveHTTPSAuthenticationMechanism(request.request().getHeader(LocalVCServletService.AUTHORIZATION_HEADER), user); } -catch (LocalVCAuthException ignored) { +catch (LocalVCAuthException e) { return AuthenticationMechanism.AUTH_HEADER_MISSING; +} +catch (Exception e) { + log.warn("Unexpected error when resolving authentication mechanism: {}", e.getMessage()); + return AuthenticationMechanism.AUTH_HEADER_MISSING; }
349-363: Consider using Optional for commit hash retrieval.The method returns an empty string when it fails to retrieve the commit hash, which might not clearly indicate an error condition versus a repository with no commits.
Consider refactoring to use Optional to make the error case more explicit:
-private String getCommitHash(LocalVCRepositoryUri localVCRepositoryUri) { +private Optional<String> getCommitHash(LocalVCRepositoryUri localVCRepositoryUri) { try { - return getLatestCommitHash(repositories.get(localVCRepositoryUri.getRelativeRepositoryPath().toString())); + return Optional.ofNullable(getLatestCommitHash(repositories.get(localVCRepositoryUri.getRelativeRepositoryPath().toString()))); } catch (GitAPIException e) { log.warn("Failed to obtain commit hash for repository {}. Error: {}", localVCRepositoryUri.getRelativeRepositoryPath().toString(), e.getMessage()); } - return ""; + return Optional.empty(); }Then update the calling code to use the Optional:
var commitHash = getCommitHash(localVCRepositoryUri).orElse("");
333-347: Consider more comprehensive error handling in switch cases.The pattern matching in the switch statement is a good use of Java's modern features, but there's no default case to handle unexpected contexts.
Consider adding a default case to handle unexpected context types:
switch (context) { case AuthenticationContext.Session ignored -> { return AuthenticationMechanism.SSH; } case AuthenticationContext.Request request -> { try { return resolveHTTPSAuthenticationMechanism(request.request().getHeader(LocalVCServletService.AUTHORIZATION_HEADER), user); } catch (LocalVCAuthException ignored) { return AuthenticationMechanism.AUTH_HEADER_MISSING; } } + default -> { + log.warn("Unexpected authentication context type: {}", context.getClass().getName()); + return AuthenticationMechanism.OTHER; + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/AuthenticationMechanism.java
🧰 Additional context used
📓 Path-based instructions (1)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/localvc/LocalVCServletService.java
🧠 Learnings (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java (1)
Learnt from: janthoXO
PR: ls1intum/Artemis#9406
File: src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java:209-209
Timestamp: 2025-02-11T12:05:49.151Z
Learning: In ProgrammingExerciseParticipationResource, exam-related authorization checks and sensitive information filtering for results and feedbacks are handled by resultService.filterSensitiveInformationIfNecessary().
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: client-style
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: Analyse
🔇 Additional comments (4)
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java (4)
259-267: Good addition of error handling for tracking failed authorization.The try-catch block properly logs failed authorization attempts and delegates to a dedicated method for recording the failures in the VCS access log. This is a valuable addition that addresses the PR objective.
289-292: Improved code organization with commit hash extraction.The code now uses a centralized method for retrieving the commit hash, which follows the principle of code reuse and reduces duplication.
294-313: Well-structured method for logging failed access attempts.This method encapsulates the logic for recording failed authorization attempts, properly separating concerns and supporting the PR's main objective.
307-307:✅ Verification successful
Forced type casting might be unsafe.
The code casts
exercisetoProgrammingExercisewithout checking if it's actually an instance of that type.
🏁 Script executed:
#!/bin/bash # Check if there are different exercise types that might be passed to this method # Search for implementations of the Exercise interface ast-grep --pattern 'class $CLASS implements Exercise' ast-grep --pattern 'class $CLASS extends $_ implements Exercise'Length of output: 370
Below is an updated verification script that excludes test resources (which were causing UTF‑8 errors) by searching only in the main source directory. This should help us reliably verify whether any classes other than
ProgrammingExerciseimplement or extend theExerciseinterface:Please run the above script and share the output so that we can confirm whether any non‑
ProgrammingExercisetypes exist. This verification will determine if the forced cast invar participation = tryToLoadParticipation(false, repositoryTypeOrUserName, localVCRepositoryUri, (ProgrammingExercise) exercise);is indeed safe or if a runtime type check is needed.
🏁 Script executed:
#!/bin/bash # Re-run ast-grep search in the main source directory to avoid UTF-8 errors from test resources. echo "Searching for classes implementing the Exercise interface in src/main/java..." find src/main/java -type f -name "*.java" -print0 | xargs -0 ast-grep --pattern 'class $CLASS implements Exercise' echo "Searching for classes extending another class that implements Exercise in src/main/java..." find src/main/java -type f -name "*.java" -print0 | xargs -0 ast-grep --pattern 'class $CLASS extends $_ implements Exercise'Length of output: 600
Forced cast verification: No issues found
After verifying the main source directory, no alternate implementations of the
Exerciseinterface were detected. It appears that onlyProgrammingExerciseis used in this context. The forced type cast is therefore safe under the current codebase assumptions. Please note that if additional implementations ofExerciseare added in the future, runtime type checks may need to be reconsidered.
- File:
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java(line 307)- Action: No changes needed at this time.
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCServletService.java
Show resolved
Hide resolved
|
Just for my understanding: Do you need to distinguish a wrong password from no password? |
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java (3)
106-107: Fix duplicate parameter documentationThere's a duplicate
@param newWorkingTimein the method documentation. One of these should probably document theoldWorkingTimeparameter instead./** * Send a working time update to the specified student. * * @param studentExam The student exam the time was updated for * @param newWorkingTime The new working time in seconds - * @param newWorkingTime The new working time in seconds + * @param oldWorkingTime The old working time in seconds * @param oldWorkingTime The old working time in seconds * @param courseWide set to true if this event is caused by a course wide update that affects all students; false otherwise */
124-126: Fix duplicate JavaDoc comment startThere are two
/**comment starters, which creates malformed JavaDoc.-/** /** + /** * Send a problem statement update to all affected students. *
140-143: Remove duplicate parameter documentationThe
@param studentExamand@param exerciselines are duplicated in the method documentation./** * @param studentExam The student exam containing the exercise with updated problem statement * @param exercise The updated exercise - * @param studentExam The student exam containing the exercise with updated problem statement - * @param exercise The updated exercise * @param message The message to send */src/main/webapp/app/shared/user-import/users-import-button.component.ts (1)
33-33:❓ Verification inconclusive
Improved event naming from
finishtoimportDoneThe renaming of the output event from
finishtoimportDoneis a good change that makes the purpose of the event more descriptive and specific. This improves code readability and maintainability.Make sure all parent components consuming this event have been updated to use the new event name to avoid breaking changes.
Also applies to: 52-52
🏁 Script executed:
#!/bin/bash # Check for any components still using the old 'finish' event with this component echo "Searching for components that might still be using the old 'finish' event with user-import-button..." rg -l '\(finish\)' --type html | grep -v 'users-import-button.component.ts'Length of output: 292
Below is an updated review comment. In addition to noting the clear improvement in event naming, please manually verify in all parent components (both in HTML templates and any related TypeScript code) that there are no remaining references to the old event name (
finish). Our initial automated search (adjusted in the new shell script below) produced no hits, but manual inspection is advised given the low-quality signal of the previous run.
File(s) to check:
src/main/webapp/app/shared/user-import/users-import-button.component.ts(Lines 33 and 52)- Any components consuming this output event
Revised verification command:
#!/bin/bash echo "Searching for Angular bindings using the old 'finish' event..." rg -l '\(finish\)=' --type html | grep -v 'users-import-button.component.ts'Please ensure no parent component is still using
(finish)="..."so that the renaming toimportDonedoes not introduce breaking changes.Action: Verify that all consumers have updated their event binding to importDone.
Ensure all references to the old
finishevent have been updated toimportDoneThe renaming from
finishtoimportDoneis a clear improvement for code clarity. However, please manually verify in all parent components (including both HTML templates and any associated TypeScript files) that they no longer reference thefinishevent. Our automated search using the shell script above did not return any usage, but due to the low-quality signal from previous runs, a manual inspection is highly recommended.
- Action Items:
- Confirm that any Angular binding using
(finish)="..."has been updated to(importDone)="...".- Verify that this change is applied consistently across all files referencing the
users-import-button.component.
🏁 Script executed:
#!/bin/bash echo "Searching for Angular event bindings using the old 'finish' event..." rg -l '\(finish\)=' --type htmlLength of output: 308
Revised Review Comment:
The change from the
finishevent toimportDoneinsrc/main/webapp/app/shared/user-import/users-import-button.component.ts(Lines 33 & 52) significantly improves the clarity and specificity of the event’s purpose. This enhancement aids in code readability and maintainability.Action Required:
- Manual Verification: Although the automated search didn’t return any references to the old
finishevent in both.htmland.tsfiles (aside from the component itself), please manually verify that all parent components consuming this output event have been updated to useimportDonerather thanfinish.
🏁 Script executed:
#!/bin/bash echo "Performing a comprehensive search for any 'finish' references in .ts and .html files (excluding users-import-button.component.ts)..." rg -l "finish" --type ts,html | grep -v "users-import-button.component.ts"Length of output: 507
Revised Review Comment:
The renaming of the output event from
finishtoimportDonein thesrc/main/webapp/app/shared/user-import/users-import-button.component.tscomponent (Lines 33 & 52) improves clarity and intent, which enhances code readability and maintainability.Action Items:
- Manual Verification: Although our updated automated search (using the commands above) did not yield any references to the old
finishevent, please verify manually that all parent components—both in TypeScript and HTML templates—have updated their event bindings to useimportDoneinstead offinish.
🏁 Script executed:
#!/bin/bash echo "Searching for 'finish' references in TypeScript files (excluding users-import-button.component.ts)..." rg -l finish -g '*.ts' | grep -v 'users-import-button.component.ts' echo "Searching for 'finish' references in HTML files (excluding users-import-button.component.ts)..." rg -l finish -g '*.html' | grep -v 'users-import-button.component.ts'Length of output: 5670
The renaming of the output event from
finishtoimportDonein thesrc/main/webapp/app/shared/user-import/users-import-button.component.tscomponent (Lines 33 & 52) is a valuable change for clarity and maintainability. However, our automated search for occurrences of the stringfinishin the codebase returned many references (in unrelated files as well), which means that relying solely on an automated search might be misleading.Action Required:
- Manual Verification: Please manually inspect the parent components—both in HTML templates and TypeScript files—to verify that they do not still reference the old
(finish)binding. Ensure that any event binding related to this component has been updated to(importDone).src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIInfoContributor.java (2)
45-56: Well-structured handling of Docker resource flags.The implementation efficiently processes Docker resource flags and contributes them to the info builder. The code uses modern switch expression syntax and properly handles different flag types.
Consider validating that the list always contains paired elements to avoid
IndexOutOfBoundsExceptionif the list has an odd number of elements.List<String> defaultDockerFlags = programmingLanguageConfiguration.getDefaultDockerFlags(); +// Ensure we have pairs of values (flag + value) +if (defaultDockerFlags.size() % 2 != 0) { + log.warn("Docker flags list has odd number of elements, which may cause issues when processing flag-value pairs"); +} for (int i = 0; i < defaultDockerFlags.size(); i += 2) { String flag = defaultDockerFlags.get(i); + // Check if we have a value for this flag + if (i + 1 >= defaultDockerFlags.size()) { + log.warn("Missing value for Docker flag: {}", flag); + break; + } String value = defaultDockerFlags.get(i + 1);
60-73: Well-implemented utility method for memory conversion.The method correctly handles different memory size suffixes for Docker resource limits and converts them to MB. The implementation is comprehensive and handles various cases.
Note that the current implementation with
replaceAll("[^0-9]", "")removes all non-numeric characters, which may be overly aggressive for more complex memory specifications.Consider using more precise pattern matching to extract just the numeric part while preserving the format for debugging or logging purposes.
src/main/webapp/i18n/en/programmingExercise.json (1)
746-757: Clear and well-structured Docker resource configuration labelsThe new Docker resource configuration options are well-structured and include helpful descriptive text. These additions will enable users to configure container CPU count and memory limits directly from the UI.
Consider adding references to acceptable value ranges in the help text (e.g., valid CPU counts or memory limits) to guide users further.
src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.html (1)
48-50: Conditional rendering of send buttonThe conditional rendering of the send button (only when
editType()is notEditType.UPDATE) adds functionality while maintaining clean separation of concerns based on the editor's mode.There's a commented-out line (78) that should be removed for cleanliness.
- <!-- <ng-container *ngTemplateOutlet="sendButton; context: {text: 'hallo'}" /> -->Also applies to: 77-80
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.jsonsrc/main/resources/config/application.ymlis excluded by!**/*.ymlpackage-lock.jsonis excluded by!**/package-lock.jsonsrc/main/resources/config/application-buildagent.ymlis excluded by!**/*.ymlsrc/main/resources/config/application.ymlis excluded by!**/*.ymlsrc/main/resources/templates/aeolus/javascript/default_static.yamlis excluded by!**/*.yamlsrc/main/resources/templates/javascript/staticCodeAnalysis/test/package-lock.jsonis excluded by!**/package-lock.jsonsrc/main/resources/templates/javascript/test/package-lock.jsonis excluded by!**/package-lock.jsonsupporting_scripts/course-scripts/quick-course-setup/config.iniis excluded by!**/*.ini
📒 Files selected for processing (109)
build.gradle(6 hunks)gradle.properties(2 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)jest.config.js(1 hunks)package.json(6 hunks)src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java(2 hunks)src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.html(5 hunks)src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts(6 hunks)src/main/webapp/app/shared/metis/posting-markdown-editor/posting-markdown-editor.component.html(1 hunks)gradle.properties(1 hunks)package.json(1 hunks)build.gradle(1 hunks)package.json(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java(5 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java(2 hunks)build.gradle(1 hunks)docs/user/exercises/programming-exercise-features.inc(1 hunks)package.json(5 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/StandardizedCompetencyResource.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/StaticCodeAnalysisConfigurer.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java(1 hunks)src/main/webapp/app/exam/participate/exam-participation-live-events.service.ts(1 hunks)src/main/webapp/app/exercises/programming/shared/service/aeolus.service.ts(1 hunks)build.gradle(8 hunks)docs/requirements.txt(1 hunks)docs/user/exercises/programming-exercise-features.inc(1 hunks)gradle.properties(1 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)jest.config.js(1 hunks)package.json(8 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/repository/KnowledgeAreaRepository.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/web/StandardizedCompetencyResource.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerFlagsDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerRunConfig.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java(4 hunks)src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java(2 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/StaticCodeAnalysisConfigurer.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamAttendanceCheckEvent.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamWideAnnouncementEvent.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ProblemStatementUpdateEvent.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/domain/event/WorkingTimeUpdateEvent.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamAttendanceCheckEventDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamLiveEventBaseDTO.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamWideAnnouncementEventDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ProblemStatementUpdateEventDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/WorkingTimeUpdateEventDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java(0 hunks)src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildJobRepository.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseBuildConfigService.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIInfoContributor.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java(1 hunks)src/main/resources/templates/aeolus/javascript/default_static.sh(1 hunks)src/main/resources/templates/java/test/blackbox/projectTemplate/testsuite/config/default.exp(2 hunks)src/main/resources/templates/javascript/staticCodeAnalysis/test/eslint.config.mjs(1 hunks)src/main/resources/templates/javascript/staticCodeAnalysis/test/package.json(1 hunks)src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-announcement-dialog/exam-live-announcement-create-modal.component.ts(0 hunks)src/main/webapp/app/exam/manage/students/exam-students.component.html(1 hunks)src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts(2 hunks)src/main/webapp/app/exam/participate/exam-participation-live-events.service.ts(0 hunks)src/main/webapp/app/exam/shared/events/exam-live-event.component.html(0 hunks)src/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.html(1 hunks)src/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.ts(6 hunks)src/main/webapp/app/exercises/programming/shared/service/aeolus.service.ts(1 hunks)src/main/webapp/app/index.d.ts(0 hunks)src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.html(0 hunks)src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.scss(0 hunks)src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.ts(0 hunks)src/main/webapp/app/shared/course-group/course-group.component.html(1 hunks)src/main/webapp/app/shared/layouts/profiles/profile-info.model.ts(1 hunks)src/main/webapp/app/shared/layouts/profiles/profile.service.ts(1 hunks)src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.html(2 hunks)src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts(1 hunks)src/main/webapp/app/shared/metis/posting-markdown-editor/posting-markdown-editor.component.html(1 hunks)src/main/webapp/app/shared/standardized-competencies/standardized-competency.service.ts(0 hunks)src/main/webapp/app/shared/user-import/users-import-button.component.ts(2 hunks)src/main/webapp/app/shared/util/markdown.conversion.util.ts(3 hunks)src/main/webapp/i18n/de/error.json(1 hunks)src/main/webapp/i18n/de/exam.json(0 hunks)src/main/webapp/i18n/de/programmingExercise.json(2 hunks)src/main/webapp/i18n/en/error.json(1 hunks)src/main/webapp/i18n/en/exam.json(0 hunks)src/main/webapp/i18n/en/programmingExercise.json(2 hunks)src/test/java/de/tum/cit/aet/artemis/atlas/competency/StandardizedCompetencyIntegrationTest.java(0 hunks)src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java(0 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIInfoContributorTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java(1 hunks)src/test/javascript/spec/component/exam/shared/events/exam-live-event.component.spec.ts(0 hunks)src/test/javascript/spec/component/programming-exercise/programming-exercise-build-configuration.component.spec.ts(3 hunks)src/test/javascript/spec/component/shared/circular-progress-bar.component.spec.ts(0 hunks)src/test/javascript/spec/component/standardized-competencies/standardized-competency.service.spec.ts(1 hunks)src/test/javascript/spec/service/exam-participation-live-events.service.spec.ts(0 hunks)src/test/playwright/e2e/exam/ExamParticipation.spec.ts(0 hunks)src/test/playwright/support/pageobjects/exam/ExamManagementPage.ts(0 hunks)src/test/playwright/support/pageobjects/exam/ExamParticipationActions.ts(0 hunks)src/test/playwright/support/pageobjects/exam/ModalDialogBox.ts(0 hunks)supporting_scripts/course-scripts/quick-course-setup/add_users_to_course.py(1 hunks)supporting_scripts/course-scripts/quick-course-setup/create_course.py(2 hunks)supporting_scripts/course-scripts/quick-course-setup/create_users.py(1 hunks)supporting_scripts/course-scripts/quick-course-setup/large_course_main.py(1 hunks)supporting_scripts/course-scripts/quick-course-setup/manage_programming_exercise.py(3 hunks)supporting_scripts/course-scripts/quick-course-setup/utils.py(2 hunks)
💤 Files with no reviewable changes (19)
- src/main/webapp/app/index.d.ts
- src/main/webapp/app/shared/standardized-competencies/standardized-competency.service.ts
- src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamLiveEventBaseDTO.java
- src/test/javascript/spec/service/exam-participation-live-events.service.spec.ts
- src/test/playwright/e2e/exam/ExamParticipation.spec.ts
- src/main/webapp/app/exam/shared/events/exam-live-event.component.html
- src/main/webapp/i18n/de/exam.json
- src/main/webapp/i18n/en/exam.json
- src/test/playwright/support/pageobjects/exam/ExamParticipationActions.ts
- src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.html
- src/main/java/de/tum/cit/aet/artemis/atlas/repository/KnowledgeAreaRepository.java
- src/test/playwright/support/pageobjects/exam/ModalDialogBox.ts
- src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.scss
- src/test/playwright/support/pageobjects/exam/ExamManagementPage.ts
- src/test/javascript/spec/component/exam/shared/events/exam-live-event.component.spec.ts
- src/main/webapp/app/shared/circular-progress-bar/circular-progress-bar.component.ts
- src/test/java/de/tum/cit/aet/artemis/atlas/competency/StandardizedCompetencyIntegrationTest.java
- src/main/webapp/app/exam/manage/exams/exam-checklist-component/exam-announcement-dialog/exam-live-announcement-create-modal.component.ts
- src/test/javascript/spec/component/shared/circular-progress-bar.component.spec.ts
✅ Files skipped from review due to trivial changes (5)
- jest.config.js
- src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java
- docs/requirements.txt
- supporting_scripts/course-scripts/quick-course-setup/large_course_main.py
- src/main/resources/templates/javascript/staticCodeAnalysis/test/package.json
🧰 Additional context used
📓 Path-based instructions (6)
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
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/localci/LocalCIProgrammingLanguageFeatureService.javasrc/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.javasrc/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ProblemStatementUpdateEventDTO.javasrc/main/java/de/tum/cit/aet/artemis/core/config/Constants.javasrc/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamAttendanceCheckEventDTO.javasrc/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamAttendanceCheckEvent.javasrc/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.javasrc/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/WorkingTimeUpdateEventDTO.javasrc/main/java/de/tum/cit/aet/artemis/exam/domain/event/WorkingTimeUpdateEvent.javasrc/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.javasrc/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamWideAnnouncementEvent.javasrc/main/java/de/tum/cit/aet/artemis/atlas/web/StandardizedCompetencyResource.javasrc/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerFlagsDTO.javasrc/main/java/de/tum/cit/aet/artemis/programming/repository/BuildJobRepository.javasrc/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerRunConfig.javasrc/main/java/de/tum/cit/aet/artemis/exam/domain/event/ProblemStatementUpdateEvent.javasrc/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIInfoContributor.javasrc/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseBuildConfigService.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.javasrc/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamWideAnnouncementEventDTO.javasrc/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java
`src/main/webapp/**/*.html`: @if and @for are new and valid ...
src/main/webapp/**/*.html: @if and @for are new and valid Angular syntax replacing *ngIf and *ngFor. They should always be used over the old style.
src/main/webapp/app/shared/course-group/course-group.component.htmlsrc/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.htmlsrc/main/webapp/app/shared/metis/posting-markdown-editor/posting-markdown-editor.component.htmlsrc/main/webapp/app/exam/manage/students/exam-students.component.htmlsrc/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.html
`src/main/webapp/**/*.ts`: angular_style:https://angular.io/...
src/main/webapp/app/exam/participate/exam-participation-live-events.service.tssrc/main/webapp/app/exercises/programming/shared/service/aeolus.service.tssrc/main/webapp/app/shared/user-import/users-import-button.component.tssrc/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.tssrc/main/webapp/app/shared/layouts/profiles/profile.service.tssrc/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.tssrc/main/webapp/app/shared/layouts/profiles/profile-info.model.tssrc/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.tssrc/main/webapp/app/shared/util/markdown.conversion.util.ts
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
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/AbstractSpringIntegrationLocalCILocalVCTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIInfoContributorTest.javasrc/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java
`src/test/javascript/spec/**/*.ts`: jest: true; mock: NgMock...
src/test/javascript/spec/**/*.ts: jest: true; mock: NgMocks; bad_practices: avoid_full_module_import; perf_improvements: mock_irrelevant_deps; service_testing: mock_http_for_logic; no_schema: avoid_NO_ERRORS_SCHEMA; expectation_specificity: true; solutions: {boolean: toBeTrue/False, reference: toBe, existence: toBeNull/NotNull, undefined: toBeUndefined, class_obj: toContainEntries/toEqual, spy_calls: {not_called: not.toHaveBeenCalled, once: toHaveBeenCalledOnce, with_value: toHaveBeenCalledWith|toHaveBeenCalledExactlyOnceWith}}
src/test/javascript/spec/component/standardized-competencies/standardized-competency.service.spec.tssrc/test/javascript/spec/component/programming-exercise/programming-exercise-build-configuration.component.spec.ts
`src/main/webapp/i18n/de/**/*.json`: German language transla...
src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
src/main/webapp/i18n/de/programmingExercise.jsonsrc/main/webapp/i18n/de/error.json
🧠 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (2)
Learnt from: magaupp
PR: ls1intum/Artemis#9573
File: src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java:154-157
Timestamp: 2024-12-06T01:01:18.563Z
Learning: In `StaticCodeAnalysisConfigurer.java`, default penalties and category states for static code analysis categories should not be set in the code, as these should be configured by instructors using the UI on a case-by-case basis.
Learnt from: iyannsch
PR: ls1intum/Artemis#8965
File: src/main/java/de/tum/in/www1/artemis/config/TheiaConfiguration.java:18-18
Timestamp: 2024-11-12T12:51:40.391Z
Learning: When suggesting the use of `EnumMap` in a Spring Boot application, consider the potential additional configuration effort and ensure it works seamlessly with Spring Boot.
src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (1)
Learnt from: valentin-boehm
PR: ls1intum/Artemis#7384
File: src/test/java/de/tum/in/www1/artemis/exam/StudentExamIntegrationTest.java:988-993
Timestamp: 2024-11-12T12:51:46.554Z
Learning: The `testSubmitStudentExam_differentUser` method does not require additional checks to verify the state of `studentExam1` after receiving a `HttpStatus.FORBIDDEN` because the control flow in the `StudentExamResource` is straightforward and ensures no state change occurs.
🔇 Additional comments (129)
jest.config.js (1)
94-97: Good job on increasing test coverage thresholds.The slight increases in coverage thresholds for statements (88.69 → 88.72), branches (74.35 → 74.42), and lines (88.75 → 88.78) reflect that the new code for logging failed authorization attempts is properly covered by tests. This is a positive practice that ensures the new functionality is reliable and maintainable.
src/main/java/de/tum/cit/aet/artemis/programming/repository/BuildJobRepository.java (1)
47-51: Improved SQL query construction for better database compatibility and clarity.The change from a shorthand wildcard syntax to explicit CONCAT functions improves cross-database compatibility and makes the query structure more explicit. This approach helps prevent potential SQL dialect issues across different database systems and makes the wildcard application more readable.
While this change appears unrelated to the PR's main objective of logging failed authorization attempts, it's a good practice improvement for SQL queries in JPA repositories.
src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java (2)
64-64: Good refactoring: Removed ConversationRepository dependencyThe constructor has been updated to remove the direct dependency on ConversationRepository, which aligns with good dependency injection practices by delegating repository access through services instead of direct repository usage.
92-95: Well-structured conversation retrieval implementationThis refactoring improves code organization by:
- Extracting the conversation ID into a variable improves readability
- Adding a helpful comment explaining the need for participants in group chats
- Using service methods instead of direct repository access
- Implementing proper fallback logic for conversation retrieval
The code now correctly handles cases where users might not be conversation members but need access to group chats.
src/main/resources/templates/java/test/blackbox/projectTemplate/testsuite/config/default.exp (1)
118-118: Fixed escape sequence notation in regex patternsThe changes from
\[\r\n\]to\[\\r\\n\]in the regular expression patterns are important fixes. In TCL's expect script syntax, when using the-reflag for regular expressions, you need to properly escape backslashes. The original code was incorrectly matching line breaks because the escape characters weren't properly escaped themselves.This change ensures that the regex patterns correctly match actual carriage return (
\r) and newline (\n) characters in the program output rather than treating them as literal 'r' and 'n' characters preceded by a single backslash.Also applies to: 121-121, 124-124, 172-172, 175-175, 178-178
src/test/java/de/tum/cit/aet/artemis/shared/base/AbstractSpringIntegrationLocalCILocalVCTest.java (1)
226-226: Appropriate visibility modifier change for testing needs.Changing the
dockerClientMockaccess modifier from private to protected allows subclasses to access this mock directly. This supports the PR's goal of adding logging for failed authorization attempts, as test classes extending this base class will need to configure the mock for simulating authentication failures.src/test/javascript/spec/component/standardized-competencies/standardized-competency.service.spec.ts (1)
4-4:❓ Verification inconclusive
Import statement change appears unrelated to PR objectives
The change in the import statement (adding KnowledgeAreaDTO) seems unrelated to the PR objective of "Add logging of failed authorization attempts to the VCS access log". While the code itself looks correct, this modification doesn't appear to contribute to the stated goal of enhancing VCS access logging for failed authorization attempts.
The tests are well-structured and follow the specified coding guidelines, using appropriate assertion methods and properly mocking HTTP requests.
🏁 Script executed:
#!/bin/bash # Let's verify if this file is intended to be part of this PR by checking the PR description and files # Check if the PR description mentions anything about competency models or standardized competencies gh pr view 10369 --json body | jq -r '.body' | grep -i 'competenc\|knowledge' # List files changed in the PR to see if there are other related changes gh pr view 10369 --json files | jq -r '.files[].path' | grep -E 'competenc|knowledge|standardized'Length of output: 178
Standardized Competency Import Change Unrelated to Logging Objectives
The inclusion of
KnowledgeAreaDTOin the import statement of
src/test/javascript/spec/component/standardized-competencies/standardized-competency.service.spec.ts
does not appear to support the PR's stated objective of adding logging for failed authorization attempts to the VCS access log. Although the test file remains structured and correctly implemented, this import change seems out of scope.Please verify whether this modification was intentional or inadvertently included in the PR. If it is not required, consider removing it to maintain focus on the PR objective.
supporting_scripts/course-scripts/quick-course-setup/create_users.py (1)
24-24: API endpoint path updated for consistencyThe URL has been updated from
/admin/usersto/core/admin/users, which aligns with a broader API restructuring pattern seen in this PR.supporting_scripts/course-scripts/quick-course-setup/add_users_to_course.py (1)
27-27: API endpoint path updated for consistencyThe URL has been updated from
/courses/{course_id}/{user_group}/{user_name}to/core/courses/{course_id}/{user_group}/{user_name}, which aligns with the API restructuring pattern across this PR.supporting_scripts/course-scripts/quick-course-setup/utils.py (2)
19-19: API endpoint path updated for consistencyThe URL has been updated from
/courses/{course_id}/{user_group}/{user_name}to/core/courses/{course_id}/{user_group}/{user_name}, maintaining consistency with other endpoint changes in this PR.
28-28: API endpoint path updated for consistencyThe authentication endpoint has been updated from
/public/authenticateto/core/public/authenticate, following the same pattern of adding the/coreprefix.supporting_scripts/course-scripts/quick-course-setup/manage_programming_exercise.py (4)
12-12: API endpoint path updated with domain-specific prefixThe URL has been updated from
/programming-exercises/setupto/programming/programming-exercises/setup, following a domain-specific restructuring pattern.
49-49: API endpoint path updated with domain-specific prefixThe URL has been updated from
/exercises/{exercise_id}/participationsto/exercise/exercises/{exercise_id}/participations, following a domain-specific restructuring pattern.
64-64: API endpoint path updated with domain-specific prefixThe URL has been updated from
/repository/{participation_id}/committo/programming/repository/{participation_id}/commit, following a domain-specific restructuring pattern.
42-42:✅ Verification successful
Verify if commenting out cleanup is intentional
The
run_cleanup()function call has been commented out in the error handling for when a programming exercise already exists. Ensure this is intentional and won't lead to resource leaks or incomplete cleanup.
🏁 Script executed:
#!/bin/bash # Search for other occurrences of run_cleanup() to understand its usage pattern grep -r "run_cleanup" --include="*.py" .Length of output: 735
Action: Confirm that disabling the cleanup call is deliberate
After verifying, we note that the call to
run_cleanup()is consistently commented out in bothmanage_programming_exercise.pyandlarge_course_main.py(and only imported increate_course.pywithout an active invocation). This pattern indicates that, at present, the cleanup functionality is intentionally bypassed. If this is by design (for example, if cleanup is handled elsewhere or deferred), please update the inline comment or documentation to clarify the rationale. Otherwise, reassess whether omitting this call might cause resource leaks or incomplete cleanup.supporting_scripts/course-scripts/quick-course-setup/create_course.py (2)
35-35:✅ Verification successful
API endpoint updated to include '/core' segment
The course creation endpoint has been updated to use the new API path structure. This change aligns with the broader API restructuring throughout the application.
🏁 Script executed:
#!/bin/bash # Check if other API endpoints in related files have been consistently updated echo "Checking other API endpoints for consistent patterns:" rg -n "SERVER_URL.*admin/courses" --type py rg -n "SERVER_URL.*core/admin/courses" --type pyLength of output: 449
Verified API Endpoint Update to '/core/admin/courses'
The update in
supporting_scripts/course-scripts/quick-course-setup/create_course.py(line 35) correctly reflects the new API path structure. The grep consistency check confirms that the endpoint now uniformly includes the/coresegment as expected. No further changes are needed.
21-21:❓ Verification inconclusive
Configuration source moved to more appropriate section
The special character regex configuration has been moved from 'Settings' to the more logically appropriate 'CourseSettings' section, improving configuration organization.
🏁 Script executed:
#!/bin/bash # Verify config.ini has the special_character_regex in the CourseSettings section grep -n "special_character_regex" config.iniLength of output: 88
Configuration update requires manual verification
It appears that the expected configuration file (config.ini) wasn’t found in the repository, so we cannot automatically confirm that the 'CourseSettings' section contains thespecial_character_regexconfiguration. Please manually verify that the configuration change (moving thespecial_character_regexsetting from 'Settings' to 'CourseSettings') has been applied in the correct configuration file or check if an alternative configuration file is being used.
- Confirm that the configuration file has been added or renamed appropriately.
- Ensure that the 'CourseSettings' section includes the
special_character_regexentry as referenced increate_course.py:21.src/main/webapp/app/exam/participate/exam-participation-live-events.service.ts (1)
184-195:❓ Verification inconclusive
API endpoint path has been updated
The URL path for fetching previous exam events has been changed to include the
/examsegment. This change aligns with what appears to be a reorganization of the API endpoints structure.Please confirm that the corresponding backend endpoint has been properly updated to match this new path structure to avoid any potential 404 errors when fetching exam live events.
🏁 Script executed:
#!/bin/bash # Check if the new endpoint is defined in the backend controllers rg -g "*.java" "courses/\$\{?.*\}?/exams/\$\{?.*\}?/student-exams/live-events" --context=3Length of output: 90
Action Required: Confirm Updated API Endpoint in Backend
The updated API endpoint now includes the
/examsegment. However, our initial search did not yield conclusive evidence that the backend controller has been updated accordingly. Please manually verify (or re-run a refined search) to ensure that the backend endpoint matching the path/api/exam/courses/{courseId}/exams/{examId}/student-exams/live-eventsis correctly defined.
- Verify that the backend controller method (likely annotated with
@GetMappingor@RequestMapping) uses the updated URL structure.- If convenient, consider running a refined search command such as:
#!/bin/bash # Search for endpoint definitions in backend controllers that include 'exam/courses' and 'live-events' rg -g "*.java" '@(GetMapping|RequestMapping)\s*\(.*exam/courses/.*exams/.*student-exams/live-events' --context=3src/test/java/de/tum/cit/aet/artemis/exam/StudentExamIntegrationTest.java (2)
869-869: The assertion now uses login instead of name for createdBy.The test has been updated to use the user's login identifier rather than the user's name in the assertion for
result.createdBy(). This change aligns with modifications in the DTO structure where user identification has changed from name to login.
900-900: The assertion now uses login instead of name for createdBy.Similar to the previous change, this assertion for the attendance check event has been updated to verify against the user's login identifier rather than the user's name.
src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamWideAnnouncementEvent.java (1)
32-32: Parameter createdBy has been removed from DTO construction.The
createdByparameter has been removed from theExamWideAnnouncementEventDTOconstructor call. This matches the broader architectural change where the DTOs no longer track the creator's information in the same way, shifting from storing the user's name to using their login identifier directly.src/main/java/de/tum/cit/aet/artemis/exam/domain/event/WorkingTimeUpdateEvent.java (1)
61-61: Method updated correctly to reflect DTO signature changeThe method has been properly updated to match the new signature of
WorkingTimeUpdateEventDTOwhich no longer includes thecreatedByparameter. This change is consistent with the broader refactoring to remove creator information from DTOs.src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/WorkingTimeUpdateEventDTO.java (1)
11-11: DTO record properly defined without creator informationThe record has been correctly updated to remove the
createdByfield while maintaining all other necessary data. This follows the Java coding guidelines which recommend using records for DTOs (dtos:{java_records}) and keeping DTOs with minimal data (dtos:{min_data}).src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamAttendanceCheckEventDTO.java (2)
13-13: DTO record updated to remove creator informationThe record has been properly updated to remove the
createdByfield while maintaining the essential data structure. This follows the Java coding guidelines for using records for DTOs and maintaining minimal data in DTOs.
16-16: Static factory method updated to match new record structureThe
ofmethod has been correctly modified to reflect the removal of thecreatedByparameter. This ensures consistency with the record's new structure and maintains the code's readability and maintainability.src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ExamAttendanceCheckEvent.java (1)
32-32: Method updated correctly to reflect DTO signature changeThe
asDTO()method has been properly updated to match the new signature ofExamAttendanceCheckEventDTOwhich no longer includes thecreatedByparameter. This change is consistent with the broader refactoring to remove creator information from DTOs.src/main/java/de/tum/cit/aet/artemis/exam/domain/event/ProblemStatementUpdateEvent.java (1)
65-65: Consistent DTO construction with other event classes.The parameter
this.getCreatedBy()has been removed from the constructor call, aligning with similar changes in other event classes to simplify DTOs by excluding creator information.src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ExamWideAnnouncementEventDTO.java (1)
11-11: Clean DTO simplification.The
createdByfield has been removed from the record declaration, maintaining consistency with other DTOs in the system that have undergone the same change.src/main/java/de/tum/cit/aet/artemis/exam/dto/examevent/ProblemStatementUpdateEventDTO.java (1)
11-11: Simplified DTO structure.The
createdByfield has been removed from the record parameters, aligning with the system-wide refactoring to remove creator information from event-related DTOs.src/main/java/de/tum/cit/aet/artemis/exercise/service/ExerciseService.java (1)
799-799: Method call updated to match refactored signature.The
instructorparameter has been removed from thecreateAndSendProblemStatementUpdateEventmethod call, consistent with the DTO changes that no longer track creator information.src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.java (6)
70-79: LGTM: Simplified method signature by removing User parameterThe removal of the
User sentByparameter simplifies the API and is consistent with the other changes in this class. This approach removes the tracking of who created the event, which aligns with the PR objectives.
89-99: LGTM: Simplified method signature by removing User parameterThe removal of the
User sentByparameter continues the consistent approach of simplifying the API and removing creator tracking.
110-122: LGTM: Simplified method signature by removing User parameterRemoving the
User sentByparameter is consistent with the other changes and supports the overall refactoring effort.
132-136: LGTM: Simplified method signature and updated method callThe removal of the
User instructorparameter and the corresponding update to the method call inside the lambda function is consistent with the overall changes.
145-157: LGTM: Simplified method signature by removing User parameterThe removal of the
User sentByparameter from this method completes the consistent refactoring across all methods in this class.
131-132:❌ Incorrect review comment
Verify @async annotation import
The
@Asyncannotation is used here, but I don't see its import in the imports section. Make sure you're importingorg.springframework.scheduling.annotation.Async.
🏁 Script executed:
#!/bin/bash # Check if the correct @Async import is present grep -r "import org.springframework.scheduling.annotation.Async;" src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.javaLength of output: 144
I'll execute an additional verification step to check if the file is using the
@Asyncannotation at all. This will help confirm whether the missing import is an oversight in a file that actually uses@Asyncor if the comment might be based on outdated information. For that, please run the following script:
🏁 Script executed:
#!/bin/bash # Check if @Async annotation is used anywhere in the file grep -R "@Async" src/main/java/de/tum/cit/aet/artemis/exam/service/ExamLiveEventsService.javaLength of output: 93
No Async Annotation Usage Found in ExamLiveEventsService.java
Upon re-verifying the file, there is no usage of the
@Asyncannotation in the code. As a result, the import statement fororg.springframework.scheduling.annotation.Asyncis not necessary.
- The file does not contain any instance of
@Async.- No import for
org.springframework.scheduling.annotation.Asyncis required.- The review comment appears to be based on an outdated or incorrect assumption.
Likely an incorrect or invalid review comment.
gradle/wrapper/gradle-wrapper.properties (1)
3-3: Good upgrade to stable Gradle versionUpgrading from a release candidate (
gradle-8.13-rc-2-bin.zip) to the stable release (gradle-8.13-bin.zip) is a good practice for production environments.src/main/webapp/app/shared/course-group/course-group.component.html (1)
9-9: Event binding name updated correctlyThe event binding has been updated from
(finish)to(importDone), which is more descriptive and consistent with the renamed output property in theUsersImportButtonComponent. This change improves code clarity by using semantically meaningful event names.src/main/resources/templates/javascript/staticCodeAnalysis/test/eslint.config.mjs (1)
1-9: New ESLint configuration looks goodThis ESLint configuration provides a solid foundation for JavaScript static code analysis:
- Correctly imports the necessary packages
- Sets up Node.js globals
- Uses the recommended ESLint configurations
- Follows modern ESM format for ESLint configuration
The configuration is minimal but effective for establishing code quality standards.
src/main/webapp/app/exercises/programming/shared/service/aeolus.service.ts (1)
13-13:✅ Verification successful
Correct API endpoint URL
The change fixes a typo in the resource URL, changing from
'api/programming/paeolus'to'api/programming/aeolus'. This ensures that HTTP requests are sent to the correct endpoint.
🏁 Script executed:
#!/bin/bash # Verify that the API endpoint exists in backend controllers rg -t java "api/programming/aeolus" --glob "src/main/java/**/*.java" # Check if there are any remaining references to the old incorrect endpoint rg -t java "api/programming/paeolus" --glob "src/main/java/**/*.java" # Check if there are any API calls using the old endpoint in TypeScript files rg -t ts "api/programming/paeolus" --glob "src/main/webapp/**/*.ts" | grep -v "aeolus.service.ts"Length of output: 366
Verified: API endpoint correction is accurate
- Confirmed that the correct endpoint (
api/programming/aeolus) is used in the backend (e.g., inAeolusTemplateResource.java).- No references to the old endpoint (
api/programming/paeolus) were found in both Java and TypeScript files.- The typo fix ensures HTTP requests target the correct API endpoint.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
72-72: Feature enhancement for JavaScript language support.The update adds static code analysis support for JavaScript by changing the second boolean parameter in the
ProgrammingLanguageFeatureconstructor fromfalsetotrue. This aligns with other changes in the codebase that enhance JavaScript support.src/main/webapp/app/exam/manage/students/exam-students.component.html (2)
19-19: Improved event naming clarity.The event name change from
(finish)to(importDone)provides a more descriptive and specific event name that clearly indicates the completion of a user import operation.
22-22: Improved event naming clarity.The event name change from
(finish)to(uploadDone)provides a more descriptive and specific event name that clearly indicates the completion of a student image upload operation.src/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerRunConfig.java (1)
6-6:✅ Verification successful
Enhanced Docker container resource management.
The record has been expanded to include three additional configuration parameters for Docker containers:
cpuCount: Controls the number of CPUs allocatedmemory: Specifies the memory limitmemorySwap: Defines the swap memory limitThis enhancement provides more fine-grained control over Docker container resources.
Since this is a record type, ensure all usages of this class are updated to provide values for the new parameters.
🏁 Script executed:
#!/bin/bash # Find all places where DockerRunConfig is instantiated to ensure they've been updated rg -A 2 "new DockerRunConfig" --type javaLength of output: 455
Enhanced Docker Container Resource Management Verified
The updated record now includes
cpuCount,memory, andmemorySwapto provide more precise control over Docker container resources. The instantiation insrc/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseBuildConfigService.javaconfirms that the new parameters are correctly provided.
- File:
src/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerRunConfig.java— Record definition updated with additional configuration fields.- File:
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseBuildConfigService.java— Verified instantiation now includes all new parameters.All usage points have been updated accordingly.
src/main/webapp/app/shared/layouts/profiles/profile-info.model.ts (1)
38-40: Added Docker container resource configuration to profile info.The addition of these optional properties aligns with the backend changes for Docker container resource management:
defaultContainerCpuCount: Default number of CPUs to allocatedefaultContainerMemoryLimitInMB: Default memory limit in megabytesdefaultContainerMemorySwapLimitInMB: Default swap memory limit in megabytesThese fields are correctly marked as optional with the
?suffix for backward compatibility.src/main/java/de/tum/cit/aet/artemis/buildagent/dto/DockerFlagsDTO.java (1)
5-5: LGTM: Addition of Docker resource configuration parameters.The addition of integer fields for CPU count, memory, and memory swap to the DockerFlagsDTO record is well-structured and provides better control over Docker container resource allocation.
src/main/resources/templates/aeolus/javascript/default_static.sh (2)
1-33: Well-structured shell script for JavaScript project analysis.The script follows good practices with clear function separation, proper error handling, and consistent directory management between operations.
11-11:✅ Verification successful
Verify intentional error suppression in static code analysis.
The
|| [ $? -eq 1 ]condition allows the linting process to continue even with an exit code of 1 (warnings). Is this intentional to distinguish between warnings (exit code 1) and errors (exit codes > 1)?
🏁 Script executed:
#!/bin/bash # Check for other JavaScript linting configurations in the codebase to verify consistency rg -A 3 "lint:ci" --type js --type jsonLength of output: 1076
Clarify Intentional Lint Exit Code Handling
It appears that the use of the
|| [ $? -eq 1 ]condition in
src/main/resources/templates/aeolus/javascript/default_static.sh
is deliberately designed to let the linting process continue when ESLint exits with code 1 (presumably indicating warnings) while still failing for exit codes greater than 1 (indicating errors). This behavior is consistent with similar configurations found in our JavaScript and TypeScript test setups (see their respectivepackage.jsonfiles). Please confirm that distinguishing warning-level lint issues by allowing an exit code of 1 aligns with your intended static analysis strategy.src/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.html (1)
95-117: LGTM: Docker resource configuration UI addition.The new input fields for CPU count, memory, and memory swap are well-structured and consistently implemented with the existing UI patterns, including proper labels and help icons.
gradle.properties (1)
21-21: Dependency version updates look good.The updates to multiple library versions appear to be coordinated minor updates focused on security fixes and improvements, with testing libraries (JUnit, Mockito) being updated together for compatibility.
Also applies to: 24-26, 30-32, 34-35, 37-46, 51-51
src/main/webapp/app/shared/metis/posting-markdown-editor/posting-markdown-editor.component.html (1)
24-26:✅ Verification successful
Enhancement to the markdown editor with additional bindings
The addition of these three new property bindings (
isButtonLoading,isFormGroupValid, andeditType) improves the component's reactivity by allowing the markdown editor to respond to form validation state and loading states. This is good for improving user experience.
🌐 Web query:
What is the latest Angular syntax for property binding?💡 Result:
Property binding in Angular (including Angular 17) uses square bracket syntax (
[]) to dynamically set values for DOM element properties, component inputs, and directive properties. This syntax enables one-way data flow from component to template while optimizing performance and maintainability.Core Syntax
- Basic format:
[property]="expression"- DOM properties:
<button [disabled]="isFormValid">Save</button> <img [src]="imageURL">- Component inputs:
<app-user-profile [user]="currentUser"></app-user-profile>Key Features
Dynamic DOM Updates
Binds directly to native element properties likedisabled,value, andsrc[2][6]Component Communication
Passes data to child components through@Inputproperties[2][5]Directive Integration
Binds to directive properties likengSrcfor optimized images[2]Special Cases:
- Attribute binding for non-DOM properties:
<div [attr.aria-label]="accessibilityText"></div>- Style/Class binding:
<div [class.active]="isActive" [style.width.px]="columnWidth"></div>Best Practices
Prefer property binding over interpolation for dynamic values:
<!-- Recommended --> <input [value]="userName"> <!-- Avoid --> <input value="{{userName}}">Avoid side effects in binding expressions[2]
Use TypeScript type safety for expression values[5]
The syntax remains consistent in Angular 17, continuing Angular's pattern of explicit, declarative template binding. For two-way binding, Angular uses separate
[(ngModel)]syntax combining property and event binding[3][4].Citations:
- 1: https://angular.dev/tutorials/learn-angular/6-property-binding
- 2: https://angular.dev/guide/templates/binding
- 3: https://www.angularminds.com/blog/how-to-implement-two-way-data-binding-in-angular
- 4: https://www.pluralsight.com/resources/blog/guides/angular-data-binding-overview
- 5: https://www.codecademy.com/learn/intro-to-angular/modules/angular-data-binding-and-directives-next-steps/cheatsheet
- 6: https://www.youtube.com/watch?v=ATdf91NWnVg
- 7: https://v17.angular.io/guide/property-binding
- 8: https://angular.io/guide/binding-syntax
Verified Enhancement: Angular Property Bindings are Correct
The new property bindings using
[isButtonLoading],[isFormGroupValid], and[editType]in the markdown editor adhere to the latest Angular syntax, as confirmed by Angular's binding best practices. The changes improve component reactivity and support clear one-way data flow, enhancing user experience without any syntax issues.docs/user/exercises/programming-exercise-features.inc (1)
96-96: Documentation updated to reflect TypeScript static code analysis supportThe documentation has been correctly updated to indicate that TypeScript now supports static code analysis in Local CI but not in Jenkins. This accurately reflects the capabilities of the system and provides clear information to users.
src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIIntegrationTest.java (1)
631-647: Well-structured test for Docker performance flagsThe new test appropriately validates that performance-related Docker flags (CPU count, memory, and memory swap limits) are correctly applied to the container configuration. The test follows JUnit5 best practices with proper annotations and assertions.
A few observations:
- The test correctly verifies the Docker host configuration using argument matchers
- The conversion calculations for memory units are properly implemented (MB to bytes)
- The test follows the same pattern as other tests in this class
src/main/webapp/app/shared/layouts/profiles/profile.service.ts (1)
91-93: LGTM! Added new container resource configuration properties.The addition of these three properties (
defaultContainerCpuCount,defaultContainerMemoryLimitInMB, anddefaultContainerMemorySwapLimitInMB) to the profile info correctly implements the container resource configuration needed for Docker containers. The naming follows Angular's camelCase convention for variables/properties.src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseService.java (4)
135-136: Well-documented constant for Docker memory limit.The addition of this constant with a clear comment explaining the Docker limitation is a good practice.
1119-1121: Proper validation for Docker memory limits.Good validation check that ensures Docker containers have at least the minimum required memory (6MB).
1123-1125: Proper validation for Docker CPU count.Good validation check that ensures Docker containers have at least one CPU core.
1127-1129: Proper validation for Docker memory swap.Good validation check that ensures the memory swap limit is non-negative.
src/main/java/de/tum/cit/aet/artemis/core/config/Constants.java (1)
290-294: LGTM! Added Docker resource configuration constants.The new constants follow Java naming conventions for constants (all uppercase with underscores) and provide consistent keys for Docker container resource configuration.
src/test/java/de/tum/cit/aet/artemis/programming/icl/LocalCIInfoContributorTest.java (2)
9-9: Added import for ProgrammingLanguageConfiguration.Correctly added the new import needed for the constructor parameter.
17-17: Updated constructor call with new required parameter.The test correctly accommodates the constructor change in LocalCIInfoContributor class, which now requires a ProgrammingLanguageConfiguration parameter.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIInfoContributor.java (2)
5-5: Good addition of specific import.The explicit import of
java.util.Listis cleaner than using wildcard imports, adhering to Java best practices.
29-33: Proper dependency injection with constructor.Good implementation of constructor-based dependency injection, following the coding guidelines for DI.
src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobExecutionService.java (3)
240-242: Good initialization of Docker resource limit variables.Proper initialization of the Docker resource limit variables to their default values.
246-248: Clean extraction of resource values from configuration.Good approach to extract Docker resource limits from the configuration when available.
252-252:✅ Verification successful
Method call updated with resource limits.
The method call correctly passes the resource limit parameters to the
configureContainermethod.The TODO comment at line 286 about reducing parameters is becoming more relevant with these additional parameters. Consider implementing that refactoring to improve maintainability.
🏁 Script executed:
#!/bin/bash # Find the configureContainer method signature to understand what changed rg -A 5 "configureContainer.*\(" --type javaLength of output: 2039
Resource Limits Correctly Passed – Consider Refactoring Parameter List
The updated method call inBuildJobExecutionService.javanow correctly passes the new resource limit parameters (cpuCount,memory,memorySwap) toconfigureContainer(), as verified by the current method signature inBuildJobContainerService.java.
- The integration of resource limits is confirmed by the method signature, so no functional changes are needed.
- However, the TODO at line 286 regarding parameter reduction is even more pertinent with these additional parameters; please consider refactoring (perhaps by grouping similar parameters) to improve maintainability.
src/main/webapp/app/exam/manage/students/upload-images/students-upload-images-button.component.ts (2)
32-32: Improved event emitter naming for better semantics.Renaming from
finishtouploadDoneimproves code clarity by making the purpose of the event more explicit. This follows Angular best practices for clear, descriptive output event names.
48-48: Updated event emission to match renamed output property.Consistent update of the event emission to use the renamed output property.
src/main/webapp/i18n/en/error.json (1)
101-103: Added clear validation error messages for Docker resource limits.The error messages are clear, informative, and provide specific guidance on valid values for memory limits, CPU count, and memory swap limits. These messages align well with the resource limit validation being implemented elsewhere in the codebase.
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (2)
3-4: New imports for Collections and EnumMap added.The imports added support the refactoring of
languageToDefaultCategoriesinitialization to use an EnumMap instead of direct Map creation.
90-103: Good refactoring to use EnumMap for better performance with enum keys.The refactoring improves the initialization of
languageToDefaultCategoriesby:
- Using an EnumMap which is more efficient for enum keys than a general HashMap
- Initializing the map in a static block for better readability
- Maintaining immutability by wrapping with Collections.unmodifiableMap()
This change allows for more efficient map operations while maintaining the same functionality.
src/main/webapp/i18n/de/error.json (1)
101-103: Good addition of clear error messages for resource limits.The error messages for memory limit, CPU count, and memory swap limit clearly explain the requirements for each setting. They adhere to the coding guidelines by using informal German (dutzen) with "du/dein" forms rather than formal "sie/ihr" forms.
src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseBuildConfigService.java (5)
59-63: Added initialization for Docker resource limit parameters.Good implementation of the new Docker resource limit parameters (network, environment, CPU count, memory, and memory swap), properly initializing all parameters with sensible defaults.
67-69: Properly extracting resource limits from DTO.Correctly assigns the CPU count, memory, and memory swap values from the dockerFlagsDTO when it's available.
74-75: Updated method calls to include resource parameters.The
createDockerRunConfigmethod calls now include the CPU count, memory, and memory swap parameters. This ensures these values are properly passed through the service chain.Also applies to: 81-81
99-100: Updated method signature and null check logic.Method signature appropriately updated to accept CPU count, memory, and memory swap parameters. The null check also properly includes these new parameters to determine if no Docker configuration is needed.
114-114: Updated Docker configuration creation.The DockerRunConfig creation now includes the CPU count, memory, and memory swap parameters, ensuring they're properly passed to the Docker configuration.
src/main/webapp/i18n/de/programmingExercise.json (2)
113-114: Updated labels for better clarity.Changed "Build Skript anpassen" to "Build Konfiguration anpassen" for both regular and Aeolus options, improving the clarity and accuracy of the labels.
747-758: Added translations for new Docker resource configuration UI elements.Well-structured translations for CPU count, memory size, and swap memory size configuration options. Each includes both a clear label and helpful description text. The translations follow the coding guideline of using informal German (dutzen) with "du/dein" forms.
src/main/webapp/i18n/en/programmingExercise.json (1)
123-124: UI terminology update looks goodThe change from "Build Script" to "Build Configuration" improves user-facing terminology consistency. This better reflects the actual functionality, which involves configuring build settings rather than just scripts.
src/test/javascript/spec/component/programming-exercise/programming-exercise-build-configuration.component.spec.ts (3)
56-106: Well-structured tests for new profile valuesThe test case has been appropriately renamed from "should set timeout options" to "should set profile values" to reflect its expanded scope. The new expectations for container CPU and memory configuration are comprehensive.
The test covers both undefined values and specific numeric values, ensuring the component behaves correctly in both scenarios.
131-138: Thorough test coverage for Docker flag changesThe test implementation for changing Docker resource settings is comprehensive. It verifies that:
- The UI events properly update component state
- The component correctly serializes these values into the Docker flags JSON string
This ensures the resource settings are properly handled throughout the component lifecycle.
157-164: Test for supported languages updated correctlyThe test now first sets the language to
EMPTY(unsupported) and then toSWIFT(supported), providing better test coverage by checking both states of theisLanguageSupportedflag.src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.html (3)
27-27: Improved file upload footer alignmentThe addition of
align-items-centerandborder-topclasses improves the visual consistency and separation of the file upload section.
45-47: Better help link positioningThe help link has been moved and given an
mx-1class for better spacing within the layout.
276-288: Well-structured send button templateThe button template is well-designed with:
- Icon and label support
- Loading state integration
- Proper disable conditions
- Flexible CSS class customization
- Test ID support for automated testing
All properties have appropriate defaults and follow Angular best practices.
src/main/webapp/app/shared/util/markdown.conversion.util.ts (3)
1-8: Improved imports structureThe import structure has been modernized to use direct imports from libraries rather than re-exports. This improves maintainability and TypeScript type checking.
53-79: Enhanced Markdown renderer configurationThe Markdown renderer initialization and plugin usage have been updated to use a more modern approach with
MarkdownItdirectly. The addition of theMarkdownitTagClassplugin with table styling improves the rendering of Markdown tables.
101-149: Well-implemented class mapping for Markdown tokensThe new
MarkdownitTagClassplugin and its supporting functions are extremely well-documented with clear explanations of:
- The purpose of each function
- How token nesting works
- The token structure
The implementation is also efficient, recursively handling nested tokens and properly preserving existing classes.
This feature will make it easier to style specific Markdown elements consistently across the application, improving UI consistency.
src/main/webapp/app/shared/markdown-editor/monaco/markdown-editor-monaco.component.ts (6)
58-58: LGTM: Import correctly updated to include PostingEditTypeThe import has been properly updated to include PostingEditType alongside the existing ReferenceType import.
75-76: LGTM: New component import addedNew PostingButtonComponent import is correctly added to be used in the component.
134-134: LGTM: PostingButtonComponent added to imports arrayThe PostingButtonComponent is correctly added to the imports array to make it available in the template.
245-248: LGTM: New input properties addedThe input properties for handling button loading state, form validation, and edit type are correctly defined.
318-318: LGTM: Font Awesome icon imported and exposedThe faPaperPlane icon is correctly imported and exposed as a protected readonly property for use in the template.
327-327: LGTM: EditType reference addedEditType property correctly references PostingEditType to make it available in the template.
build.gradle (8)
22-22: LGTM: Gradle plugin version updatedThe com.gorylenko.gradle-git-properties plugin has been updated to the latest version.
40-40: LGTM: Gradle wrapper version updatedThe Gradle wrapper has been updated from release candidate to stable version.
108-108: LGTM: JUnit module exclusion addedThe JUnit module exclusion has been added to avoid conflicts with JUnit 5.
162-162: LGTM: GitLab API dependency updatedThe GitLab API dependency has been updated to a newer version.
227-227: LGTM: JSoup dependency updatedThe JSoup dependency has been updated to the latest version.
326-326: LGTM: Security-related dependency updatednimbus-jose-jwt dependency has been updated to a newer version, improving security.
429-433: LGTM: JUnit Jupiter dependencies addedJUnit 5 dependencies properly added for testing support.
435-436: LGTM: JUnit Platform dependencies addedJUnit Platform dependencies correctly specified with consistent versioning.
src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildJobContainerService.java (5)
79-86: LGTM: Container resource limit configuration properties addedThe configuration properties for container CPU and memory limits are properly defined using Spring's @value annotation with sensible defaults.
101-107: Function signature updated to include resource parametersThe configureContainer method signature has been updated to include CPU and memory parameters, which correctly aligns with the PR objectives.
119-135: LGTM: Resource limit logic implementedThe implementation correctly applies CPU and memory limits when provided, respecting maximum configured values.
151-155: LGTM: Helper method for host config addedNew method correctly copies the host configuration while applying the resource limits.
157-159: LGTM: Memory conversion utility method addedUseful utility method added to convert memory values from MB to bytes.
src/main/webapp/app/exercises/programming/manage/update/update-components/custom-build-plans/programming-exercise-build-configuration/programming-exercise-build-configuration.component.ts (7)
12-12: LGTM: Simplified network disabled languages constantThe constant has been updated to only include EMPTY language, removing SWIFT and HASKELL.
17-19: LGTM: Docker flags interface extendedThe DockerFlags interface has been properly updated to include the new resource limit properties.
40-42: LGTM: Component properties added for resource limitsNew properties for CPU count and memory limits added to the component class.
83-91: LGTM: Resource limit initialization in ngOnInitThe component correctly initializes the resource limit properties from profileInfo values.
103-111: LGTM: Docker flags initialization updatedThe initDockerFlags method properly initializes the resource limit properties from existing Docker flags.
125-138: LGTM: Event handlers added for resource limit inputsNew event handlers correctly update the resource limit properties and regenerate the Docker flags string.
172-172: LGTM: Docker flags generation updatedThe parseDockerFlagsToString method now correctly includes the resource limit properties in the generated Docker flags.
package.json (14)
16-27: Angular Dependencies Update
The Angular packages were updated to the 19.2.x patch versions. These appear to be routine version bumps to address bug fixes and minor improvements. Please verify that these versions are fully compatible with your current Angular application and that none of the breaking changes (if any) affect your project.
39-39: Sentry Angular SDK Version Bump
The update to@sentry/angularfrom 9.2.0 to 9.5.0 may provide better error reporting features or bug fixes. Please review the Sentry changelog to confirm that no unexpected changes affect your logging behavior—especially given the recent focus on enhanced logging in the PR.
47-47: Core-js Update
The update ofcore-jsto 3.41.0 should help keep polyfills current. Ensure that this bump does not introduce any incompatibility issues with the browsers supported by your application or affect any polyfill-dependent functionality.
75-75: Simple-Statistics Version Update
The version increment forsimple-statisticsto 7.8.8 appears to be a minor patch. It would be good to re-run any statistical or data-analysis related tests to confirm that the updated version does not alter any expected results.
103-105: ESLint Override Update for @typescript-eslint/utils
The override for@typescript-eslint/utilsnow specifies ESLint version "^9.22.0". Please verify that this version aligns well with your overall ESLint configuration and does not cause any unexpected linting issues or conflicts with other overrides.
112-113: ESLint Plugin Deprecation Override Updated
The override foreslint-plugin-deprecationhas been updated to use ESLint "^9.22.0". This should help maintain consistency across your linting environment. Confirm that all deprecation warnings are still being reported accurately without conflicts.
127-127: Vite Version Increment
Vite has been updated to 6.2.1. Make sure that your development server, bundling process, and any Vite-specific configurations continue to function as expected after this update.
137-141: Angular ESLint and Build Tools Updates in DevDependencies
Multiple Angular ESLint and build-related packages have been updated to 19.2.1. These upgrades should improve development tooling consistency; however, please run your lint and build scripts to confirm no regressions occur.
155-155: @types/node Upgrade
The upgrade of@types/nodeto 22.13.10 should provide improved type definitions. Verify that your Node.js version (as noted in the engines section) remains fully compatible with these definitions.
162-163: TypeScript ESLint Plugin and Parser Upgrades
The updates to both@typescript-eslint/eslint-pluginand@typescript-eslint/parserto version 8.26.0 look routine. Please ensure that your existing lint rules continue to apply correctly and that no new linting errors are introduced as a result of this change.
166-167: ESLint and Prettier Configuration Adjustments
Updatingeslintto 9.22.0 andeslint-config-prettierto 10.1.1 should enhance your linting setup. It's advisable to run a full linting pass to catch any configuration mismatches.
188-189: TypeScript and TypeScript-ESLint Updates
The bump to TypeScript 5.8.2 and the update totypescript-eslint(8.26.0) are significant enough to warrant a re-run of your type-checking and builds. Please confirm that there are no type errors or build issues after this upgrade.
191-191: Vitest Update
Vitest has been updated to 3.0.8. Ensure that your test suite runs correctly with the new version, and verify that any custom configurations for Vitest remain compatible.
1-224: Overall Dependency and Script Updates
The collection of dependency and devDependency updates, as well as script adjustments, are largely standard maintenance. It is important to perform comprehensive manual and automated testing to catch any unexpected integration issues after these version bumps.
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
At the moment the VCS access log, logs only fully successful interactions, and interactions failing at the authentication step already.
When pushing/pulling fails at the authorization, Artemis writes no VCS access log, in the case of HTTPS not even a server log.
Description
Steps for Testing
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes