From c6e9f5d02b56d8c9b1edff498cf1136ff3554ff6 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 15 Dec 2023 12:09:50 -0500 Subject: [PATCH 1/8] fix: Remove Prod console.logs, unrelated --- src/order-history/service.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/order-history/service.js b/src/order-history/service.js index 0de5db22..fc71f88c 100644 --- a/src/order-history/service.js +++ b/src/order-history/service.js @@ -29,12 +29,10 @@ export async function getOrders(page = 1, pageSize = 20) { let callCC = false; if (data.count > 0) { callCC = data.results[0].enable_hoist_order_history; - console.log('REV-2577 LOG: ecommerce data.results', data.results); } - console.log('REV-2577 LOG: enable_hoist_order_history flag is: ', callCC); if (callCC) { - console.log('REV-2577 LOG: about to call commerce-coordinator'); + // REV-2577: enable_hoist_order_history flag is on, about to call commerce-coordinator const newData = await httpClient.get(orderFetchingUrl, { params: { username, @@ -43,7 +41,6 @@ export async function getOrders(page = 1, pageSize = 20) { }, }); data = newData.data; - console.log('REV-2577 LOG: CC response.data.results', data.results); } // [END] TEMPORARY CODE for rollout testing/confirmation=========== From b2fc2c33ecd840264639b28a331611a202c4a6da Mon Sep 17 00:00:00 2001 From: julianajlk Date: Wed, 13 Dec 2023 22:23:42 -0500 Subject: [PATCH 2/8] fix: Modify order-history initial state to loading true --- src/order-history/reducer.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/order-history/reducer.js b/src/order-history/reducer.js index 63bbb554..ef9383b7 100644 --- a/src/order-history/reducer.js +++ b/src/order-history/reducer.js @@ -1,7 +1,7 @@ import { fetchOrders } from './actions'; export const initialState = { - loading: false, + loading: true, loadingError: false, orders: [], count: 0, @@ -16,12 +16,12 @@ const orderHistoryPage = (state = initialState, action = {}) => { case fetchOrders.TRIGGER: return { ...state, - loading: true, loadingError: false, }; case fetchOrders.SUCCESS: return { ...state, + loading: false, orders: action.payload.orders, count: action.payload.count, next: action.payload.next, @@ -32,6 +32,7 @@ const orderHistoryPage = (state = initialState, action = {}) => { case fetchOrders.FAILURE: return { ...state, + loading: false, loadingError: true, }; case fetchOrders.FULFILL: From 57bc7c75d3e6e1689eb73a14fc282145f69ee1b5 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Wed, 13 Dec 2023 22:24:06 -0500 Subject: [PATCH 3/8] fix: Modify subscriptions initial state to loading true --- src/subscriptions/reducer.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/subscriptions/reducer.js b/src/subscriptions/reducer.js index 53d589f2..7d532af9 100644 --- a/src/subscriptions/reducer.js +++ b/src/subscriptions/reducer.js @@ -1,7 +1,7 @@ import { fetchStripeCustomerPortalURL, fetchSubscriptions } from './actions'; export const initialState = { - loading: false, + loading: true, loadingError: false, subscriptions: [], stripeCustomerPortalURL: null, @@ -15,17 +15,18 @@ const subscriptionsReducer = (state = initialState, action = {}) => { case fetchSubscriptions.TRIGGER: return { ...state, - loading: true, loadingError: false, }; case fetchSubscriptions.SUCCESS: return { ...state, + loading: false, subscriptions: action.payload, }; case fetchSubscriptions.FAILURE: return { ...state, + loading: false, loadingError: true, }; case fetchSubscriptions.FULFILL: From f4ed2335dfdb0915f049e3312bde1e83286b0567 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Wed, 13 Dec 2023 22:27:13 -0500 Subject: [PATCH 4/8] refactor: Check for subs and move heading to only show if subs --- .../OrdersAndSubscriptionsPage.jsx | 47 +++++++++++-------- .../OrdersAndSubscriptionsPage.messages.js | 4 +- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx index cc2ada04..23ec9559 100644 --- a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx +++ b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx @@ -9,6 +9,7 @@ import { BasicAlert, PageLoading } from '../components'; import OrderHistory, { fetchOrders } from '../order-history'; import Subscriptions, { fetchSubscriptions } from '../subscriptions'; +import { subscriptionsSelector } from '../subscriptions/selectors'; import { errorSelector, loadingSelector, @@ -21,6 +22,8 @@ const OrdersAndSubscriptionsPage = () => { const dispatch = useDispatch(); const isLoading = useSelector(loadingSelector); const hasError = useSelector(errorSelector); + const { subscriptions } = useSelector(subscriptionsSelector); + /** * TODO: PON-299 - Remove this selector after the MVP. */ @@ -30,11 +33,17 @@ const OrdersAndSubscriptionsPage = () => { getConfig().ENABLE_B2C_SUBSCRIPTIONS?.toLowerCase() === 'true' ); + const hasSubscriptions = subscriptions.length > 0; + const isSubscriptionDisabled = !isB2CSubsEnabled || !shouldShowSubscriptionSection; + const shouldShowOrderHistoryOnly = isSubscriptionDisabled || (!isLoading && !hasSubscriptions); + useEffect(() => { if (isB2CSubsEnabled) { + dispatch(fetchSubscriptions()); + } + if (!isSubscriptionDisabled && hasSubscriptions) { document.title = 'Orders and Subscriptions | edX'; sendTrackEvent('edx.bi.user.subscription.order-page.viewed'); - dispatch(fetchSubscriptions()); } // TODO: We should fetch based on the route (ex: /orders/?orderPage=1) dispatch(fetchOrders(1)); @@ -48,24 +57,6 @@ const OrdersAndSubscriptionsPage = () => { ); const renderOrdersandSubscriptions = () => ( - <> - - - - ); - - /** - * TODO: PON-299 - Remove the extra condition i.e. shouldShowSubscriptionSection after the MVP. - */ - if (!isB2CSubsEnabled || !shouldShowSubscriptionSection) { - return ( -
- -
- ); - } - - return (
@@ -84,9 +75,25 @@ const OrdersAndSubscriptionsPage = () => { {(text) => {text}}
- {isLoading ? renderLoading() : renderOrdersandSubscriptions()} + +
); + + const renderOrderHistoryOnly = () => ( +
+ + +
+ ); + + if (isLoading) { + return renderLoading(); + } + if (shouldShowOrderHistoryOnly) { + return renderOrderHistoryOnly(); + } + return renderOrdersandSubscriptions(); }; export default OrdersAndSubscriptionsPage; diff --git a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.messages.js b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.messages.js index c9eb2694..96d7ee44 100644 --- a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.messages.js +++ b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.messages.js @@ -3,8 +3,8 @@ import { defineMessages } from '@edx/frontend-platform/i18n'; const messages = defineMessages({ 'ecommerce.order.history.loading': { id: 'ecommerce.order.history.loading', - defaultMessage: 'Loading orders and subscriptions...', - description: 'Message when orders and subscriptions page is loading.', + defaultMessage: 'Loading orders...', + description: 'Message when order history page is loading.', }, }); From b8a05dd7ac83e2bbb8961ef0773009fdd9ac8239 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Thu, 14 Dec 2023 22:32:23 -0500 Subject: [PATCH 5/8] test: Update OrdersAndSubscriptionsPage component tests --- .../OrdersAndSubscriptionsPage.test.jsx | 105 +++++++++++++++--- 1 file changed, 90 insertions(+), 15 deletions(-) diff --git a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.test.jsx b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.test.jsx index 232126d1..30e90ad2 100644 --- a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.test.jsx +++ b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.test.jsx @@ -14,24 +14,62 @@ const { queryByTestId, } = screen; -const testHeadings = (hasSections = true) => { - // Assert the main heading - expect(getByText('My orders and subscriptions')).toBeInTheDocument(); - expect( - getByText('Manage your program subscriptions and view your order history.'), - ).toBeInTheDocument(); - - if (hasSections) { +const testHeadings = (hasSections = true, hasSubscriptions = true) => { + if (hasSections && hasSubscriptions) { + // Assert the main heading is present + expect(getByText('My orders and subscriptions')).toBeInTheDocument(); + expect( + getByText('Manage your program subscriptions and view your order history.'), + ).toBeInTheDocument(); // Assert Subscription and Order History sections are rendered expect(getByText('Subscriptions')).toBeInTheDocument(); expect(getByText('Order History')).toBeInTheDocument(); - } else { - // Assert Subscription and Order History sections are not rendered + } else if (!hasSections && !hasSubscriptions) { + // Assert only Order History section is rendered + expect(queryByText('My orders and subscriptions')).toBeNull(); + expect( + queryByText('Manage your program subscriptions and view your order history.'), + ).toBeNull(); + expect(getByText('Order History')).toBeInTheDocument(); + expect(queryByText('Subscriptions')).toBeNull(); + } +}; + +const testHeadingsLoading = (hasSections = true, hasSubscriptions = true) => { + if (!hasSections && !hasSubscriptions) { + // Assert loading, nothing is rendered + expect(queryByText('My orders and subscriptions')).toBeNull(); + expect( + queryByText('Manage your program subscriptions and view your order history.'), + ).toBeNull(); expect(queryByText('Subscriptions')).toBeNull(); expect(queryByText('Order History')).toBeNull(); } }; +const testHeadingsError = (hasSections = true, hasSubscriptions = true) => { + if (!hasSections && !hasSubscriptions) { + // Error with no subscriptions + // Assert only Order History sections is rendered + expect(queryByText('My orders and subscriptions')).toBeNull(); + expect( + queryByText('Manage your program subscriptions and view your order history.'), + ).toBeNull(); + expect(queryByText('Subscriptions')).toBeNull(); + expect(getByText('Order History')).toBeInTheDocument(); + } else if (hasSections && hasSubscriptions) { + // Error but has subscriptions + // Assert the main heading is present + expect(getByText('My orders and subscriptions')).toBeInTheDocument(); + expect( + getByText('Manage your program subscriptions and view your order history.'), + ).toBeInTheDocument(); + // Assert Subscription and Order History sections are rendered + expect(getByText('Subscriptions')).toBeInTheDocument(); + expect(getByText('Order History')).toBeInTheDocument(); + } +}; + describe('', () => { describe('Renders correctly in various states', () => { it('renders with orders and subscriptions', () => { @@ -42,7 +80,23 @@ describe('', () => { expect(queryByTestId('basic-alert')).toBeNull(); }); - it('renders alerts on errors', () => { + it('renders with orders only', () => { + const ordersOnlyMocks = { + orderHistory: { + ...storeMocks.orderHistory, + }, + subscriptions: { + ...emptyStoreMocks.subscriptions, + }, + }; + render(, ordersOnlyMocks); + testHeadings(false, false); + + // Assert alerts are not rendered + expect(queryByTestId('basic-alert')).toBeNull(); + }); + + it('renders alerts on errors no subscriptions', () => { const storeMocksWithErrors = { orderHistory: { ...emptyStoreMocks.orderHistory, @@ -55,13 +109,34 @@ describe('', () => { }; render(, storeMocksWithErrors); - testHeadings(); + testHeadingsError(false, false); expect(getByTestId('basic-alert')).toBeInTheDocument(); // Assert Subscription section renders empty state expect(queryByTestId('section-subscription-cards')).toBeNull(); - expect(getByTestId('section-subscription-upsell')).toBeInTheDocument(); + expect(queryByTestId('section-subscription-upsell')).toBeNull(); + }); + + it('renders alerts on errors with subscriptions', () => { + const storeMocksWithErrors = { + orderHistory: { + ...emptyStoreMocks.orderHistory, + loadingError: true, + }, + subscriptions: { + ...storeMocks.subscriptions, + loadingError: false, + }, + }; + + render(, storeMocksWithErrors); + testHeadingsError(true, true); + + expect(getByTestId('basic-alert')).toBeInTheDocument(); + + expect(getByTestId('section-subscription-cards')).toBeInTheDocument(); + expect(queryByTestId('section-subscription-upsell')).toBeNull(); }); it('renders with loading', () => { @@ -77,10 +152,10 @@ describe('', () => { }; render(, storeMocksWithLoading); - testHeadings(false); + testHeadingsLoading(false, false); // Assert loading message is rendered - expect(getByText('Loading orders and subscriptions...')) + expect(getByText('Loading orders...')) .toBeInTheDocument(); // Assert alerts are not rendered From c3b4aca63e6fbdc9a32b75d77632982061ed81ff Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 15 Dec 2023 11:06:03 -0500 Subject: [PATCH 6/8] fix: Reducer changes based on feedback --- src/subscriptions/reducer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/subscriptions/reducer.js b/src/subscriptions/reducer.js index 7d532af9..72cb4e61 100644 --- a/src/subscriptions/reducer.js +++ b/src/subscriptions/reducer.js @@ -15,18 +15,17 @@ const subscriptionsReducer = (state = initialState, action = {}) => { case fetchSubscriptions.TRIGGER: return { ...state, + loading: true, loadingError: false, }; case fetchSubscriptions.SUCCESS: return { ...state, - loading: false, subscriptions: action.payload, }; case fetchSubscriptions.FAILURE: return { ...state, - loading: false, loadingError: true, }; case fetchSubscriptions.FULFILL: From fbceaa6c150e29fcfe7b5962a945e8fd1faf2554 Mon Sep 17 00:00:00 2001 From: julianajlk Date: Fri, 15 Dec 2023 12:33:02 -0500 Subject: [PATCH 7/8] fix: Prevent infinite loading if ENABLE_B2C_SUBSCRIPTIONS config not set up --- src/order-history/selectors.js | 4 ++++ .../OrdersAndSubscriptionsPage.jsx | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/order-history/selectors.js b/src/order-history/selectors.js index ddffcbc2..a79f97ae 100644 --- a/src/order-history/selectors.js +++ b/src/order-history/selectors.js @@ -4,3 +4,7 @@ export const storeName = 'orderHistory'; export const pageSelector = state => ({ ...state[storeName], }); + +export const loadingOrderHistorySelector = (state) => ( + state[storeName].loading +); diff --git a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx index 23ec9559..e598feb7 100644 --- a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx +++ b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx @@ -15,12 +15,14 @@ import { loadingSelector, showSubscriptionSelector, } from './selectors'; +import { loadingOrderHistorySelector } from '../order-history/selectors'; import messages from './OrdersAndSubscriptionsPage.messages'; const OrdersAndSubscriptionsPage = () => { const { formatMessage } = useIntl(); const dispatch = useDispatch(); const isLoading = useSelector(loadingSelector); + const isLoadingOrderHistoryOnly = useSelector(loadingOrderHistorySelector); const hasError = useSelector(errorSelector); const { subscriptions } = useSelector(subscriptionsSelector); @@ -87,9 +89,16 @@ const OrdersAndSubscriptionsPage = () => { ); - if (isLoading) { + // Now that loading initial state is true, if subscriptions is not enabled, + // it will never fetch subscriptions, want to prevent from local infinite loading + if (isSubscriptionDisabled) { + if (isLoadingOrderHistoryOnly) { + return renderLoading(); + } + } else if (isLoading) { return renderLoading(); } + if (shouldShowOrderHistoryOnly) { return renderOrderHistoryOnly(); } From 5f4e7f8e43d06a023c312d079b17b9c98161405c Mon Sep 17 00:00:00 2001 From: julianajlk Date: Mon, 18 Dec 2023 20:02:04 -0500 Subject: [PATCH 8/8] fix: Update order-history initial state loading based on feedback --- src/order-history/reducer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/order-history/reducer.js b/src/order-history/reducer.js index ef9383b7..dab058de 100644 --- a/src/order-history/reducer.js +++ b/src/order-history/reducer.js @@ -16,12 +16,12 @@ const orderHistoryPage = (state = initialState, action = {}) => { case fetchOrders.TRIGGER: return { ...state, + loading: true, loadingError: false, }; case fetchOrders.SUCCESS: return { ...state, - loading: false, orders: action.payload.orders, count: action.payload.count, next: action.payload.next, @@ -32,7 +32,6 @@ const orderHistoryPage = (state = initialState, action = {}) => { case fetchOrders.FAILURE: return { ...state, - loading: false, loadingError: true, }; case fetchOrders.FULFILL: