Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Fix] Welcome Notification #1108

Merged
merged 2 commits into from Sep 29, 2023
Merged

[Fix] Welcome Notification #1108

merged 2 commits into from Sep 29, 2023

Conversation

jkasten2
Copy link
Member

@jkasten2 jkasten2 commented Sep 29, 2023 •
edited by jwilm

Description

1 Line Summary

Fix welcome notifications not showing after subscribing.

Details

This was disabled in code during the User Model beta and a replacement implementation hasn't been done on the backend. We will continue to use include_player_ids until there is a backend replacement which needs to be done by 2025.

Validation

Tests

Tested on Windows 11 with Chrome, ensuring the welcome notification showed up.

Checklist

  • All the automated tests pass or I explained why that is not possible
  • I have personally tested this on my machine or explained why that is not possible
  • I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

  • Don't use default export
  • New interfaces are in model files

Functions:

  • Don't use default export
  • All function signatures have return types
  • Helpers should not access any data but rather be given the data to operate on.

Typescript:

  • No Typescript warnings
  • Avoid silencing null/undefined warnings with the exclamation point

Other:

  • Iteration: refrain from using elem of array syntax. Prefer forEach or use map
  • Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Checklist

  • I have included screenshots/recordings of the intended results or explained why they are not needed

Related Tickets

This change is Reviewable

@jkasten2
Re-enable welcome notifications
ff9a216 
This existing code still uses include_player_ids on the OneSignal REST
API POST /notifications. include_player_ids will be dropped by 2025 so
a replacement will needed by then. This will most likely be a backend
feature which will be a hook on a new push subscription create.
@jkasten2
rm double welcome send work around logic
03a65de 
The event logic was changed in User Model so this is longer an issue
with v16.
@jkasten2 jkasten2 merged commit beffd4f into main Sep 29, 2023
2 checks passed
@jkasten2 jkasten2 deleted the fix/welcome_notification branch September 29, 2023 01:10
@jkasten2 jkasten2 mentioned this pull request Sep 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgomezp rgomezp rgomezp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jkasten2 @rgomezp