enhance(apps/frontend-manage): limit group activity creation to courses where group creation is enabled #4259
Conversation
…es where group creation is enabled
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.
|
WalkthroughWalkthroughThe pull request introduces significant modifications across multiple components and hooks in the frontend application, primarily focusing on the handling of course selection and group creation functionalities. Key changes include the addition of the Changes
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 (
|
|
![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 (1)
packages/i18n/messages/de.ts (1)
Line range hint
1202-1203: Translate the message to German for consistency.The
groupActivityNamemessage is in English, while the rest of the file is in German.Apply this diff to fix the issue:
- groupActivityName: - 'This name should allow you to distinguish this group activity from others. It will not be shown to the participants, please use the display name (next field) for this.', + groupActivityName: + 'Dieser Name sollte es Ihnen ermöglichen, diese Gruppenaktivität von anderen zu unterscheiden. Er wird den Teilnehmenden nicht angezeigt, bitte verwenden Sie dafür den Anzeigenamen (nächstes Feld).',
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (16)
- apps/frontend-manage/src/components/sessions/creation/ElementCreation.tsx (6 hunks)
- apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityInformationStep.tsx (3 hunks)
- apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivitySettingsStep.tsx (2 hunks)
- apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx (7 hunks)
- apps/frontend-manage/src/components/sessions/creation/liveQuiz/LiveSessionWizard.tsx (4 hunks)
- apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (4 hunks)
- apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (4 hunks)
- apps/frontend-manage/src/lib/hooks/useCoursesGamificationSplit.tsx (1 hunks)
- apps/frontend-manage/src/lib/hooks/useCoursesGroupsSplit.tsx (1 hunks)
- apps/frontend-manage/src/lib/hooks/useGroupsCourseGrouping.tsx (1 hunks)
- packages/graphql/src/graphql/ops/QGetUserCourses.graphql (1 hunks)
- packages/graphql/src/ops.ts (2 hunks)
- packages/graphql/src/public/client.json (1 hunks)
- packages/graphql/src/public/server.json (1 hunks)
- packages/i18n/messages/de.ts (3 hunks)
- packages/i18n/messages/en.ts (3 hunks)
Additional comments not posted (35)
packages/graphql/src/graphql/ops/QGetUserCourses.graphql (1)
9-9: LGTM!The addition of the
isGroupCreationEnabledfield to theGetUserCoursesquery is a valid change that enhances the functionality by providing additional context about the courses. The field name is clear and descriptive, and it is correctly placed within theuserCoursesobject, maintaining the structure of the query.apps/frontend-manage/src/lib/hooks/useGroupsCourseGrouping.tsx (1)
1-30: LGTM!The code changes are approved. The custom hook
useGroupsCourseGroupingis well-structured, follows best practices, and provides a reusable way to group courses based on whether they have groups or not. The use of TypeScript and theuseTranslationshook for internationalization is also commendable.apps/frontend-manage/src/lib/hooks/useCoursesGamificationSplit.tsx (1)
1-32: LGTM!The custom hook
useCoursesGamificationSplitis well-structured and follows a clear logic for splitting the providedcourseSelectionarray into two arrays based on theisGamifiedproperty. The use of thereducefunction is appropriate for categorizing the courses, and the hook handles the case whencourseSelectionisundefinedto avoid potential errors. The addition of thedataproperty with thecyproperty for testing purposes is a good practice. The hook is exported as a default export, making it easy to import and use in other components.The code changes are approved.
apps/frontend-manage/src/lib/hooks/useCoursesGroupsSplit.tsx (1)
1-32: LGTM!The
useCoursesGroupsSplithook is correctly splitting thecourseSelectionarray into two arrays based on theisGroupCreationEnabledproperty of each course object. It is also adding adataproperty with acykey to each course object in the split arrays, which could be used for testing purposes. The hook is returning an object with the two split arrays, which is a good way to return multiple values from a hook. It is also using optional chaining and nullish coalescing to handle the case wherecourseSelectionis undefined, which is a good practice to avoid errors.The code changes are approved.
apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityInformationStep.tsx (4)
21-22: LGTM!The changes to the component parameters are consistent with the new approach to managing course data.
28-28: LGTM!The introduction of the
noCoursesWithGroupsvariable simplifies the logic and improves clarity.
Line range hint
48-54: LGTM!The conditional rendering of the
UserNotificationcomponent based onnoCoursesWithGroupsprovides a user-friendly message when there are no courses with groups available.
138-138: LGTM!Deriving the
continueDisabledprop fromnoCoursesWithGroupsenhances the component's responsiveness to the current state of course data.apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivitySettingsStep.tsx (3)
3-3: LGTM!The import statement change is approved.
34-39: LGTM!The function call change is approved.
26-27: LGTM!The function parameter change is approved.
apps/frontend-manage/src/components/sessions/creation/ElementCreation.tsx (3)
34-35: LGTM!The changes to the
ElementSelectCoursetype are approved. The addition of theisGroupCreationEnabledproperty and making thedataproperty optional align with the goals mentioned in the AI-generated summary.
113-118: LGTM!The changes to the course mapping are approved. Including the
isGroupCreationEnabledproperty in the mapped object aligns with the goals mentioned in the AI-generated summary and ensures that the property is available for use in the component's logic.Also applies to: 122-122
174-174: LGTM!The change to the
coursesprop assignment is approved. Using the nullish coalescing operator (??) to provide a fallback to an empty array is a good practice to prevent passingnullorundefinedvalues to thecoursesprop.apps/frontend-manage/src/components/sessions/creation/liveQuiz/LiveSessionWizard.tsx (3)
12-12: LGTM!The import statement for the
useCoursesGamificationSplithook is correctly written.
49-49: LGTM!The changes to the
LiveSessionWizardPropsinterface are approved.Consolidating the
gamifiedCoursesandnonGamifiedCoursesprops into a singlecoursesprop simplifies the interface and centralizes the course data handling. TheuseCoursesGamificationSplithook can be used to split thecoursesprop within the component.
77-80: LGTM!The usage of the
useCoursesGamificationSplithook in theLiveSessionWizardcomponent is approved.Invoking the hook with the
coursesprop to derive thegamifiedCoursesandnonGamifiedCoursesvariables improves the component's logic by centralizing the course data handling and enhancing the separation of concerns. The derived variables are correctly passed as props to theLiveQuizSettingsStepcomponent.apps/frontend-manage/src/components/sessions/creation/microLearning/MicroLearningWizard.tsx (2)
Line range hint
59-92: LGTM!The changes to the
MicroLearningWizardPropsinterface and the usage of theuseCoursesGamificationSplithook are a good refactoring that simplifies the component's props and improves the separation of concerns.
Line range hint
280-320: LGTM!The usage of
gamifiedCoursesandnonGamifiedCoursesin theMicroLearningInformationStepandMicroLearningSettingsStepcomponents is consistent with the changes made to theMicroLearningWizardcomponent.Also applies to: 340-380
apps/frontend-manage/src/components/sessions/creation/practiceQuiz/PracticeQuizWizard.tsx (4)
10-10: LGTM!The code change is approved.
60-60: LGTM!The code change is approved. The interface change simplifies the props by consolidating the course properties.
71-71: LGTM!The code change is approved. The function parameter change is consistent with the interface change.
94-96: LGTM!The code change is approved. The hook is correctly used to derive
gamifiedCoursesandnonGamifiedCoursesfrom thecoursesprop.apps/frontend-manage/src/components/sessions/creation/groupActivity/GroupActivityWizard.tsx (3)
10-10: Approve the introduction of theuseCoursesGroupsSplithook.The changes to the component's props and the introduction of the
useCoursesGroupsSplithook are approved. The hook handles the splitting of courses based on the presence of groups, which aligns with the new categorization of courses.Also applies to: 90-92
326-328: Approve the updated logic for determining the disabled state of the continue button.The logic for determining the disabled state of the continue button in the
GroupActivityInformationStephas been updated to check the length ofcoursesWithGroups. This change is approved as it simplifies the control flow and aligns with the new categorization of courses.
365-366: Approve the changes to the props passed to theGroupActivitySettingsStep.The
GroupActivitySettingsStepnow receives thecoursesWithGroupsandcoursesWithoutGroupsprops, which is consistent with the changes made to the component's props and the new categorization of courses. These changes are approved.packages/graphql/src/public/client.json (1)
139-139: Acknowledge hash value update for "GetUserCourses".The hash value for the "GetUserCourses" key has been updated, indicating a change in the underlying implementation or data.
Ensure that this change is thoroughly tested to confirm that the "GetUserCourses" functionality behaves as expected and does not introduce any regressions.
Run the following script to verify the usage of "GetUserCourses":
Verification successful
Ensure thorough testing of "GetUserCourses" functionality.
The "GetUserCourses" query is used in several parts of the codebase, including:
packages/graphql/src/ops.ts: Defines the query type and document.apps/frontend-manage/src/components/sessions/creation/ElementCreation.tsx: Utilizes the query in a component.apps/frontend-manage/src/pages/courses/index.tsx: Utilizes the query in a page.Given the hash update, please ensure that these components and pages are thoroughly tested to confirm that they work correctly with the updated query implementation.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of "GetUserCourses" in the codebase. # Test: Search for the usage of "GetUserCourses". Expect: Only valid usages with the updated implementation. rg --type typescript --type javascript --type graphql -A 5 $'GetUserCourses'Length of output: 110
Script:
#!/bin/bash # Description: Verify the usage of "GetUserCourses" in the codebase. # Test: Search for the usage of "GetUserCourses". Expect: Only valid usages with the updated implementation. rg --type ts --type js -A 5 $'GetUserCourses'Length of output: 11206
packages/graphql/src/public/server.json (1)
139-140: LGTM!The addition of the
isGroupCreationEnabledfield to theuserCoursesobject in theGetUserCoursesquery looks good. It provides more information about each user course, specifically whether group creation is enabled for that course. This change aligns with the PR objective to limit group activity creation to courses where group creation is enabled.The change is additive and does not break existing functionality.
packages/i18n/messages/en.ts (2)
208-209: LGTM!The new localized strings
withGroupsandwithoutGroupshave been added correctly.
1163-1163: Looks good!The modified
groupActivityNoCoursestring provides clearer instructions to users about the requirements for creating group activities.packages/i18n/messages/de.ts (3)
208-209: LGTM!The code changes are approved. The new entries
withGroupsandwithoutGroupsenhance the German localization for group-related functionalities.
1170-1171: LGTM!The code changes are approved. The updated
groupActivityNoCoursemessage provides clearer instructions to the user about the prerequisites for creating group activities.
Line range hint
1173-1174: LGTM!The code changes are approved. The new
groupActivityIntroductionNamemessage provides helpful guidance to the user when creating a group activity.packages/graphql/src/ops.ts (2)
3268-3268: LGTM!The code changes are approved.
3519-3519: LGTM!The code changes are approved.
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
limit-group-activity-creation
|
| Run status |
|
| Run duration | 09m 06s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
43
|
| View all changes introduced in this branch ↗︎ | |
klicker-uzh
|
||||||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3
|
| Run status |
|
| Run duration | 09m 24s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
1
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
42
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/K-group-activity-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Create and solve a group activity > create a group activity and edit it |
Screenshots
|
|
No description provided.