Skip to content

Commit

Permalink
chore: PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@brobro10000
brobro10000 committed Oct 4, 2024
1 parent bb53284 commit 4315514
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 84 deletions.
24 changes: 11 additions & 13 deletions src/components/course/CourseSidebarPrice.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const CourseSidebarPrice = () => {
description="Message to indicate that the price has been reduced."
/>
</span>
${originalPriceDisplay} {currency}
{originalPriceDisplay} {currency}
</del>
);

Expand All @@ -61,15 +61,13 @@ const CourseSidebarPrice = () => {
);
}

const hasDiscountedPrice = coursePrice.discounted
&& sumOfArray(coursePrice.discounted) < sumOfArray(coursePrice.listRange);
const hasDiscountedPrice = coursePrice.discountedList
&& sumOfArray(coursePrice.discountedList) < sumOfArray(coursePrice.listRange);
// Case 2: No subsidies found but learner can request a subsidy
if (!hasDiscountedPrice && canRequestSubsidy) {
return (
<span style={{ whiteSpace: 'pre-wrap' }} data-testid="browse-and-request-pricing">
<s>
${originalPriceDisplay} {currency}
</s><br />
<s>{originalPriceDisplay} {currency}</s><br />
<FormattedMessage
id="enterprise.course.about.course.sidebar.price.free.when.approved"
defaultMessage="Free to me{br}(when approved)"
Expand All @@ -84,7 +82,7 @@ const CourseSidebarPrice = () => {
if (!hasDiscountedPrice) {
return (
<span className="d-block">
${originalPriceDisplay} {currency}
{originalPriceDisplay} {currency}
</span>
);
}
Expand Down Expand Up @@ -114,22 +112,22 @@ const CourseSidebarPrice = () => {
});
}
}
const discountedPriceDisplay = `${getContentPriceDisplay(coursePrice.discounted)} ${currency}`;
const discountedPriceDisplay = `${getContentPriceDisplay(coursePrice.discountedList)} ${currency}`;
return (
<>
<div className={classNames({ 'mb-2': coursePrice.discounted > 0 || showOrigPrice })}>
{/* discounted > 0 means partial discount */}
<div className={classNames({ 'mb-2': coursePrice.discountedList > 0 || showOrigPrice })}>
{/* discountedList > 0 means partial discount */}
{showOrigPrice && <>{crossedOutOriginalPrice}{' '}</>}
{sumOfArray(coursePrice.discounted) > 0 && (
{sumOfArray(coursePrice.discountedList) > 0 && (
<>
<span className="sr-only">
<FormattedMessage
id="enterprise.course.about.price.discounted"
defaultMessage="Discounted price:"
description="Message to indicate that the price has been discounted."
description="Message to indicate that the price has been discountedList."
/>
</span>
${discountedPriceDisplay}
{discountedPriceDisplay}
</>
)}
</div>
Expand Down
16 changes: 8 additions & 8 deletions src/components/course/data/hooks.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ export function useCoursePacingType(courseRun) {

/**
* @typedef {Object} CoursePrice
* @property {number} list The list price.
* @property {number} discounted The discounted price.
* @property {number} listRange The list price.
* @property {number} discountedList The discountedList price.
*/

/**
Expand Down Expand Up @@ -208,29 +208,29 @@ export const useCoursePriceForUserSubsidy = ({

if (userSubsidyApplicableToCourse) {
const { discountType, discountValue } = userSubsidyApplicableToCourse;
let discountedPrice = [];
let discountedPriceList = [];

if (discountType && discountType.toLowerCase() === SUBSIDY_DISCOUNT_TYPE_MAP.PERCENTAGE.toLowerCase()) {
discountedPrice = onlyListPrice.listRange.map(
discountedPriceList = onlyListPrice.listRange.map(
(individualPrice) => individualPrice - (individualPrice * (discountValue / 100)),
);
}

if (discountType && discountType.toLowerCase() === SUBSIDY_DISCOUNT_TYPE_MAP.ABSOLUTE.toLowerCase()) {
discountedPrice = onlyListPrice.listRange.map(
discountedPriceList = onlyListPrice.listRange.map(
(individualPrice) => Math.max(individualPrice - discountValue, 0),
);
}

if (isDefinedAndNotNull(discountedPrice)) {
if (isDefinedAndNotNull(discountedPriceList)) {
return {
...onlyListPrice,
discounted: discountedPrice,
discountedList: discountedPriceList,
};
}
return {
...onlyListPrice,
discounted: onlyListPrice.listRange,
discountedList: onlyListPrice.listRange,
};
}

Expand Down
32 changes: 16 additions & 16 deletions src/components/course/data/tests/hooks.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ describe('useCoursePriceForUserSubsidy', () => {
userSubsidyApplicableToCourse,
}));
const { coursePrice } = result.current;
expect(coursePrice).toEqual({ listRange: [100], discounted: [90] });
expect(coursePrice).toEqual({ listRange: [100], discountedList: [90] });
});

it('should return the correct course price when a user subsidy is applicable with unknown discount type', () => {
Expand All @@ -818,7 +818,7 @@ describe('useCoursePriceForUserSubsidy', () => {
userSubsidyApplicableToCourse,
}));
const { coursePrice } = result.current;
expect(coursePrice).toEqual({ listRange: [100], discounted: [] });
expect(coursePrice).toEqual({ listRange: [100], discountedList: [] });
});

it('should return the correct course price when a user subsidy is applicable with absolute discount', () => {
Expand All @@ -835,7 +835,7 @@ describe('useCoursePriceForUserSubsidy', () => {
userSubsidyApplicableToCourse,
}));
const { coursePrice } = result.current;
expect(coursePrice).toEqual({ listRange: [150], discounted: [140] });
expect(coursePrice).toEqual({ listRange: [150], discountedList: [140] });
});

it('should return the correct course price when a user subsidy is not applicable', () => {
Expand Down Expand Up @@ -1676,47 +1676,47 @@ describe('useCourseListPrice', () => {
beforeEach(() => {
jest.clearAllMocks();
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: mockListPrice } });
useCourseMetadata.mockReturnValue(mockListPrice || getCoursePrice(baseCourseMetadataValue));
useCourseMetadata.mockReturnValue({ data: mockListPrice || getCoursePrice(baseCourseMetadataValue) });
});
it('should return the list price if one exist', () => {
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
expect(result.current).toEqual(mockListPrice);
expect(result.current).toEqual({ data: mockListPrice });
});
it('should not return the list price if one doesnt, first fallback, fixed_price_usd', () => {
it('should not return the list price if one doesnt exist, first fallback price, fixed_price_usd', () => {
const updatedListPrice = undefined;
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
useCourseMetadata.mockReturnValue(updatedListPrice || getCoursePrice(baseCourseMetadataValue));
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
expect(result.current).toEqual([baseCourseMetadataValue.activeCourseRun.fixedPriceUsd]);
expect(result.current).toEqual({ data: [baseCourseMetadataValue.activeCourseRun.fixedPriceUsd] });
});
it('should not return the list price if one doesnt, second fallback, firstEnrollablePaidSeatPrice', () => {
it('should not return the list price if one doesnt exist, second fallback price, firstEnrollablePaidSeatPrice', () => {
const updatedListPrice = undefined;
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
delete baseCourseMetadataValue.activeCourseRun.fixedPriceUsd;
useCourseMetadata.mockReturnValue(updatedListPrice || getCoursePrice(baseCourseMetadataValue));
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
expect(result.current).toEqual([baseCourseMetadataValue.activeCourseRun.firstEnrollablePaidSeatPrice]);
expect(result.current).toEqual({ data: [baseCourseMetadataValue.activeCourseRun.firstEnrollablePaidSeatPrice] });
});
it('should not return the list price if one doesnt, third fallback, entitlements', () => {
it('should not return the list price if one doesnt exit, third fallback price, entitlements', () => {
const updatedListPrice = undefined;
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
delete baseCourseMetadataValue.activeCourseRun.fixedPriceUsd;
delete baseCourseMetadataValue.activeCourseRun.firstEnrollablePaidSeatPrice;
useCourseMetadata.mockReturnValue(updatedListPrice || getCoursePrice(baseCourseMetadataValue));
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
expect(result.current).toEqual([baseCourseMetadataValue.entitlements[0].price]);
expect(result.current).toEqual({ data: [baseCourseMetadataValue.entitlements[0].price] });
});
it('should not return the list price if one doesnt exist or the course metadata doesnt include it', () => {
const updatedListPrice = undefined;
Expand All @@ -1725,12 +1725,12 @@ describe('useCourseListPrice', () => {
...baseCourseMetadataValue,
entitlements: [],
};
useCourseMetadata.mockReturnValue(updatedListPrice || getCoursePrice(updatedCourseMetadata));
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(updatedCourseMetadata) });
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
expect(result.current).toEqual(null);
expect(result.current).toEqual({ data: null });
});
});

Expand Down
23 changes: 5 additions & 18 deletions src/components/course/data/utils.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import XSeriesSvgIcon from '../../../assets/icons/xseries.svg';
import CreditSvgIcon from '../../../assets/icons/credit.svg';
import { PROGRAM_TYPE_MAP } from '../../program/data/constants';
import { programIsMicroMasters, programIsProfessionalCertificate } from '../../program/data/utils';
import { hasValidStartExpirationDates } from '../../../utils/common';
import { formatPrice, hasValidStartExpirationDates } from '../../../utils/common';
import { LICENSE_STATUS } from '../../enterprise-user-subsidy/data/constants';
import {
findHighestLevelEntitlementSku,
Expand Down Expand Up @@ -167,19 +167,9 @@ export const getContentPriceDisplay = (priceRange) => {
const minPrice = Math.min(...priceRange);
const maxPrice = Math.max(...priceRange);
if (maxPrice !== minPrice) {
return `${numberWithPrecision(minPrice)} - ${numberWithPrecision(maxPrice)}`;
return `${formatPrice(minPrice)} - ${formatPrice(maxPrice)}`;

Check warning on line 170 in src/components/course/data/utils.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/course/data/utils.jsx#L170 

Added line #L170 was not covered by tests
}
return numberWithPrecision(priceRange[0]);
};

export const formatPrice = (price, options = {}) => {
const USDollar = new Intl.NumberFormat('en-US', {
style: 'currency',
currency: 'USD',
minimumFractionDigits: 0,
...options,
});
return USDollar.format(Math.abs(price));
return formatPrice(priceRange[0]);
};

/**
Expand Down Expand Up @@ -831,7 +821,7 @@ export function getLinkToCourse(course, slug) {
*/
export function getEntitlementPrice(entitlements) {
if (entitlements?.length) {
return Number(entitlements[0].price);
return parseFloat(entitlements[0].price);
}
return undefined;
}
Expand All @@ -846,10 +836,7 @@ export function getEntitlementPrice(entitlements) {
*/
export function getCoursePrice(course) {
if (course.activeCourseRun?.fixedPriceUsd) {
if (typeof course.activeCourseRun.fixedPriceUsd === 'string') {
return [Number(course.activeCourseRun.fixedPriceUsd)];
}
return [course.activeCourseRun?.fixedPriceUsd];
return [parseFloat(course.activeCourseRun.fixedPriceUsd)];
}
if (course.activeCourseRun?.firstEnrollablePaidSeatPrice) {
return [course.activeCourseRun?.firstEnrollablePaidSeatPrice];
Expand Down
24 changes: 12 additions & 12 deletions src/components/course/tests/CourseSidebarPrice.test.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('<CourseSidebarPrice/> ', () => {
jest.clearAllMocks();
useEnterpriseCustomer.mockReturnValue({ data: mockEnterpriseCustomer });
useUserSubsidyApplicableToCourse.mockReturnValue({ userSubsidyApplicableToCourse: null });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [7.5] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [7.5] }, currency: 'USD' });
useIsCourseAssigned.mockReturnValue({ isCourseAssigned: false });
useCanUserRequestSubsidyForCourse.mockReturnValue(false);
});
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('<CourseSidebarPrice/> ', () => {
subsidyType: ENTERPRISE_OFFER_SUBSIDY_TYPE,
};
useUserSubsidyApplicableToCourse.mockReturnValue({ userSubsidyApplicableToCourse: mockEnterpriseOfferSubsidy });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
render(<CourseSidebarPriceWrapper />);
expect(screen.getByText('Priced reduced from:')).toBeInTheDocument();
expect(screen.getByText(/\$7.50 USD/)).toBeInTheDocument();
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('<CourseSidebarPrice/> ', () => {
});

test('subscription license subsidy, shows no price, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: 7.5, discounted: 0 }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: { subsidyType: LICENSE_SUBSIDY_TYPE },
});
Expand All @@ -138,7 +138,7 @@ describe('<CourseSidebarPrice/> ', () => {
});

test('coupon code 100% subsidy, shows no price, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: FULL_COUPON_CODE_SUBSIDY,
});
Expand All @@ -151,8 +151,8 @@ describe('<CourseSidebarPrice/> ', () => {
expect(screen.queryByText('This course is assigned to you. The price of this course is already covered by your organization.')).not.toBeInTheDocument();
});

test('coupon code non-full subsidy, shows discounted price only, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [3.75] }, currency: 'USD' });
test('coupon code non-full subsidy, shows discountedList price only, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [3.75] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: PARTIAL_COUPON_CODE_SUBSIDY,
});
Expand All @@ -166,7 +166,7 @@ describe('<CourseSidebarPrice/> ', () => {

test('assigned course, shows no price, correct message', () => {
useIsCourseAssigned.mockReturnValue({ isCourseAssigned: true });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: { subsidyType: LEARNER_CREDIT_SUBSIDY_TYPE },
});
Expand All @@ -190,7 +190,7 @@ describe('<CourseSidebarPrice/> ', () => {
expect(screen.queryByText('This course is assigned to you. The price of this course is already covered by your organization.')).not.toBeInTheDocument();
});
test('subscription license subsidy, shows orig crossed out price, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: { subsidyType: LICENSE_SUBSIDY_TYPE },
});
Expand All @@ -202,7 +202,7 @@ describe('<CourseSidebarPrice/> ', () => {
expect(screen.queryByText('This course is assigned to you. The price of this course is already covered by your organization.')).not.toBeInTheDocument();
});
test('coupon code 100% subsidy, shows orig price, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: FULL_COUPON_CODE_SUBSIDY,
});
Expand All @@ -213,8 +213,8 @@ describe('<CourseSidebarPrice/> ', () => {
expect(screen.queryByText("This course can be purchased with your organization's learner credit")).not.toBeInTheDocument();
expect(screen.queryByText('This course is assigned to you. The price of this course is already covered by your organization.')).not.toBeInTheDocument();
});
test('coupon code non-full subsidy, shows orig and discounted price only, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [3.75] }, currency: 'USD' });
test('coupon code non-full subsidy, shows orig and discountedList price only, correct message', () => {
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [3.75] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: PARTIAL_COUPON_CODE_SUBSIDY,
});
Expand All @@ -228,7 +228,7 @@ describe('<CourseSidebarPrice/> ', () => {
});
test('assigned course, shows orig price, correct message', () => {
useIsCourseAssigned.mockReturnValue({ isCourseAssigned: true });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discounted: [0] }, currency: 'USD' });
useCoursePrice.mockReturnValue({ coursePrice: { listRange: [7.5], discountedList: [0] }, currency: 'USD' });
useUserSubsidyApplicableToCourse.mockReturnValue({
userSubsidyApplicableToCourse: { subsidyType: LEARNER_CREDIT_SUBSIDY_TYPE },
});
Expand Down
11 changes: 4 additions & 7 deletions src/components/executive-education-2u/components/CourseSummaryCard.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,21 @@ import {
import { FormattedMessage } from '@edx/frontend-platform/i18n';

import {
DATE_FORMAT,
getContentPriceDisplay,
numberWithPrecision,
useMinimalCourseMetadata,
ZERO_PRICE,
DATE_FORMAT, getContentPriceDisplay, useMinimalCourseMetadata, ZERO_PRICE,
} from '../../course/data';
import { formatPrice } from '../../../utils/common';

const CourseSummaryCard = ({ enrollmentCompleted }) => {
const { data: minimalCourseMetadata } = useMinimalCourseMetadata();

let coursePrice = null;
const precisePrice = minimalCourseMetadata.priceDetails?.price ? `$${getContentPriceDisplay(
const precisePrice = minimalCourseMetadata.priceDetails?.price ? `${getContentPriceDisplay(
minimalCourseMetadata.priceDetails.price,
)} ${minimalCourseMetadata.priceDetails.currency}` : '-';
if (enrollmentCompleted && minimalCourseMetadata.priceDetails?.price) {
coursePrice = (
<><del>{precisePrice}</del>
${numberWithPrecision(ZERO_PRICE)} {minimalCourseMetadata.priceDetails.currency}
{formatPrice(ZERO_PRICE)} {minimalCourseMetadata.priceDetails.currency}
</>
);
} else {
Expand Down
Loading

0 comments on commit 4315514

Please sign in to comment.