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.

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

Support notification priority in FCM relay #1630

Merged
merged 3 commits into from
Mar 27, 2023
Merged

Conversation

vstpme
Copy link
Member

@vstpme vstpme commented Mar 27, 2023

What this PR does

Adds support for notification priority in FCM relay + add some tests on FCM relay.

Which issue(s) this PR fixes

Related to #1550 and #1612.

Checklist

  • Unit, integration, and e2e (if applicable) tests updated

No changelog update since notification priority is "Unreleased": https://github.com/grafana/oncall/blob/a2caeae3c7d15c81e72c70108e32617d68cf1a7b/CHANGELOG.md#unreleased

@vstpme vstpme added pr:no public docs Added to a PR that does not require public documentation updates pr:no changelog labels Mar 27, 2023
@vstpme vstpme marked this pull request as ready for review March 27, 2023 10:54
@vstpme vstpme requested a review from a team March 27, 2023 10:54
def fcm_relay_async(token, data, apns):
message = Message(token=token, data=data, apns=deserialize_apns(apns))
def fcm_relay_async(token, data, apns, android=None):
message = _get_message_from_request_data(token, data, apns, android)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need the android argument here? this field should be available on the FCMDevice object fetched just a few lines below. See the type column in the fcm_django_fcmdevice table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The android field is needed to pass any Android related fields to FCM relay (e.g. priority). We don’t store any OSS device data in FCM relay, so the only way to get this data is to extract it from the request. Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense 👍 I sometimes get confused w/ the relay where certain data is being stored

@vstpme vstpme enabled auto-merge March 27, 2023 11:58
@vstpme vstpme disabled auto-merge March 27, 2023 12:48
@vstpme vstpme disabled auto-merge March 27, 2023 12:48
@vstpme vstpme disabled auto-merge March 27, 2023 12:49
@vstpme vstpme disabled auto-merge March 27, 2023 12:52
@vstpme vstpme disabled auto-merge March 27, 2023 13:36
@vstpme vstpme merged commit ec5472c into dev Mar 27, 2023
@vstpme vstpme deleted the vadimkerr/fcm-relay-update branch March 27, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants