-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Programming exercises
: Display error message for title and shortname conflicts on localVCS
#8523
Programming exercises
: Display error message for title and shortname conflicts on localVCS
#8523
Conversation
Programming exercises
: Display error message for existing exercises von LocalVCSProgramming exercises
: Display error message for title and shortname conflicts on localVCS
...ava/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseExportImportResource.java
Show resolved
Hide resolved
…t-if-exercise-folder-exists-before-import
WalkthroughThis update introduces a new method for alert parameter construction in Changes
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Out of diff range and nitpick comments (2)
src/main/java/de/tum/in/www1/artemis/web/rest/errors/InternalServerErrorAlertException.java (1)
18-20
: Review exception handling strategy.Consider whether it's appropriate to handle all internal server errors with this new exception class. Depending on the application's error handling strategy, it might be beneficial to differentiate between different types of internal server errors.
src/main/java/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseExportImportResource.java (1)
Line range hint
241-264
: Review error handling inimportProgrammingExercise
.The method
importProgrammingExercise
uses a genericException
catch block. Consider using more specific exception types to handle different scenarios more effectively. This will also help in providing more descriptive error messages to the client.
src/main/java/de/tum/in/www1/artemis/web/rest/errors/InternalServerErrorAlertException.java
Show resolved
Hide resolved
src/main/java/de/tum/in/www1/artemis/web/rest/errors/InternalServerErrorAlertException.java
Show resolved
Hide resolved
src/main/java/de/tum/in/www1/artemis/web/rest/errors/HttpStatusException.java
Show resolved
Hide resolved
...ava/de/tum/in/www1/artemis/web/rest/programming/ProgrammingExerciseExportImportResource.java
Show resolved
Hide resolved
The failing server LocalVCIntegrationTest testUserTriesToDeleteBranch() test is flaky, the Cypress E2E tests are known failing E2E tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Demonstrated in Testing Session, LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
demontrated in testing - code also lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm!
Checklist
General
on a test serverlocally.Server
Client
Motivation and Context
In a previous Artemis version (7.0.3 right now), it was possible that when an error occurred during creating/importing/deleting a programming exercise the respective folders for that programming exercise were not cleaned up.
This means that the short names that are related to the folder names stay blocked, however, there was no error message shown in this case, for the blocked short names.
Description
title
andshort title
Steps for Testing Locally
Prerequisites:
local-vcs-repos
andrepos
a foldercoursePrefix
+BLOCKEDSHORTNAME
will be createdlocal-vcs-repos
and rename it tocoursePrefix
+BLOCKEDSHORTNAME1
coursePrefix
+BLOCKEDSHORTNAME1
and see that an error message is displayed that explains that the shortname is already takenTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Now using actual translations instead of hardcoded english messages