Exam mode: Use events instead of notifications for exercise updates - V2
#8578
Conversation
# Conflicts: # src/main/java/de/tum/in/www1/artemis/web/rest/TextExerciseResource.java
# Conflicts: # src/test/java/de/tum/in/www1/artemis/text/TextExerciseIntegrationTest.java
…te-events # Conflicts: # src/main/java/de/tum/in/www1/artemis/web/rest/FileUploadExerciseResource.java # src/main/webapp/app/exam/participate/exam-participation.component.ts # src/test/javascript/spec/component/exam/participate/exam-participation.component.spec.ts
…te-events # Conflicts: # src/main/java/de/tum/in/www1/artemis/service/ExerciseService.java
…te-events # Conflicts: # src/main/java/de/tum/in/www1/artemis/service/ExerciseService.java
github-actions
bot
added
tests
server
WalkthroughThe recent updates focus on enhancing the event and notification systems within an educational platform. Key changes include the introduction of a new event type for problem statement updates during exams, restructuring of notification handling in exercise services, and adjustments to service dependencies. These changes aim to improve real-time communication and interaction efficiency during exams and exercise updates. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (30)
Files not reviewed due to errors (1)
Additional Context UsedPath-based Instructions (28)
Learnings (7)
Additional comments not posted (64)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| if (this.workingTimeUpdateEventsSubscription) { | ||
| this.workingTimeUpdateEventsSubscription.unsubscribe(); | ||
| } | ||
| this.liveEventsSubscription = this.liveEventsService.observeNewEventsAsSystem([ExamLiveEventType.WORKING_TIME_UPDATE]).subscribe((event: WorkingTimeUpdateEvent) => { | ||
| // Create new object to make change detection work, otherwise the date will not update | ||
| this.studentExam = { ...this.studentExam, workingTime: event.newWorkingTime! }; | ||
| this.examParticipationService.currentlyLoadedStudentExam.next(this.studentExam); | ||
| this.individualStudentEndDate = dayjs(startDate).add(this.studentExam.workingTime!, 'seconds'); | ||
| this.individualStudentEndDateWithGracePeriod = this.individualStudentEndDate.clone().add(this.exam.gracePeriod!, 'seconds'); | ||
| this.liveEventsService.acknowledgeEvent(event, false); | ||
| }); | ||
| this.workingTimeUpdateEventsSubscription = this.liveEventsService | ||
| .observeNewEventsAsSystem([ExamLiveEventType.WORKING_TIME_UPDATE]) | ||
| .subscribe((event: WorkingTimeUpdateEvent) => { | ||
| // Create new object to make change detection work, otherwise the date will not update | ||
| this.studentExam = { ...this.studentExam, workingTime: event.newWorkingTime! }; | ||
| this.examParticipationService.currentlyLoadedStudentExam.next(this.studentExam); | ||
| this.individualStudentEndDate = dayjs(startDate).add(this.studentExam.workingTime!, 'seconds'); | ||
| this.individualStudentEndDateWithGracePeriod = this.individualStudentEndDate.clone().add(this.exam.gracePeriod!, 'seconds'); | ||
| this.liveEventsService.acknowledgeEvent(event, false); | ||
| }); | ||
| } | ||
|
|
||
| private subscribeToProblemStatementUpdates() { | ||
| if (this.problemStatementUpdateEventsSubscription) { | ||
| this.problemStatementUpdateEventsSubscription.unsubscribe(); | ||
| } | ||
| this.problemStatementUpdateEventsSubscription = this.liveEventsService | ||
| .observeNewEventsAsSystem([ExamLiveEventType.PROBLEM_STATEMENT_UPDATE]) | ||
| .subscribe((event: ProblemStatementUpdateEvent) => { | ||
| this.examExerciseUpdateService.updateLiveExamExercise(event.exerciseId, event.problemStatement); | ||
| this.liveEventsService.acknowledgeEvent(event, false); | ||
| }); |
There was a problem hiding this comment.
Optimize event subscription logic.
The methods subscribeToWorkingTimeUpdates and subscribeToProblemStatementUpdates handle the subscription to the new event types. However, consider refactoring to reduce redundancy and improve error handling. For instance, unsubscribing before subscribing can be extracted into a utility function to avoid duplication.
private manageSubscription(subscription: Subscription, newSubscriptionCallback: () => Subscription): void {
if (subscription) {
subscription.unsubscribe();
}
subscription = newSubscriptionCallback();
}Then use this utility method in both subscribeToWorkingTimeUpdates and subscribeToProblemStatementUpdates.
| }, | ||
| "from": "von", | ||
| "acknowledge": "Bestätigen", | ||
| "acknowledgeAll": "Alle bestätigen", | ||
| "navigateToExercise": "Navigiere zur Aufgabe", |
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that the formal pronoun "Sie" is used in various German translation files, including the exam.json file which contains the specific translation in question. This suggests that the translation does not consistently maintain an informal tone as required.
Here are the specific instances where the formal tone is used:
- student-dashboard.json
- Multiple instances where "Sie" is used in explanations about grades and bonus points.
- tutorialGroups.json
- Used in the context of explaining the timezone feature.
- exam.json
- Used in descriptions and announcements within the exam context.
- featureOverview.json
- Used extensively in descriptions about exam processes and features.
- exerciseChatbot.json
- Used in descriptions about the functionalities of a chatbot.
- competency.json
- Mentioned in the context of optional competencies.
Given these findings, it is clear that the informal tone is not consistently applied across the German translations in the application. This includes the specific line under review in exam.json.
Action Required:
- Review and revise the translations to ensure consistency with the informal tone requirement across all German translation files.
Analysis chain
Verify that the translation for "Navigate to exercise" is consistent with the informal tone required.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the informal tone in German translations.
# Test: Search for formal tone usage. Expect: No occurrences of formal tone.
rg --type json 'Sie' src/main/webapp/i18n/de/**/*.json
Length of output: 130
Script:
#!/bin/bash
# Description: Re-verify the informal tone in German translations by correcting the path issue.
# Test: Search for formal tone usage. Expect: No occurrences of formal tone.
rg --type json 'Sie' src/main/webapp/i18n/de/
Length of output: 3451
| @SpyBean | ||
| private ExamLiveEventsService examLiveEventsService; | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
The ExamLiveEventsService is declared in TextExerciseIntegrationTest.java but none of its methods are called within the test cases. This might indicate an oversight or unnecessary declaration. It would be beneficial to review the necessity of this service in the test setup.
TextExerciseIntegrationTest.java:ExamLiveEventsServiceis declared but not utilized.
Analysis chain
Ensure ExamLiveEventsService is appropriately utilized in the test cases where its methods are expected to be called.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of `ExamLiveEventsService` in TextExerciseIntegrationTest.java.
# Test: Search for method calls to `ExamLiveEventsService`. Expect: At least one method call.
rg --type java --files-with-matches 'examLiveEventsService' src/test/java/de/tum/in/www1/artemis/text/TextExerciseIntegrationTest.java
Length of output: 209
Script:
#!/bin/bash
# Description: Extract specific method calls to `ExamLiveEventsService` in TextExerciseIntegrationTest.java.
# Test: Search for specific method calls to `examLiveEventsService`.
rg 'examLiveEventsService\.\w+' src/test/java/de/tum/in/www1/artemis/text/TextExerciseIntegrationTest.java
Length of output: 108
| @case (ExamLiveEventType.PROBLEM_STATEMENT_UPDATE) { | ||
| <div> | ||
| <div> | ||
| {{ 'artemisApp.exam.events.messages.problemStatementUpdate.description' | artemisTranslate: { exerciseName: problemStatementUpdateEvent.exerciseName } }} | ||
| </div> | ||
| @if (problemStatementUpdateEvent.text) { | ||
| <div>{{ 'artemisApp.exam.events.messages.problemStatementUpdate.instructorMessage' | artemisTranslate }}</div> | ||
| <div [innerHTML]="problemStatementUpdateEvent.text | htmlForMarkdown"></div> | ||
| } | ||
| </div> | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The search for the i18n keys problemStatementUpdate.description and problemStatementUpdate.instructorMessage within the language-specific directories (de and en) did not yield any results. This confirms that these translation keys are not present in the i18n files, which is necessary for the proper display and localization of the new case for ProblemStatementUpdateEvent in the application.
Action Required: Add Missing i18n Keys
- The translation keys
problemStatementUpdate.descriptionandproblemStatementUpdate.instructorMessageare missing from the i18n files. Please add these keys to the respective language files to ensure the application displays the correct localized messages.
Analysis chain
The new case for ProblemStatementUpdateEvent correctly displays the event details. Ensure that the translations for problemStatementUpdate.description and problemStatementUpdate.instructorMessage are provided in the i18n files.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of new i18n keys for problem statement updates.
# Test: Search for the new i18n keys in translation files. Expect: Keys exist.
rg 'problemStatementUpdate.description|problemStatementUpdate.instructorMessage' -- src/main/webapp/i18n
Length of output: 106
Script:
#!/bin/bash
# Description: List all i18n translation files to confirm the scope of the search.
# List all files in the i18n directory to ensure all were considered in the initial search.
ls src/main/webapp/i18n
Length of output: 31
Script:
#!/bin/bash
# Description: Search for the i18n keys within language-specific directories.
# Search for the specified i18n keys in both 'de' and 'en' translation files.
rg 'problemStatementUpdate.description|problemStatementUpdate.instructorMessage' -- src/main/webapp/i18n/de
rg 'problemStatementUpdate.description|problemStatementUpdate.instructorMessage' -- src/main/webapp/i18n/en
Length of output: 219
| @if (showAcknowledge) { | ||
| <button class="btn btn-primary w-100 mt-2" (click)="acknowledgeEvent()"> | ||
| <fa-icon [icon]="faCheck" /> | ||
| <span jhiTranslate="artemisApp.exam.events.acknowledge"></span> | ||
| </button> | ||
| } | ||
| @if (event.eventType === ExamLiveEventType.PROBLEM_STATEMENT_UPDATE) { | ||
| <button class="btn btn-primary w-100 mt-2" (click)="navigateToExercise()"> | ||
| <span jhiTranslate="artemisApp.exam.events.navigateToExercise"></span> | ||
| </button> |
There was a problem hiding this comment.
Tip
Codebase Verification
The Angular version used in the project is 17.3.8, which supports the event binding syntax (click)="methodName()" as seen in the HTML snippet. However, the conditional rendering syntax @if is not standard Angular syntax; Angular uses *ngIf for such purposes. This suggests a potential issue with the conditional rendering in the provided HTML code.
- The event handlers
acknowledgeEvent()andnavigateToExercise()are correctly implemented and bound inexam-live-event.component.ts.
Recommendation:
- Replace
@ifwith*ngIfin the HTML file to align with standard Angular practices.
Analysis chain
The buttons for acknowledging and navigating to exercises are implemented using the new Angular syntax. Verify that this syntax is supported in the project's Angular version and that the event handlers are correctly bound.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify support for new Angular syntax and correct binding of event handlers.
# Test: Check Angular version and search for event handler implementations. Expect: Supported syntax and correct implementations.
cat package.json | jq '.dependencies.angular'
rg --type ts '(acknowledgeEvent|navigateToExercise)' -- src/main/webapp/app/exam/shared/events
Length of output: 335
Script:
#!/bin/bash
# Correcting the query to fetch Angular version from package.json
cat package.json | jq '.dependencies["@angular/core"]'
Length of output: 65
undernagruzez
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
GitHub Actions
Inactive
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Changes affecting Programming Exercises
Motivation and Context
Description
Steps for Testing
Prerequisites:
Exam Mode Testing
Prerequisites:
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Test Coverage
Screenshots