Skip to content
/ api Public

fix: move NotificationsService to parent MatchEventProcessor#135

Open
Flegma wants to merge 2 commits intomainfrom
fix/match-map-status-event-di
Open

fix: move NotificationsService to parent MatchEventProcessor#135
Flegma wants to merge 2 commits intomainfrom
fix/match-map-status-event-di

Conversation

@Flegma
Copy link
Copy Markdown
Contributor

@Flegma Flegma commented Mar 28, 2026

Summary

  • Move NotificationsService into the parent MatchEventProcessor constructor so all event processors inherit it
  • This fixes this.notifications being undefined in MatchMapStatusEvent, which crashed with TypeError: Cannot read properties of undefined (reading 'sendMatchMapPauseNotification') when a match map entered "Paused" status
  • Remove redundant constructors from MatchMapStatusEvent and MatchMapResetRoundEvent to align with sibling event classes

Test plan

  • Verify matches can start and progress through map statuses without errors
  • Verify pausing a match map triggers the pause notification without crashing
  • Verify no regression in other match event processors

Without @Injectable() on the child class, NestJS cannot generate
design:paramtypes metadata for the child's constructor. It falls back
to the parent's metadata (4 params), so the 5th param
(NotificationsService) is never injected, causing a TypeError when
sendMatchMapPauseNotification is called during match map pause events.
@Flegma Flegma requested a review from lukepolo March 28, 2026 09:05
@lukepolo
Copy link
Copy Markdown
Contributor

so this will also cause moe memory usage, we might want to just add notification service on to the parent class. the request scope means each time it comes into will generate the notification service (new) each time.

@lukepolo
Copy link
Copy Markdown
Contributor

TLDR; just add the notification service to the parent

Instead of adding @Injectable to the child class, move
NotificationsService into the parent constructor so all event
processors can access it without needing their own decorator.
Also removes redundant constructors from MatchMapStatusEvent
and MatchMapResetRoundEvent.
@Flegma
Copy link
Copy Markdown
Contributor Author

Flegma commented Mar 30, 2026

@lukepolo Updated:

  • NotificationsService is on the parent class

  • No child-level @Injectable decorator - avoids the per-child scope issue he flagged

  • Redundant constructors removed from both MatchMapStatusEvent and MatchMapResetRoundEvent, matching the pattern of all the other sibling event classes

  • The fix is preserved - this.notifications.sendMatchMapPauseNotification() still works in MatchMapStatusEvent, it just comes from the inherited parent now

@Flegma Flegma changed the title fix: add Injectable decorator to MatchMapStatusEvent for proper DI fix: move NotificationsService to parent MatchEventProcessor Mar 30, 2026
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.

2 participants