nabeelio / phpvms

virtual airline management
http://www.phpvms.net
Other
169 stars 135 forks source link

Discord Notification Dispatched on Nightly Cron (Rank Change) #1688

Open FatihKoz opened 7 months ago

FatihKoz commented 7 months ago

Discord Notification about Rank Changes is dispatched every night during cron checks even without changes to pilot ranks.

Version 7.0.0-dev+231122.a0ed24 (latest dev as of this report creation)

To Reproduce No need to re-produce, either wait for nightly cron to run or trigger it manually

Expected behavior If there is no change (due to stats calculation or corrections), notification should NOT be dispatched each night with same ranks over and over.

Screenshots image

arthurpar06 commented 7 months ago

I think this issue and the previous one are related. The Notification is dispatched when the UserStatsChanged event is dispatched https://github.com/nabeelio/phpvms/blob/dev/app/Notifications/NotificationEventsHandler.php#L289-L297.

But this event isn't dispatched only when a rank changes. For example here the event is dispatched when a pirep is filed and the user has changed of location https://github.com/nabeelio/phpvms/blob/dev/app/Services/PirepService.php#L727

I think adding a check in the onUserStatsChanged method like $event->stat_name == 'rank' should be a good solution for both issues.

FatihKoz commented 7 months ago

Probably yes, I just did not had time to check how this is implemented. At first it was just dispatching the same rank over and over on each pirep, I fixed it but looks like this still needs some more changes/updates.

arthurpar06 commented 7 months ago

I think #1704 closes this issue

FatihKoz commented 7 months ago

Strangely enough, it does not solve the issue fully... Pirep part is ok, it is solved but I am still getting them with nightly cron (all new members, no pireps, VA being built up, everyone is -new pilot-)

arthurpar06 commented 7 months ago

I can't reproduce it so it's complicated to debug it but it definitely comes from here (the only place where the event is fired during nightly cron). We should try to debug the differences between $original_rank_id and $user->rank_id and understand why they are not the same

FatihKoz commented 7 months ago

Yeah, it is not happening on my two running installs either.

Only happening at freshly installed va, no pireps, everyone is still new pilot.

I will change some users to first officer manually to monitor. It may be related to new pilot (default rank).

Sent from mobile device / Mobil cihaz ile gönderildi

10 Ara 2023 Pzr 17:46 tarihinde Arthur Parienté @.***> şunu yazdı:

I can't reproduce it so it's complicated to debug it but it definitely comes from here https://github.com/nabeelio/phpvms/blob/dev/app/Services/UserService.php#L485-L504 (the only place where the event is fired during nightly cron). We should try to debug the differences between $original_rank_id and $user->rank_id and understand why they are not the same

— Reply to this email directly, view it on GitHub https://github.com/nabeelio/phpvms/issues/1688#issuecomment-1848985069, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARXKVMLBDGNEP2SOVQN5PXLYIXDM7AVCNFSM6AAAAAA722VT2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBYHE4DKMBWHE . You are receiving this because you authored the thread.Message ID: @.***>

arthurpar06 commented 6 months ago

I was unable to reproduce it locally even after removing all pireps and setting all users to 0 flights. Do you still have this bug on your freshly installed va?

FatihKoz commented 6 months ago

Hi Arthur,

Yes it is still happening for new pilot rank only, it does not matter if they have any pireps or not.

When you change a pilot to next rank, with no time/pireps it does not occur.

I simply stopped thinking about possible causes, maybe new pilot/new member triggers it during nightly cron too

Sent from mobile device / Mobil cihaz ile gönderildi

26 Ara 2023 Sal 14:39 tarihinde Arthur Parienté @.***> şunu yazdı:

I was unable to reproduce it locally even after removing all pireps and setting all users to 0 flights. Do you still have this bug on your freshly installed va?

— Reply to this email directly, view it on GitHub https://github.com/nabeelio/phpvms/issues/1688#issuecomment-1869479904, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARXKVMMLYOT3IOBXPBRC5F3YLKZPHAVCNFSM6AAAAAA722VT2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRZGQ3TSOJQGQ . You are receiving this because you authored the thread.Message ID: @.***>

arthurpar06 commented 6 months ago

I really can't reproduce it... If you want to try to debug from your install I think that the notification is sent just after this if is passed, this is where it would be interesting to debug $user->rank_id and $original_rank_id https://github.com/nabeelio/phpvms/blob/dev/app/Services/UserService.php#L500