-
Notifications
You must be signed in to change notification settings - Fork 297
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
: Add ProgrammingExercise by ProjectKey endpoint
#9406
base: feature/re-key
Are you sure you want to change the base?
Programming exercises
: Add ProgrammingExercise by ProjectKey endpoint
#9406
Conversation
WalkthroughThe pull request introduces a new method Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Outdated
Show resolved
Hide resolved
should be replaced by AtLeastStudentInExercise but this has to be implemented without the macro, because no exerciseID is passed |
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Outdated
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Outdated
Show resolved
Hide resolved
ProgrammingExercise
: Add ProgrammingExercise by ProjectKey endpoint
ProgrammingExercise
: Add ProgrammingExercise by ProjectKey endpointProgramming Exercise
: Add ProgrammingExercise by ProjectKey endpoint
Programming Exercise
: Add ProgrammingExercise by ProjectKey endpointProgramming exercises
: Add ProgrammingExercise by ProjectKey endpoint
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, but you also should add some simple server tests.
good point thanks |
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Outdated
Show resolved
Hide resolved
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java
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.
Code
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
🧹 Outside diff range and nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
553-560
: Enhance method documentation.The method documentation should include more details about the possible response status codes and parameter description.
Apply this diff to improve the documentation:
/** * GET /programming-exercises : Queries a programming exercise by its project key. * - * + * @param projectKey the unique identifier of the programming exercise in the version control system * - * @return the ProgrammingExercise of this project key in an ResponseEntity or 404 Not Found if no exercise exists + * @return the ResponseEntity with status: + * - 200 (OK) and the ProgrammingExercise in body + * - 403 (Forbidden) if the user doesn't have access + * - 404 (Not Found) if no exercise exists for the given project key */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseResource.java (1)
561-568
:⚠️ Potential issueOptimize authorization check and enhance error handling.
The current implementation has the following issues:
- Performs authorization check after fetching the exercise, which is inefficient
- Lacks request logging
- Missing explicit error handling
Apply this diff to improve the implementation:
@GetMapping("programming-exercises/project-key/{projectKey}") @EnforceAtLeastStudent public ResponseEntity<ProgrammingExercise> getExerciseByProjectKey(@PathVariable String projectKey) { + log.debug("REST request to get ProgrammingExercise by project key : {}", projectKey); + try { final ProgrammingExercise exercise = programmingExerciseRepository.findOneByProjectKeyOrThrow(projectKey, false); - authCheckService.checkIsAtLeastRoleInExerciseElseThrow(Role.STUDENT, exercise.getId()); + // Check if user has access to this specific exercise + authCheckService.checkIsAtLeastRoleInExerciseElseThrow(Role.STUDENT, exercise.getId()); return ResponseEntity.ok(exercise); + } catch (EntityNotFoundException e) { + return ResponseEntity.notFound().build(); + } catch (AccessForbiddenException e) { + return ResponseEntity.status(HttpStatus.FORBIDDEN).build(); + } }Likely invalid or redundant comment.
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
…ture/programming-exercise-by-projectkey
…ture/programming-exercise-by-projectkey
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
…ture/programming-exercise-by-projectkey
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.
Tested the new endpoint on TS9, works as expected
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.
Works on Ts9, code 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
Server
Changes affecting Programming Exercises
Description
Adds an endpoint that fetches a programming exercise by its projectKey
Steps for Testing
Postman testing or code review:
Postman Testing:
{{base_url}}/api/public/authenticate?tool=SCORPIO
{{base_url}}/api/courses/for-dashboard
{{base_url}}/api/programming-exercises/project-key/{{projectKey}}
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
@coderabbitai pause