Skip to content

Fix negative volunteer counts caused by observer bug#826

Merged
edwh merged 3 commits intodevelopfrom
RES-2057_negative_volunteer_counts_in_events
Apr 23, 2026
Merged

Fix negative volunteer counts caused by observer bug#826
edwh merged 3 commits intodevelopfrom
RES-2057_negative_volunteer_counts_in_events

Conversation

@edwh
Copy link
Copy Markdown
Collaborator

@edwh edwh commented Jan 15, 2026

Summary

  • Fixes bug where deleting event invitations incorrectly decremented the volunteer count
  • The EventsUsersObserver::deleted() method passed 4 parameters to removed() which only accepts 3, causing the status check to be silently ignored
  • Every deleted events_users record was decrementing the count, even for invited (non-confirmed) users who were never counted

Root Cause

Line 89 in EventsUsersObserver.php:

// Bug: 4th parameter is silently ignored
$this->removed($event, $user, true, $eu->status == 1);

// Fix: Pass status check as the $count parameter
$this->removed($event, $user, $eu->status == 1);

Changes

  1. One-line fix in EventsUsersObserver.php - correctly pass $eu->status == 1 as the $count parameter
  2. Migration to fix all existing events with incorrect volunteer counts
  3. Three new tests to prevent regression:
    • testDeletingInvitedVolunteerDoesNotDecrementCount
    • testDeletingConfirmedVolunteerDecrementsCount
    • testMultipleInvitationDeletionsCauseNegativeCount
  4. Updated CLAUDE.md with Task/Docker documentation

Test plan

  • New unit tests pass locally
  • Verify negative counts are fixed after migration runs on staging
  • Manual test: invite user to event, delete invitation, verify count stays at 0

The EventsUsersObserver::deleted() method was incorrectly decrementing
the volunteer count for ALL deleted events_users records, not just
confirmed volunteers.

Bug: Line 89 passed 4 parameters to removed() which only accepts 3.
The 4th parameter ($eu->status == 1) was silently ignored, so $count
was always true, causing incorrect decrements when invitations
(non-confirmed users) were deleted.

Fix: Pass $eu->status == 1 as the 3rd parameter instead of true.

Also includes:
- Migration to fix existing incorrect volunteer counts
- Three new tests to prevent regression
- Updated CLAUDE.md with Task/Docker documentation
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
25.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

…counts

Migration was too aggressive - volunteer counts can be manually adjusted,
so mismatches aren't necessarily wrong. Only negative counts are always
wrong. Replace with targeted artisan command (dev:fixvolunteercount).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
29.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Brings in Laravel 10 + group tags. Conflict resolutions:

- app/Observers/EventsUsersObserver.php: kept HEAD's 3-arg removed()
  call (which is the actual fix for this PR — the 4-arg call was the
  bug) but added develop's null guard, which is required because L10's
  type hints now make removed(User $user) strict and $user can be null
  when iduser is unset.
- app/Console/Commands/FixVolunteerCount.php: kept HEAD's more
  detailed docblock, dropped the obsolete @return mixed tag.
- CLAUDE.md: combined HEAD's detailed task:docker command reference
  with develop's "Run Vite for HMR" note.
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
29.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@edwh edwh merged commit d7c29d1 into develop Apr 23, 2026
2 of 3 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.

1 participant