-
Notifications
You must be signed in to change notification settings - Fork 11
fix(classes): reset user data when removing congregation #1646
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
Conversation
WalkthroughThis update modifies the logic for removing a user from a congregation, introducing role-based handling. If the user's role is 'vip', their congregation and related data are cleared; if the role is 'pocket', the user is deleted entirely. Additional cleanup steps are added to the congregation removal process. Changes
Sequence Diagram(s)sequenceDiagram
participant AdminController
participant User
participant UsersList
participant Congregation
AdminController->>User: get user by ID
alt user.role == 'vip'
AdminController->>User: removeCongregation()
User->>User: Clear congregation, settings, sessions, reports
User->>Congregation: reloadMembers()
else user.role == 'pocket'
AdminController->>UsersList: delete(user.id)
end
AdminController->>Congregation: reloadMembers()
AdminController-->>AdminController: return updated user list
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ 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 (
|
|
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
🧹 Nitpick comments (1)
src/v3/controllers/admin_controller.ts (1)
859-865
: Consistent role-based logic maintained across controllers.The implementation matches the logic in
congregation_admin_controller.ts
exactly, ensuring consistent behavior regardless of which admin interface is used to remove users. This consistency is crucial for maintaining predictable system behavior.Consider extracting this role-based removal logic into a shared utility function to reduce code duplication:
// utils/user_removal_utils.ts export async function removeUserBasedOnRole(user: User) { if (user.profile.role === 'vip') { await user.removeCongregation(); } if (user.profile.role === 'pocket') { await UsersList.delete(user.id); } }This would centralize the logic and make future changes easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/v3/classes/User.ts
(1 hunks)src/v3/controllers/admin_controller.ts
(1 hunks)src/v3/controllers/congregation_admin_controller.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/v3/controllers/congregation_admin_controller.ts (1)
src/v3/classes/Users.ts (1)
UsersList
(139-139)
src/v3/controllers/admin_controller.ts (1)
src/v3/classes/Users.ts (1)
UsersList
(139-139)
🔇 Additional comments (2)
src/v3/classes/User.ts (1)
306-320
: Comprehensive data cleanup implementation looks good.The enhanced
removeCongregation
method now performs thorough cleanup by:
- Resetting all user settings to empty strings
- Clearing all active sessions (good security practice)
- Removing all user-specific data (reports, studies, delegated reports)
This ensures users have a clean slate when removed from a congregation, which aligns with the PR objective to "reset user data when removing congregation."
src/v3/controllers/congregation_admin_controller.ts (1)
729-735
: Role-based user removal logic implemented correctly.The conditional approach is well-designed:
vip
users: CallsremoveCongregation()
which performs comprehensive cleanup while preserving the user accountThis differentiation makes sense for different user account types and aligns with the enhanced cleanup logic in the User class.
🎉 This PR is included in version 3.27.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.