MemberJunction / MemberJunction/MJ
Bulk enable/disable rollback leaves already-correct users showing the wrong status
- Dominant language
- TSQL
- Stars
- 29
- Forks
- 6
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 308
Description
**Found by** the PR gauntlet on #4275 (`fix/4260-new-user-roles-default`), 2026-09-07. Not caused by that PR: verified pre-existing at `f2ec156daf31670b42d7704e994686b09daf14df`. That PR repaired the neighbouring `toggleUserStatus` for a related reason and deliberately did not widen into the bulk paths.
### Where
`packages/Angular/Explorer/explorer-settings/src/lib/user-management/user-management.component.ts:900-906` (the rollback), with `executeBulkAction`'s catch at ~:874-880.
### What happens
When the transaction group submit fails, the rollback restores every selected user to the **opposite of the requested target** rather than to that user's own prior value:
```ts
for (const user of users) {
user.IsActive = !isActive; // `isActive` is the REQUESTED target, not this user's previous state
}
```
A selection that includes a user who was *already* in the target state therefore ends up rendered in the wrong state after a rollback. `executeBulkAction`'s catch does not call `loadInitialData()`, so nothing subsequently corrects the grid — it keeps disagreeing with the database until the page is reloaded.
### Why it matters
The grid silently misreports account status, which is exactly the class of problem this screen must not have: an administrator reading "inactive" for an active account may take further action based on it.
It also stopped being rare. Since the `MJ: Users` privilege-elevation guard shipped (issue #4260), *every* bulk status change attempted by a non-Owner administrator fails at the server — so this rollback path, previously an edge case, is now the normal outcome for a whole class of users.
### Repro
```bash
# As an administrator whose User.Type is 'User' (so the server guard refuses the writes):
# 1. Settings -> User Management.
# 2. Select two users: one ALREADY active, one inactive.
# 3. Bulk -> Enable. The submit fails at the guard.
# Observed: the already-active user is now displayed as INACTIVE. Reload to see it was never changed.
```
### Evidence
`user-management.component.ts:900-906` — the loop above, keyed on `isActive` rather than a captured per-user value. Contrast the single-row path `toggleUserStatus`, which reverts the one user it actually flipped.
### Suggested fix
Prefer calling `loadInitialData()` in the failure path and letting the server be the source of truth: it is smaller than per-user bookkeeping, cannot drift as the method grows, and is already the pattern used elsewhere in this component after a mutation. If a reload is too heavy, capture each user's `IsActive` **before** submitting and restore per-user from that snapshot — but do not key the restore on the requested target.
### Definition of done
- [ ] A failing test that reproduces it (mixed selection, failing submit, assert the already-correct user is unchanged), then green
- [ ] After a failed bulk action the grid matches the database for every selected user
- [ ] Existing suite and gates green; no changed expectation in an existing test
### Verify by
Running the repro: after the failed bulk enable, the already-active user still shows as active, and a reload agrees.
Contributor guide
Research direction
Start in packages/Angular/Explorer/explorer-settings/src/lib/user-management/user-management.component.ts, especially executeBulkAction and the rollback around lines 874-906. Reproduce the failed mixed-selection bulk action, then locate or add the component test covering a rejected submit. Done means the failed action leaves every selected user's displayed status matching the database and the existing suite and gates remain green.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- angular, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 86/100