Development: Add endpoint to fetch participation by repo name
#10715
Conversation
github-actions
bot
added
tests
server
janthoXO
changed the title
Programming exercises: Add endpoint to fetch ProgrammingExerciseStudentParticipation by repo identifier
WalkthroughA new constructor was added to Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (9)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (1)
229-244: Consider adding null checks for feedback.testCaseId.The code conditionally loads test cases based on whether submissions exist, which is a good optimization. However, when processing feedbacks, there's no explicit check to handle cases where a feedback has a testCaseId that doesn't correspond to an existing test case.
When joining feedback with test cases on the client side, consider adding a safeguard to handle the case of feedbacks referencing non-existent test cases (which could happen if test cases were deleted but feedbacks remain). This could be a client-side check or an additional check in the DTO conversion to ensure data consistency.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (1)
759-762: Consider improving the repository name extraction method.The current implementation assumes a specific URI format and may break if the format changes. A more robust approach would use regex or URI parsing.
- String extractRepoName(String repoUrl) { - // <server.url>/git/<project_key>/<repo-name>.git - return repoUrl.substring(repoUrl.lastIndexOf("/") + 1, repoUrl.length() - 4); - } + String extractRepoName(String repoUrl) { + // <server.url>/git/<project_key>/<repo-name>.git + try { + URI uri = new URI(repoUrl); + String path = uri.getPath(); + if (path.endsWith(".git")) { + String fileName = path.substring(path.lastIndexOf('/') + 1); + return fileName.substring(0, fileName.length() - 4); // Remove .git + } + return ""; + } catch (URISyntaxException e) { + return ""; + } + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/de/tum/cit/aet/artemis/assessment/domain/Feedback.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/domain/VcsRepositoryUri.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/dto/RepoIdentifierProgrammingStudentParticipationDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingSubmissionRepository.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java(7 hunks)src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/main/java/**/*.java`: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,de...
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/repository/ProgrammingSubmissionRepository.javasrc/main/java/de/tum/cit/aet/artemis/programming/domain/VcsRepositoryUri.javasrc/main/java/de/tum/cit/aet/artemis/assessment/domain/Feedback.javasrc/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.javasrc/main/java/de/tum/cit/aet/artemis/programming/dto/RepoIdentifierProgrammingStudentParticipationDTO.java
`src/test/java/**/*.java`: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_...
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java
🧬 Code Graph Analysis (1)
src/main/java/de/tum/cit/aet/artemis/programming/domain/VcsRepositoryUri.java (1)
src/main/webapp/app/exercise/exercise-scores/exercise-scores.component.ts (1)
projectKey(304-306)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: validate-pr-title
- GitHub Check: validate-pr-title
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build Docker Image for ls1intum/artemis
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
🔇 Additional comments (25)
src/main/java/de/tum/cit/aet/artemis/assessment/domain/Feedback.java (2)
85-89: Looks good - added field to efficiently access test case ID without loading the entity.The new
testCaseIdfield withinsertable = false, updatable = falseannotations is a good approach to directly access the test case ID without triggering lazy loading of the fullProgrammingExerciseTestCaseentity. This design choice improves performance in the new API endpoint.
256-258: Well-implemented getter for new field.The getter method follows Java bean conventions and completes the implementation of the new field.
src/main/java/de/tum/cit/aet/artemis/programming/repository/ProgrammingSubmissionRepository.java (2)
21-21: Appropriate import addition.The added import supports the return type of the new repository method.
146-155: Efficient query implementation for retrieving latest submission with results.The JPQL query efficiently fetches the latest submission with its latest result and associated feedbacks in a single database operation. The query intelligently uses subqueries to get the latest submission and result, and the fetch joins ensure that related entities are loaded eagerly, preventing N+1 query issues.
The return type
Optional<Submission>(superclass) instead ofOptional<ProgrammingSubmission>(concrete class) provides flexibility without compromising type safety.src/main/java/de/tum/cit/aet/artemis/programming/domain/VcsRepositoryUri.java (1)
36-46: 🛠️ Refactor suggestionConsider adding validation for repository name format.
The constructor extracts the project key by splitting the repository name at the first hyphen. While this works for the expected format, it will fail with an
ArrayIndexOutOfBoundsExceptionif the repository name doesn't contain a hyphen.public VcsRepositoryUri(String vcBaseUrl, String repositoryName) throws URISyntaxException { - var projectKey = repositoryName.split("-")[0]; + String[] parts = repositoryName.split("-", 2); + if (parts.length < 1) { + throw new IllegalArgumentException("Repository name must contain at least one character"); + } + var projectKey = parts[0]; this.uri = new URI(vcBaseUrl + "/git/" + projectKey.toUpperCase() + "/" + repositoryName + ".git"); }Likely an incorrect or invalid review comment.
src/main/java/de/tum/cit/aet/artemis/programming/web/ProgrammingExerciseParticipationResource.java (8)
81-84: Good dependency management for new functionality.Properly adding the repository and configuration field needed for the new endpoint.
116-117: Appropriate dependency addition.The test case repository is needed to fetch test cases in the new endpoint.
118-143: Well-structured constructor with appropriate dependencies.The constructor correctly initializes all required dependencies for the new functionality.
191-210: Well-documented endpoint with proper security annotations.The endpoint is well-documented with clear Javadoc, appropriate endpoint path mapping, and security annotations. The method signature is clear, and the
@AllowedToolsannotation properly restricts access to specific tools.
208-213: Good error handling for URI construction.The code properly wraps the URI construction in a try-catch block and throws a meaningful error message if the repository URL is invalid.
215-223: Comprehensive authorization checks.The code performs proper authorization checks to ensure the user has permission to access the participation and that the exercise is released.
224-226: Efficient loading of submission data.The code uses the new repository method to efficiently load the latest submission with its latest result and feedbacks, and properly handles the case where no submission exists by setting an empty set.
245-246: Clean DTO conversion.The endpoint properly returns a specialized DTO for the response, avoiding potential lazy loading issues and providing a clean API contract.
src/main/java/de/tum/cit/aet/artemis/programming/dto/RepoIdentifierProgrammingStudentParticipationDTO.java (7)
30-49: Well-structured main DTO definition with comprehensive data model.The Java record approach for DTOs is an excellent choice here, providing immutability and reducing boilerplate. The static
ofmethod handles null safety properly with Optional and includes comprehensive data conversion.
51-70: Proper implementation of submission DTO with appropriate fields.The submission DTO correctly includes all necessary fields for the client and properly handles collection conversion with null-safety through Optional.
72-91: Results DTO correctly models test case statistics.All the critical fields for representing test results are properly included, with appropriate null handling and collection transformation.
93-108: Feedback DTO includes essential testCaseId for traceability.The inclusion of
testCaseIdin the feedback DTO is a good design choice, enabling the client to correlate feedback with specific test cases without requiring additional requests.
110-133: ProgrammingExercise DTO provides comprehensive exercise data.This record includes all essential fields for displaying exercise information, with proper null handling and collection transformation. The inclusion of test cases is particularly useful for the client's needs.
135-148: Minimal but sufficient Course DTO.The Course DTO only includes the essential fields (id, title, shortName) needed for display, adhering to the principle of minimizing DTO data to only what's required.
150-165: TestCase DTO includes all relevant fields for test case display.The TestCase DTO properly includes visibility information that will be essential for displaying appropriate test case information to students based on their permissions.
src/test/java/de/tum/cit/aet/artemis/programming/ProgrammingExerciseParticipationIntegrationTest.java (5)
660-676: Basic repository name retrieval test is properly implemented.The test verifies the basic functionality of retrieving a participation by repository name, including checking that the returned data has the expected structure and that test cases are null when not eagerly loaded. The helper method
extractRepoNamecorrectly parses the repository name from the URI.
678-714: Comprehensive test for retrieving participation with results and feedbacks.This test thoroughly verifies that:
- The endpoint returns both participation and related data
- The latest submission and result are correctly included
- Feedback data includes test case IDs
- Test cases are appropriately included in the exercise data
The test coverage is comprehensive and validates all aspects of the DTO structure.
716-734: Not-found case is properly tested.The test correctly verifies the 404 response for non-existent repository names, using a robust approach to generate a valid but non-existent repository URL to avoid false positives.
736-743: Access control for unauthorized users is properly tested.The test correctly verifies that a student cannot access another student's participation by repository name, expecting and asserting the appropriate 403 Forbidden status code.
745-757: Exam-specific access control is properly tested.The test verifies that exam participations cannot be accessed through this endpoint, correctly ensuring that exam security is maintained.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test9.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test5.artemis.cit.tum.de
|
Quick question: are the (new) api endpoints in the "Steps for Testing" section correct? I had trouble using these for testing |
janthoXO
changed the title
Programming exercises: Add endpoint to fetch ProgrammingExerciseStudentParticipation by repo identifierProgramming exercises: Add endpoint to fetch ProgrammingExerciseStudentParticipation by repo name
You are right the description should be updated now |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
|
Looking good to me :) |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
…hub.com:ls1intum/Artemis into feature/programming-exercise-by-repo-identifier
janthoXO
dismissed stale reviews from konrad2002, Mtze, and coderabbitai[bot]
via
e12ed75
janthoXO
requested review from
konrad2002,
iyannsch,
krusche,
Auxburger and
Mtze
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Reopening of too long open PR.
Checklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
The VSCode extension should be able to display the problem statement corresponding to an open repository by the user. Therefore it should be able to fetch the participation by the repository url. All other information like submissions, results will then be fetch on demand via different endpoints.
Description
Adds an endpoint to fetch a student programming participation by the repo name (
<vc.url>/git/<project_key>/<repo_name>.git) with the exercise, course.This endpoint will be used to automatically detect an participation from a repository url and display the problem statement to it (therefore course, exercise are needed).
Steps for Testing
Postman testing or code review:
Postman Testing with a student account:
{{base_url}}/api/core/public/authenticate?tool=SCORPIOto login{{base_url}}/api/core/courses/{courseId}/for-dashboard). The repo url should look like<vc.url>/git/<project_key>/<repo_name>.gitrepo_namefrom that url{{base_url}}/api/programming/programming-exercise-participations?repoName={repo_name}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
Performance Review
Code Review
Manual Tests
Performance Tests
Test Coverage
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores