feat: add possibility to extend microlearnings and group activities before end date #4283
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.
|
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces enhancements to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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 (
|
klicker-uzh
|
||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
extend-activities
|
| Run status |
|
| Run duration | 09m 59s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
2
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
47
|
| View all changes introduced in this branch ↗︎ | |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Outside diff range and nitpick comments (9)
packages/graphql/src/graphql/ops/MExtendGroupActivity.graphql (1)
1-1: Consider adding a comment to explain the mutation's purpose.To improve code readability and maintainability, it would be helpful to add a brief comment explaining the purpose of this mutation and any important details about its usage.
Example:
# Extends the end date of a group activity # id: The unique identifier of the group activity # endDate: The new end date for the group activity mutation ExtendGroupActivity($id: String!, $endDate: Date!) { # ... (rest of the mutation) }packages/graphql/src/graphql/ops/MExtendMicroLearning.graphql (1)
1-6: LGTM! Consider adding a comment for clarity.The
ExtendMicroLearningmutation is well-structured and aligns with the PR objective. It correctly takes anidandendDateas parameters, and returns the updatedidandscheduledEndAt.Consider adding a brief comment at the top of the file to explain the purpose of this mutation, for example:
# Mutation to extend the end date of a microlearning mutation ExtendMicroLearning($id: String!, $endDate: Date!) { # ... rest of the mutation }This would enhance code readability and provide context for other developers.
packages/graphql/src/public/schema.graphql (1)
776-777: LGTM! Consider adding descriptions for new mutations.The new mutations
extendGroupActivityandextendMicroLearningare well-implemented and consistent with the PR objectives. They follow the existing naming conventions and use appropriate argument and return types.For improved documentation and consistency with other mutations in the schema, consider adding descriptions to these new mutations. For example:
+ """ + Extends the end date of a group activity. + """ extendGroupActivity(endDate: Date!, id: String!): GroupActivity + """ + Extends the end date of a micro learning. + """ extendMicroLearning(endDate: Date!, id: String!): MicroLearningpackages/graphql/src/services/groups.ts (1)
1690-1708: LGTM! Consider adding a check for the minimum extension period.The
extendGroupActivityfunction looks good overall. It correctly updates thescheduledEndAtfield of the group activity, ensuring that only the owner can extend it and that the current end date is in the future. However, there are a couple of suggestions for improvement:
Consider adding a check to ensure the new
endDateis later than the currentscheduledEndAt. This would prevent accidental shortening of the activity period.You might want to add a minimum extension period (e.g., 1 hour) to prevent very small extensions that might not be meaningful.
Here's a suggested implementation with these improvements:
export async function extendGroupActivity( { id, endDate, }: { id: string endDate: Date }, ctx: ContextWithUser ) { + const currentActivity = await ctx.prisma.groupActivity.findUnique({ + where: { id, ownerId: ctx.user.sub, scheduledEndAt: { gt: new Date() } }, + select: { scheduledEndAt: true }, + }); + + if (!currentActivity) { + throw new Error('Group activity not found or not eligible for extension'); + } + + const minExtensionPeriod = 60 * 60 * 1000; // 1 hour in milliseconds + if (endDate.getTime() - currentActivity.scheduledEndAt.getTime() < minExtensionPeriod) { + throw new Error('Extension period must be at least 1 hour'); + } const updatedGroupActivity = await ctx.prisma.groupActivity.update({ where: { id, ownerId: ctx.user.sub, scheduledEndAt: { gt: new Date() } }, data: { scheduledEndAt: endDate, }, }); return updatedGroupActivity; }This implementation adds checks for the current activity's existence and ensures a minimum extension period.
packages/graphql/src/public/server.json (2)
42-42: LGTM! Consider adding a success flag to the response.The
ExtendGroupActivitymutation is well-structured and includes the necessary fields. It correctly uses non-nullable types for required parameters.Consider adding a
successboolean field to the response to explicitly indicate whether the operation was successful. This can be helpful for error handling on the client side.mutation ExtendGroupActivity($id: String!, $endDate: Date!) { extendGroupActivity(id: $id, endDate: $endDate) { id scheduledEndAt + success __typename } }
43-43: LGTM! Consider adding a success flag to the response.The
ExtendMicroLearningmutation is well-structured and consistent with theExtendGroupActivitymutation. This consistency is good for maintainability and ease of use.As with the
ExtendGroupActivitymutation, consider adding asuccessboolean field to the response:mutation ExtendMicroLearning($id: String!, $endDate: Date!) { extendMicroLearning(id: $id, endDate: $endDate) { id scheduledEndAt + success __typename } }packages/i18n/messages/en.ts (2)
1549-1554: New translations for extending microlearning look good.The added translations for extending microlearning are clear, concise, and consistent with the existing style. They provide necessary information for the user about the functionality.
A minor suggestion to improve clarity:
Consider modifying the
extendMicroLearningDescriptionto be more explicit about the current end date:extendMicroLearningDescription: - 'Use this dialogue to modify the end date of the microlearning. Please note that only future dates can be set as end dates.', + 'Use this dialogue to modify the end date of the microlearning. Please note that only dates after the current end date can be set.',This change makes it clearer that the new end date must be after the existing end date, not just any future date.
1592-1594: New translations for extending group activity are consistent.The added translations for extending group activity mirror those for microlearning, maintaining consistency across similar features.
However, to maintain absolute consistency with the microlearning translations (if that's desired), consider this minor adjustment:
extendGroupActivityDescription: - 'Use this dialogue to modify the end date of the group activity. Please note that only future dates can be set as end dates.', + 'Use this dialogue to modify the end date of the group activity. Please note that only dates after the current end date can be set.',This change would make the description identical to the suggested microlearning description, ensuring perfect consistency between similar features.
packages/graphql/src/ops.schema.json (1)
10822-10911: Overall assessment: Well-implemented schema changes with a minor suggestionThe additions to the schema are consistent, well-implemented, and align perfectly with the PR objective. Both
extendGroupActivityandextendMicroLearningmutations provide the necessary functionality to extend group activities and microlearnings before their end dates. The use of non-null types for arguments ensures data integrity, and the return types are appropriate.As a minor suggestion for improvement:
Consider adding descriptions to these new mutations and their arguments. This would enhance the self-documentation of the schema and improve the developer experience when working with these new mutations.
For example:
"Extends the end date of a group activity" extendGroupActivity( "The new end date for the group activity" endDate: Date!, "The ID of the group activity to extend" id: String! ): GroupActivity "Extends the end date of a microlearning" extendMicroLearning( "The new end date for the microlearning" endDate: Date!, "The ID of the microlearning to extend" id: String! ): MicroLearningThis addition would make the schema more self-explanatory and easier to use for other developers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (17)
- apps/frontend-manage/src/components/courses/GroupActivityElement.tsx (5 hunks)
- apps/frontend-manage/src/components/courses/MicroLearningElement.tsx (8 hunks)
- apps/frontend-manage/src/components/courses/modals/ExtensionModal.tsx (1 hunks)
- cypress/cypress/e2e/G-microlearning-workflow.cy.ts (1 hunks)
- cypress/cypress/e2e/K-group-activity-workflow.cy.ts (1 hunks)
- packages/graphql/src/graphql/ops/MExtendGroupActivity.graphql (1 hunks)
- packages/graphql/src/graphql/ops/MExtendMicroLearning.graphql (1 hunks)
- packages/graphql/src/ops.schema.json (1 hunks)
- packages/graphql/src/ops.ts (4 hunks)
- packages/graphql/src/public/client.json (1 hunks)
- packages/graphql/src/public/schema.graphql (1 hunks)
- packages/graphql/src/public/server.json (1 hunks)
- packages/graphql/src/schema/mutation.ts (2 hunks)
- packages/graphql/src/services/groups.ts (1 hunks)
- packages/graphql/src/services/microLearning.ts (1 hunks)
- packages/i18n/messages/de.ts (2 hunks)
- packages/i18n/messages/en.ts (2 hunks)
🔇 Additional comments (26)
packages/graphql/src/graphql/ops/MExtendGroupActivity.graphql (1)
1-6: LGTM! Well-structured GraphQL mutation.The
ExtendGroupActivitymutation is well-defined with appropriate parameters and return fields. It follows GraphQL best practices and provides a clear interface for extending a group activity's end date.packages/graphql/src/public/client.json (1)
42-43: New operations added for extending group activities and micro-learnings.Two new entries have been added to the client.json file:
- "ExtendGroupActivity" with hash "c3788f2608170940bedbec90fe77a623fb05249e20cd11ad2e4a529b5068149a"
- "ExtendMicroLearning" with hash "bf40877047d36fc8549bf3050e75e858066f7a8515c97ad1207af5c3c3e227a6"
These additions align with the PR objective of adding the possibility to extend microlearnings and group activities before the end date.
To ensure these are the only changes and that they are correctly placed within the file, let's run the following script:
✅ Verification successful
Verification Successful: New Entries Correctly Added
- The new entries "ExtendGroupActivity" and "ExtendMicroLearning" are present and correctly placed.
- No unexpected changes found related to "Extend".
- Total number of entries is consistent with the addition of these two entries.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new entries and their placement in the file # Test 1: Check if the new entries exist and are correctly placed rg -A 1 -B 1 "ExtendGroupActivity" packages/graphql/src/public/client.json rg -A 1 -B 1 "ExtendMicroLearning" packages/graphql/src/public/client.json # Test 2: Ensure no other unexpected changes were made rg "Extend" packages/graphql/src/public/client.json # Test 3: Verify the total number of entries in the file jq 'length' packages/graphql/src/public/client.jsonLength of output: 1008
cypress/cypress/e2e/K-group-activity-workflow.cy.ts (1)
662-715: Overall assessment of the new test caseThe new test case for extending a group activity is a valuable addition to the test suite. It covers the main scenarios for the extension feature, including successful extensions and handling of invalid dates.
To further improve this test:
- Consider implementing the suggested refactorings to improve code organization and reduce duplication.
- Add more assertions to verify UI feedback and state changes.
- If possible, extend the test to verify the impact of the extension on student access to the activity.
These enhancements will make the test more robust, maintainable, and comprehensive, ensuring the group activity extension feature works as expected across various scenarios.
cypress/cypress/e2e/G-microlearning-workflow.cy.ts (3)
1355-1357: LGTM: Test case setup is correct.The test case is properly defined using the
itfunction, and themicroLearningNameconstant is correctly initialized.
1358-1361: LGTM: Navigation to course overview is well-implemented.The test correctly navigates to the courses page and selects the specific course using appropriate data-cy attributes.
1362-1369: LGTM: Extension modal interaction is comprehensive.The test thoroughly checks the extension modal functionality, including:
- Navigating to the microlearnings tab
- Opening the actions menu
- Clicking the extend option
- Testing the cancel functionality
- Reopening the modal
This ensures a robust test of the user interaction flow.
packages/graphql/src/public/server.json (1)
42-43: Overall, good additions. Consider updating related queries and documentation.The new mutations
ExtendGroupActivityandExtendMicroLearningare well-structured and consistent with each other. They provide valuable functionality for extending end dates of activities.To fully integrate these new mutations:
- Ensure that the backend resolvers for these mutations are implemented correctly and handle potential edge cases (e.g., extending to a date earlier than the current end date).
- Update any related queries or subscriptions that fetch group activities or micro learnings to reflect the possibility of extended end dates.
- Update the API documentation to include these new mutations and their usage.
- Consider adding integration tests to verify the behavior of these mutations in various scenarios.
To check for any existing queries or subscriptions that might need updating, we can search for references to
scheduledEndAtin group activities and micro learnings:✅ Verification successful
Verified: Related queries and subscriptions already reference
scheduledEndAt.The existing queries and subscriptions appropriately utilize the
scheduledEndAtfield for both group activities and micro learnings. No additional updates to related queries are necessary at this time.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash echo "Searching for queries/subscriptions referencing scheduledEndAt in group activities:" rg --type=json 'query.*groupActivit.*scheduledEndAt' echo "\nSearching for queries/subscriptions referencing scheduledEndAt in micro learnings:" rg --type=json 'query.*microLearning.*scheduledEndAt'Length of output: 10024
packages/i18n/messages/en.ts (1)
Line range hint
1-1594: Overall, the new translations are well-integrated and consistent.The additions for extending microlearnings and group activities are clear and fit well within the existing translation structure. They provide necessary information for users about the new functionality.
Two minor suggestions were made to improve clarity and maintain consistency between similar features. These changes are not critical but could enhance the user experience slightly.
packages/graphql/src/ops.schema.json (2)
10823-10866: LGTM:extendGroupActivitymutation is well-definedThe
extendGroupActivitymutation is correctly implemented with appropriate non-null arguments (endDateandid) and return type (GroupActivity). This aligns well with the PR objective of adding the possibility to extend group activities before the end date.
10868-10911: LGTM:extendMicroLearningmutation is well-definedThe
extendMicroLearningmutation is correctly implemented with appropriate non-null arguments (endDateandid) and return type (MicroLearning). This aligns well with the PR objective of adding the possibility to extend microlearnings before the end date. The structure is consistent with theextendGroupActivitymutation, which is a good practice.apps/frontend-manage/src/components/courses/modals/ExtensionModal.tsx (2)
1-148: Overall, the component is well-implemented and follows best practicesThe
ExtensionModalcomponent is well-structured, with clear separation of concerns. The use of Formik for form handling, Yup for validation, and Apollo Client for GraphQL mutations aligns with industry best practices. Styling and internationalization are also appropriately managed.
62-63: Verify the date format for the initial end dateWhen setting the initial value for
endDate, ensure that the date format matches the expected input format ofFormikDateField. This prevents any potential issues with date parsing or incorrect default values.Run the following script to check the expected date format:
apps/frontend-manage/src/components/courses/GroupActivityElement.tsx (2)
2-6: ImportingfaCalendaricon for new functionalityThe addition of
faCalendarto the imports is appropriate for the new extension feature.
309-318: Integration ofExtensionModalcomponentThe
ExtensionModalcomponent is correctly integrated with the appropriate props, ensuring users can extend group activities seamlessly.apps/frontend-manage/src/components/courses/MicroLearningElement.tsx (6)
41-41:ExtensionModalimport added correctlyThe
ExtensionModalcomponent is imported appropriately, allowing for its use within this file.
64-64: State management forextensionModalis properly initializedThe state variable
extensionModalis correctly initialized usinguseState(false), which will control the visibility of the extension modal.
74-74: Logical determination ofisActivestatus is correctThe
isActivevariable accurately assesses whether the current date falls between the scheduled start and end dates of the microlearning, which is essential for conditionally rendering components based on the active status.
278-298: Extend option appropriately added for active microlearningsThe addition of the "Extend MicroLearning" option in the dropdown menu when
isActiveis true is correctly implemented. This allows users to extend microlearnings that are currently active.
367-376: Integration ofExtensionModalcomponent with correct propsThe
ExtensionModalis integrated into the component with all necessary props, includingtype,id,currentEndDate, and control handlers foropenandsetOpen. This ensures that the modal functions correctly when triggered.
2-6: Verify icon imports for consistencyThe icons
faCalendar,faClock, andfaTrashCanare being imported from@fortawesome/free-regular-svg-icons. Ensure that these icons are available in thefree-regular-svg-iconspackage and confirm that this is consistent with how other icons are imported in the project. If these icons are typically imported from@fortawesome/free-solid-svg-icons, consider updating the import statement for consistency.Run the following script to check where these icons are imported from in the codebase:
✅ Verification successful
Icon imports are consistent with the majority of the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the import sources of faCalendar, faClock, and faTrashCan icons. # Search for import statements of the specified icons in the codebase rg "import\s+\{[^}]*\b(faCalendar|faClock|faTrashCan)\b[^}]*\}\s+from\s+'@fortawesome/free-(solid|regular)-svg-icons'" --type js --type tsLength of output: 2017
packages/graphql/src/schema/mutation.ts (1)
1292-1304: EnsureendDatevalidation inextendGroupActivitymutationThe
extendGroupActivitymutation allows users to extend theendDateof a group activity. Please verify thatendDateis properly validated withinGroupService.extendGroupActivityto prevent setting anendDatethat is in the past or before the originalendDate.Run the following script to verify that
endDatevalidation is implemented:#!/bin/bash # Description: Verify that `extendGroupActivity` enforces proper `endDate` validation. # Test: Search for validation logic in the `extendGroupActivity` method. Expect to find validation for `endDate`. ast-grep --lang typescript --pattern $'export async function extendGroupActivity($_args, $_ctx) { $$$ }' packages/graphql/src/services/groups.ts | xargs rg 'validate.*endDate'packages/i18n/messages/de.ts (2)
1568-1572: Translations are accurate and consistent.The new keys for extending microlearning sessions are correctly added with appropriate German translations, maintaining consistency with existing entries.
1611-1613: Addition of group activity extension translations is correct.The translations for extending group activities are accurately provided and align well with the existing localization structure.
packages/graphql/src/ops.ts (3)
847-848: Ensure Consistency in Mutation OptionalityThe mutations
extendGroupActivityandextendMicroLearning(lines 847-848) are defined as optional (with?), whereasfinalRandomGroupAssignmentsis defined without the optional marker. Please verify if the optionality of these mutations is intentional and consistent with other mutations in the schema.
1216-1225: Approve Mutation Argument DefinitionsThe new mutation argument types
MutationExtendGroupActivityArgs(lines 1216-1219) andMutationExtendMicroLearningArgs(lines 1222-1225) are correctly defined with the requiredidandendDatefields of appropriate types.
2775-2790: Approve Addition of Mutation TypesThe definitions for
ExtendGroupActivityMutationVariables,ExtendGroupActivityMutation,ExtendMicroLearningMutationVariables, andExtendMicroLearningMutation(lines 2775-2790) correctly specify the variables and response types for the new mutations. This aligns with the existing patterns in the codebase.
apps/frontend-manage/src/components/courses/GroupActivityElement.tsx
Outdated
Show resolved
Hide resolved
apps/frontend-manage/src/components/courses/GroupActivityElement.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/i18n/messages/de.ts (2)
Line range hint
1569-1614: Consider grouping related translations for better organization.While the new translations are correctly placed within the
coursesection, consider grouping all microlearning-related translations together and all group activity-related translations together. This could improve readability and make future updates easier.For example:
microlearning: { edit: '...', duplicate: '...', extend: '...', extendDescription: '...', newEndDate: '...', futureEndDateRequired: '...', publish: '...', unpublish: '...', convert: '...', delete: '...', }, groupActivity: { edit: '...', delete: '...', publish: '...', unpublish: '...', extend: '...', extendDescription: '...', grade: '...', },This structure would make it easier to locate and maintain related translations.
Line range hint
1-1614: Consider implementing a more structured approach to translation management.While the current structure of the translation file is functional, as the application grows, it may become challenging to maintain. Consider the following suggestions to improve organization and maintainability:
Split the translations into multiple files based on major sections of the application (e.g.,
auth.ts,manage.ts,pwa.ts, etc.). This would make it easier to locate and update specific translations.Implement a consistent naming convention for keys, such as using camelCase for all keys or separating words with underscores.
Add comments to group related translations or provide context for complex phrases.
Consider using a translation management system that can help maintain consistency across different languages and provide additional features like version control and collaboration tools.
Implement type checking for translations to catch missing or extra keys early in the development process.
These changes could significantly improve the long-term maintainability of the translation files as the application continues to grow and evolve.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- packages/graphql/src/ops.ts (4 hunks)
- packages/graphql/src/public/client.json (1 hunks)
- packages/graphql/src/public/server.json (1 hunks)
- packages/i18n/messages/de.ts (2 hunks)
- packages/i18n/messages/en.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/graphql/src/public/client.json
- packages/graphql/src/public/server.json
- packages/i18n/messages/en.ts
🔇 Additional comments (6)
packages/i18n/messages/de.ts (2)
1569-1573: New translations for extending microlearnings look good.The newly added translations for extending microlearnings are clear and consistent with the existing style. They provide appropriate German translations for the functionality of extending microlearnings.
1612-1614: New translations for extending group activities are consistent.The translations added for extending group activities follow the same pattern as those for microlearnings. They are clear and maintain consistency with the existing translations.
packages/graphql/src/ops.ts (4)
847-848: LGTM!The added mutations
extendGroupActivityandextendMicroLearningare correctly defined in theMutationtype.
1216-1226: LGTM!The argument types for
extendGroupActivityandextendMicroLearningmutations are properly specified.
2775-2790: LGTM!The type definitions for the new mutations and their variables are correctly implemented.
3718-3719: Refactor Large JSON Literals for ReadabilityThe previous suggestion to refactor large JSON literals for better readability remains applicable.
…d group activity lie in future
|
klicker-uzh
|
||||||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3
|
| Run status |
|
| Run duration | 09m 25s |
| Commit |
|
| Committer | Julius Schlapbach |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
1
|
|
Flaky
|
0
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
46
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/E-course-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Test course creation and editing functionalities > Test the assignment of random groups in the seeded test course |
Test Replay
Screenshots
|
|
With this PR, it is now possible to extend running microlearnings and group activities through a corresponding modal on the course overview.