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

Allow default value for cloud message type to be overridden with SETTINGS #699

Closed
wants to merge 9 commits into from
1 change: 1 addition & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ For WNS, you need both the ``WNS_PACKAGE_SECURITY_KEY`` and the ``WNS_SECRET_KEY
- ``USER_MODEL``: Your user model of choice. Eg. ``myapp.User``. Defaults to ``settings.AUTH_USER_MODEL``.
- ``UPDATE_ON_DUPLICATE_REG_ID``: Transform create of an existing Device (based on registration id) into a update. See below `Update of device with duplicate registration ID`_ for more details.
- ``UNIQUE_REG_ID``: Forces the ``registration_id`` field on all device models to be unique.
- ``DEFAULT_CLOUD_MESSAGE_TYPE``: The default cloud message type to use when registering a FCM/GCM device. Defaults to ``"FCM"``. Can be set to ``"GCM"`` to use Google Cloud Messaging instead.

**APNS settings**

Expand Down
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",
Copy link

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.

help_text="You should choose FCM or GCM",
Copy link

Choose a reason for hiding this comment

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

Could you please rebase and recreate this migration? The help_text changed in #702

max_length=3,
verbose_name="Cloud Message Type",
),
),
]
6 changes: 5 additions & 1 deletion push_notifications/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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(),
Copy link

Choose a reason for hiding this comment

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

Suggested change
choices=CLOUD_MESSAGE_TYPES, default=get_default_cloud_message_type(),
choices=CLOUD_MESSAGE_TYPES, default=get_default_cloud_message_type,

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()
Expand Down
31 changes: 30 additions & 1 deletion tests/test_models.py
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
Expand Down Expand Up @@ -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
Copy link

Choose a reason for hiding this comment

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

Suggested change
@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")
@override_settings()
def test_cloud_message_type_is_set_to_gcm(self) -> None:
settings.PUSH_NOTIFICATIONS_SETTINGS.update({
"DEFAULT_CLOUD_MESSAGE_TYPE": "GCM",
})
device = GCMDevice.objects.create(registration_id="a valid registration id")
self.assertEqual(device.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",
})
device = GCMDevice.objects.create(registration_id="a valid registration id")
self.assertEqual(device.cloud_message_type, "FCM")


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
Copy link

@tuky tuky Apr 17, 2024

Choose a reason for hiding this comment

The 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.

Copy link

@tuky tuky Apr 17, 2024

Choose a reason for hiding this comment

The 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 cloud_message_type (see #699 (comment))

Loading