Programming exercises: Add open repository button for editors and instructors in LocalVC
#9898
Conversation
github-actions
bot
added
the
client
SimonEntholzer
changed the title
Exam: Add open repository button for editors and instructors in LocalVCProgramming exercises: Add open repository button for editors and instructors in LocalVC
WalkthroughThe changes in this pull request involve modifications to the 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
|
![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
🧹 Outside diff range and nitpick comments (4)
src/main/webapp/app/utils/navigation.utils.ts (1)
155-164: Implementation aligns well with PR objectivesThe function successfully implements the URL generation needed for the "open repository" feature, supporting both regular and exam scenarios. This enables instructors to access students' repositories during assessment, which was the main goal of the PR.
Consider adding unit tests to verify the URL generation for different scenarios:
- Regular course repository access
- Exam repository access
- Test run repository access
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html (1)
108-114: Consider enhancing link accessibility.While the implementation is solid, consider adding an aria-label to provide more context for screen readers, especially since the link contains an icon in the local repository case.
<a class="ms-2 me-5" href="{{ participation?.userIndependentRepositoryUri }}" target="_blank" rel="noopener noreferrer" + aria-label="{{ 'artemisApp.exerciseAssessmentDashboard.programmingExercise.goToRepo' | translate }}" jhiTranslate="artemisApp.exerciseAssessmentDashboard.programmingExercise.goToRepo" ></a>And for the local repository link:
<a class="ms-2 me-5" [routerLink]="localRepositoryLink" target="_blank" rel="noopener noreferrer" + aria-label="{{ 'artemisApp.exerciseAssessmentDashboard.programmingExercise.goToRepo' | translate }}" jhiTranslate="artemisApp.exerciseAssessmentDashboard.programmingExercise.goToRepo" >Also applies to: 116-124
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts (2)
87-87: Add JSDoc documentation for the new properties.While the property names are self-descriptive, adding JSDoc documentation would improve maintainability by explaining:
- The purpose of
localRepositoryLinkarray- The implications of
isAtLeastEditorflag- When these properties are used
Example documentation:
+/** Array containing the repository link segments for local version control */ localRepositoryLink: string[]; +/** Flag indicating whether the current user has at least editor permissions */ isAtLeastEditor = false; +/** FontAwesome icon for external links */ faExternalLink = faExternalLink;Also applies to: 92-92, 116-116
193-193: Consider using explicit boolean conversion.The code is functionally correct, but the boolean conversion could be more explicit for better readability.
Consider this refactor:
-this.isAtLeastEditor = !!this.exercise.isAtLeastEditor; +this.isAtLeastEditor = Boolean(this.exercise.isAtLeastEditor);The initialization of
localRepositoryLinkis well-structured and includes all necessary parameters.Also applies to: 201-208
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html(1 hunks)src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts(4 hunks)src/main/webapp/app/utils/navigation.utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html (1)
Pattern 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/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts (1)
src/main/webapp/app/utils/navigation.utils.ts (1)
🔇 Additional comments (3)
src/main/webapp/app/utils/navigation.utils.ts (1)
155-164:
Several improvements needed for the getLocalRepositoryLink function
- The test run case appears to return an assessment URL instead of a repository URL, which seems inconsistent with the function's purpose.
- Missing JSDoc documentation as required by the coding guidelines.
- Return type annotation should be explicit for better type safety.
Here's the suggested implementation:
+/**
+ * Generates a URL array for accessing a local repository
+ * @param courseId - The ID of the course
+ * @param exerciseId - The ID of the exercise
+ * @param participationId - The ID of the participation
+ * @param exerciseGroupId - The ID of the exercise group (optional, default: 0)
+ * @param examId - The ID of the exam (optional, default: 0)
+ * @param isTestRun - Flag indicating if this is a test run (optional, default: false)
+ * @returns Array of URL segments for router navigation
+ */
-export const getLocalRepositoryLink = (courseId: number, exerciseId: number, participationId: number, exerciseGroupId: number = 0, examId = 0, isTestRun = false): string[] => {
+export const getLocalRepositoryLink = (courseId: number, exerciseId: number, participationId: number, exerciseGroupId: number = 0, examId = 0, isTestRun = false): string[] => {
if (isTestRun) {
- return ['/course-management', courseId.toString(), 'exams', examId.toString(), 'test-runs', 'assess'];
+ return ['/course-management', courseId.toString(), 'exams', examId.toString(), 'test-runs', 'repository'];
}
const suffix = ['programming-exercises', exerciseId.toString(), 'participations', participationId.toString(), 'repository'];
return examId > 0
? ['/course-management', courseId.toString(), 'exams', examId.toString(), 'exercise-groups', exerciseGroupId.toString(), ...suffix]
: ['/course-management', courseId.toString(), ...suffix];
};Let's verify the usage of this function and similar patterns in the codebase:
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.html (1)
106-125: LGTM! Clean implementation of repository access controls.
The implementation correctly follows the requirements by:
- Gating repository access to users with editor permissions
- Supporting both local and external repository scenarios
- Using proper security attributes for external links
- Following the new Angular @if syntax
src/main/webapp/app/exercises/programming/assess/code-editor-tutor-assessment-container.component.ts (1)
32-32: LGTM: Import statements are properly organized.
The new imports follow Angular's style guide and are correctly placed with related imports.
Also applies to: 36-36
SimonEntholzer
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
GitHub Actions
Inactive
HawKhiem
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
sawys777
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
GitHub Actions
Inactive
SimonEntholzer
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
FelixTJDietrich
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
GitHub Actions
Inactive
FelixTJDietrich
added
maintainer-approved
Checklist
General
Client
Motivation and Context
Instructors and editors are not able to open the student's repository when they are assessing submissions.
Description
Adds the "open repository" link, identically how it is for external repositories for LocalVC repositories.
Steps for Testing
Exam Mode Testing
Note: For test-runs, the button we do not show the repository button, as the repository view is not supported for test-run participation repositories yet. This will be added in a follow up.
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
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Client
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Documentation