From c59989db54a7ec3fcc81e52d1e4f8cedd2e87145 Mon Sep 17 00:00:00 2001 From: Matti Lamppu Date: Wed, 30 Aug 2023 10:25:26 +0300 Subject: [PATCH] Rename BannerNotification type to level See issue with using type as a name with mutations: https://github.com/graphql-python/graphene/issues/1519 --- api/graphql/banner_notification/filtersets.py | 6 ++-- api/graphql/banner_notification/types.py | 2 +- common/admin/banner_notification.py | 4 +-- common/admin/forms/banner_notification.py | 4 +-- common/choices/__init__.py | 4 +-- common/choices/banner_notification.py | 2 +- .../management/commands/create_test_data.py | 20 ++++++------- ...tification_type_priority_index_and_more.py | 26 +++++++++++++++++ common/models/banner_notification.py | 10 +++---- common/querysets/banner_notification.py | 14 +++++----- tests/factories/banner_notification.py | 4 +-- .../test_banner_notification/test_sorting.py | 28 +++++++++---------- 12 files changed, 75 insertions(+), 49 deletions(-) create mode 100644 common/migrations/0003_remove_bannernotification_type_priority_index_and_more.py diff --git a/api/graphql/banner_notification/filtersets.py b/api/graphql/banner_notification/filtersets.py index ac2b427a4..4e6731059 100644 --- a/api/graphql/banner_notification/filtersets.py +++ b/api/graphql/banner_notification/filtersets.py @@ -7,8 +7,8 @@ class BannerNotificationOrderingFilter(CustomOrderingFilter): @staticmethod - def order_by_type(qs: BannerNotificationQuerySet, desc: bool) -> BannerNotificationQuerySet: - return qs.order_by_type(desc) + def order_by_level(qs: BannerNotificationQuerySet, desc: bool) -> BannerNotificationQuerySet: + return qs.order_by_level(desc) @staticmethod def order_by_target(qs: BannerNotificationQuerySet, desc: bool) -> BannerNotificationQuerySet: @@ -29,7 +29,7 @@ class BannerNotificationFilterSet(django_filters.FilterSet): ("active_until", "ends"), ), custom_fields=( - "type", + "level", "state", "target", ), diff --git a/api/graphql/banner_notification/types.py b/api/graphql/banner_notification/types.py index 59c143dfa..86e18a5f2 100644 --- a/api/graphql/banner_notification/types.py +++ b/api/graphql/banner_notification/types.py @@ -26,7 +26,7 @@ class Meta: "message_en", "message_sv", "draft", - "type", + "level", "target", "active_from", "active_until", diff --git a/common/admin/banner_notification.py b/common/admin/banner_notification.py index f34e30dfb..215a8955c 100644 --- a/common/admin/banner_notification.py +++ b/common/admin/banner_notification.py @@ -10,12 +10,12 @@ class BannerNotificationAdmin(TranslationAdmin): form = BannerNotificationAdminForm list_display = [ "name", - "type", + "level", "target", "active_from", "active_until", ] list_filter = [ - "type", + "level", "target", ] diff --git a/common/admin/forms/banner_notification.py b/common/admin/forms/banner_notification.py index 2581c7810..cc276fa9d 100644 --- a/common/admin/forms/banner_notification.py +++ b/common/admin/forms/banner_notification.py @@ -11,7 +11,7 @@ class Meta: fields = [ "name", "message", - "type", + "level", "target", "active_from", "active_until", @@ -23,7 +23,7 @@ class Meta: help_texts = { "name": gettext_lazy("Name of the notification. Should be unique."), "message": gettext_lazy("Notification body."), - "type": gettext_lazy("Type of the notification."), + "level": gettext_lazy("Level of the notification."), "target": gettext_lazy("Target user interface of the notification."), "active_from": gettext_lazy( "Start date of the notification. If empty, 'active_until' must also be empty.", diff --git a/common/choices/__init__.py b/common/choices/__init__.py index f545e0a5e..a9c48f284 100644 --- a/common/choices/__init__.py +++ b/common/choices/__init__.py @@ -1,7 +1,7 @@ -from .banner_notification import BannerNotificationState, BannerNotificationTarget, BannerNotificationType +from .banner_notification import BannerNotificationLevel, BannerNotificationState, BannerNotificationTarget __all__ = [ - "BannerNotificationType", + "BannerNotificationLevel", "BannerNotificationTarget", "BannerNotificationState", ] diff --git a/common/choices/banner_notification.py b/common/choices/banner_notification.py index d9e5954f5..4ec556529 100644 --- a/common/choices/banner_notification.py +++ b/common/choices/banner_notification.py @@ -2,7 +2,7 @@ from django.utils.translation import gettext_lazy -class BannerNotificationType(models.TextChoices): +class BannerNotificationLevel(models.TextChoices): EXCEPTION = "EXCEPTION", gettext_lazy("Exception") WARNING = "WARNING", gettext_lazy("Warning") NORMAL = "NORMAL", gettext_lazy("Normal") diff --git a/common/management/commands/create_test_data.py b/common/management/commands/create_test_data.py index a916c1974..e4c93d48b 100644 --- a/common/management/commands/create_test_data.py +++ b/common/management/commands/create_test_data.py @@ -84,7 +84,7 @@ from tilavarauspalvelu.utils.commons import WEEKDAYS from users.models import User -from ...choices import BannerNotificationTarget, BannerNotificationType +from ...choices import BannerNotificationLevel, BannerNotificationTarget from ...models import BannerNotification from ._utils import ( batched, @@ -2091,7 +2091,7 @@ def _create_banner_notifications(): today = now() banner_notifications: list[BannerNotification] = [] for target in BannerNotificationTarget.values: - for type_ in BannerNotificationType.values: + for level in BannerNotificationLevel.values: draft_message_fi = faker_fi.sentence() active_message_fi = faker_fi.sentence() scheduled_message_fi = faker_fi.sentence() @@ -2099,48 +2099,48 @@ def _create_banner_notifications(): banner_notifications += [ BannerNotification( - name=f"Draft {type_} notification for {target}", + name=f"Draft {level} notification for {target}", message=draft_message_fi, message_fi=draft_message_fi, message_en=faker_en.sentence(), message_sv=faker_sv.sentence(), - type=type_, + level=level, target=target, draft=True, active_from=None, active_until=None, ), BannerNotification( - name=f"Active {type_} notification for {target}", + name=f"Active {level} notification for {target}", message=active_message_fi, message_fi=active_message_fi, message_en=faker_en.sentence(), message_sv=faker_sv.sentence(), - type=type_, + level=level, target=target, draft=False, active_from=today - timedelta(days=1), active_until=today + timedelta(days=7), ), BannerNotification( - name=f"Scheduled {type_} notification for {target}", + name=f"Scheduled {level} notification for {target}", message=scheduled_message_fi, message_fi=scheduled_message_fi, message_en=faker_en.sentence(), message_sv=faker_sv.sentence(), - type=type_, + level=level, target=target, draft=False, active_from=today + timedelta(days=7), active_until=today + timedelta(days=14), ), BannerNotification( - name=f"Past {type_} notification for {target}", + name=f"Past {level} notification for {target}", message=past_message_fi, message_fi=past_message_fi, message_en=faker_en.sentence(), message_sv=faker_sv.sentence(), - type=type_, + level=level, target=target, draft=False, active_from=today - timedelta(days=7), diff --git a/common/migrations/0003_remove_bannernotification_type_priority_index_and_more.py b/common/migrations/0003_remove_bannernotification_type_priority_index_and_more.py new file mode 100644 index 000000000..96524e512 --- /dev/null +++ b/common/migrations/0003_remove_bannernotification_type_priority_index_and_more.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.4 on 2023-08-30 07:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0002_alter_bannernotification_target_and_more'), + ] + + operations = [ + migrations.RemoveIndex( + model_name='bannernotification', + name='type_priority_index', + ), + migrations.RenameField( + model_name='bannernotification', + old_name='type', + new_name='level', + ), + migrations.AddIndex( + model_name='bannernotification', + index=models.Index(models.Case(models.When(level='EXCEPTION', then=models.Value(1)), models.When(level='WARNING', then=models.Value(2)), models.When(level='NORMAL', then=models.Value(3)), default=models.Value(4)), name='level_priority_index'), + ), + ] diff --git a/common/models/banner_notification.py b/common/models/banner_notification.py index de8435ee8..a0c2ca1be 100644 --- a/common/models/banner_notification.py +++ b/common/models/banner_notification.py @@ -4,17 +4,17 @@ from django.utils import timezone from django.utils.translation import gettext_lazy -from common.choices import BannerNotificationState, BannerNotificationTarget, BannerNotificationType +from common.choices import BannerNotificationLevel, BannerNotificationState, BannerNotificationTarget from common.fields import ChoiceField from common.querysets import BannerNotificationQuerySet -from common.querysets.banner_notification import BANNER_TARGET_SORT_ORDER, BANNER_TYPE_SORT_ORDER +from common.querysets.banner_notification import BANNER_LEVEL_SORT_ORDER, BANNER_TARGET_SORT_ORDER class BannerNotification(models.Model): name: str = models.CharField(max_length=100, null=False, blank=False, unique=True) message: str = models.TextField(max_length=1_000, blank=True, default="") draft: bool = models.BooleanField(default=True) - type: str = ChoiceField(choices=BannerNotificationType.choices) + level: str = ChoiceField(choices=BannerNotificationLevel.choices) target: str = ChoiceField(choices=BannerNotificationTarget.choices) active_from: datetime | None = models.DateTimeField(null=True, blank=True, default=None) active_until: datetime | None = models.DateTimeField(null=True, blank=True, default=None) @@ -51,8 +51,8 @@ class Meta: base_manager_name = "objects" indexes = [ models.Index( - BANNER_TYPE_SORT_ORDER, - name="type_priority_index", + BANNER_LEVEL_SORT_ORDER, + name="level_priority_index", ), models.Index( BANNER_TARGET_SORT_ORDER, diff --git a/common/querysets/banner_notification.py b/common/querysets/banner_notification.py index 8732fb755..a1df13f81 100644 --- a/common/querysets/banner_notification.py +++ b/common/querysets/banner_notification.py @@ -5,7 +5,7 @@ from django.db import models from django.db.models.functions import Now -from common.choices import BannerNotificationTarget, BannerNotificationType +from common.choices import BannerNotificationLevel, BannerNotificationTarget from permissions.helpers import can_manage_banner_notifications from ._base import BaseQuerySet @@ -48,8 +48,8 @@ def hidden(self, user: User | AnonymousUser) -> Self: return self.none() - def order_by_type(self, desc: bool = False) -> Self: - return self.order_by_expression(alias="__type", expression=BANNER_TYPE_SORT_ORDER, desc=desc) + def order_by_level(self, desc: bool = False) -> Self: + return self.order_by_expression(alias="__level", expression=BANNER_LEVEL_SORT_ORDER, desc=desc) def order_by_state(self, desc: bool = False) -> Self: return self.order_by_expression(alias="__state", expression=BANNER_STATE_SORT_ORDER, desc=desc) @@ -58,17 +58,17 @@ def order_by_target(self, desc: bool = False) -> Self: return self.order_by_expression(alias="__target", expression=BANNER_TARGET_SORT_ORDER, desc=desc) -BANNER_TYPE_SORT_ORDER = models.Case( +BANNER_LEVEL_SORT_ORDER = models.Case( models.When( - type=BannerNotificationType.EXCEPTION, + level=BannerNotificationLevel.EXCEPTION, then=models.Value(1), ), models.When( - type=BannerNotificationType.WARNING, + level=BannerNotificationLevel.WARNING, then=models.Value(2), ), models.When( - type=BannerNotificationType.NORMAL, + level=BannerNotificationLevel.NORMAL, then=models.Value(3), ), default=models.Value(4), diff --git a/tests/factories/banner_notification.py b/tests/factories/banner_notification.py index 8530ab81b..cc9f749d8 100644 --- a/tests/factories/banner_notification.py +++ b/tests/factories/banner_notification.py @@ -5,7 +5,7 @@ from django.utils import timezone from factory import fuzzy -from common.choices import BannerNotificationTarget, BannerNotificationType +from common.choices import BannerNotificationLevel, BannerNotificationTarget from common.models import BannerNotification from ._base import GenericDjangoModelFactory @@ -18,7 +18,7 @@ class Meta: name = factory.Faker("text", max_nb_chars=100) message = factory.Faker("text", max_nb_chars=1_000) draft = True - type = fuzzy.FuzzyChoice(BannerNotificationType.values) + level = fuzzy.FuzzyChoice(BannerNotificationLevel.values) target = fuzzy.FuzzyChoice(BannerNotificationTarget.values) active_from = None active_until = None diff --git a/tests/test_graphql_api/test_banner_notification/test_sorting.py b/tests/test_graphql_api/test_banner_notification/test_sorting.py index 36b507ee7..2e6947628 100644 --- a/tests/test_graphql_api/test_banner_notification/test_sorting.py +++ b/tests/test_graphql_api/test_banner_notification/test_sorting.py @@ -5,7 +5,7 @@ from django.utils import timezone from freezegun import freeze_time -from common.choices import BannerNotificationTarget, BannerNotificationType +from common.choices import BannerNotificationLevel, BannerNotificationTarget from tests.factories import BannerNotificationFactory from tests.factories.user import UserFactory from tests.helpers import load_content, parametrize_helper @@ -368,39 +368,39 @@ def test_sort_banner_notifications_by_target(graphql, order_by, expected): **parametrize_helper( { "Ascending order": OrderingParams( - order_by="type", + order_by="level", expected=[ - {"node": {"message": "3", "type": "EXCEPTION"}}, - {"node": {"message": "2", "type": "WARNING"}}, - {"node": {"message": "1", "type": "NORMAL"}}, + {"node": {"message": "3", "level": "EXCEPTION"}}, + {"node": {"message": "2", "level": "WARNING"}}, + {"node": {"message": "1", "level": "NORMAL"}}, ], ), "Descending order": OrderingParams( - order_by="-type", + order_by="-level", expected=[ - {"node": {"message": "1", "type": "NORMAL"}}, - {"node": {"message": "2", "type": "WARNING"}}, - {"node": {"message": "3", "type": "EXCEPTION"}}, + {"node": {"message": "1", "level": "NORMAL"}}, + {"node": {"message": "2", "level": "WARNING"}}, + {"node": {"message": "3", "level": "EXCEPTION"}}, ], ), }, ) ) -def test_sort_banner_notifications_by_type(graphql, order_by, expected): +def test_sort_banner_notifications_by_level(graphql, order_by, expected): # given: # - There are banner notification for all types in the system # - Notification manager user is using the system BannerNotificationFactory.create_active( message="1", - type=BannerNotificationType.NORMAL, + level=BannerNotificationLevel.NORMAL, ) BannerNotificationFactory.create_active( message="2", - type=BannerNotificationType.WARNING, + level=BannerNotificationLevel.WARNING, ) BannerNotificationFactory.create_active( message="3", - type=BannerNotificationType.EXCEPTION, + level=BannerNotificationLevel.EXCEPTION, ) user = UserFactory.create_with_general_permissions(perms=["can_manage_notifications"]) graphql.force_login(user) @@ -414,7 +414,7 @@ def test_sort_banner_notifications_by_type(graphql, order_by, expected): edges { node { message - type + level } } }