-
Notifications
You must be signed in to change notification settings - Fork 14
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
fix: add loading states where required and update live quiz deletion logic #4314
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
klicker-uzh Run #3261
Run Properties:
|
Project |
klicker-uzh
|
Run status |
Passed #3261
|
Run duration | 11m 36s |
Commit |
6ff6b65ce1 ℹ️: Merge b6eb9eaa51a9dfe93024f7180a179b18bbe39512 into 24e1bb99210c57dd6d58ea4257cb...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
138
|
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces various enhancements across multiple components in the frontend application, primarily focusing on the implementation of loading states for various mutation operations. Components such as Changes
Possibly related PRs
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
CodeRabbit Configuration File (
|
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 (4)
apps/frontend-manage/src/components/courses/groupActivity/GroupActivityGradingStack.tsx (1)
370-370
: Approve the change with a minor suggestion.The modification to the
disabled
prop improves the button's behavior by disabling it when grading is completed. This aligns well with the PR objective of updating quiz-related logic.However, consider retaining the
isSubmitting
condition to prevent potential multiple submissions:-disabled={!isValid || gradingCompleted} +disabled={!isValid || isSubmitting || gradingCompleted}This ensures the button remains disabled during form submission, preventing accidental multiple clicks.
apps/frontend-manage/src/components/sessions/Session.tsx (3)
43-53
: LGTM: Improved startSession mutation with loading state and refetchThe addition of the
startingQuiz
loading state enhances user feedback during session start. TherefetchQueries
option ensures that the list of running sessions is updated after starting a new session, improving data consistency.Consider adding error handling to provide user feedback if the session start fails.
Line range hint
55-74
: LGTM: Updated deleteLiveQuiz mutationThe renaming of the mutation from
deleteSession
todeleteLiveQuiz
aligns with the PR objective of updating the live quiz deletion logic. The optimistic response has been correctly updated to maintain consistency.Consider adding error handling to provide user feedback if the deletion fails.
248-267
: LGTM: Extended edit functionality to scheduled sessionsThe edit session button is now displayed for both prepared and scheduled sessions, providing more flexibility in session management. This change allows users to make modifications to sessions even after they've been scheduled.
Consider adding a tooltip or info icon to explain the implications of editing a scheduled session, especially if it might affect participants who have already joined or prepared for the session.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/frontend-control/src/components/layout/Header.tsx (2 hunks)
- apps/frontend-manage/src/components/courses/actions/StartLiveQuizButton.tsx (2 hunks)
- apps/frontend-manage/src/components/courses/actions/UnpublishMicroLearningButton.tsx (1 hunks)
- apps/frontend-manage/src/components/courses/groupActivity/GroupActivityGradingStack.tsx (1 hunks)
- apps/frontend-manage/src/components/courses/groupActivity/GroupActivityUnpublishButton.tsx (1 hunks)
- apps/frontend-manage/src/components/sessions/Session.tsx (6 hunks)
- apps/frontend-manage/src/pages/index.tsx (3 hunks)
- apps/frontend-pwa/src/components/course/SuspendedGroupView.tsx (1 hunks)
- apps/frontend-pwa/src/components/practiceQuiz/Bookmark.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/frontend-control/src/components/layout/Header.tsx
- apps/frontend-manage/src/components/courses/actions/StartLiveQuizButton.tsx
- apps/frontend-manage/src/components/courses/actions/UnpublishMicroLearningButton.tsx
- apps/frontend-manage/src/components/courses/groupActivity/GroupActivityUnpublishButton.tsx
- apps/frontend-manage/src/pages/index.tsx
- apps/frontend-pwa/src/components/course/SuspendedGroupView.tsx
- apps/frontend-pwa/src/components/practiceQuiz/Bookmark.tsx
🧰 Additional context used
🔇 Additional comments (4)
apps/frontend-manage/src/components/sessions/Session.tsx (4)
16-16
: LGTM: Updated import for deletion mutationThe change from
DeleteSessionDocument
toDeleteLiveQuizDocument
suggests a more specific mutation for deleting live quizzes. This change aligns with the PR objective of updating the live quiz deletion logic.
193-193
: LGTM: Disabled start session button during loadingThe addition of the
disabled
prop controlled by thestartingQuiz
loading state prevents multiple clicks on the start session button while the session is being started. This improves user experience and prevents potential errors.
355-355
: LGTM: Updated DeletionModal to use deleteLiveQuizThe
deleteElement
prop in the DeletionModal component has been correctly updated to usedeleteLiveQuiz
instead ofdeleteSession
. This change maintains consistency with the renamed deletion mutation.
269-277
: LGTM: Extended delete functionality to completed sessionsThe delete session button is now displayed for prepared, scheduled, and completed sessions, providing more control over session management. This aligns with the PR objective of updating the live quiz deletion logic.
Please verify that deleting completed sessions doesn't have unintended consequences, such as loss of important data or breaking references in other parts of the system. Consider running the following script to check for any references to completed sessions:
✅ Verification successful
Verified: No unintended consequences found when deleting completed sessions
Deleting completed sessions does not appear to adversely affect other parts of the system based on the current references. The
session.status === SessionStatus.Completed
checks are used solely for display and filtering purposes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to completed sessions in the codebase rg --type typescript --type javascript 'session.*status.*Completed' -C 3Length of output: 109
Script:
#!/bin/bash # Search for references to completed sessions in the codebase with corrected file types rg --type ts --type tsx --type js --type jsx 'session.*status.*Completed' -C 3Length of output: 108
Script:
#!/bin/bash # Search for references to completed sessions in the codebase using glob patterns rg -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx" 'session.*status.*Completed' -C 3Length of output: 1339
Quality Gate passedIssues Measures |
klicker-uzh Run #3267
Run Properties:
|
Project |
klicker-uzh
|
Run status |
Passed #3267
|
Run duration | 11m 35s |
Commit |
3ab1a1ab32: fix: add loading states where required and update live quiz deletion logic (#431...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
138
|
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
These updates aim to improve user interactions and streamline the management of sessions and quizzes within the application.