Skip to content

fix(group): block admins from leaving a group with clear in-app guidance#7

Merged
jgmontoya merged 1 commit into
masterfrom
feat/admin-leave-flow
May 19, 2026
Merged

fix(group): block admins from leaving a group with clear in-app guidance#7
jgmontoya merged 1 commit into
masterfrom
feat/admin-leave-flow

Conversation

@jgmontoya

@jgmontoya jgmontoya commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • New Features
    • Admin users attempting to leave a group now receive an info popup explaining they must step down as admin first. A distinct message appears if the user is the only admin.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown

Walkthrough

The PR prevents group admins from leaving a group without stepping down. It exposes the is_admin helper function as public, adds an early admin check to the group-detail "L" key handler, and adds test coverage for the admin-leave and non-admin-leave flows with context-appropriate popup messages.

Changes

Admin-Leave Protection

Layer / File(s) Summary
Expose is_admin helper
src/screen/group_detail.rs
The is_admin function is made public and its parameter documentation is updated from "npub" to "pubkey", enabling safe public use.
Block admin users from leaving group
src/app.rs
The L key handler in handle_group_detail_key adds an early admin check; admins see an Info popup with a context-specific message (only admin or step down first), while non-admins continue to receive the leave confirmation. Three new tests verify both admin and non-admin flows.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: blocking admins from leaving groups with in-app guidance. It accurately reflects the primary functionality added across both modified files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/admin-leave-flow

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/screen/group_detail.rs (1)

22-31: 💤 Low value

The npub field check is not dead code; the documentation is misleading.

The parameter pubkey actually receives bech32-encoded npub values (from member_npub() and self.account), not raw hex as the doc comment claims. The npub field check on line 30 is a live code path that matches when the admins list contains objects with npub fields.

However, the doc comment saying "hex, as returned by the daemon" is inaccurate. Consider clarifying that the function matches bech32-encoded npub values or raw pubkey strings, not hex.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/screen/group_detail.rs` around lines 22 - 31, The doc comment for
is_admin is inaccurate: the pubkey parameter can be a bech32-encoded npub (from
member_npub() or self.account) as well as raw pubkey strings, so update the
documentation to reflect that the function matches either bech32 npub values or
raw pubkey strings (and that admins may contain bare strings or objects with
pubkey/npub fields); leave the implementation (is_admin) unchanged but change
the comment to something like “pubkey (bech32 npub or raw pubkey string)”.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/screen/group_detail.rs`:
- Around line 22-31: The doc comment for is_admin is inaccurate: the pubkey
parameter can be a bech32-encoded npub (from member_npub() or self.account) as
well as raw pubkey strings, so update the documentation to reflect that the
function matches either bech32 npub values or raw pubkey strings (and that
admins may contain bare strings or objects with pubkey/npub fields); leave the
implementation (is_admin) unchanged but change the comment to something like
“pubkey (bech32 npub or raw pubkey string)”.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf76e12b-d619-4e43-9f00-d44874d26ef3

📥 Commits

Reviewing files that changed from the base of the PR and between e342a7c and 7511b4a.

📒 Files selected for processing (2)
  • src/app.rs
  • src/screen/group_detail.rs

@jgmontoya
jgmontoya merged commit f9d6016 into master May 19, 2026
5 checks passed
@jgmontoya
jgmontoya deleted the feat/admin-leave-flow branch May 19, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants