From b48a1291b96d8db30966a6077a4535ba455e1cd1 Mon Sep 17 00:00:00 2001 From: Jonathan Reveille Date: Thu, 30 Jan 2025 19:06:46 +0100 Subject: [PATCH] =?UTF-8?q?fixup!=20=E2=9C=A8(backend)=20add=20start=20and?= =?UTF-8?q?=20end=20date=20on=20order=20groups=20model?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../0053_alter_ordergroup_and_more.py | 10 +- src/backend/joanie/core/models/products.py | 23 +- src/backend/joanie/core/serializers/admin.py | 27 ++- src/backend/joanie/core/utils/ordergroup.py | 27 +++ .../tests/core/test_api_admin_order_group.py | 227 ++++++++++++++++++ .../tests/core/test_models_order_group.py | 28 --- .../tests/core/utils/test_utils_ordergroup.py | 92 +++++++ .../joanie/tests/swagger/admin-swagger.json | 49 ++-- src/backend/joanie/tests/swagger/swagger.json | 6 +- 9 files changed, 414 insertions(+), 75 deletions(-) create mode 100644 src/backend/joanie/core/utils/ordergroup.py create mode 100644 src/backend/joanie/tests/core/utils/test_utils_ordergroup.py diff --git a/src/backend/joanie/core/migrations/0053_alter_ordergroup_and_more.py b/src/backend/joanie/core/migrations/0053_alter_ordergroup_and_more.py index bbe137b3a..453700bde 100644 --- a/src/backend/joanie/core/migrations/0053_alter_ordergroup_and_more.py +++ b/src/backend/joanie/core/migrations/0053_alter_ordergroup_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.18 on 2025-01-29 16:04 +# Generated by Django 4.2.18 on 2025-01-30 13:33 from django.db import migrations, models @@ -13,16 +13,12 @@ class Migration(migrations.Migration): migrations.AddField( model_name='ordergroup', name='end', - field=models.DateTimeField(blank=True, null=True, verbose_name='order group end'), + field=models.DateTimeField(blank=True, help_text='order group’s end date and time', null=True, verbose_name='order group end datetime'), ), migrations.AddField( model_name='ordergroup', name='start', - field=models.DateTimeField(blank=True, null=True, verbose_name='order group start'), - ), - migrations.AddConstraint( - model_name='ordergroup', - constraint=models.CheckConstraint(check=models.Q(models.Q(('start__isnull', True), _negated=True), ('end__isnull', True), _connector='XOR'), name='both_start_and_end_dates_must_be_set_or_neither', violation_error_message='Both start and end dates must be set, or neither.'), + field=models.DateTimeField(blank=True, help_text='order group’s start date and time', null=True, verbose_name='order group start datetime'), ), migrations.AddConstraint( model_name='ordergroup', diff --git a/src/backend/joanie/core/models/products.py b/src/backend/joanie/core/models/products.py index 8fd3bcb09..f72559d4c 100755 --- a/src/backend/joanie/core/models/products.py +++ b/src/backend/joanie/core/models/products.py @@ -379,19 +379,22 @@ class OrderGroup(BaseModel): on_delete=models.CASCADE, ) is_active = models.BooleanField(_("is active"), default=True) - # Available start to end period - start = models.DateTimeField(_("order group start"), blank=True, null=True) - end = models.DateTimeField(_("order group end"), blank=True, null=True) + # Available start to end period of activation of the OrderGroup + start = models.DateTimeField( + help_text=_("order group’s start date and time"), + verbose_name=_("order group start datetime"), + blank=True, + null=True, + ) + end = models.DateTimeField( + help_text=_("order group’s end date and time"), + verbose_name=_("order group end datetime"), + blank=True, + null=True, + ) class Meta: constraints = [ - models.CheckConstraint( - check=~models.Q(start__isnull=True) ^ models.Q(end__isnull=True), - name="both_start_and_end_dates_must_be_set_or_neither", - violation_error_message=_( - "Both start and end dates must be set, or neither." - ), - ), models.CheckConstraint( check=models.Q(start__lte=models.F("end")), name="check_start_before_end", diff --git a/src/backend/joanie/core/serializers/admin.py b/src/backend/joanie/core/serializers/admin.py index dbd42d839..8e8c404a5 100755 --- a/src/backend/joanie/core/serializers/admin.py +++ b/src/backend/joanie/core/serializers/admin.py @@ -19,6 +19,7 @@ ThumbnailDetailField, ) from joanie.core.utils import Echo +from joanie.core.utils.ordergroup import is_active from joanie.payment import models as payment_models @@ -451,10 +452,7 @@ class AdminOrderGroupSerializer(serializers.ModelSerializer): .validators[1] .limit_value, ) - is_active = serializers.BooleanField( - required=False, - default=models.OrderGroup._meta.get_field("is_active").default, - ) + is_active = serializers.SerializerMethodField(read_only=True) nb_available_seats = serializers.SerializerMethodField(read_only=True) class Meta: @@ -475,6 +473,25 @@ def get_nb_available_seats(self, order_group) -> int: """Return the number of available seats for this order group.""" return order_group.nb_seats - order_group.get_nb_binding_orders() + def get_is_active(self, order_group): + """ + Return if the order group is active if start or end dates are setted. Else, it returns + the value in the database. + """ + return is_active(order_group) + + def update(self, instance, validated_data): + """ + Update instance and ensure `is_active` is only updated if explicitly + passed in the request. Else, don't update the field. + """ + is_active_explicit = "is_active" in self.initial_data + + if is_active_explicit: + instance.is_active = self.initial_data["is_active"] + + return super().update(instance, validated_data) + @extend_schema_serializer(exclude_fields=("course_product_relation",)) class AdminOrderGroupCreateSerializer(AdminOrderGroupSerializer): @@ -485,6 +502,8 @@ class AdminOrderGroupCreateSerializer(AdminOrderGroupSerializer): the order group. """ + is_active = serializers.BooleanField(required=False) + class Meta(AdminOrderGroupSerializer.Meta): fields = [*AdminOrderGroupSerializer.Meta.fields, "course_product_relation"] diff --git a/src/backend/joanie/core/utils/ordergroup.py b/src/backend/joanie/core/utils/ordergroup.py new file mode 100644 index 000000000..a38c72771 --- /dev/null +++ b/src/backend/joanie/core/utils/ordergroup.py @@ -0,0 +1,27 @@ +""" +Util to manage OrderGroup +""" + +from django.utils import timezone + + +def is_active(order_group): + """ + Determine the value of `is_active` of an OrderGroup object. If `start` nor `end` datetime + fields are set, we return the value setted in the database. Otherwise when `start`, `end` + or both datetime are setted, we should compare it to timezone.now() calculate if it's + the window period to set `is_active`. + """ + if not order_group.start and not order_group.end: + return order_group.is_active + + now = timezone.now() + + if order_group.start and order_group.end: + return order_group.start <= now <= order_group.end + if order_group.start: + return now >= order_group.start + if order_group.end: + return now <= order_group.end + + return False diff --git a/src/backend/joanie/tests/core/test_api_admin_order_group.py b/src/backend/joanie/tests/core/test_api_admin_order_group.py index 80a1b4850..a1b43ef22 100644 --- a/src/backend/joanie/tests/core/test_api_admin_order_group.py +++ b/src/backend/joanie/tests/core/test_api_admin_order_group.py @@ -2,8 +2,11 @@ Test suite for OrderGroup Admin API. """ +from datetime import datetime from http import HTTPStatus from operator import itemgetter +from unittest import mock +from zoneinfo import ZoneInfo from django.db import IntegrityError from django.test import TestCase @@ -439,3 +442,227 @@ def test_admin_api_order_group_patch_start_date_or_end_date(self): self.assertEqual(response.status_code, HTTPStatus.OK) content = response.json() self.assertEqual(content["end"], data["end"]) + + def test_admin_api_order_group_patch_start_date_should_compute_is_active(self): + """ + The `is_active` field is dynamically computed when the `start` datetime field is set. + Otherwise, it should retrieve the stored value from the database. + """ + admin = factories.UserFactory(is_staff=True, is_superuser=True) + self.client.login(username=admin.username, password="password") + relation = factories.CourseProductRelationFactory() + order_group = factories.OrderGroupFactory( + course_product_relation=relation, is_active=False + ) + + response = self.client.get( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/" + ) + + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertFalse(order_group.is_active) # Stored value in database + self.assertFalse(content["is_active"]) + + # Update the `start` field that should make is_active to True (start <= now) + mocked_now = datetime(2025, 1, 29, 16, 20, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "start": "2025-01-10T00:00:00Z", + }, + ) + + order_group.refresh_from_db() + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertEqual(content["start"], "2025-01-10T00:00:00Z") + self.assertTrue(content["is_active"]) + self.assertFalse(order_group.is_active) # Stored value in database + + # Let's pretend that start date is tomorrow from the mocked date + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "start": "2025-01-30T00:00:00Z", + "is_active": True, + }, + ) + + order_group.refresh_from_db() + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertEqual(content["start"], "2025-01-30T00:00:00Z") + self.assertFalse(content["is_active"]) + self.assertTrue(order_group.is_active) + + def test_admin_api_order_group_patch_end_date_should_compute_is_active(self): + """ + The `is_active` field is dynamically computed when the `end` datetime field is set. + Otherwise, it should retrieve its stored value from the database. + """ + admin = factories.UserFactory(is_staff=True, is_superuser=True) + self.client.login(username=admin.username, password="password") + relation = factories.CourseProductRelationFactory() + order_group = factories.OrderGroupFactory( + course_product_relation=relation, is_active=False + ) + + response = self.client.get( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/" + ) + + content = response.json() + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertFalse(order_group.is_active) + self.assertFalse(content["is_active"]) + + mocked_now = datetime(2025, 1, 30, 16, 20, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "end": "2025-02-20T00:00:00Z", + }, + ) + + order_group.refresh_from_db() + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertEqual(content["end"], "2025-02-20T00:00:00Z") + self.assertTrue(content["is_active"]) + self.assertFalse(order_group.is_active) + + # Let's pretend that we passed over the end datetime and we set the `is_active` to True + # in for the object in database, the serializer should return the computed value instead + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "end": "2025-01-29T00:00:00Z", + "is_active": True, + }, + ) + + order_group.refresh_from_db() + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertEqual(content["end"], "2025-01-29T00:00:00Z") + self.assertFalse(content["is_active"]) + self.assertTrue(order_group.is_active) + + def test_api_admin_is_active_start_and_end_date_should_compute_is_active(self): + """ + When the `start` and `end` date on an order group correspond to the activation window, + computed is_active should return True + """ + admin = factories.UserFactory(is_staff=True, is_superuser=True) + self.client.login(username=admin.username, password="password") + relation = factories.CourseProductRelationFactory() + order_group = factories.OrderGroupFactory( + course_product_relation=relation, is_active=False + ) + + response = self.client.get( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/" + ) + + content = response.json() + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertFalse(order_group.is_active) + self.assertFalse(content["is_active"]) + + mocked_now = datetime(2025, 1, 30, 16, 20, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "start": "2025-01-15T00:00:00Z", + "end": "2025-02-20T00:00:00Z", + }, + ) + + order_group.refresh_from_db() + content = response.json() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertEqual(content["start"], "2025-01-15T00:00:00Z") + self.assertEqual(content["end"], "2025-02-20T00:00:00Z") + self.assertTrue(content["is_active"]) + self.assertFalse(order_group.is_active) + + def test_api_admin_order_group_force_to_is_active_false_when_is_active_due_to_start_and_end( + self, + ): + """ + When we want to force an order group to stop suddenly, we need to set the start + and end date to None in order to update `is_active` in the database. + """ + admin = factories.UserFactory(is_staff=True, is_superuser=True) + self.client.login(username=admin.username, password="password") + relation = factories.CourseProductRelationFactory() + + order_group = factories.OrderGroupFactory( + course_product_relation=relation, + is_active=True, + start="2025-01-15T00:00:00Z", + end="2025-02-20T00:00:00Z", + ) + + mocked_now = datetime(2025, 1, 30, 16, 20, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.get( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/" + ) + + content = response.json() + order_group.refresh_from_db() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertTrue(content["is_active"]) + self.assertTrue(order_group.is_active) + + # If we want to force it to stop and return is_active False + # We should set the start and end date to None + # Because : When there are date, the truth is the computed `is_active` value. + # When there are no dates at all, `is_active` in database value is the truth. + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + response = self.client.patch( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/", + content_type="application/json", + data={ + "start": None, + "end": None, + "is_active": False, + }, + ) + + content = response.json() + order_group.refresh_from_db() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertFalse(content["is_active"]) + self.assertFalse(order_group.is_active) + + response = self.client.get( + f"{self.base_url}/{relation.id}/order-groups/{order_group.id}/" + ) + + content = response.json() + order_group.refresh_from_db() + + self.assertEqual(response.status_code, HTTPStatus.OK) + self.assertFalse(content["is_active"]) + self.assertFalse(order_group.is_active) diff --git a/src/backend/joanie/tests/core/test_models_order_group.py b/src/backend/joanie/tests/core/test_models_order_group.py index fc5f8afdf..df26569c8 100644 --- a/src/backend/joanie/tests/core/test_models_order_group.py +++ b/src/backend/joanie/tests/core/test_models_order_group.py @@ -45,34 +45,6 @@ def test_model_order_group_check_start_before_end(self): ' check constraint "check_start_before_end"' in str(context.exception) ) - def test_model_order_group_set_end_date_but_not_start_date(self): - """ - When there is an end date set, the start date should be set, else it raises - an Integrity error. - """ - with self.assertRaises(IntegrityError) as context: - factories.OrderGroupFactory(end=timezone.now()) - - self.assertTrue( - 'new row for relation "core_ordergroup" violates check' - ' constraint "both_start_and_end_dates_must_be_set_or_neither"' - in str(context.exception) - ) - - def test_model_order_group_set_start_date_but_not_end_date(self): - """ - When there is an start date set, the end date should be set, else it raises - an Integrity error. - """ - with self.assertRaises(IntegrityError) as context: - factories.OrderGroupFactory(start=timezone.now()) - - self.assertTrue( - 'new row for relation "core_ordergroup" violates check' - ' constraint "both_start_and_end_dates_must_be_set_or_neither"' - in str(context.exception) - ) - def test_model_order_group_set_start_and_end_date(self): """ When we set a start date that is not greater than the end date, the order diff --git a/src/backend/joanie/tests/core/utils/test_utils_ordergroup.py b/src/backend/joanie/tests/core/utils/test_utils_ordergroup.py new file mode 100644 index 000000000..43c943c2d --- /dev/null +++ b/src/backend/joanie/tests/core/utils/test_utils_ordergroup.py @@ -0,0 +1,92 @@ +"""Test suite for utils ordergroup methods""" + +from datetime import datetime +from unittest import mock +from zoneinfo import ZoneInfo + +from django.test import TestCase + +from joanie.core.factories import OrderGroupFactory +from joanie.core.utils.ordergroup import is_active + + +class UtilsOrderGroupTestCase(TestCase): + """Test suite for utils ordergroup methods""" + + def test_utils_ordergroup_is_active_should_return_false_when_no_start_nor_end_date_setted( + self, + ): + """ + When there are no start nor end datetime, the method `is_active()` should return + the value set on the object's field. + """ + order_group_1 = OrderGroupFactory(is_active=True, start=None, end=None) + order_group_2 = OrderGroupFactory(is_active=False, start=None, end=None) + + self.assertTrue(is_active(order_group_1)) + self.assertFalse(is_active(order_group_2)) + + def test_utils_ordergroup_is_active_when_only_start_date_setted(self): + """ + When there is a start datetime, the object's field value `is_active` does not matter + anymore, the value for `is_active` is computed with timezone.now() and the start datetime + setted. + """ + order_group = OrderGroupFactory( + is_active=True, + start=datetime(2025, 1, 15, 12, 0, tzinfo=ZoneInfo("UTC")), + end=None, + ) + + # Datetime inferior to start date + mocked_now = datetime(2025, 1, 10, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertFalse(is_active(order_group)) + + # Datetime superior to start date + mocked_now = datetime(2025, 1, 18, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertTrue(is_active(order_group)) + + def test_utils_ordergroup_is_active_when_only_end_date_setted(self): + """ + When there is a end datetime, the object's field value `is_active` does not matter anymore, + the value for `is_active` is computed with timezone.now() and the end datetime setted. + """ + order_group = OrderGroupFactory( + is_active=True, + start=None, + end=datetime(2025, 1, 15, 12, 0, tzinfo=ZoneInfo("UTC")), + ) + + # Datetime superior to end date: last minute orders are finished + mocked_now = datetime(2025, 1, 18, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertFalse(is_active(order_group)) + + # Datetime inferior to end date : last minite orders are opened + mocked_now = datetime(2025, 1, 10, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertTrue(is_active(order_group)) + + def test_utils_ordergroup_is_active_when_both_datetime_start_and_end_date(self): + """ + When there is a start and end datetime, the object's field value `is_active` does not + matter anymore, the value for `is_active` is computed with timezone.now() and the + start and end datetime values setted. + """ + order_group = OrderGroupFactory( + is_active=True, + start=datetime(2025, 1, 15, 12, 0, tzinfo=ZoneInfo("UTC")), + end=datetime(2025, 2, 15, 12, 0, tzinfo=ZoneInfo("UTC")), + ) + + # Datetime outside the range of start and end datetime + mocked_now = datetime(2025, 3, 18, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertFalse(is_active(order_group)) + + # Datetime inside the range of start and end datetime + mocked_now = datetime(2025, 2, 12, 12, 0, tzinfo=ZoneInfo("UTC")) + with mock.patch("django.utils.timezone.now", return_value=mocked_now): + self.assertTrue(is_active(order_group)) diff --git a/src/backend/joanie/tests/swagger/admin-swagger.json b/src/backend/joanie/tests/swagger/admin-swagger.json index 2ac8b7c49..062518086 100644 --- a/src/backend/joanie/tests/swagger/admin-swagger.json +++ b/src/backend/joanie/tests/swagger/admin-swagger.json @@ -6398,8 +6398,8 @@ "description": "The maximum number of orders that can be validated for a given order group" }, "is_active": { - "type": "boolean", - "default": true + "type": "string", + "readOnly": true }, "nb_available_seats": { "type": "integer", @@ -6420,19 +6420,22 @@ "type": "string", "format": "date-time", "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } }, "required": [ "can_edit", "created_on", "id", + "is_active", "nb_available_seats" ] }, @@ -6455,8 +6458,7 @@ "description": "The maximum number of orders that can be validated for a given order group" }, "is_active": { - "type": "boolean", - "default": true + "type": "boolean" }, "nb_available_seats": { "type": "integer", @@ -6477,13 +6479,15 @@ "type": "string", "format": "date-time", "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } }, "required": [ @@ -6506,20 +6510,21 @@ "description": "The maximum number of orders that can be validated for a given order group" }, "is_active": { - "type": "boolean", - "default": true + "type": "boolean" }, "start": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } } }, @@ -6535,21 +6540,19 @@ "title": "Number of seats", "description": "The maximum number of orders that can be validated for a given order group" }, - "is_active": { - "type": "boolean", - "default": true - }, "start": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } } }, @@ -8589,21 +8592,19 @@ "title": "Number of seats", "description": "The maximum number of orders that can be validated for a given order group" }, - "is_active": { - "type": "boolean", - "default": true - }, "start": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } } }, diff --git a/src/backend/joanie/tests/swagger/swagger.json b/src/backend/joanie/tests/swagger/swagger.json index 9362d4dc9..fec3c1bc7 100644 --- a/src/backend/joanie/tests/swagger/swagger.json +++ b/src/backend/joanie/tests/swagger/swagger.json @@ -6197,14 +6197,16 @@ "format": "date-time", "readOnly": true, "nullable": true, - "title": "Order group start" + "title": "Order group start datetime", + "description": "order group’s start date and time" }, "end": { "type": "string", "format": "date-time", "readOnly": true, "nullable": true, - "title": "Order group end" + "title": "Order group end datetime", + "description": "order group’s end date and time" } }, "required": [