feat: notify on chat when new record is established#26
Open
feat: notify on chat when new record is established#26
Conversation
rubasace
requested changes
Feb 23, 2026
| private void saveSession(final ChatInfo chatInfo, final UserInfo userInfo, final LocalDate gameDay, final GameSession gameSession, final GameInfo gameInfo, final TelegramGroup telegramGroup) { | ||
| gameSessionRepository.saveAndFlush(gameSession); | ||
| applicationEventPublisher.publishEvent(new GameSessionRegistrationEvent(this, chatInfo, userInfo, gameInfo, gameSession.getDuration(), gameDay, | ||
| applicationEventPublisher.publishEvent(new GameSessionRegistrationEvent(this, chatInfo, userInfo, gameInfo, gameSession.getGame(), gameSession.getDuration(), gameDay, |
Owner
There was a problem hiding this comment.
Isn't the game already inferable from gameInfo?
Collaborator
Author
There was a problem hiding this comment.
You're right — already addressed in the refactoring commit. GameInfo now includes the GameType, and the redundant parameter was removed from the event. gameInfo.type() is used everywhere instead.
Collaborator
Author
There was a problem hiding this comment.
Applied in the refactoring commit: GameInfo now includes the GameType as the first field, and the redundant gameType parameter was removed from GameSessionRegistrationEvent. All consumers now correctly use gameInfo.type().
Collaborator
Author
|
Hi @rubasace! Just wanted to flag that the GameType redundancy feedback has been addressed in a previous commit:
Could you please take another look when you get a chance? Thanks! |
GameInfo now contains the GameType as its first field, eliminating the need to pass both gameInfo and gameType separately to events and other components. Changes: - Updated GameInfo record to include GameType as first parameter - Modified GameTypeAdapter to populate type field - Simplified GameSessionRegistrationEvent constructor - Updated all event consumers to use gameInfo.type() - Fixed test fixtures to use new GameInfo constructor
daimonbot
force-pushed
the
feature/record-notification-reimpl
branch
from
6673411 to
031aead
Compare
Collaborator
Author
|
Rebased on current main (no conflicts). CI running on the updated commits. Summary of changes in this PR:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #7
Implements notifications when a user establishes a new record time for a game in a group chat.
Changes:
Behavior: