-
-
Notifications
You must be signed in to change notification settings - Fork 614
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
Allow default value for cloud message type to be overridden with SETTINGS #699
Changes from all commits
5117e64
1f31b13
af2b614
7fe75a7
dba0957
f27bea0
9558516
1269c60
53759b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# Generated by Django 4.2.10 on 2024-04-12 16:29 | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
("push_notifications", "0009_alter_apnsdevice_device_id"), | ||
] | ||
|
||
operations = [ | ||
migrations.AlterField( | ||
model_name="gcmdevice", | ||
name="cloud_message_type", | ||
field=models.CharField( | ||
choices=[ | ||
("FCM", "Firebase Cloud Message"), | ||
("GCM", "Google Cloud Message"), | ||
], | ||
default="FCM", | ||
help_text="You should choose FCM or GCM", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please rebase and recreate this migration? The |
||
max_length=3, | ||
verbose_name="Cloud Message Type", | ||
), | ||
), | ||
] |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -87,6 +87,10 @@ def send_message(self, message, **kwargs): | |||||
return messaging.BatchResponse(responses) | ||||||
|
||||||
|
||||||
def get_default_cloud_message_type() -> str: | ||||||
return SETTINGS.get("DEFAULT_CLOUD_MESSAGE_TYPE", "FCM") | ||||||
|
||||||
|
||||||
class GCMDevice(Device): | ||||||
# device_id cannot be a reliable primary key as fragmentation between different devices | ||||||
# can make it turn out to be null and such: | ||||||
|
@@ -98,7 +102,7 @@ class GCMDevice(Device): | |||||
registration_id = models.TextField(verbose_name=_("Registration ID"), unique=SETTINGS["UNIQUE_REG_ID"]) | ||||||
cloud_message_type = models.CharField( | ||||||
verbose_name=_("Cloud Message Type"), max_length=3, | ||||||
choices=CLOUD_MESSAGE_TYPES, default="FCM", | ||||||
choices=CLOUD_MESSAGE_TYPES, default=get_default_cloud_message_type(), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
having the callable method as default makes it easier to override the setting in tests. also requires the regeneration of the migration |
||||||
help_text=_("You should choose FCM, GCM is deprecated") | ||||||
) | ||||||
objects = GCMDeviceManager() | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,6 +1,9 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import importlib | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from unittest import mock | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from django.test import TestCase | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from django.conf import settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from django.test import TestCase, override_settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from django.utils import timezone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from firebase_admin import messaging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from firebase_admin.exceptions import InvalidArgumentError | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -468,3 +471,29 @@ def test_can_save_wsn_device(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.assertIsNotNone(device.pk) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.assertIsNotNone(device.date_created) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.assertEqual(device.date_created.date(), timezone.now().date()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def test_gcm_is_default_cloud_message_type_when_not_specified_in_settings(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self._validate_device_cloud_message_type(expected_cloud_message_type="GCM") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@override_settings() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def test_cloud_message_type_is_set_to_gcm(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
settings.PUSH_NOTIFICATIONS_SETTINGS.update({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"DEFAULT_CLOUD_MESSAGE_TYPE": "GCM", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self._validate_device_cloud_message_type(expected_cloud_message_type="GCM") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
@override_settings() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def test_cloud_message_type_is_set_to_fcm(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
settings.PUSH_NOTIFICATIONS_SETTINGS.update({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
"DEFAULT_CLOUD_MESSAGE_TYPE": "FCM", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self._validate_device_cloud_message_type(expected_cloud_message_type="FCM") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+478
to
+490
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
def _validate_device_cloud_message_type(self, expected_cloud_message_type: str) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# Reload the models so that cached model is evicted and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
# field default value is re-read from settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import push_notifications.models | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
importlib.reload(push_notifications.models) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
from push_notifications.models import GCMDevice | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
field_object = GCMDevice._meta.get_field("cloud_message_type") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.assertEqual(field_object.default, expected_cloud_message_type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+492
to
+499
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all of this should become obsolete and can be removed after switching to the callable default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. instead I would recommend to create model instances in each of the tests and assert they have been created with the correct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, this line is the main reason for a callable default. otherwise the migration would be hard coded to
"FCM"
and projects using a different default in their settings would complain about a missing migration.