-
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
Development
: Inline endpoint constants
#8438
Conversation
This comment was marked as spam.
This comment was marked as spam.
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: 34
src/test/java/de/tum/in/www1/artemis/assessment/AssessmentComplaintIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/de/tum/in/www1/artemis/assessment/AssessmentComplaintIntegrationTest.java
Outdated
Show resolved
Hide resolved
src/test/java/de/tum/in/www1/artemis/assessment/AssessmentComplaintIntegrationTest.java
Outdated
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Show resolved
Hide resolved
...java/de/tum/in/www1/artemis/exercise/programmingexercise/ProgrammingExerciseTestService.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/in/www1/artemis/service/dto/athena/ProgrammingSubmissionDTO.java
Outdated
Show resolved
Hide resolved
src/test/java/de/tum/in/www1/artemis/exercise/AthenaResourceIntegrationTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thank you!
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.
Looks good now
5ef17d2
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.
Reapprove after Merge
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: 0
Out of diff range and nitpick comments (4)
src/main/webapp/app/exercises/quiz/manage/apollon-diagrams/apollon-diagram-create-form.component.ts (1)
Line range hint
23-23
: The save method correctly handles the diagram creation logic. However, consider adding loading indicators or disabling form elements during the save operation to improve user feedback and prevent duplicate submissions.src/main/webapp/app/admin/standardized-competencies/import/admin-import-standardized-competencies.component.ts (1)
Line range hint
31-31
: TheimportCompetencies
method handles the API call for importing competencies. Consider implementing error handling and user feedback mechanisms to enhance the robustness of the import process.src/main/webapp/app/exercises/quiz/manage/apollon-diagrams/apollon-diagram-detail.component.ts (1)
Line range hint
39-39
: ThesaveDiagram
method handles the saving of the diagram state. Consider adding more robust error handling and user feedback mechanisms to improve the reliability of the save operation.src/test/java/de/tum/in/www1/artemis/authorization/AuthorizationTestService.java (1)
Line range hint
79-79
: ThetestEndpoint
method is a wrapper for specific tests on a single endpoint. This method should be expanded to include more comprehensive tests based on the application's security requirements.
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.
reapprove code
f4a423f
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.
Reapprove code
Checklist
General
Motivation and Context
Some endpoints use concatenated string constants as their URL. This can make these endpoints harder to find when using a typically search function as a developer. We want to therefore use string literals when defining the endpoint url.
Description
This PR uses the automatic IntelliJ refactoring tools to inline these constants. I then went over these changes and cleaned up instances like
"/api/programming-exercises/import/{sourceExerciseId}".replace("{sourceExerciseId}", sourceExercise.getId().toString())
to"/api/programming-exercises/import/" + sourceExercise.getId()
.You can review the PR commit by commit to see the process.
Steps for Testing
code review
Review Progress
Code Review
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests