Skip to content

feat: prevent adminless group [WPB-25246]#21364

Open
e-maad wants to merge 10 commits into
devfrom
feat/adminless-group-WPB-25246
Open

feat: prevent adminless group [WPB-25246]#21364
e-maad wants to merge 10 commits into
devfrom
feat/adminless-group-WPB-25246

Conversation

@e-maad
Copy link
Copy Markdown
Contributor

@e-maad e-maad commented May 21, 2026

TaskWPB-25246 [Web] Admin Replacement Modal (Flow 1)

Summary

This PR introduces a safeguard for cases where the last admin of a group attempts to leave, ensuring groups are never left without an admin.

Changes Included

  • Added a new LeaveGroupAdminModal

    • Shown when the final admin tries to leave a group
    • Requires the user to either:
      • promote another participant to admin, or
      • delete the group
  • Added Zustand-based modal state management

    • Handles modal visibility
    • Tracks selected participant
    • Manages loading state
  • Implemented AdminSearchInput

    • Searchable participant selector for admin promotion
    • Supports optional clearing of group content
  • Added supporting styles and utilities

    • Modal/component styling
    • User filtering utility for search behavior
  • Integrated modal into existing leave-group flow

    • Added feature flag checks
    • Added eligible participant filtering for admin promotion
  • Added new i18n strings

    • Covers modal copy and related actions/messages

Result

Prevents groups from being left without an admin and improves the overall group management experience.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 62.13592% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.87%. Comparing base (4b93781) to head (174c795).
⚠️ Report is 56 commits behind head on dev.

Files with missing lines Patch % Lines
...s/Modals/LeaveGroupAdminModal/AdminSearchInput.tsx 0.00% 19 Missing ⚠️
...dals/LeaveGroupAdminModal/LeaveGroupAdminModal.tsx 75.86% 5 Missing and 2 partials ⚠️
...pt/components/Modals/LeaveGroupAdminModal/utils.ts 0.00% 6 Missing ⚠️
...s/webapp/src/script/view_model/ActionsViewModel.ts 77.27% 4 Missing and 1 partial ⚠️
...aveGroupAdminModal/useLeaveGroupAdminModalStore.ts 85.71% 1 Missing ⚠️
apps/webapp/src/script/view_model/MainViewModel.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #21364      +/-   ##
==========================================
+ Coverage   45.83%   45.87%   +0.04%     
==========================================
  Files        1698     1703       +5     
  Lines       43840    43943     +103     
  Branches     9127     9146      +19     
==========================================
+ Hits        20092    20159      +67     
- Misses      21472    21502      +30     
- Partials     2276     2282       +6     
Flag Coverage Δ
app_webapp 44.11% <62.13%> (+0.05%) ⬆️
lib_api_client 51.74% <ø> (ø)
lib_core 58.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rc/script/components/AppContainer/AppContainer.tsx 0.00% <ø> (ø)
...t/components/Modals/LeaveGroupAdminModal/styles.ts 100.00% <100.00%> (ø)
...s/api-client/src/team/feature/featureList.types.ts 100.00% <ø> (ø)
...aveGroupAdminModal/useLeaveGroupAdminModalStore.ts 85.71% <85.71%> (ø)
apps/webapp/src/script/view_model/MainViewModel.ts 0.00% <0.00%> (ø)
...s/webapp/src/script/view_model/ActionsViewModel.ts 25.56% <77.27%> (+9.33%) ⬆️
...pt/components/Modals/LeaveGroupAdminModal/utils.ts 0.00% <0.00%> (ø)
...dals/LeaveGroupAdminModal/LeaveGroupAdminModal.tsx 75.86% <75.86%> (ø)
...s/Modals/LeaveGroupAdminModal/AdminSearchInput.tsx 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

🔗 Download Full Report Artifact

🧪 Playwright Test Summary

  • Passed: 15
  • Failed: 0
  • Skipped: 0
  • 🔁 Flaky: 0
  • 📊 Total: 15
  • Total Runtime: 130.4s (~ 2 min 10 sec)

Comment thread apps/webapp/src/script/view_model/ActionsViewModel.ts
Comment thread apps/webapp/src/script/view_model/ActionsViewModel.ts Outdated
Comment thread apps/webapp/src/script/view_model/ActionsViewModel.ts
Comment thread apps/webapp/src/script/view_model/ActionsViewModel.ts Outdated
Comment thread libraries/api-client/src/team/feature/featureList.types.ts
Comment thread apps/webapp/src/script/view_model/ActionsViewModel.ts Outdated
).length;
const isLastAdmin = isSelfAdmin && otherAdminCount === 0;

if (isLastAdmin) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add tests for "last admin leaves" branches: eligible users, no eligible users, and self-exclusion case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I planned to add them in next PR as this PR is already huge.
I will add in this PR.

@screendriver
Copy link
Copy Markdown
Member

This change introduces new branching behavior in leaveConversation plus a new modal/store flow, but I don’t see corresponding tests.

Please add tests for:

  1. feature flag off -> existing leave flow unchanged
  2. feature flag on + user is not last admin -> existing leave flow unchanged
  3. feature flag on + user is last admin + eligible users exist -> modal opens
  4. eligible users list excludes self (to avoid self-promotion no-op)
  5. promote selected user -> role assignment called before leave
  6. no eligible users -> “no eligible users” modal variant and no leave action shown
  7. role assignment failure -> leave is not executed and loading state resets

This is a high-risk path (permissions + destructive actions), so deterministic tests are needed before merge

@e-maad e-maad requested a review from screendriver May 26, 2026 10:09
@sonarqubecloud
Copy link
Copy Markdown

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.

3 participants