fix(user-mgmt): single toast + stable editor on group update (closes #998) - #1002
Conversation
…998) Two bugs on the User Management expand panel: 1. Duplicate toasts: document.addEventListener for group-assign-checkbox was called on every renderUsers() invocation, stacking N listeners for N renders. A single checkbox change fired all N handlers -> N toasts. Fix: module-level AbortController (groupAssignListenerCtrl); abort + recreate on each renderUsers() call so only one listener is ever live. 2. Panel collapse + first-click unresponsive: toggleUserGroup called loadUsers() on success, which called renderUsers(), which replaced the entire table innerHTML and reset every expand row to hidden. The next expand-button click reacted correctly (new element, one listener), but the UX felt broken because the row collapsed mid-edit. Fix: patch the user in allUsers in-memory, then re-render only the affected expand panel and groups-cell badge in place. No full re-render, no collapse. Tests: 8 new tests covering single toast (incl. listener-stacking regression case), panel stays visible after toggle, expand-on-first-click, and in-memory state patching. All 204 users tests pass; full suite green.
|
@coderabbitai review |
|
Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references. Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesGroup Assignment UX Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
renderUsers()call added anotherdocument.addEventListenerforgroup-assign-checkboxchange events, so N renders produced N toasts per toggle).toggleUserGrouppreviously calledloadUsers()which calledrenderUsers()which replaced the whole table innerHTML, resetting all expand rows tohidden.Root cause
setupUserTableListeners()(called byrenderUsers()) registered the group-assign handler with a baredocument.addEventListener. No cleanup. Every re-render stacked another listener, so after N group saves the N-th toast fired N times.toggleUserGroupcalledloadUsers()on success which calledrenderUsers()which nuked the DOM and collapsed the panel before the user could make a second group change.Fix
AbortController(groupAssignListenerCtrl). EachsetupUserTableListeners()call aborts the previous controller and creates a fresh one, so only one listener is ever active. Same pattern assettings.ts:1564-1565.toggleUserGroupno longer callsloadUsers(). Instead: patch the user inallUsersin-memory, then re-render only the affected expand panel (panel.innerHTML = renderUserExpandPanel(updatedUser)) and the groups-cell badge in the main row. The expand row stays open; the user can make multiple group changes without the panel collapsing.Test plan
8 new tests added to
users.test.tsundergroup assignment UX (issue #998):renderUsers()calls before a toggle still emit exactly one toast (direct regression test for listener stacking)renderUsersrenderUserscallsallUsersstate reflects added group after toggleallUsersstate reflects removed group after toggleAll 204 users tests pass; full 71-suite frontend test run green;
tsc --noEmitclean;npm run buildsucceeds.closes #998
Summary by CodeRabbit
Bug Fixes
Tests