refactor: Modify initial loading state for better control of subs sec…

…tion (#356) REV-3799
openedx-unsupported · Dec 19, 2023 · 2a35e69 · 2a35e69
1 parent 5cec05d
commit 2a35e69
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 43 deletions.
diff --git 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,

diff --git 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/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===========
 

diff --git a/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx b/src/orders-and-subscriptions/OrdersAndSubscriptionsPage.jsx
@@ -9,18 +9,23 @@ import { BasicAlert, PageLoading } from '../components';
 import OrderHistory, { fetchOrders } from '../order-history';
 import Subscriptions, { fetchSubscriptions } from '../subscriptions';
 
+import { subscriptionsSelector } from '../subscriptions/selectors';
 import {
   errorSelector,
   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);
+
   /**
    * TODO: PON-299 - Remove this selector after the MVP.
    */
@@ -30,11 +35,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 +59,6 @@ const OrdersAndSubscriptionsPage = () => {
   );
 
   const renderOrdersandSubscriptions = () => (
-    <>
-      <Subscriptions />
-      <OrderHistory isB2CSubsEnabled />
-    </>
-  );
-
-  /**
-   * TODO: PON-299 - Remove the extra condition i.e. shouldShowSubscriptionSection after the MVP.
-   */
-  if (!isB2CSubsEnabled || !shouldShowSubscriptionSection) {
-    return (
-      <div className="page__orders-and-subscriptions container-fluid py-5">
-        <OrderHistory isB2CSubsEnabled={false} />
-      </div>
-    );
-  }
-
-  return (
     <div className="page__orders-and-subscriptions container-fluid py-4.5">
       <div className="section">
         <BasicAlert isVisible={hasError} />
@@ -84,9 +77,32 @@ const OrdersAndSubscriptionsPage = () => {
           {(text) => <span className="text-dark-900">{text}</span>}
         </FormattedMessage>
       </div>
-      {isLoading ? renderLoading() : renderOrdersandSubscriptions()}
+      <Subscriptions />
+      <OrderHistory isB2CSubsEnabled />
+    </div>
+  );
+
+  const renderOrderHistoryOnly = () => (
+    <div className="page__orders-and-subscriptions container-fluid py-5">
+      <BasicAlert isVisible={hasError} />
+      <OrderHistory isB2CSubsEnabled={false} />
     </div>
   );
+
+  // 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();
+  }
+  return renderOrdersandSubscriptions();
 };
 
 export default OrdersAndSubscriptionsPage;
diff --git 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.',
   },
 });
 

diff --git 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('<OrdersAndSubscriptions />', () => {
   describe('Renders correctly in various states', () => {
     it('renders with orders and subscriptions', () => {
@@ -42,7 +80,23 @@ describe('<OrdersAndSubscriptions />', () => {
       expect(queryByTestId('basic-alert')).toBeNull();
     });
 
-    it('renders alerts on errors', () => {
+    it('renders with orders only', () => {
+      const ordersOnlyMocks = {
+        orderHistory: {
+          ...storeMocks.orderHistory,
+        },
+        subscriptions: {
+          ...emptyStoreMocks.subscriptions,
+        },
+      };
+      render(<OrdersAndSubscriptionsPage />, 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('<OrdersAndSubscriptions />', () => {
       };
 
       render(<OrdersAndSubscriptionsPage />, 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(<OrdersAndSubscriptionsPage />, 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('<OrdersAndSubscriptions />', () => {
       };
 
       render(<OrdersAndSubscriptionsPage />, 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

diff --git 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,