Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: convert CEUs to decimal #3217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cms/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,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)}
Expand Down
23 changes: 23 additions & 0 deletions cms/migrations/0073_alter_certificatepage_ceus.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Generated by Django 4.2.16 on 2024-11-06 06:35

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("cms", "0072_add_hybrid_courseware_format_option"),
]

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,
),
),
]
22 changes: 16 additions & 6 deletions cms/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,9 @@ def get_context(self, request, *args, **kwargs):
"techniques": self.techniques,
"propel_career": self.propel_career,
"news_and_events": self.news_and_events,
"ceus": self.certificate_page.CEUs if self.certificate_page else None,
"ceus": self.certificate_page.normalized_ceus
if self.certificate_page
else None,
}
Comment on lines +1077 to 1080
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Keeping the same format of how we are using other properties, we might just want to move the if/else inside the normalized_ceus method.


def save(self, clean=True, user=None, log_action=False, **kwargs): # noqa: FBT002
Expand Down Expand Up @@ -2142,11 +2144,12 @@ 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,
help_text="Optional field for CEU (continuing education unit).",
Comment on lines +2150 to +2152
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows -ve values too. We will need to add validators.

)

partner_logo = models.ForeignKey(
Expand Down Expand Up @@ -2242,6 +2245,13 @@ def parent(self):
"""
return self.get_parent().specific

@property
def normalized_ceus(self):
"""
Normalizes the CEUs from decimal to string. Removes the trailing zeros if any.
"""
return f"{self.CEUs.normalize():f}" if self.CEUs else None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a simple str() conversion be better?


def get_context(self, request, *args, **kwargs):
preview_context = {}
context = {}
Expand All @@ -2255,14 +2265,14 @@ def get_context(self, request, *args, **kwargs):
"end_date": self.parent.product.first_unexpired_run.end_date
if self.parent.product.first_unexpired_run
else datetime.now() + timedelta(days=45), # noqa: DTZ005
"CEUs": self.CEUs,
"CEUs": self.normalized_ceus,
}
elif self.certificate:
# Verify that the certificate in fact is for this same course
if self.parent.product.id != self.certificate.get_courseware_object_id():
raise Http404
start_date, end_date = self.certificate.start_end_dates
CEUs = self.CEUs
CEUs = self.normalized_ceus

for override in self.overrides:
if (
Expand Down
17 changes: 10 additions & 7 deletions cms/models_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import json
from datetime import date, datetime, timedelta
from decimal import Decimal

import factory
import pytest
Expand Down Expand Up @@ -1389,12 +1390,13 @@ 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.normalized_ceus == "1.8"
assert certificate_page.product_name == "product_name"
assert certificate_page.partner_logo.title == "Partner Logo"
for signatory in certificate_page.signatories:
Expand Down Expand Up @@ -1424,13 +1426,14 @@ 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.normalized_ceus == "2.8"
assert certificate_page.product_name == "product_name"
assert certificate_page.partner_logo.title == "Partner Logo"
for signatory in certificate_page.signatories:
Expand Down Expand Up @@ -1989,7 +1992,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,
)

Expand Down Expand Up @@ -2032,7 +2035,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,
)
Expand Down Expand Up @@ -2073,7 +2076,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,
)

Expand Down
2 changes: 1 addition & 1 deletion cms/views_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,6 @@ 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"] == "12"
else:
assert resp.context["ceus"] is None
4 changes: 2 additions & 2 deletions courses/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def build_program_credential(certificate: ProgramCertificate) -> dict:
"name": certificate.program.title,
"description": certificate.program.page.description,
"numberOfCredits": {
"value": certificate.program.page.certificate_page.CEUs
"value": certificate.program.page.certificate_page.normalized_ceus
},
"startDate": start_date.isoformat(),
"endDate": end_date.isoformat(),
Expand Down Expand Up @@ -72,7 +72,7 @@ def build_course_run_credential(certificate: CourseRunCertificate) -> dict:
"courseCode": course.readable_id,
"name": course.title,
"description": course.page.description,
"numberOfCredits": {"value": course.page.certificate_page.CEUs},
"numberOfCredits": {"value": course.page.certificate_page.normalized_ceus},
"startDate": start_date.isoformat(),
"endDate": end_date.isoformat(),
},
Expand Down
4 changes: 2 additions & 2 deletions courses/credentials_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def test_build_program_credential(user):
"type": "EducationalOccupationalProgram",
"name": program.title,
"description": program.page.description,
"numberOfCredits": {"value": program.page.certificate_page.CEUs},
"numberOfCredits": {"value": program.page.certificate_page.normalized_ceus},
"startDate": course_run.start_date.isoformat(),
"endDate": course_run.end_date.isoformat(),
},
Expand Down Expand Up @@ -92,7 +92,7 @@ def test_build_course_run_credential():
"courseCode": course.readable_id,
"name": course.title,
"description": course.page.description,
"numberOfCredits": {"value": course.page.certificate_page.CEUs},
"numberOfCredits": {"value": course.page.certificate_page.normalized_ceus},
"startDate": start_date.isoformat(),
"endDate": end_date.isoformat(),
},
Expand Down
4 changes: 2 additions & 2 deletions courses/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def get_video_url(self, instance):
def get_credits(self, instance):
"""Returns the credits for this Course"""
return (
instance.page.certificate_page.CEUs
instance.page.certificate_page.normalized_ceus
if instance.page and instance.page.certificate_page
else None
)
Expand Down Expand Up @@ -415,7 +415,7 @@ def get_video_url(self, instance):
def get_credits(self, instance):
"""Returns the credits for this Course"""
return (
instance.page.certificate_page.CEUs
instance.page.certificate_page.normalized_ceus
if instance.page and instance.page.certificate_page
else None
)
Expand Down
9 changes: 5 additions & 4 deletions courses/serializers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""

from datetime import UTC, datetime, timedelta
from decimal import Decimal

import factory
import pytest
Expand Down Expand Up @@ -60,7 +61,7 @@ def test_base_program_serializer():
"2 Months",
"2 Hours",
"http://www.testvideourl.com",
"2 Test CEUs",
"2.0",
"https://www.testexternalcourse1.com",
"fb4f5b79-test-4972-92c3-test",
),
Expand Down Expand Up @@ -156,7 +157,7 @@ def test_serialize_program( # noqa: PLR0913
"duration": duration,
"format": program_format,
"video_url": video_url,
"credits": ceus,
"credits": str(Decimal(ceus).normalize()) if ceus else None,
"is_external": is_external,
"external_marketing_url": external_marketing_url,
"marketing_hubspot_form_id": marketing_hubspot_form_id,
Expand Down Expand Up @@ -193,7 +194,7 @@ def test_base_course_serializer():
"2 Months",
"2 Hours",
"http://www.testvideourl.com",
"2 Test CEUs",
"2",
"http://www.testexternalmarketingurl.com",
"fb4f5b79-test-4972-92c3-test",
),
Expand Down Expand Up @@ -291,7 +292,7 @@ def test_serialize_course( # noqa: PLR0913
"duration": duration 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": str(Decimal(ceus).normalize()) 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": (
Expand Down
3 changes: 2 additions & 1 deletion courses/sync_external_courses/emeritus_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import re
import time
from datetime import timedelta
from decimal import Decimal
from enum import Enum
from pathlib import Path

Expand Down Expand Up @@ -120,7 +121,7 @@ def __init__(self, emeritus_course_json):
self.format = emeritus_course_json.get("format")
self.category = emeritus_course_json.get("Category", None)
self.image_name = emeritus_course_json.get("image_name", None)
self.CEUs = str(emeritus_course_json.get("ceu") or "")
self.CEUs = Decimal(str(emeritus_course_json.get("ceu"))) or None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails on null CEU values

self.learning_outcomes_list = (
parse_emeritus_data_str(emeritus_course_json.get("learning_outcomes"))
if emeritus_course_json.get("learning_outcomes")
Expand Down
11 changes: 6 additions & 5 deletions courses/sync_external_courses/emeritus_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import logging
import random
from datetime import datetime, timedelta
from decimal import Decimal
from pathlib import Path

import pytest
Expand Down Expand Up @@ -276,12 +277,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()
Expand All @@ -290,7 +291,7 @@ def test_create_or_update_certificate_page(
external_course_page, EmeritusCourse(emeritus_course_data)
)
certificate_page = certificate_page.revisions.last().as_object()
assert certificate_page.CEUs == emeritus_course_data["ceu"]
assert certificate_page.CEUs == Decimal(str(emeritus_course_data["ceu"]))
assert is_created == (not existing_cert_page)
assert is_updated == existing_cert_page

Expand Down Expand Up @@ -481,7 +482,7 @@ def test_update_emeritus_course_runs( # noqa: PLR0915
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"])
Expand Down Expand Up @@ -553,7 +554,7 @@ def test_update_emeritus_course_runs( # noqa: PLR0915
CertificatePage
)
assert certificate_page
assert certificate_page.CEUs == emeritus_course_run["ceu"]
assert certificate_page.CEUs == Decimal(str(emeritus_course_run["ceu"]))


def test_fetch_emeritus_courses_success(settings, mocker):
Expand Down
2 changes: 1 addition & 1 deletion ecommerce/mail_api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def test_send_ecommerce_order_receipt(mocker, receipt_data, settings):
"start_date": None,
"end_date": None,
"content_title": "test_run_title",
"CEUs": line.product_version.product.content_object.course.page.certificate_page.CEUs,
"CEUs": line.product_version.product.content_object.course.page.certificate_page.normalized_ceus,
}
],
"order_total": "100.00",
Expand Down
4 changes: 2 additions & 2 deletions ecommerce/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ def get_lines(self, instance):
certificate_page = program.page.certificate_page

if certificate_page:
CEUs = certificate_page.CEUs
CEUs = certificate_page.normalized_ceus
for override in certificate_page.overrides:
if (
override.value.get("readable_id")
Expand All @@ -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"],
Expand Down
2 changes: 1 addition & 1 deletion ecommerce/serializers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ def test_serialize_order_receipt(receipt_data):
"tax_paid": "0.00",
"total_before_tax": str(line.quantity * product_version.price),
"quantity": line.quantity,
"CEUs": product_version.product.content_object.course.page.certificate_page.CEUs,
"CEUs": product_version.product.content_object.course.page.certificate_page.normalized_ceus,
}
],
"order": {
Expand Down
Loading