fix(user): allow unblocking users promoted to admin - #39192
Open
loveulvu wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change cleanly separates “effective blocked status” from “blocking relationship exists” and includes a targeted regression test for the reported failure mode.
Pull request overview
This PR fixes an edge case in the user blocking feature where a previously-blocked user promoted to admin could no longer be unblocked because admin users are intentionally treated as “not blocked” by IsUserBlockedBy.
Changes:
- Introduces
user_model.HasBlockingto check for an existing persisted blocking relationship independent of admin status. - Updates
CanUnblockUserto useHasBlockingso unblock can proceed even if the blockee is now an admin. - Adds a regression test ensuring admins are still treated as not blocked while existing block relationships remain removable.
File summaries
| File | Description |
|---|---|
| services/user/block.go | Switches unblock eligibility to check the persisted relationship (HasBlocking) rather than effective blocked status (IsUserBlockedBy). |
| services/user/block_test.go | Adds regression coverage for unblock-after-promotion-to-admin behavior. |
| models/user/block.go | Refactors IsUserBlockedBy to delegate to new HasBlocking, preserving admin “not blocked” semantics. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #39189.
IsUserBlockedByintentionally treats admin users as not blocked, butCanUnblockUserwas also using it to determine whether a blocking relationship exists. If a previously blocked user is later promoted to admin, the existinguser_blockingrecord remains but can no longer be removed.This change separates those two concerns by adding
HasBlockingfor checking the persisted blocking relationship.CanUnblockUseruses that relationship check whileIsUserBlockedBykeeps its existing admin-user behavior.A regression test verifies that an admin is still not considered blocked while an existing blocking relationship can still be unblocked.
Tests:
go test ./models/user ./services/user -count=1