Skip to content

feat(M-833): notify the remaining admins when a member leaves a band space - #898

Merged
Cryde merged 1 commit into
masterfrom
M-833
Aug 21, 2026
Merged

feat(M-833): notify the remaining admins when a member leaves a band space#898
Cryde merged 1 commit into
masterfrom
M-833

Conversation

@Cryde

@Cryde Cryde commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Closes #833.

BandSpaceLeaveProcessor recorded the departure in the activity feed and dispatched nothing, unlike every sibling action: a kick dispatches BandSpaceMemberRemovedEvent, a role change dispatches BandSpaceMemberRoleChangedEvent. So a member could quit, have their finance recurrences deactivated and their planned entries deleted, and the admins would only find out by scrolling the activity log.

What changed

BandSpaceMemberLeftEvent plus BandSpaceMemberLeftListener, following the established notification shape: one event, one listener with a single __invoke, the whole body in try/catch with a log so it can never roll back or 500 the departure. Recipients are the remaining active admins, excluding the actor. Dispatched after wrapInTransaction commits, so a failed flush cannot announce a departure that did not happen.

Admins only rather than the whole roster, because the departure leaves book-keeping only an admin can act on. The payload matches the band_space_member_removed shape exactly (band_space_id, band_space_name, actor_id, actor_username), and the feed gets a distinct icon and colour from a kick, since quitting and being removed sit side by side in the same list.

BandSpaceMembershipRepository::findActiveAdmins() filters on both role = Admin and status = Active, so a departed or removed admin is never notified.

Review

Confirmed by execution rather than by reading: the post-commit dispatch, the admin-query filtering, the fault isolation, and that the frontend branch reads exactly the keys the listener writes.

It also confirmed a gap the implementation had already disclosed: WithdrawUserFromBandSpacesProcedure performs the identical status = Left transition on the account-deletion path and does not dispatch this event. That is left as is deliberately, and the reasoning is now recorded in that class rather than left implicit. Worth knowing the original justification was wrong: the concern was scattering fresh copies of a deleted member's username, but DeleteAccountProcedure anonymises first and withdraws second, so a notification there would carry the anonymised handle. The real reason not to send it is that admins would be told deleted_<uuid> left, which carries the fact but not the person, and changing that is a behaviour change on the deletion path that belongs in its own issue.

Verification

Full suite 2331 green, PHPStan level 8 clean, biome clean, build succeeds. New tests cover the notify-admins-only path with a full-body payload assertion, an admin leaving not notifying themselves, and a fault-isolation case proving a throwing notification path still returns 204 with the membership marked Left.

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.45%. Comparing base (925598b) to head (ea8c5bd).

Files with missing lines Patch % Lines
...rc/EventSubscriber/BandSpaceMemberLeftListener.php 95.83% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #898      +/-   ##
============================================
+ Coverage     88.43%   88.45%   +0.01%     
- Complexity     4818     4824       +6     
============================================
  Files           805      807       +2     
  Lines         17618    17656      +38     
============================================
+ Hits          15581    15618      +37     
- Misses         2037     2038       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 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.

@Cryde
Cryde merged commit a9c85e8 into master Aug 21, 2026
4 checks passed
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.

Band Space: notify admins when a member leaves

1 participant