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.

Sign up for GitHub

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

Jump to bottom

fix: check length on empty array for list price #1211

Merged
merged 2 commits into from
Oct 21, 2024
Merged

fix: check length on empty array for list price #1211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b96c4f1
fix: check length on empty array for list price
brobro10000 Oct 18, 2024
f34670a
chore: update tests
brobro10000 Oct 18, 2024
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 src/components/course/data/hooks.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ export function useUserHasSubsidyRequestForCourse(courseKey) {

export function useCourseListPrice() {
const { data: { listPrice } } = useCourseRedemptionEligibility();
const resolveListPrice = ({ transformed }) => listPrice || getCoursePrice(transformed);
const resolveListPrice = ({ transformed }) => (listPrice.length > 0 ? listPrice : getCoursePrice(transformed));
return useCourseMetadata({
select: resolveListPrice,
});
Expand Down
33 changes: 22 additions & 11 deletions src/components/course/data/tests/hooks.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,7 @@ describe('useCourseListPrice', () => {
},
],
};
const mockedListPrice = [mockListPrice];

const Wrapper = ({ children }) => (
<AppContext.Provider value={mockAuthenticatedUser}>
Expand All @@ -1675,25 +1676,29 @@ describe('useCourseListPrice', () => {
);
beforeEach(() => {
jest.clearAllMocks();
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: mockListPrice } });
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: mockedListPrice } });
// NOTE: `useCourseMetadata`'s mocked return value assumes the returned value
// from the `select` function passed to the hook.
useCourseMetadata.mockReturnValue({ data: mockListPrice || getCoursePrice(baseCourseMetadataValue) });
useCourseMetadata.mockReturnValue({
data: mockedListPrice.length > 0 ? mockedListPrice : getCoursePrice(baseCourseMetadataValue),
});
});
it('should return the list price if one exist', () => {
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
);
const expectedListPrice = mockListPrice;
const expectedListPrice = [mockListPrice];
const courseMetadataSelectFn = useCourseMetadata.mock.calls[0][0].select;
expect(expectedListPrice).toEqual(courseMetadataSelectFn({ transformed: baseCourseMetadataValue }));
expect(result.current).toEqual({ data: expectedListPrice });
});
it('should not return the list price if one doesnt exist, fall back to fixed_price_usd from getCoursePrice', () => {
const updatedListPrice = undefined;
const updatedListPrice = [];
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
useCourseMetadata.mockReturnValue(
{ data: updatedListPrice.length > 0 ? updatedListPrice : getCoursePrice(baseCourseMetadataValue) },
);
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
Expand All @@ -1704,10 +1709,12 @@ describe('useCourseListPrice', () => {
expect(result.current).toEqual({ data: expectedListPrice });
});
it('should not return the list price if one doesnt exist, fall back to firstEnrollablePaidSeatPrice from getCoursePrice', () => {
const updatedListPrice = undefined;
const updatedListPrice = [];
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
delete baseCourseMetadataValue.activeCourseRun.fixedPriceUsd;
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
useCourseMetadata.mockReturnValue(
{ data: updatedListPrice.length > 0 ? updatedListPrice : getCoursePrice(baseCourseMetadataValue) },
);
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
Expand All @@ -1718,11 +1725,13 @@ describe('useCourseListPrice', () => {
expect(result.current).toEqual({ data: expectedListPrice });
});
it('should not return the list price if one doesnt exit, fall back to entitlements from getCoursePrice', () => {
const updatedListPrice = undefined;
const updatedListPrice = [];
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
delete baseCourseMetadataValue.activeCourseRun.fixedPriceUsd;
delete baseCourseMetadataValue.activeCourseRun.firstEnrollablePaidSeatPrice;
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(baseCourseMetadataValue) });
useCourseMetadata.mockReturnValue(
{ data: updatedListPrice > 0 ? updatedListPrice : getCoursePrice(baseCourseMetadataValue) },
);
const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
Expand All @@ -1733,7 +1742,7 @@ describe('useCourseListPrice', () => {
expect(result.current).toEqual({ data: expectedListPrice });
});
it('should not return the list price if one doesnt exist or the course metadata doesnt include it', () => {
const updatedListPrice = undefined;
const updatedListPrice = [];
useCourseRedemptionEligibility.mockReturnValue({ data: { listPrice: updatedListPrice } });
const updatedCourseMetadata = {
...baseCourseMetadataValue,
Expand All @@ -1744,7 +1753,9 @@ describe('useCourseListPrice', () => {
},
entitlements: [],
};
useCourseMetadata.mockReturnValue({ data: updatedListPrice || getCoursePrice(updatedCourseMetadata) });
useCourseMetadata.mockReturnValue(
{ data: updatedListPrice.length > 0 ? updatedListPrice : getCoursePrice(updatedCourseMetadata) },
);
Comment on lines -1747 to +1758
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this missing a sub-key under the data top-level key? When we call useCourseMetdata, it doesn't normally just plop down the price as a value to the data key, does it?

Copy link
Member Author

@brobro10000 brobro10000 Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In this case it does because of how useCourseListPrice utilizes the select within the useCourseMetadata arguments. It passes in the data from the transformed useCourseMetadata return value to resolve within getCoursePrice or the listPrice (acting as a no-op on the original course metadata)

export function useCourseListPrice() {
  const { data: { listPrice } } = useCourseRedemptionEligibility();
  const resolveListPrice = ({ transformed }) => listPrice || getCoursePrice(transformed);
  return useCourseMetadata({
    select: resolveListPrice,
  });
}

const { result } = renderHook(
() => useCourseListPrice(),
{ wrapper: Wrapper },
Expand Down
Loading