BUM: Improved user creation #13538
BUM: Improved user creation #13538
Conversation
github-actions
bot
added
DEV: backend
AlexVelezLl
force-pushed
the
improved-user-creation
branch
from
0c2a054 to
b276fd7
Compare
AlexVelezLl
force-pushed
the
improved-user-creation
branch
from
b276fd7 to
b4728e9
Compare
Build Artifacts
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis change restructures the user creation workflow in the facility user management module. It introduces a new "New Users" page and side panel for streamlined user creation and bulk actions, adds date-based filtering for recently added users, removes legacy user management components, and updates constants, routes, composables, and supporting utilities to support the new workflow and UI. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🧹 Nitpick comments (4)
kolibri/plugins/facility/assets/src/views/users/NewUsersPage.vue (1)
126-127: Consider making the threshold configurableThe hardcoded
MAX_NEW_USER_DAYS = 30might be better as a configurable constant, potentially moved to a configuration file or made into a facility setting, to allow administrators to adjust the "new user" period.-// Constant for the maximum number of days to consider a user as a "new user" -const MAX_NEW_USER_DAYS = 30; +// TODO: Consider making this configurable via facility settings +const MAX_NEW_USER_DAYS = 30;kolibri/plugins/facility/assets/src/utils.js (1)
33-48: Consider adding defensive checks tooverrideRoutefunction.The function correctly merges route parameters and queries. However, consider adding validation to ensure
routeandnewRouteare valid objects.export function overrideRoute(route, newRoute) { // Override the route with a new one, preserving the params and query + if (!route || !newRoute) { + logging.error('Invalid route objects provided to overrideRoute'); + return newRoute || route || {}; + } const { params, query } = route; return { ...newRoute, params: { ...params, ...newRoute.params, }, query: { ...query, ...newRoute.query, }, }; }kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/ClassesSelect.vue (1)
119-123: Remove unused CSS class.The
.selected-chipsclass is defined but not used in the template.- .selected-chips { - display: flex; - flex-wrap: wrap; - gap: 8px; - }kolibri/plugins/facility/assets/src/views/users/common/UsersTable.vue (1)
1-655: Consider splitting this large component for better maintainability.At 655 lines, this component handles many responsibilities. Consider extracting:
- User selection logic into a composable
- Table header configuration into a separate utility
- Modal handling into a separate component or composable
This would improve maintainability and reusability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
kolibri/core/auth/api.py(3 hunks)kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/index.vue(2 hunks)kolibri/plugins/facility/assets/src/composables/useUserManagement.js(3 hunks)kolibri/plugins/facility/assets/src/constants.js(1 hunks)kolibri/plugins/facility/assets/src/modules/pluginModule.js(2 hunks)kolibri/plugins/facility/assets/src/routes.js(2 hunks)kolibri/plugins/facility/assets/src/utils.js(3 hunks)kolibri/plugins/facility/assets/src/views/UserCreatePage.vue(0 hunks)kolibri/plugins/facility/assets/src/views/UserEditPage.vue(1 hunks)kolibri/plugins/facility/assets/src/views/UserPage/index.vue(0 hunks)kolibri/plugins/facility/assets/src/views/users/NewUsersPage.vue(1 hunks)kolibri/plugins/facility/assets/src/views/users/UsersRootPage/__test__/UsersRootPage.spec.js(1 hunks)kolibri/plugins/facility/assets/src/views/users/UsersRootPage/index.vue(1 hunks)kolibri/plugins/facility/assets/src/views/users/common/DeleteUserModal.vue(2 hunks)kolibri/plugins/facility/assets/src/views/users/common/ResetUserPasswordModal.vue(2 hunks)kolibri/plugins/facility/assets/src/views/users/common/UsersTable.vue(1 hunks)kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/ClassesSelect.vue(1 hunks)kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/index.vue(1 hunks)packages/kolibri-common/components/PaginatedListContainerWithBackend.vue(1 hunks)packages/kolibri-common/components/labels/CoreInfoIcon/index.vue(1 hunks)packages/kolibri-common/components/userAccounts/FullNameTextbox.vue(1 hunks)packages/kolibri-common/components/userAccounts/PasswordTextbox.vue(1 hunks)packages/kolibri-common/components/userAccounts/UsernameTextbox.vue(1 hunks)packages/kolibri-common/strings/bulkUserManagementStrings.js(2 hunks)packages/kolibri-common/strings/enhancedQuizManagementStrings.js(0 hunks)packages/kolibri-common/uiText/userKinds.js(2 hunks)packages/kolibri/uiText/commonCoreStrings.js(2 hunks)
💤 Files with no reviewable changes (3)
- packages/kolibri-common/strings/enhancedQuizManagementStrings.js
- kolibri/plugins/facility/assets/src/views/UserCreatePage.vue
- kolibri/plugins/facility/assets/src/views/UserPage/index.vue
🧰 Additional context used
🧬 Code Graph Analysis (3)
kolibri/plugins/facility/assets/src/modules/pluginModule.js (1)
kolibri/plugins/facility/assets/src/constants.js (2)
PageNames(1-26)PageNames(1-26)
packages/kolibri-common/uiText/userKinds.js (1)
packages/kolibri/uiText/commonCoreStrings.js (2)
coreStrings(9-1608)coreStrings(9-1608)
kolibri/plugins/facility/assets/src/routes.js (2)
kolibri/plugins/facility/assets/src/utils.js (1)
getSidePanelRoutes(82-92)kolibri/plugins/facility/assets/src/constants.js (2)
PageNames(1-26)PageNames(1-26)
🔇 Additional comments (42)
packages/kolibri-common/components/labels/CoreInfoIcon/index.vue (1)
15-18: Verify KTooltip’sappendToOverlaypropWe weren’t able to locate KTooltip’s prop definitions in the repository—if the installed KDS version doesn’t expose
appendToOverlay, it will silently no-op.• File: packages/kolibri-common/components/labels/CoreInfoIcon/index.vue
Lines: 15–18<KTooltip reference="icon" appendToOverlay :class="{ ltr: !isRtl }"Please confirm against the Kolibri Design System version in use that KTooltip defines a prop named
appendToOverlay. If it does not, update to the correct portal prop (e.g.appendToBody,appendTo) or bump the KDS dependency accordingly.kolibri/plugins/facility/assets/src/views/UserEditPage.vue (1)
157-157: Import path for IdentifierTextbox is correct and consistent
- Verified that
IdentifierTextbox.vueexists atkolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/IdentifierTextbox.vue.- The only other import is in
kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/index.vue, which correctly uses a relative import (./IdentifierTextbox).No further updates are required.
packages/kolibri-common/components/userAccounts/UsernameTextbox.vue (1)
100-105: LGTM! Clean implementation of reset functionality.The
resetmethod provides a clean way to reset the validation state by clearing theblurredflag. This is consistent with similar methods in other user account components and follows good practices for form reset functionality.packages/kolibri-common/components/userAccounts/FullNameTextbox.vue (1)
75-80: LGTM! Consistent reset functionality across user account components.The
resetmethod implementation is consistent with similar methods in other user account components and provides a clean way to reset the validation state.kolibri/plugins/facility/assets/src/views/users/common/ResetUserPasswordModal.vue (2)
74-74: Event emission standardization looks good.The change from 'cancel' to 'close' event emission after successful password reset is consistent with the modal's closing behavior pattern.
10-10: Verified: No remaining ‘@cancel’ listeners on ResetUserPasswordModal
- ResetUserPasswordModal is only used in UsersTable.vue.
- No instances of
@cancelhandlers remain.- UsersTable.vue correctly listens for
@close="closeModal".All parent components handle the new
closeevent.packages/kolibri-common/components/PaginatedListContainerWithBackend.vue (1)
4-4: LGTM! Good optimization for conditional rendering.The conditional rendering of the
<KGrid>component based on slot presence is a sensible optimization that prevents unnecessary DOM elements when no filters are provided. This improves performance and keeps the DOM clean.kolibri/plugins/facility/assets/src/views/users/common/DeleteUserModal.vue (1)
9-9: Good standardization of modal events.The change from
canceltoclosestandardizes modal closure handling, and adding thechangeevent after successful deletion enables proper reactive updates in parent components.Also applies to: 48-49
kolibri/plugins/facility/assets/src/views/users/UsersRootPage/__test__/UsersRootPage.spec.js (1)
5-5: Path adjustments align with directory restructuring.The import path updates correctly reflect the new directory structure after the user management components were reorganized.
Also applies to: 11-11
kolibri/plugins/coach/assets/src/views/quizzes/CreateExamPage/index.vue (1)
121-121: Good centralization of common UI strings.Moving the
saveAndClose$string fromenhancedQuizManagementStringstocoreStringspromotes consistency and reduces duplication of common UI text across modules.Also applies to: 172-172
packages/kolibri-common/components/userAccounts/PasswordTextbox.vue (1)
132-139: Well-implemented reset method for form state management.The new
reset()method provides a clean way to reset internal validation state without affecting the main password value. The implementation is consistent with similar methods in other user input components and properly documented.kolibri/core/auth/api.py (1)
34-34: Properly implemented date-based filtering for user management.The new
date_joined__gtanddate_joined__ltfilters are correctly implemented using Django REST framework patterns. The import, filter definitions, and Meta class registration are all properly done to support the new "New Users" feature.Also applies to: 320-327, 374-375
kolibri/plugins/facility/assets/src/composables/useUserManagement.js (3)
8-8: LGTM: Good refactor to options object patternThe change from a single
activeFacilityIdparameter to an options object with destructuring is a good practice that makes the function more extensible and self-documenting.
30-30: LGTM: Proper null safety with optional chainingThe use of optional chaining (
dateJoinedGt?.toISOString()) properly handles cases wheredateJoinedGtmight be undefined, and thepickByfunction will filter out null/undefined values.
65-68: LGTM: Enhanced watcher with deep comparisonUsing
isEqualfor deep comparison of filter arrays is appropriate and prevents unnecessaryfetchUserscalls when the filter values haven't actually changed.kolibri/plugins/facility/assets/src/modules/pluginModule.js (2)
29-29: LGTM: Consistent with route refactorRemoving the
USER_CREATE_PAGEexception from the condition is correct since this page is being replaced by the side panel approach. The logic now only preserves state when navigating fromUSER_MGMT_PAGEtoUSER_EDIT_PAGE.
85-85: LGTM: Updated to use new side panel routeThe route name change from
USER_CREATE_PAGEtoADD_NEW_USER_SIDE_PANEL__NEW_USERSis consistent with the new constants and the side panel-based user creation flow.packages/kolibri/uiText/commonCoreStrings.js (2)
138-142: LGTM: Well-documented translation stringsThe new translation keys are properly formatted with clear messages and helpful context for translators. The
saveAndClosestring supports the side panel workflow, whileadminsLabelandsuperAdminsLabelsupport the user role management features.
292-299: LGTM: Consistent role label stringsThe
adminsLabelandsuperAdminsLabelstrings follow the established pattern and provide clear context for their usage in user role management.kolibri/plugins/facility/assets/src/constants.js (2)
8-8: LGTM: Consistent page name constantThe
NEW_USERS_PAGEconstant follows the established naming pattern and supports the new "new users" functionality.
15-25: LGTM: Consistent side panel constantsThe new side panel constants with
__NEW_USERSsuffix follow the established pattern and enable context-specific side panels for the new users page. This approach maintains consistency while providing the necessary routing flexibility.kolibri/plugins/facility/assets/src/views/users/NewUsersPage.vue (6)
140-141: LGTM: Proper date calculation for filteringThe date threshold calculation correctly subtracts the number of days from the current date to create the filter for "new users". This works well with the
dateJoinedGtparameter in the composable.
151-154: LGTM: Proper composable integrationThe integration with the updated
useUserManagementcomposable correctly passes bothactiveFacilityIdanddateJoinedGtparameters, enabling the "new users" filtering functionality.
26-35: LGTM: Conditional rendering and proper propsThe conditional rendering based on
facilityUsers.lengthis appropriate, and the props passed toUsersTableare correctly structured. ThefilterPageNameprop correctly uses the new__NEW_USERSvariant.
79-99: LGTM: Well-designed empty stateThe empty state provides a clear message and appropriate call-to-action button, following good UX practices. The styling is clean and the content is helpful for users.
36-77: LGTM: Consistent bulk action implementationThe bulk action buttons for assigning coaches, enrolling in classes, removing from classes, and deleting users all follow the same pattern and correctly use the
__NEW_USERSvariants of the side panel routes.
208-238: LGTM: Clean and appropriate stylingThe scoped CSS provides clean, responsive styling for both the header and empty state. The use of flexbox and proper spacing creates a good user experience.
kolibri/plugins/facility/assets/src/utils.js (3)
6-11: LGTM! Import reorganization reflects new directory structure.The imports have been properly updated to reflect the new directory structure under
views/users/sidePanels/.
75-80: LGTM! User creation side panel route added correctly.The new route for
ADD_NEW_USER_SIDE_PANELis properly integrated into thesidePanelRoutesarray.
82-92: LGTM! Function signature updated to support route name suffixing.The change from variadic arguments to an array parameter with optional suffix is a good improvement for flexibility and type safety.
kolibri/plugins/facility/assets/src/views/users/UsersRootPage/index.vue (1)
1-214: LGTM! Well-structured user management component.The component is well-implemented with:
- Proper use of Vue 3 composition API
- Clean separation of concerns between template, script, and styles
- Appropriate integration with routing and state management
- Good use of composables for shared functionality
kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/ClassesSelect.vue (1)
1-116: LGTM! Well-implemented multi-select component.The component correctly handles:
- Multi-selection with "All Classes" option
- Proper state management with computed properties
- Clean event handling and prop validation
kolibri/plugins/facility/assets/src/routes.js (3)
15-16: LGTM! Import statements updated for new component structure.The imports correctly reflect the new user management components and their location.
79-93: LGTM! Route configuration updated correctly.The USER_MGMT_PAGE route now uses UsersRootPage component with proper side panel routes configuration.
96-114: LGTM! New route for recently created users.The NEW_USERS_PAGE route is properly configured with:
- Correct component and path
- Facility parameter guard
- Side panel routes with 'NEW_USERS' suffix for proper namespacing
kolibri/plugins/facility/assets/src/views/users/common/UsersTable.vue (2)
1-655: LGTM! Comprehensive user table implementation.The component is well-implemented with:
- Proper URL-based state management for pagination, filtering, and sorting
- Good use of Vue 3 composition API
- Effective debouncing for search input
- Proper cleanup in lifecycle hooks
- Comprehensive accessibility features
121-122: Verify empty KOptionalText usage for “Date Joined” columnI noticed that in UsersTable.vue the sixth column renders
<KOptionalText :text="''" />– resulting in an always-empty cell. This is likely meant to show each user’s
date_joinedvalue instead. Please:• Confirm what the
tableRowscomputed property is supplying at index 6.
• If it’s intended to display the join date, replace the empty string with the actual field, for example:<KOptionalText :text="row.date_joined" />• Otherwise, clarify why an empty placeholder is needed here.
packages/kolibri-common/uiText/userKinds.js (2)
1-14: Well-structured refactoring!The extraction of
getUserKindDisplayMapas a standalone function improves code reusability and follows the DRY principle. The function correctly handles the conditional logic for coach types and learner labels.
16-34: Clean integration with existing component APIThe computed property correctly uses the extracted function while maintaining the component's existing prop interface.
kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/index.vue (2)
320-324: Good handling of optional password requirementThe logic correctly sets the password to
NOT_SPECIFIEDwhen password input is not required based on facility configuration.
228-230: Fix type mismatch in kind initializationThe
kindref is initialized with an object fromuserTypeOptions, but should be initialized with just the first option to maintain consistency.-kind.value = userTypeOptions[0]; // Reset to Learner +kind.value = userTypeOptions[0];Note: The current implementation is actually correct since
kindis used as an object throughout the component (e.g.,kind.value.valueon lines 247 and 253).packages/kolibri-common/strings/bulkUserManagementStrings.js (1)
153-160: Well-structured localization stringsAll new strings follow the established patterns with clear messages and helpful context descriptions for translators. The naming conventions are consistent with the existing codebase.
Also applies to: 263-299
kolibri/plugins/facility/assets/src/views/users/sidePanels/UserCreate/index.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
I've given the code a solid first pass and nothing jumped out at me. I've also tested locally and things worked as expected. Great work @AlexVelezLl -- on to full QA!
|
An additional note of something to focus on, particularly with a11y in mind: Multiple-selection classes dropdown At the bottom of the creation form, you'll see a dropdown for assigning to classes which opens to present multiple options. I've tested this w/ keyboard nav successfully, but it'd be a great place for your focus. No need to focus on much outside the workflow itself for adding multiple users. See the notes below about existing known issues __ @AlexVelezLl I found two items of note after I submitted my review (cc @radinamatic so you're aware)
|
|
On the first cursory QA pass while testing in both Firefox and Chrome on Ubuntu, all workflows seem to be working as specified! 👍🏽 Two questions/comments:
bum-new-user.mp4 |
|
Thanks for the review @radinamatic - for your note (1) we could change the text on that button from "Close" to "Save & Close" when the form has been changed (and back when it's an empty form)? We won't block this PR on it but we can follow-up on this workflow. As for (2) it's just a page listing the users added in the last 30 days. Perhaps text explaining that to the user here could help? |
|
Hi @AlexVelezLl some additional observations:
assign.to.all.classes.mp4
2025-07-10_11-47-27.mp4
|
|
Thanks @pcenov!
|
There was a problem hiding this comment.
Code changes all look good to me - I tested things out locally and the issues @pcenov mentioned appear to be resolved 👍
There was a problem hiding this comment.
Excellent @AlexVelezLl - I have filed 2 follow-up issues as suggested and confirm that the ones that you mention as fixed are indeed fixed now.
…a learner or non learner user
AlexVelezLl
force-pushed
the
improved-user-creation
branch
from
b910dc3 to
0631dc8
Compare
|
Just noting I smoke tested after the rebase and confirmed that bulk enrollment still works as expected along w/ user creation and it was really nice doing both at once right now :) Good to merge once the checks are all green |
Summary
usersfolder to have a better file organization.identifierinfo icon tooltip in users table.Screencast
Screenshare.-.2025-07-08.10_15_35.AM.mp4
References
Closes #13410.
Reviewer guidance
save and add anotherandsave and continueactions