From dea13e80e763c475c83a1d6a7db52ee1189eac44 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Mon, 17 Jun 2024 22:46:38 +0200 Subject: [PATCH 1/5] Display certificate price for free courses with paid option --- .../LearningResourceListCard.tsx | 89 +++++++++++++++++-- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 16f5c9a843..59384bbdee 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -94,29 +94,104 @@ const getEmbedlyUrl = (url: string, isMobile: boolean) => { }) } -const getPrice = (resource: LearningResource) => { +type Prices = { + course: null | number + certificate: null | number +} + +const getPrices = (resource: LearningResource) => { + const prices: Prices = { + course: null, + certificate: null, + } + if (!resource) { + return prices + } + + const resourcePrices = resource.prices + + if (resourcePrices.length > 1) { + /* The resource is free and offers a paid certificate option, e.g. + * { prices: [0, 49], free: false, certification: true } + */ + if (resource.certification && !Number(resourcePrices[0])) { + return { + certificate: resourcePrices[1], + course: 0, + } + } + + /* We are not expecting multiple prices for courses with no certificate option. + * For resourses always certificated, there is one price that includes the certificate. + */ + } else if (resourcePrices.length === 1) { + if (!Number(resourcePrices[0])) { + /* Sometimes price info is missing, but the free flag is reliable. + */ + if (!resource.free) { + return { + course: +Infinity, + certificate: null, + } + } + + return { + course: 0, + certificate: null, + } + } else { + /* If the course has no free option, the price of the certificate + * is included in the price of the course. + */ + return { + course: Number(resourcePrices[0]), + certificate: null, + } + } + } + + // if (!price && !resource.free) { + // prices.course = +Infinity + // return prices + // } + // if (resource.platform?.code === PlatformEnum.Ocw || price === 0) { + // return "Free" + // } + return prices +} + +const getDisplayPrice = (price: number | null) => { + if (price === null) { return null } - const price = resource.prices?.[0] - if (resource.platform?.code === PlatformEnum.Ocw || price === 0) { + if (price === 0) { return "Free" } - return price ? `$${price}` : null + if (price === +Infinity) { + return "Paid" + } + return `$${price}` } +/* This displays a single price for courses with no free option + * (price includes the certificate). For free courses with the + * option of a paid certificate, the certificate price displayed + * in the certificate badge alongside the course "Free" price. + */ const Info = ({ resource }: { resource: LearningResource }) => { - const price = getPrice(resource) + const prices = getPrices(resource) + getDisplayPrice(+Infinity) return ( <> {getReadableResourceType(resource.resource_type)} {resource.certification && ( - Certificate + Certificate {getDisplayPrice(prices?.certificate)} )} - {price && {price}} + {getDisplayPrice(prices?.course)} ) } From c1c9bf385dca013a0d865e3d12a49e266099a998 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Tue, 18 Jun 2024 14:27:32 +0200 Subject: [PATCH 2/5] Display price ranges. Tests. --- .../LearningResourceListCard.test.tsx | 64 +++++++++++++++++++ .../LearningResourceListCard.tsx | 55 ++++++++++++---- 2 files changed, 106 insertions(+), 13 deletions(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx index 9e9171c0ea..98e236fedc 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx @@ -169,4 +169,68 @@ describe("Learning Resource List Card", () => { expect(matching.length).toBe(1) expect(matching[0]).toHaveAttribute("alt", expected.alt) }) + + describe("Price display", () => { + test('Free course without certificate option displays "Free"', () => { + const resource = factories.learningResources.resource({ + certification: false, + free: true, + prices: [0], + }) + setup(resource) + screen.getByText("Free") + }) + + test('Free course with paid certificate option displays the certificate price and "Free"', () => { + const resource = factories.learningResources.resource({ + certification: true, + free: true, + prices: [0, 49], + }) + setup(resource) + screen.getByText("Certificate: $49 - $99") + screen.getByText("Free") + }) + + test('Free course with paid certificate option range displays the certificate price range and "Free"', () => { + const resource = factories.learningResources.resource({ + certification: true, + free: true, + prices: [0, 49, 99], + }) + setup(resource) + screen.getByText("Certificate: $49 - $99") + screen.getByText("Free") + }) + + test("Paid course without certificate option displays the course price", () => { + const resource = factories.learningResources.resource({ + certification: false, + free: false, + prices: [49], + }) + setup(resource) + screen.getByText("$49") + }) + + test('Free course with empty prices array displays "Free"', () => { + const resource = factories.learningResources.resource({ + certification: false, + free: true, + prices: [], + }) + setup(resource) + screen.getByText("Free") + }) + + test('Course that is not free and has zero price (prices not ingested) displays "Paid"', () => { + const resource = factories.learningResources.resource({ + certification: false, + free: false, + prices: [0], + }) + setup(resource) + screen.getByText("Paid") + }) + }) }) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 59384bbdee..1dd11d8862 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -109,16 +109,43 @@ const getPrices = (resource: LearningResource) => { return prices } - const resourcePrices = resource.prices + const resourcePrices = resource.prices.sort() if (resourcePrices.length > 1) { /* The resource is free and offers a paid certificate option, e.g. - * { prices: [0, 49], free: false, certification: true } + * { prices: [0, 49], free: true, certification: true } */ - if (resource.certification && !Number(resourcePrices[0])) { + if (resource.certification && resource.free) { + const certificatedPrices = resourcePrices.filter((price) => price > 0) return { - certificate: resourcePrices[1], course: 0, + certificate: + certificatedPrices.length === 1 + ? certificatedPrices[0] + : [ + certificatedPrices[0], + certificatedPrices[certificatedPrices.length - 1], + ], + } + } + + /* The resource is not free and has a range of prices, e.g. + * { prices: [950, 999], free: false, certification: true|false } + */ + if (resource.certification && !resource.free && Number(resourcePrices[0])) { + return { + course: [resourcePrices[0], resourcePrices[resourcePrices.length - 1]], + certificate: null, + } + } + + /* The resource is not free but has a zero price option (prices not ingested correctly) + * { prices: [0, 999], free: false, certification: true|false } + */ + if (!resource.free && !Number(resourcePrices[0])) { + return { + course: +Infinity, + certificate: null, } } @@ -149,19 +176,17 @@ const getPrices = (resource: LearningResource) => { certificate: null, } } + } else if (resourcePrices.length === 0) { + return { + course: resource.free ? 0 : +Infinity, + certificate: null, + } } - // if (!price && !resource.free) { - // prices.course = +Infinity - // return prices - // } - // if (resource.platform?.code === PlatformEnum.Ocw || price === 0) { - // return "Free" - // } return prices } -const getDisplayPrice = (price: number | null) => { +const getDisplayPrice = (price: number | number[] | null) => { if (price === null) { return null } @@ -171,6 +196,9 @@ const getDisplayPrice = (price: number | null) => { if (price === +Infinity) { return "Paid" } + if (Array.isArray(price)) { + return `$${price[0]} - $${price[1]}` + } return `$${price}` } @@ -188,7 +216,8 @@ const Info = ({ resource }: { resource: LearningResource }) => { {resource.certification && ( - Certificate {getDisplayPrice(prices?.certificate)} + Certificate{prices?.certificate ? ":" : ""}{" "} + {getDisplayPrice(prices?.certificate)} )} {getDisplayPrice(prices?.course)} From 869ded5e650d72304636b33dddd605a117fc2fb6 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Tue, 18 Jun 2024 15:15:56 +0200 Subject: [PATCH 3/5] Update for prices as strings. Price display precision --- .../LearningResourceListCard.test.tsx | 24 +++++++++++++------ .../LearningResourceListCard.tsx | 13 +++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx index 98e236fedc..bfbd8b1af7 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx @@ -175,7 +175,7 @@ describe("Learning Resource List Card", () => { const resource = factories.learningResources.resource({ certification: false, free: true, - prices: [0], + prices: ["0"], }) setup(resource) screen.getByText("Free") @@ -185,10 +185,10 @@ describe("Learning Resource List Card", () => { const resource = factories.learningResources.resource({ certification: true, free: true, - prices: [0, 49], + prices: ["0", "49"], }) setup(resource) - screen.getByText("Certificate: $49 - $99") + screen.getByText("Certificate: $49") screen.getByText("Free") }) @@ -196,7 +196,7 @@ describe("Learning Resource List Card", () => { const resource = factories.learningResources.resource({ certification: true, free: true, - prices: [0, 49, 99], + prices: ["0", "49", "99"], }) setup(resource) screen.getByText("Certificate: $49 - $99") @@ -207,12 +207,22 @@ describe("Learning Resource List Card", () => { const resource = factories.learningResources.resource({ certification: false, free: false, - prices: [49], + prices: ["49"], }) setup(resource) screen.getByText("$49") }) + test("Amount with currency subunits are displayed to 2 decimal places", () => { + const resource = factories.learningResources.resource({ + certification: false, + free: false, + prices: ["49.50"], + }) + setup(resource) + screen.getByText("$49.50") + }) + test('Free course with empty prices array displays "Free"', () => { const resource = factories.learningResources.resource({ certification: false, @@ -223,11 +233,11 @@ describe("Learning Resource List Card", () => { screen.getByText("Free") }) - test('Course that is not free and has zero price (prices not ingested) displays "Paid"', () => { + test('Paid course that has zero price (prices not ingested) displays "Paid"', () => { const resource = factories.learningResources.resource({ certification: false, free: false, - prices: [0], + prices: ["0"], }) setup(resource) screen.getByText("Paid") diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 1dd11d8862..3f21b53505 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -109,7 +109,7 @@ const getPrices = (resource: LearningResource) => { return prices } - const resourcePrices = resource.prices.sort() + const resourcePrices = resource.prices.map((price) => Number(price)).sort() if (resourcePrices.length > 1) { /* The resource is free and offers a paid certificate option, e.g. @@ -186,6 +186,13 @@ const getPrices = (resource: LearningResource) => { return prices } +const getDisplayPrecision = (price: number) => { + if (Number.isInteger(price)) { + return price.toFixed(0) + } + return price.toFixed(2) +} + const getDisplayPrice = (price: number | number[] | null) => { if (price === null) { return null @@ -197,9 +204,9 @@ const getDisplayPrice = (price: number | number[] | null) => { return "Paid" } if (Array.isArray(price)) { - return `$${price[0]} - $${price[1]}` + return `$${getDisplayPrecision(price[0])} - $${getDisplayPrecision(price[1])}` } - return `$${price}` + return `$${getDisplayPrecision(price)}` } /* This displays a single price for courses with no free option From 34f06cfd627050e6d809002c73ea1a6145983d63 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Tue, 18 Jun 2024 16:56:34 +0200 Subject: [PATCH 4/5] Price sort --- .../LearningResourceCard/LearningResourceListCard.test.tsx | 4 ++-- .../LearningResourceCard/LearningResourceListCard.tsx | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx index bfbd8b1af7..3ccac0b182 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.test.tsx @@ -192,11 +192,11 @@ describe("Learning Resource List Card", () => { screen.getByText("Free") }) - test('Free course with paid certificate option range displays the certificate price range and "Free"', () => { + test('Free course with paid certificate option range displays the certificate price range and "Free". Prices are sorted correctly', () => { const resource = factories.learningResources.resource({ certification: true, free: true, - prices: ["0", "49", "99"], + prices: ["0", "99", "49"], }) setup(resource) screen.getByText("Certificate: $49 - $99") diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx index 3f21b53505..bafa231907 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.tsx @@ -109,7 +109,9 @@ const getPrices = (resource: LearningResource) => { return prices } - const resourcePrices = resource.prices.map((price) => Number(price)).sort() + const resourcePrices = resource.prices + .map((price) => Number(price)) + .sort((a, b) => a - b) if (resourcePrices.length > 1) { /* The resource is free and offers a paid certificate option, e.g. From 02d865bf4ee0caea2754f27d306a9a9425182873 Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Tue, 18 Jun 2024 17:09:23 +0200 Subject: [PATCH 5/5] Faker and Storybook for prices --- .../test-utils/factories/learningResources.ts | 4 ++- .../LearningResourceListCard.stories.tsx | 29 +++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index fb45aaf3ee..26376b54c6 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -224,6 +224,7 @@ const learningResourceCourseNumber: Factory = ( const _learningResourceShared = (): Partial< Omit > => { + const free = Math.random() < 0.5 return { id: uniqueEnforcerId.enforce(() => faker.number.int()), professional: faker.datatype.boolean(), @@ -233,7 +234,8 @@ const _learningResourceShared = (): Partial< image: learningResourceImage(), offered_by: maybe(learningResourceOfferor) ?? null, platform: maybe(learningResourcePlatform) ?? null, - prices: ["0.00"], + free, + prices: free ? ["0"] : [faker.finance.amount({ min: 0, max: 100 })], readable_id: faker.lorem.slug(), course_feature: repeat(faker.lorem.word), runs: [], diff --git a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx index 716fce9c09..ce142c091f 100644 --- a/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx +++ b/frontends/ol-components/src/components/LearningResourceCard/LearningResourceListCard.stories.tsx @@ -73,11 +73,26 @@ export default meta type Story = StoryObj -export const Course: Story = { +export const PaidCourse: Story = { args: { resource: makeResource({ resource_type: ResourceTypeEnum.Course, runs: [factories.learningResources.run()], + free: false, + certification: true, + prices: ["999"], + }), + }, +} + +export const FreeCourse: Story = { + args: { + resource: makeResource({ + resource_type: ResourceTypeEnum.Course, + runs: [factories.learningResources.run()], + free: true, + certification: true, + prices: ["0", "400"], }), }, } @@ -96,13 +111,19 @@ export const Program: Story = { export const Podcast: Story = { args: { - resource: makeResource({ resource_type: ResourceTypeEnum.Podcast }), + resource: makeResource({ + resource_type: ResourceTypeEnum.Podcast, + free: true, + }), }, } export const PodcastEpisode: Story = { args: { - resource: makeResource({ resource_type: ResourceTypeEnum.PodcastEpisode }), + resource: makeResource({ + resource_type: ResourceTypeEnum.PodcastEpisode, + free: true, + }), }, } @@ -111,6 +132,7 @@ export const Video: Story = { resource: makeResource({ resource_type: ResourceTypeEnum.Video, url: "https://www.youtube.com/watch?v=4A9bGL-_ilA", + free: true, }), }, } @@ -119,6 +141,7 @@ export const VideoPlaylist: Story = { args: { resource: makeResource({ resource_type: ResourceTypeEnum.VideoPlaylist, + free: true, }), }, }