From 6337ba35c96cebb660735ee8d5ff4feead98208a Mon Sep 17 00:00:00 2001 From: Asad Ali Date: Tue, 7 Jan 2025 16:34:37 +0500 Subject: [PATCH] feat: convert CEUs to decimal (#3217) * feat: convert CEUs to decimal review changes revert unintended change recreate migration * recreate migration * refactor: return CEUs as decimal * fix tests * fix tests --- cms/factories.py | 2 +- .../0077_alter_certificatepage_ceus.py | 27 +++++++++++++++++++ cms/models.py | 10 ++++--- cms/models_test.py | 15 ++++++----- cms/templates/certificate_page.html | 2 +- cms/views_test.py | 3 ++- courses/serializers_test.py | 9 ++++--- .../external_course_sync_api.py | 3 ++- .../external_course_sync_api_test.py | 11 ++++---- courses/views_test.py | 7 +++++ ecommerce/serializers.py | 2 +- ecommerce/views_test.py | 15 +++++++++++ 12 files changed, 82 insertions(+), 24 deletions(-) create mode 100644 cms/migrations/0077_alter_certificatepage_ceus.py diff --git a/cms/factories.py b/cms/factories.py index 6470807cd..623ff5136 100644 --- a/cms/factories.py +++ b/cms/factories.py @@ -469,7 +469,7 @@ class CertificatePageFactory(wagtail_factories.PageFactory): """CertificatePage factory class""" product_name = factory.fuzzy.FuzzyText(prefix="product_name") - CEUs = factory.Faker("pystr_format", string_format="#.#") + CEUs = factory.fuzzy.FuzzyDecimal(low=1, high=10, precision=2) partner_logo = factory.SubFactory(wagtail_factories.ImageChooserBlockFactory) signatories = wagtail_factories.StreamFieldFactory( {"signatory": factory.SubFactory(SignatoryChooserBlockFactory)} diff --git a/cms/migrations/0077_alter_certificatepage_ceus.py b/cms/migrations/0077_alter_certificatepage_ceus.py new file mode 100644 index 000000000..6beff137a --- /dev/null +++ b/cms/migrations/0077_alter_certificatepage_ceus.py @@ -0,0 +1,27 @@ +# Generated by Django 4.2.16 on 2025-01-03 11:14 + +from decimal import Decimal + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("cms", "0076_min_max_weekly_hours"), + ] + + operations = [ + migrations.AlterField( + model_name="certificatepage", + name="CEUs", + field=models.DecimalField( + blank=True, + decimal_places=2, + help_text="Optional field for CEU (continuing education unit).", + max_digits=5, + null=True, + validators=[django.core.validators.MinValueValidator(Decimal("0.00"))], + ), + ), + ] diff --git a/cms/models.py b/cms/models.py index 14310c1aa..d2ee262b6 100644 --- a/cms/models.py +++ b/cms/models.py @@ -5,12 +5,14 @@ import re from collections import defaultdict from datetime import datetime, timedelta +from decimal import Decimal from urllib.parse import urljoin from django import forms from django.conf import settings from django.core.cache import cache from django.core.exceptions import ValidationError +from django.core.validators import MinValueValidator from django.db import models from django.db.models import Prefetch, Q, prefetch_related_objects from django.http.response import Http404 @@ -2176,11 +2178,13 @@ class PartnerLogoPlacement(models.IntegerChoices): max_length=255, null=True, blank=True, help_text="Specify the institute text" ) - CEUs = models.CharField( # noqa: DJ001 - max_length=250, + CEUs = models.DecimalField( null=True, blank=True, - help_text="Optional text field for CEU (continuing education unit).", + decimal_places=2, + max_digits=5, + validators=[MinValueValidator(Decimal("0.00"))], + help_text="Optional field for CEU (continuing education unit).", ) partner_logo = models.ForeignKey( diff --git a/cms/models_test.py b/cms/models_test.py index 7962ef8d1..5aafb096f 100644 --- a/cms/models_test.py +++ b/cms/models_test.py @@ -2,6 +2,7 @@ import json from datetime import date, datetime, timedelta +from decimal import Decimal import factory import pytest @@ -1401,12 +1402,12 @@ def test_certificate_for_course_page(): certificate_page = CertificatePageFactory.create( parent=course_page, product_name="product_name", - CEUs="1.8", + CEUs=Decimal("1.8"), partner_logo__image__title="Partner Logo", signatories__0__signatory__page=signatory, ) assert certificate_page.get_parent() == course_page - assert certificate_page.CEUs == "1.8" + assert certificate_page.CEUs == Decimal("1.8") assert certificate_page.product_name == "product_name" assert certificate_page.partner_logo.title == "Partner Logo" for signatory in certificate_page.signatories: @@ -1436,13 +1437,13 @@ def test_certificate_for_program_page(): certificate_page = CertificatePageFactory.create( parent=program_page, product_name="product_name", - CEUs="2.8", + CEUs=Decimal("2.8"), partner_logo__image__title="Partner Logo", signatories__0__signatory__page=signatory, ) assert certificate_page.get_parent() == program_page - assert certificate_page.CEUs == "2.8" + assert certificate_page.CEUs == Decimal("2.8") assert certificate_page.product_name == "product_name" assert certificate_page.partner_logo.title == "Partner Logo" for signatory in certificate_page.signatories: @@ -2001,7 +2002,7 @@ def test_certificatepage_no_signatories_internal_courseware( certificate_page = CertificatePageFactory.create( parent=page, product_name="product_name", - CEUs="2.8", + CEUs=Decimal("2.8"), partner_logo=None, ) @@ -2044,7 +2045,7 @@ def test_certificatepage_with_signatories_internal_courseware( certificate_page = CertificatePageFactory.create( parent=page, product_name="product_name", - CEUs="2.8", + CEUs=Decimal("2.8"), partner_logo=None, signatories__0__signatory__page=signatory, ) @@ -2085,7 +2086,7 @@ def test_certificatepage_saved_no_signatories_external_courseware( certificate_page = CertificatePageFactory.create( parent=page, product_name="product_name", - CEUs="2.8", + CEUs=Decimal("2.8"), partner_logo=None, ) diff --git a/cms/templates/certificate_page.html b/cms/templates/certificate_page.html index c8a528322..254b147a7 100755 --- a/cms/templates/certificate_page.html +++ b/cms/templates/certificate_page.html @@ -165,7 +165,7 @@

{% if page.CEUs %} Awarded - {{ CEUs }} + {{ CEUs|floatformat }} Continuing Education Units (CEUs)
{% endif %} diff --git a/cms/views_test.py b/cms/views_test.py index acffbdfe4..50ff87b79 100644 --- a/cms/views_test.py +++ b/cms/views_test.py @@ -2,6 +2,7 @@ import textwrap from datetime import datetime, timedelta +from decimal import Decimal from types import SimpleNamespace import factory @@ -693,7 +694,7 @@ def test_product_page_context_has_certificate( if published_certificate: assert resp.context["ceus"] is not None - assert resp.context["ceus"] == "12.0" + assert resp.context["ceus"] == Decimal("12.00") else: assert resp.context["ceus"] is None diff --git a/courses/serializers_test.py b/courses/serializers_test.py index bcad2e00f..bde188d44 100644 --- a/courses/serializers_test.py +++ b/courses/serializers_test.py @@ -3,6 +3,7 @@ """ from datetime import UTC, datetime, timedelta +from decimal import Decimal import factory import pytest @@ -64,7 +65,7 @@ def test_base_program_serializer(): 2, 4, "http://www.testvideourl.com", - "2 Test CEUs", + "2.0", "https://www.testexternalcourse1.com", "fb4f5b79-test-4972-92c3-test", ), @@ -176,7 +177,7 @@ def test_serialize_program( # noqa: PLR0913 "min_weeks": min_weeks, "format": program_format, "video_url": video_url, - "credits": ceus, + "credits": Decimal(ceus) if ceus else None, "is_external": is_external, "external_marketing_url": external_marketing_url, "marketing_hubspot_form_id": marketing_hubspot_form_id, @@ -217,7 +218,7 @@ def test_base_course_serializer(): 2, 4, "http://www.testvideourl.com", - "2 Test CEUs", + "2", "http://www.testexternalmarketingurl.com", "fb4f5b79-test-4972-92c3-test", ), @@ -331,7 +332,7 @@ def test_serialize_course( # noqa: PLR0913 "min_weeks": min_weeks if course_page else None, "format": course_format if course_page else None, "video_url": video_url if course_page else None, - "credits": ceus if course_page else None, + "credits": Decimal(ceus) if course_page and ceus else None, "is_external": is_external, "external_marketing_url": external_marketing_url if course_page else None, "marketing_hubspot_form_id": ( diff --git a/courses/sync_external_courses/external_course_sync_api.py b/courses/sync_external_courses/external_course_sync_api.py index 13c33c143..9778e534f 100644 --- a/courses/sync_external_courses/external_course_sync_api.py +++ b/courses/sync_external_courses/external_course_sync_api.py @@ -5,6 +5,7 @@ import re import time from datetime import timedelta +from decimal import Decimal from enum import Enum from pathlib import Path @@ -164,7 +165,7 @@ def __init__(self, external_course_json, keymap): self.format = external_course_json.get("format") self.category = external_course_json.get("Category", None) self.image_name = external_course_json.get("image_name", None) - self.CEUs = str(external_course_json.get("ceu") or "") + self.CEUs = Decimal(str(external_course_json.get("ceu") or "0.0")) or None self.learning_outcomes_list = ( parse_external_course_data_str( external_course_json.get("learning_outcomes") diff --git a/courses/sync_external_courses/external_course_sync_api_test.py b/courses/sync_external_courses/external_course_sync_api_test.py index f3528b1e3..d74f53c71 100644 --- a/courses/sync_external_courses/external_course_sync_api_test.py +++ b/courses/sync_external_courses/external_course_sync_api_test.py @@ -6,6 +6,7 @@ import logging import random from datetime import datetime, timedelta +from decimal import Decimal from pathlib import Path import pytest @@ -319,12 +320,12 @@ def test_create_or_update_certificate_page( ) if existing_cert_page: certificate_page = CertificatePageFactory.create( - parent=external_course_page, CEUs="" + parent=external_course_page, CEUs=None ) if publish_certificate: certificate_page.save_revision().publish() if is_live_and_draft: - certificate_page.CEUs = "1.2" + certificate_page.CEUs = Decimal("1.2") certificate_page.save_revision() else: certificate_page.unpublish() @@ -335,7 +336,7 @@ def test_create_or_update_certificate_page( ExternalCourse(external_course_data, keymap=keymap), ) certificate_page = certificate_page.revisions.last().as_object() - assert certificate_page.CEUs == external_course_data["ceu"] + assert certificate_page.CEUs == Decimal(str(external_course_data["ceu"])) assert is_created == (not existing_cert_page) assert is_updated == existing_cert_page @@ -549,7 +550,7 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 description="", ) CertificatePageFactory.create( - parent=course_page, CEUs="1.0", partner_logo=None + parent=course_page, CEUs=Decimal("1.0"), partner_logo=None ) product = ProductFactory.create(content_object=course_run) ProductVersionFactory.create(product=product, price=run["list_price"]) @@ -622,7 +623,7 @@ def test_update_external_course_runs( # noqa: PLR0915, PLR0913 CertificatePage ) assert certificate_page - assert certificate_page.CEUs == external_course_run["ceu"] + assert certificate_page.CEUs == Decimal(str(external_course_run["ceu"])) @pytest.mark.parametrize( diff --git a/courses/views_test.py b/courses/views_test.py index 99b6e6864..766c8e60c 100644 --- a/courses/views_test.py +++ b/courses/views_test.py @@ -5,6 +5,7 @@ import json import operator as op from datetime import timedelta +from decimal import Decimal from pathlib import Path import pytest @@ -135,6 +136,9 @@ def test_get_courses(user_drf_client, courses, mock_context, is_anonymous): courses_data = resp.json() assert len(courses_data) == len(courses) for course, course_data in zip(courses, courses_data): + course_data["credits"] = ( + Decimal(str(course_data["credits"])) if course_data["credits"] else None + ) assert ( course_data == CourseSerializer(instance=course, context=mock_context).data ) @@ -148,6 +152,9 @@ def test_get_course(user_drf_client, courses, mock_context, is_anonymous): course = courses[0] resp = user_drf_client.get(reverse("courses_api-detail", kwargs={"pk": course.id})) course_data = resp.json() + course_data["credits"] = ( + Decimal(str(course_data["credits"])) if course_data["credits"] else None + ) assert course_data == CourseSerializer(instance=course, context=mock_context).data diff --git a/ecommerce/serializers.py b/ecommerce/serializers.py index dedbacadd..8dba62571 100644 --- a/ecommerce/serializers.py +++ b/ecommerce/serializers.py @@ -1021,7 +1021,7 @@ def get_lines(self, instance): tax_paid=str(tax_paid), discount=str(discount), total_before_tax=str(total_before_tax), - CEUs=str(CEUs) if CEUs else None, + CEUs=CEUs if CEUs else None, **BaseProductVersionSerializer(line.product_version).data, start_date=dates["start_date"], end_date=dates["end_date"], diff --git a/ecommerce/views_test.py b/ecommerce/views_test.py index c38380223..0315feae2 100644 --- a/ecommerce/views_test.py +++ b/ecommerce/views_test.py @@ -2,6 +2,7 @@ import json from datetime import UTC, datetime, timedelta +from decimal import Decimal from urllib.parse import quote_plus, urljoin import factory @@ -604,6 +605,13 @@ def test_patch_basket_update_coupon_valid( resp = basket_client.patch(reverse("basket_api"), type="json", data=data) assert resp.status_code == status.HTTP_200_OK resp_data = resp.json() + + for item in resp_data.get("items", []): + for course in item.get("courses", []): + course["credits"] = ( + Decimal(str(course["credits"])) if course["credits"] else None + ) + assert resp_data.get("items") == original_basket.get("items") assert CouponSelection.objects.get(basket=basket).coupon.coupon_code == new_code assert len(resp_data.get("coupons")) == 1 @@ -670,6 +678,13 @@ def test_patch_basket_clear_coupon_no_auto( resp = basket_client.patch(reverse("basket_api"), type="json", data=data) assert resp.status_code == status.HTTP_200_OK resp_data = resp.json() + + for item in resp_data.get("items", []): + for course in item.get("courses", []): + course["credits"] = ( + Decimal(str(course["credits"])) if course["credits"] else None + ) + assert resp_data.get("coupons") == [] assert resp_data.get("items") == original_basket.get("items") assert CouponSelection.objects.filter(basket=basket).first() is None