From 49b75729ae1dfbd07d8217c94cfa8b4afe430855 Mon Sep 17 00:00:00 2001 From: Jonathan Reveille Date: Mon, 10 Jun 2024 18:22:56 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20deprecate=20`has?= =?UTF-8?q?=5Fconsent=5Fto=5Fterms`=20for=20Order=20model?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From now on, the terms and conditions (CGV in French) must be specific to each organization. We can no longer use a global version for the entire platform. These terms will be included directly in the contract's context, so the Order model no longer needs to track user acceptance, as this will happen during contract signing. Fix #816 --- CHANGELOG.md | 1 + src/backend/joanie/core/admin.py | 1 - ...ter_order_has_consent_to_terms_and_more.py | 23 ++++++ src/backend/joanie/core/models/products.py | 10 ++- src/backend/joanie/core/serializers/admin.py | 1 - src/backend/joanie/core/serializers/client.py | 11 --- .../joanie/tests/core/api/order/test_abort.py | 1 - .../tests/core/api/order/test_create.py | 74 ------------------- .../tests/core/test_api_admin_orders.py | 2 - .../joanie/tests/core/test_models_order.py | 17 +++++ .../joanie/tests/swagger/admin-swagger.json | 6 -- src/backend/joanie/tests/swagger/swagger.json | 9 --- 12 files changed, 50 insertions(+), 106 deletions(-) create mode 100644 src/backend/joanie/core/migrations/0034_alter_order_has_consent_to_terms_and_more.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e8d43498b..1552ec9744 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ### Changed +- Deprecated field `has_consent_to_terms` for `Order` model - Add `currency` field to `OrderPaymentSerializer` serializer - Allow an order with `no_payment` state to pay for failed installment on a payment schedule diff --git a/src/backend/joanie/core/admin.py b/src/backend/joanie/core/admin.py index 6029bbe542..7324f4c89d 100644 --- a/src/backend/joanie/core/admin.py +++ b/src/backend/joanie/core/admin.py @@ -583,7 +583,6 @@ class OrderAdmin(DjangoObjectActions, admin.ModelAdmin): readonly_fields = ( "state", "total", - "has_consent_to_terms", "invoice", "certificate", ) diff --git a/src/backend/joanie/core/migrations/0034_alter_order_has_consent_to_terms_and_more.py b/src/backend/joanie/core/migrations/0034_alter_order_has_consent_to_terms_and_more.py new file mode 100644 index 0000000000..3bf813a670 --- /dev/null +++ b/src/backend/joanie/core/migrations/0034_alter_order_has_consent_to_terms_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.13 on 2024-06-10 16:21 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0033_alter_order_state'), + ] + + operations = [ + migrations.AlterField( + model_name='order', + name='has_consent_to_terms', + field=models.BooleanField(db_column='has_consent_to_terms', default=False, editable=False, help_text='User has consented to the platform terms and conditions.', verbose_name='has consent to terms'), + ), + migrations.RenameField( + model_name='order', + old_name='has_consent_to_terms', + new_name='_has_consent_to_terms', + ), + ] diff --git a/src/backend/joanie/core/models/products.py b/src/backend/joanie/core/models/products.py index 977294b875..26de57755e 100644 --- a/src/backend/joanie/core/models/products.py +++ b/src/backend/joanie/core/models/products.py @@ -465,11 +465,12 @@ class Order(BaseModel): on_delete=models.RESTRICT, db_index=True, ) - has_consent_to_terms = models.BooleanField( + _has_consent_to_terms = models.BooleanField( verbose_name=_("has consent to terms"), editable=False, default=False, help_text=_("User has consented to the platform terms and conditions."), + db_column="has_consent_to_terms", ) state = models.CharField( default=enums.ORDER_STATE_DRAFT, @@ -1116,6 +1117,13 @@ def withdraw(self): self.flow.cancel() + @property + def has_consent_to_terms(self): + """Redefine `has_consent_to_terms` property to raise an exception if used""" + raise DeprecationWarning( + "Access denied to has_consent_to_terms: deprecated field" + ) + class OrderTargetCourseRelation(BaseModel): """ diff --git a/src/backend/joanie/core/serializers/admin.py b/src/backend/joanie/core/serializers/admin.py index df0ddc90a6..69726f9158 100755 --- a/src/backend/joanie/core/serializers/admin.py +++ b/src/backend/joanie/core/serializers/admin.py @@ -1085,7 +1085,6 @@ class Meta: "contract", "certificate", "main_invoice", - "has_consent_to_terms", ) read_only_fields = fields diff --git a/src/backend/joanie/core/serializers/client.py b/src/backend/joanie/core/serializers/client.py index 4d3bf8ef92..0182ff1551 100644 --- a/src/backend/joanie/core/serializers/client.py +++ b/src/backend/joanie/core/serializers/client.py @@ -1129,7 +1129,6 @@ class OrderSerializer(serializers.ModelSerializer): read_only=True, slug_field="id", source="certificate" ) contract = ContractSerializer(read_only=True, exclude_abilities=True) - has_consent_to_terms = serializers.BooleanField(write_only=True) payment_schedule = OrderPaymentSerializer(many=True, read_only=True) class Meta: @@ -1151,7 +1150,6 @@ class Meta: "target_enrollments", "total", "total_currency", - "has_consent_to_terms", "payment_schedule", ] read_only_fields = fields @@ -1166,14 +1164,6 @@ def get_target_enrollments(self, order) -> list[dict]: context=self.context, ).data - def validate_has_consent_to_terms(self, value): - """Check that user has accepted terms and conditions.""" - if not value: - message = _("You must accept the terms and conditions to proceed.") - raise serializers.ValidationError(message) - - return value - def create(self, validated_data): """ Create a new order and set the organization if provided. @@ -1196,7 +1186,6 @@ def update(self, instance, validated_data): validated_data.pop("organization", None) validated_data.pop("product", None) validated_data.pop("order_group", None) - validated_data.pop("has_consent_to_terms", None) return super().update(instance, validated_data) def get_total_currency(self, *args, **kwargs) -> str: diff --git a/src/backend/joanie/tests/core/api/order/test_abort.py b/src/backend/joanie/tests/core/api/order/test_abort.py index 286e089274..7b2521f85c 100644 --- a/src/backend/joanie/tests/core/api/order/test_abort.py +++ b/src/backend/joanie/tests/core/api/order/test_abort.py @@ -91,7 +91,6 @@ def test_api_order_abort(self, mock_abort_payment): "product_id": str(product.id), "course_code": course.code, "billing_address": billing_address, - "has_consent_to_terms": True, } response = self.client.post( "/api/v1.0/orders/", diff --git a/src/backend/joanie/tests/core/api/order/test_create.py b/src/backend/joanie/tests/core/api/order/test_create.py index 0ebec060bf..fa61904beb 100644 --- a/src/backend/joanie/tests/core/api/order/test_create.py +++ b/src/backend/joanie/tests/core/api/order/test_create.py @@ -60,7 +60,6 @@ def test_api_order_create_authenticated_for_course_success(self, _mock_thumbnail "course_code": course.code, "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -211,7 +210,6 @@ def test_api_order_create_authenticated_for_enrollment_success( "enrollment_id": str(enrollment.id), "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.generate_token_from_user(enrollment.user) @@ -380,7 +378,6 @@ def test_api_order_create_authenticated_for_enrollment_not_owner( "enrollment_id": str(enrollment.id), "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -419,7 +416,6 @@ def test_api_order_create_submit_authenticated_organization_not_passed(self): data = { "course_code": course.code, "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -461,7 +457,6 @@ def test_api_order_create_authenticated_organization_not_passed_one(self): data = { "course_code": course.code, "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -530,7 +525,6 @@ def test_api_order_create_authenticated_organization_passed_several(self): data = { "course_code": course.code, "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -579,7 +573,6 @@ def test_api_order_create_authenticated_has_read_only_fields(self, _mock_thumbna "product_id": str(product.id), "id": uuid.uuid4(), "amount": 0.00, - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -719,7 +712,6 @@ def test_api_order_create_authenticated_invalid_product(self): "course_code": course.code, "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -768,7 +760,6 @@ def test_api_order_create_authenticated_invalid_organization(self): "course_code": course.code, "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } token = self.get_user_token("panoramix") @@ -819,7 +810,6 @@ def test_api_order_create_authenticated_missing_product_then_course(self): response.json(), { "product_id": ["This field is required."], - "has_consent_to_terms": ["This field is required."], }, ) @@ -830,7 +820,6 @@ def test_api_order_create_authenticated_missing_product_then_course(self): HTTP_AUTHORIZATION=f"Bearer {token}", data={ "product_id": str(product.id), - "has_consent_to_terms": True, }, ) @@ -842,56 +831,6 @@ def test_api_order_create_authenticated_missing_product_then_course(self): {"__all__": ["Either the course or the enrollment field is required."]}, ) - def test_api_order_create_authenticated_product_with_contract_require_terms_consent( - self, - ): - """ - The payload must contain has_consent_to_terms sets to True to create an order. - """ - relation = factories.CourseProductRelationFactory() - token = self.get_user_token("panoramix") - - data = { - "product_id": str(relation.product.id), - "course_code": relation.course.code, - } - - # - `has_consent_to_terms` is required - response = self.client.post( - "/api/v1.0/orders/", - content_type="application/json", - HTTP_AUTHORIZATION=f"Bearer {token}", - data=data, - ) - self.assertContains( - response, - '{"has_consent_to_terms":["This field is required."]}', - status_code=HTTPStatus.BAD_REQUEST, - ) - - # - `has_consent_to_terms` must be set to True - data.update({"has_consent_to_terms": False}) - response = self.client.post( - "/api/v1.0/orders/", - content_type="application/json", - HTTP_AUTHORIZATION=f"Bearer {token}", - data=data, - ) - self.assertContains( - response, - '{"has_consent_to_terms":["You must accept the terms and conditions to proceed."]}', - status_code=HTTPStatus.BAD_REQUEST, - ) - - data.update({"has_consent_to_terms": True}) - response = self.client.post( - "/api/v1.0/orders/", - content_type="application/json", - HTTP_AUTHORIZATION=f"Bearer {token}", - data=data, - ) - self.assertEqual(response.status_code, HTTPStatus.CREATED) - def test_api_order_create_authenticated_product_course_unicity(self): """ If a user tries to create a new order while he has already a not canceled order @@ -910,7 +849,6 @@ def test_api_order_create_authenticated_product_course_unicity(self): "product_id": str(product.id), "course_code": course.code, "organization_id": str(organization.id), - "has_consent_to_terms": True, } response = self.client.post( @@ -953,7 +891,6 @@ def test_api_order_create_authenticated_billing_address_not_required(self): "product_id": str(product.id), "course_code": course.code, "organization_id": str(organization.id), - "has_consent_to_terms": True, } response = self.client.post( @@ -999,7 +936,6 @@ def test_api_order_create_authenticated_payment_binding( "organization_id": str(organization.id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } with self.assertNumQueries(23): @@ -1169,7 +1105,6 @@ def test_api_order_create_authenticated_payment_with_registered_credit_card( "product_id": str(product.id), "billing_address": billing_address, "credit_card_id": str(credit_card.id), - "has_consent_to_terms": True, } response = self.client.post( @@ -1268,7 +1203,6 @@ def test_api_order_create_authenticated_payment_failed(self, mock_create_payment "organization_id": str(organization.id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } response = self.client.post( @@ -1321,7 +1255,6 @@ def test_api_order_create_authenticated_nb_seats(self): "order_group_id": str(order_group.id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } token = self.generate_token_from_user(user) @@ -1371,7 +1304,6 @@ def test_api_order_create_authenticated_no_seats(self): "order_group_id": str(order_group.id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } token = self.generate_token_from_user(user) @@ -1402,7 +1334,6 @@ def test_api_order_create_authenticated_free_product_no_billing_address(self): "course_code": course.code, "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } response = self.client.post( "/api/v1.0/orders/", @@ -1439,7 +1370,6 @@ def test_api_order_create_authenticated_no_billing_address_to_validation(self): "course_code": course.code, "organization_id": str(organization.id), "product_id": str(product.id), - "has_consent_to_terms": True, } response = self.client.post( @@ -1488,7 +1418,6 @@ def test_api_order_create_order_group_required(self): "organization_id": str(relation.organizations.first().id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } token = self.generate_token_from_user(user) @@ -1532,7 +1461,6 @@ def test_api_order_create_order_group_unrelated(self): "organization_id": str(organization.id), "product_id": str(relation.product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } response = self.client.post( @@ -1581,7 +1509,6 @@ def test_api_order_create_several_order_groups(self): "organization_id": str(relation.organizations.first().id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } token = self.generate_token_from_user(user) @@ -1636,7 +1563,6 @@ def test_api_order_create_inactive_order_groups(self): "organization_id": str(relation.organizations.first().id), "product_id": str(product.id), "billing_address": billing_address, - "has_consent_to_terms": True, } token = self.generate_token_from_user(user) diff --git a/src/backend/joanie/tests/core/test_api_admin_orders.py b/src/backend/joanie/tests/core/test_api_admin_orders.py index b3e76e74ae..c334085926 100644 --- a/src/backend/joanie/tests/core/test_api_admin_orders.py +++ b/src/backend/joanie/tests/core/test_api_admin_orders.py @@ -564,7 +564,6 @@ def test_api_admin_orders_course_retrieve(self): "id": str(order.id), "created_on": format_date(order.created_on), "state": order.state, - "has_consent_to_terms": False, "owner": { "id": str(order.owner.id), "username": order.owner.username, @@ -710,7 +709,6 @@ def test_api_admin_orders_enrollment_retrieve(self): "id": str(order.id), "created_on": format_date(order.created_on), "state": order.state, - "has_consent_to_terms": False, "owner": { "id": str(order.owner.id), "username": order.owner.username, diff --git a/src/backend/joanie/tests/core/test_models_order.py b/src/backend/joanie/tests/core/test_models_order.py index 647051f93c..a370ab3e49 100644 --- a/src/backend/joanie/tests/core/test_models_order.py +++ b/src/backend/joanie/tests/core/test_models_order.py @@ -1007,3 +1007,20 @@ def test_models_order_submit_for_signature_check_contract_context_course_section self.assertIsInstance(contract.context["course"]["price"], str) self.assertEqual(order.total, Decimal("1202.99")) self.assertEqual(contract.context["course"]["price"], "1202.99") + + def test_models_order_has_consent_to_terms_should_raise_deprecation_warning(self): + """ + Due to the refactoring of the `has_consent_to_terms` field, it is now be a deprecated + field. So when calling the field, it should raise a DeprecationWarning error. + """ + order = factories.OrderFactory() + + with self.assertRaises(DeprecationWarning) as deprecation_warning: + # ruff : noqa : B018 + # pylint: disable=pointless-statement + order.has_consent_to_terms + + self.assertEqual( + str(deprecation_warning.exception), + "Access denied to has_consent_to_terms: deprecated field", + ) diff --git a/src/backend/joanie/tests/swagger/admin-swagger.json b/src/backend/joanie/tests/swagger/admin-swagger.json index 67e60b33b7..424432a38b 100644 --- a/src/backend/joanie/tests/swagger/admin-swagger.json +++ b/src/backend/joanie/tests/swagger/admin-swagger.json @@ -5505,11 +5505,6 @@ }, "main_invoice": { "$ref": "#/components/schemas/AdminInvoice" - }, - "has_consent_to_terms": { - "type": "boolean", - "readOnly": true, - "description": "User has consented to the platform terms and conditions." } }, "required": [ @@ -5518,7 +5513,6 @@ "course", "created_on", "enrollment", - "has_consent_to_terms", "id", "main_invoice", "order_group", diff --git a/src/backend/joanie/tests/swagger/swagger.json b/src/backend/joanie/tests/swagger/swagger.json index 66d4adb25a..8adaf95cc8 100644 --- a/src/backend/joanie/tests/swagger/swagger.json +++ b/src/backend/joanie/tests/swagger/swagger.json @@ -6306,14 +6306,9 @@ "type": "string", "format": "uuid", "description": "primary key for the record as UUID" - }, - "has_consent_to_terms": { - "type": "boolean", - "writeOnly": true } }, "required": [ - "has_consent_to_terms", "product_id" ] }, @@ -7052,10 +7047,6 @@ "type": "string", "format": "uuid", "description": "primary key for the record as UUID" - }, - "has_consent_to_terms": { - "type": "boolean", - "writeOnly": true } } },