-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: User inactiveReason
#37224
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
feat: User inactiveReason
#37224
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 7cf2410 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds an optional user Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant UI as Admin UI
participant API as API Route
participant UsersSvc as Users Service
participant DB as Database
Note over Admin,DB: Create user when manual approval enabled
Admin->>UI: Submit new user
UI->>API: POST /users
API->>UsersSvc: createUser(payload)
UsersSvc->>DB: insert { active:false, inactiveReason:'pending_approval' }
DB-->>UsersSvc: created
UsersSvc-->>API: created
API-->>UI: success
rect `#DCEBFF`
Note over UI,API: List Pending tab uses inactiveReason filter
UI->>API: GET /users/list-by-status?status=deactivated&inactiveReason=['pending_approval']
API->>UsersSvc: findPaginatedUsersByStatus(..., inactiveReason)
UsersSvc->>DB: query inactiveReason IN ['pending_approval'] OR legacy (no field)
DB-->>UsersSvc: results
UsersSvc-->>API: users
API-->>UI: render Pending tab
end
rect `#E6FFE6`
Note over UI,API: Approve user (activate)
Admin->>UI: Activate user
UI->>API: PATCH /users/:id active=true
API->>UsersSvc: setUserActive(id, true)
UsersSvc->>DB: update { active:true, $unset:{ inactiveReason:1 } }
DB-->>UsersSvc: updated
UsersSvc-->>API: updated
API-->>UI: refresh lists
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37224 +/- ##
===========================================
+ Coverage 68.82% 68.88% +0.05%
===========================================
Files 3361 3361
Lines 114265 114329 +64
Branches 20619 20643 +24
===========================================
+ Hits 78646 78753 +107
+ Misses 33530 33492 -38
+ Partials 2089 2084 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
cc1b51a to
c914302
Compare
329ef07 to
8fa1cac
Compare
c914302 to
88171c2
Compare
88171c2 to
a8079bc
Compare
KevLehman
left a comment
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.
I'm still not happy with the changeset 🙈
We also agreed with Yash to try some improvements on the queries for that page in a future task.
tassoevan
left a comment
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.
The disagreement over the changeset is rooted under the nature of this modification: it started as a simple bug report/support request but evolved into introducing a new concept (reason for a user's inactive state). @KevLehman and @gabriellsh made valid points suggesting this is a fix, but IMHO it's a feature and the changeset description must be rephrased as such. I stand over previously "What's New" entries to affirm it.
91ec45d
Proposed changes (including videos or screenshots)
A user can be deactivated for multiple reasons, which was causing some issues with new user approval flow with pending user states. This PR introduces new field on
IUser,inactiveReasonwhich specifies the exact reason for a deactivated user.Issue(s)
Steps to test or reproduce
Further comments
SUP-835
Summary by CodeRabbit
Bug Fixes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.