From 73f88b584179132e794f6fb0b04bc5ebb9c1ff30 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Fri, 14 Jun 2024 10:42:16 -0400 Subject: [PATCH 1/6] resource.prices == resorces.next_run.prices; sort run prices on save --- learning_resources/etl/loaders.py | 12 ++++-------- learning_resources/factories.py | 2 +- learning_resources/models.py | 20 +++++++++++++------- learning_resources/models_test.py | 1 + learning_resources/serializers_test.py | 11 ++++++++--- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index 0f350a1e61..e111b9ad78 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -6,8 +6,6 @@ from django.contrib.auth import get_user_model from django.db import transaction -from django.db.models import Q -from django.utils import timezone from learning_resources.constants import ( LearningResourceFormat, @@ -115,12 +113,7 @@ def load_departments( def load_next_start_date(resource: LearningResource) -> datetime.time | None: - next_upcoming_run = ( - resource.runs.filter(Q(published=True) & Q(start_date__gt=timezone.now())) - .order_by("start_date") - .first() - ) - + next_upcoming_run = resource.next_run if next_upcoming_run: resource.next_start_date = next_upcoming_run.start_date else: @@ -227,6 +220,9 @@ def load_run( image_data = run_data.pop("image", None) instructors_data = run_data.pop("instructors", []) + # Make sure any prices are unique and sorted in ascending order + run_data["prices"] = sorted(set(run_data.get("prices", []))) + with transaction.atomic(): ( learning_resource_run, diff --git a/learning_resources/factories.py b/learning_resources/factories.py index 18642783d5..85cb3b5b73 100644 --- a/learning_resources/factories.py +++ b/learning_resources/factories.py @@ -476,7 +476,7 @@ class LearningResourceRunFactory(DjangoModelFactory): constants.AvailabilityType.archived.value, ) ) - enrollment_start = factory.Faker("date_time", tzinfo=UTC) + enrollment_start = factory.Faker("future_datetime", tzinfo=UTC) enrollment_end = factory.LazyAttribute( lambda obj: ( (obj.enrollment_start + timedelta(days=45)) diff --git a/learning_resources/models.py b/learning_resources/models.py index ed25703969..67881d16e0 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -2,12 +2,12 @@ from decimal import Decimal -from django.contrib.admin.utils import flatten from django.contrib.auth.models import User from django.contrib.postgres.fields import ArrayField from django.db import models -from django.db.models import JSONField +from django.db.models import JSONField, Q from django.db.models.functions import Lower +from django.utils import timezone from learning_resources import constants from learning_resources.constants import ( @@ -239,6 +239,15 @@ def audience(self) -> str | None: return self.platform.audience return None + @property + def next_run(self): + """Returns the next run for the learning resource""" + return ( + self.runs.filter(Q(published=True) & Q(start_date__gt=timezone.now())) + .order_by("start_date") + .first() + ) + @property def prices(self) -> list[Decimal]: """Returns the prices for the learning resource""" @@ -246,11 +255,8 @@ def prices(self) -> list[Decimal]: LearningResourceType.course.name, LearningResourceType.program.name, ]: - return list( - set( - flatten([(run.prices or [Decimal(0.0)]) for run in self.runs.all()]) - ) - ) + next_run = self.next_run + return next_run.prices if next_run else [Decimal(0.00)] else: return [Decimal(0.00)] diff --git a/learning_resources/models_test.py b/learning_resources/models_test.py index fef3d9a3d9..4f0db4d45c 100644 --- a/learning_resources/models_test.py +++ b/learning_resources/models_test.py @@ -47,3 +47,4 @@ def test_course_creation(): assert resource.topics.count() > 0 assert resource.offered_by is not None assert resource.runs.count() == course.runs.count() + assert resource.prices == resource.next_run.prices diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index 4cd920094d..736e368ed8 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -206,7 +206,7 @@ def test_learning_resource_serializer( # noqa: PLR0913 "platform": serializers.LearningResourcePlatformSerializer( instance=resource.platform ).data, - "prices": resource.prices, + "prices": sorted(resource.prices), "professional": resource.professional, "certification": resource.certification, "certification_type": { @@ -214,9 +214,14 @@ def test_learning_resource_serializer( # noqa: PLR0913 "name": CertificationType[resource.certification_type].value, }, "free": ( - not resource.professional - and detail_key + detail_key not in (LearningResourceType.course.name, LearningResourceType.program.name) + or ( + not resource.professional + and ( + not resource.prices or all(price == 0 for price in resource.prices) + ) + ) ), "published": resource.published, "readable_id": resource.readable_id, From 69839398b913527d5ef924ce8258f0c118772028 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Fri, 14 Jun 2024 10:57:05 -0400 Subject: [PATCH 2/6] fix tests --- learning_resources/factories.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/learning_resources/factories.py b/learning_resources/factories.py index 85cb3b5b73..43596791be 100644 --- a/learning_resources/factories.py +++ b/learning_resources/factories.py @@ -490,10 +490,12 @@ class LearningResourceRunFactory(DjangoModelFactory): end_date = factory.LazyAttribute( lambda obj: obj.start_date + timedelta(days=90) if obj.start_date else None ) - prices = [ - decimal.Decimal(random.uniform(100, 200)) # noqa: S311 - for _ in range(random.randint(1, 3)) # noqa: S311 - ] + prices = sorted( + [ + decimal.Decimal(random.uniform(100, 200)) # noqa: S311 + for _ in range(random.randint(1, 3)) # noqa: S311 + ] + ) @factory.post_generation def instructors(self, create, extracted, **kwargs): # noqa: ARG002 From 09345e252a238b93a6cf46aff1b0f5cbc308048f Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Fri, 14 Jun 2024 11:58:23 -0400 Subject: [PATCH 3/6] tweak --- learning_resources/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/learning_resources/models.py b/learning_resources/models.py index 67881d16e0..72bbdb4780 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -256,7 +256,7 @@ def prices(self) -> list[Decimal]: LearningResourceType.program.name, ]: next_run = self.next_run - return next_run.prices if next_run else [Decimal(0.00)] + return next_run.prices if next_run else [] else: return [Decimal(0.00)] From 0a8086869878f5fd08329ae7fd95025119f7e447 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Fri, 14 Jun 2024 17:52:18 -0400 Subject: [PATCH 4/6] tweak --- learning_resources/etl/loaders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/learning_resources/etl/loaders.py b/learning_resources/etl/loaders.py index e111b9ad78..61a71c20c9 100644 --- a/learning_resources/etl/loaders.py +++ b/learning_resources/etl/loaders.py @@ -221,7 +221,7 @@ def load_run( instructors_data = run_data.pop("instructors", []) # Make sure any prices are unique and sorted in ascending order - run_data["prices"] = sorted(set(run_data.get("prices", []))) + run_data["prices"] = sorted(set(run_data.get("prices", [])), key=lambda x: float(x)) with transaction.atomic(): ( From afb3276658e5affcc599660f9a1cdbae7ef7cdbf Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Mon, 17 Jun 2024 12:19:24 -0400 Subject: [PATCH 5/6] Make prices format consistent --- frontends/api/src/generated/v1/api.ts | 42 +++++++++---------- .../test-utils/factories/learningResources.ts | 2 +- learning_resources/serializers.py | 5 ++- learning_resources/serializers_test.py | 2 +- openapi/specs/v1.yaml | 42 +++++++++---------- 5 files changed, 48 insertions(+), 45 deletions(-) diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 56300bda2f..0c8107a283 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -603,11 +603,11 @@ export interface CourseResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof CourseResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -1233,11 +1233,11 @@ export interface LearningPathResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof LearningPathResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -3710,11 +3710,11 @@ export interface PodcastEpisodeResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof PodcastEpisodeResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -3995,11 +3995,11 @@ export interface PodcastResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof PodcastResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -4518,11 +4518,11 @@ export interface ProgramResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof ProgramResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -5210,11 +5210,11 @@ export interface VideoPlaylistResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof VideoPlaylistResource */ - prices: Array + prices: Array /** * * @type {Array} @@ -5489,11 +5489,11 @@ export interface VideoResource { */ certification_type: CourseResourceCertificationType /** - * Returns the prices for the learning resource - * @type {Array} + * + * @type {Array} * @memberof VideoResource */ - prices: Array + prices: Array /** * * @type {Array} diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index b1f75ad737..fb45aaf3ee 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -233,7 +233,7 @@ const _learningResourceShared = (): Partial< image: learningResourceImage(), offered_by: maybe(learningResourceOfferor) ?? null, platform: maybe(learningResourcePlatform) ?? null, - prices: [0.0], + prices: ["0.00"], readable_id: faker.lorem.slug(), course_feature: repeat(faker.lorem.word), runs: [], diff --git a/learning_resources/serializers.py b/learning_resources/serializers.py index 69231623f6..01c799760c 100644 --- a/learning_resources/serializers.py +++ b/learning_resources/serializers.py @@ -443,7 +443,10 @@ class LearningResourceBaseSerializer(serializers.ModelSerializer, WriteableTopic ) certification = serializers.ReadOnlyField(read_only=True) certification_type = CertificateTypeField(read_only=True) - prices = serializers.ReadOnlyField() + prices = serializers.ListField( + child=serializers.DecimalField(max_digits=10, decimal_places=2), + read_only=True, + ) runs = LearningResourceRunSerializer(read_only=True, many=True, allow_null=True) image = serializers.SerializerMethodField() learning_path_parents = serializers.SerializerMethodField() diff --git a/learning_resources/serializers_test.py b/learning_resources/serializers_test.py index 736e368ed8..3e9c9815de 100644 --- a/learning_resources/serializers_test.py +++ b/learning_resources/serializers_test.py @@ -206,7 +206,7 @@ def test_learning_resource_serializer( # noqa: PLR0913 "platform": serializers.LearningResourcePlatformSerializer( instance=resource.platform ).data, - "prices": sorted(resource.prices), + "prices": sorted([f"{price:.2f}" for price in resource.prices]), "professional": resource.professional, "certification": resource.certification, "certification_type": { diff --git a/openapi/specs/v1.yaml b/openapi/specs/v1.yaml index f89fed2e23..b0ef46d0ce 100644 --- a/openapi/specs/v1.yaml +++ b/openapi/specs/v1.yaml @@ -7223,9 +7223,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -7629,9 +7629,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -9489,9 +9489,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -9734,9 +9734,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -10124,9 +10124,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -10604,9 +10604,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array @@ -10839,9 +10839,9 @@ components: prices: type: array items: - type: number - format: double - description: Returns the prices for the learning resource + type: string + format: decimal + pattern: ^-?\d{0,8}(?:\.\d{0,2})?$ readOnly: true runs: type: array From 2a93abdc6c5b48986e0e8f836dbd1b699e4db6d9 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Mon, 17 Jun 2024 15:51:50 -0400 Subject: [PATCH 6/6] thought I already made this change --- .../LearningResourceCard/LearningResourceListCard.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 16f5c9a843..d476927895 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -99,7 +99,7 @@ const getPrice = (resource: LearningResource) => { return null } const price = resource.prices?.[0] - if (resource.platform?.code === PlatformEnum.Ocw || price === 0) { + if (resource.free) { return "Free" } return price ? `$${price}` : null