Playabl-io / playabl

https://app.playabl.io
GNU Affero General Public License v3.0
7 stars 3 forks source link

Improve notifications about users joining a game #130

Closed jongrim closed 1 week ago

jongrim commented 1 year ago

Is your feature request related to a problem? Please describe. The notifications when someone joins one of your games, it lacks enough detail to understand what session was joined. This can lead to what look like duplicate notifications if someone joins multiple sessions.

Describe the solution you'd like The notification should include information about the session joined

Describe alternatives you've considered None

Additional context Example of two legitimate notifications that look the same, though the user joined two different sessions

Screenshot 2023-06-30 at 4 32 14 PM
mikeminutillo commented 7 months ago

I had a look at this. The biggest challenge I can see is converting to the game owner's timezone, which we don't have on the server. Would it make sense to pass the actual timestamps in the custom_fields property and render them client side?

Not sure how that would fit in with the email part. I haven't found that yet :)

I will try and get the app running in a dev environment and give this approach a go.

jongrim commented 7 months ago

Thanks for looking at this! I don't think we need the game owner's timezone - what were you seeing that led you down that path?

My thought for these was that the notification should include the session number that the user joined. So something like "H Guy joined session 2 of your game, game_name".

That string is built in notifyGameCreator in the processRsvp.ts file. Currently, the main handling function has the specific session being joined, but it's not aware of what session that is in the sequence. I think the change needed is to load all sessions for the game rather than the single one, and then we should be able to improve the string by adding language about which session was joined. Does that make sense? Any other ideas?

While here, I think it'd be useful to also go ahead and attach the joiner's user ID to the notification. I want to expand the ability to view other's profiles in the future, so that will set it up to be easy to do so later. It also enables potentially showing the joiner's avatar and other info. That joining user's ID could go into the custom_fields part when creating the notification record. That will make it available in the UI later.

How does that sound? Feel free to suggest alternatives or other ideas, but that is what I had planned in my head. Lastly, I need to iron out the details on how others can run the project locally and develop, so I'll look into that this weekend and will get back soon!

mikeminutillo commented 7 months ago

I was thinking of adding the date/time to the notification.

USER has joined your game GAME starting DATETIME.

Session number could be useful, but as a game creator I would probably need to backport that to a date/time anyway. Especially if I am running two one-shots of the same game on the same day (which could happen).

jongrim commented 6 months ago

Ah, I see! I do like that idea. Very nice and readable which was the goal.

So yes, you're original idea is the right one, take the session start_time (which is a unix timestamp) and add that to the custom_fields portion of the notification record so it will be available on the client. On the client side, when it builds the display for that notification type, it can use date-fns to format the time into a human readable string which will use the current browser timezone.

I did confirm that local dev should be good to go. Check out the developing doc in /docs to get started, and let me know if there's any questions!

mikeminutillo commented 5 months ago

I got everything working and raised a PR to address this https://github.com/Playabl-io/playabl/pull/268

The layout is a bit naff. I messed with it for a bit but you can probably move things around quicker than I can.