Skip to content

Commit

Permalink
♻️(backend) deprecate has_consent_to_terms for Order model
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jonathanreveille authored and kernicPanel committed Jun 14, 2024
1 parent fed7984 commit 7bbfc01
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 102 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to

### Changed

- Deprecated field `has_consent_to_terms` for `Order` model
- Rework order statuses
- Update certificate template to render logo of organization if
it has a value.
Expand Down
1 change: 0 additions & 1 deletion src/backend/joanie/core/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,6 @@ class OrderAdmin(DjangoObjectActions, admin.ModelAdmin):
readonly_fields = (
"state",
"total",
"has_consent_to_terms",
"invoice",
"certificate",
)
Expand Down
Original file line number Diff line number Diff line change
@@ -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', '0038_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',
),
]
10 changes: 9 additions & 1 deletion src/backend/joanie/core/models/products.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1134,6 +1135,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):
"""
Expand Down
1 change: 0 additions & 1 deletion src/backend/joanie/core/serializers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1085,7 +1085,6 @@ class Meta:
"contract",
"certificate",
"main_invoice",
"has_consent_to_terms",
)
read_only_fields = fields

Expand Down
11 changes: 0 additions & 11 deletions src/backend/joanie/core/serializers/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,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)
credit_card_id = serializers.SlugRelatedField(
queryset=CreditCard.objects.all(),
Expand Down Expand Up @@ -1159,7 +1158,6 @@ class Meta:
"target_enrollments",
"total",
"total_currency",
"has_consent_to_terms",
"payment_schedule",
]
read_only_fields = fields
Expand All @@ -1174,14 +1172,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.
Expand All @@ -1204,7 +1194,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:
Expand Down
74 changes: 0 additions & 74 deletions src/backend/joanie/tests/core/api/order/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,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")

Expand Down Expand Up @@ -240,7 +239,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)

Expand Down Expand Up @@ -411,7 +409,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")

Expand Down Expand Up @@ -450,7 +447,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")

Expand Down Expand Up @@ -480,7 +476,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")

Expand Down Expand Up @@ -542,7 +537,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")

Expand Down Expand Up @@ -727,7 +721,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")

Expand Down Expand Up @@ -867,7 +860,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")

Expand Down Expand Up @@ -916,7 +908,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")

Expand Down Expand Up @@ -967,7 +958,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."],
},
)

Expand All @@ -978,7 +968,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,
},
)

Expand All @@ -990,58 +979,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")
billing_address = BillingAddressDictFactory()

data = {
"product_id": str(relation.product.id),
"course_code": relation.course.code,
"billing_address": billing_address,
}

# - `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
Expand All @@ -1060,7 +997,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(
Expand Down Expand Up @@ -1103,7 +1039,6 @@ def test_api_order_create_authenticated_billing_address_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(
Expand Down Expand Up @@ -1139,7 +1074,6 @@ def test_api_order_create_authenticated_payment_binding(self, _mock_thumbnail):
"organization_id": str(organization.id),
"product_id": str(product.id),
"billing_address": billing_address,
"has_consent_to_terms": True,
}

with self.assertNumQueries(60):
Expand Down Expand Up @@ -1287,7 +1221,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)

Expand Down Expand Up @@ -1337,7 +1270,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)

Expand Down Expand Up @@ -1368,7 +1300,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/",
Expand All @@ -1395,7 +1326,6 @@ def test_api_order_create_authenticated_to_pending(self):
"course_code": course.code,
"organization_id": str(organization.id),
"product_id": str(product.id),
"has_consent_to_terms": True,
"billing_address": billing_address,
"credit_card_id": str(credit_card.id),
}
Expand Down Expand Up @@ -1432,7 +1362,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)

Expand Down Expand Up @@ -1476,7 +1405,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(
Expand Down Expand Up @@ -1527,7 +1455,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)

Expand Down Expand Up @@ -1582,7 +1509,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)

Expand Down
2 changes: 0 additions & 2 deletions src/backend/joanie/tests/core/test_api_admin_orders.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,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,
Expand Down Expand Up @@ -709,7 +708,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,
Expand Down
17 changes: 17 additions & 0 deletions src/backend/joanie/tests/core/test_models_order.py
Original file line number Diff line number Diff line change
Expand Up @@ -1087,3 +1087,20 @@ def test_models_order_has_unsigned_contract_signature(self):
submitted_for_signature_on=datetime(2023, 9, 20, 8, 0, tzinfo=timezone.utc),
)
self.assertFalse(order.has_unsigned_contract)

def test_models_order_has_consent_to_terms_should_raise_deprecation_warning(self):
"""
Due to the refactoring of `has_consent_to_terms` attribute, it is now 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",
)
Loading

0 comments on commit 7bbfc01

Please sign in to comment.