Skip to content

Commit

Permalink
PON-251: Update Existing Error Handling (#294)
Browse files Browse the repository at this point in the history
* feat: update order history error flows

* refactor: remove unused AsyncActionType, favour saga routines over this

* fix: add sr message to loading, fix order history loading

* test: add more tests for coverage
  • Loading branch information
NawfalAhmed authored May 30, 2023
1 parent 74cc3da commit c2039c6
Show file tree
Hide file tree
Showing 17 changed files with 522 additions and 155 deletions.
68 changes: 27 additions & 41 deletions src/order-history/OrderHistoryPage.jsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { connect } from 'react-redux';
import {
getConfig,
} from '@edx/frontend-platform';
import { getConfig } from '@edx/frontend-platform';
import {
injectIntl,
intlShape,
Expand Down Expand Up @@ -157,55 +155,45 @@ class OrderHistoryPage extends React.Component {
);
}

renderError() {
renderLoading() {
return (
<div>
{this.props.intl.formatMessage(messages['ecommerce.order.history.loading.error'], {
error: this.props.loadingError,
})}
</div>
<PageLoading
srMessage={this.props.intl.formatMessage(
messages['ecommerce.order.history.loading.orders'],
)}
/>
);
}

renderLoading() {
return (
<PageLoading srMessage={this.props.intl.formatMessage(messages['ecommerce.order.history.loading.orders'])} />
renderOrders() {
const hasOrders = this.props.orders.length > 0;

return hasOrders ? (
<>
<MediaQuery query="(max-width: 768px)">
{this.renderMobileOrdersTable()}
</MediaQuery>
<MediaQuery query="(min-width: 769px)">
{this.renderOrdersTable()}
</MediaQuery>
{this.renderPagination()}
</>
) : (
this.renderEmptyMessage()
);
}

render() {
const {
loading,
loadingError,
orders,
} = this.props;
const loaded = !loading && !loadingError;
const hasOrders = orders.length > 0;
const heading = this.props.intl.formatMessage(
const { loading, intl, isB2CSubsEnabled } = this.props;

const heading = intl.formatMessage(
messages['ecommerce.order.history.page.heading'],
);

return (
<section className="page__order-history">
{this.props.isB2CSubsEnabled ? <h2>{heading}</h2> : <h1>{heading}</h1>}
<div>
{loadingError ? this.renderError() : null}
{loaded && hasOrders ? (
<>
<MediaQuery query="(max-width: 768px)">
{this.renderMobileOrdersTable()}
</MediaQuery>
<MediaQuery query="(min-width: 769px)">
{this.renderOrdersTable()}
</MediaQuery>
{this.renderPagination()}
</>
) : null}
{loaded && !hasOrders ? this.renderEmptyMessage() : null}
{loading && !this.props.isB2CSubsEnabled
? this.renderLoading()
: null}
</div>
{isB2CSubsEnabled ? <h2>{heading}</h2> : <h1>{heading}</h1>}
<div>{loading ? this.renderLoading() : this.renderOrders()}</div>
</section>
);
}
Expand All @@ -230,13 +218,11 @@ OrderHistoryPage.propTypes = {
count: PropTypes.number,
currentPage: PropTypes.number,
loading: PropTypes.bool,
loadingError: PropTypes.string,
fetchOrders: PropTypes.func.isRequired,
};

OrderHistoryPage.defaultProps = {
orders: [],
loadingError: null,
loading: false,
pageCount: 0,
count: 0,
Expand Down
5 changes: 0 additions & 5 deletions src/order-history/OrderHistoryPage.messages.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@ const messages = defineMessages({
defaultMessage: 'Loading orders...',
description: 'Message when orders are being loaded',
},
'ecommerce.order.history.loading.error': {
id: 'ecommerce.order.history.loading.error',
defaultMessage: 'Error: {error}',
description: 'Message when orders are fail to load',
},
'ecommerce.order.history.view.order.detail': {
id: 'ecommerce.order.history.view.order.detail',
defaultMessage: 'View Order Details',
Expand Down
59 changes: 55 additions & 4 deletions src/order-history/OrderHistoryPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ const requiredOrderHistoryPageProps = {

// Match all media queries. This will result in rendering
// both the desktop and mobile views at the same time.
global.matchMedia = media => ({ // eslint-disable-line no-unused-vars
// eslint-disable-next-line no-unused-vars
global.matchMedia = (media) => ({
addListener: () => {},
removeListener: () => {},
matches: true,
Expand All @@ -27,13 +28,63 @@ describe('<OrderHistoryPage />', () => {
describe('Renders correctly in various states', () => {
it('renders orders table with pagination', () => {
const tree = renderer
.create((
.create(
<IntlProvider locale="en">
<Provider store={mockStore(storeMocks)}>
<ConnectedOrderHistoryPage {...requiredOrderHistoryPageProps} />
</Provider>
</IntlProvider>
))
</IntlProvider>,
)
.toJSON();
expect(tree).toMatchSnapshot();
});

it('renders empty orders', () => {
const storeMockWithoutOrders = {
...storeMocks,
orderHistory: {
...storeMocks.orders,
orders: [],
count: 0,
pageCount: 0,
currentPage: null,
},
};

const tree = renderer
.create(
<IntlProvider locale="en">
<Provider store={mockStore(storeMockWithoutOrders)}>
<ConnectedOrderHistoryPage {...requiredOrderHistoryPageProps} />
</Provider>
</IntlProvider>,
)
.toJSON();
expect(tree).toMatchSnapshot();
});

it('renders loading state', () => {
const storeMockWithLoading = {
...storeMocks,
orderHistory: {
...storeMocks.orders,
loading: true,
loadingError: false,
orders: [],
count: 0,
pageCount: 0,
currentPage: null,
},
};

const tree = renderer
.create(
<IntlProvider locale="en">
<Provider store={mockStore(storeMockWithLoading)}>
<ConnectedOrderHistoryPage {...requiredOrderHistoryPageProps} />
</Provider>
</IntlProvider>,
)
.toJSON();
expect(tree).toMatchSnapshot();
});
Expand Down
48 changes: 48 additions & 0 deletions src/order-history/__snapshots__/OrderHistoryPage.test.jsx.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,53 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<OrderHistoryPage /> Renders correctly in various states renders empty orders 1`] = `
<section
className="page__order-history"
>
<h1>
Order History
</h1>
<div>
<p>
Orders you place with localhost will appear here.
</p>
</div>
</section>
`;

exports[`<OrderHistoryPage /> Renders correctly in various states renders loading state 1`] = `
<section
className="page__order-history"
>
<h1>
Order History
</h1>
<div>
<div>
<div
className="d-flex justify-content-center align-items-center flex-column"
style={
Object {
"height": "50vh",
}
}
>
<div
className="spinner-border text-primary"
role="status"
>
<span
className="sr-only"
>
Loading orders...
</span>
</div>
</div>
</div>
</div>
</section>
`;

exports[`<OrderHistoryPage /> Renders correctly in various states renders orders table with pagination 1`] = `
<section
className="page__order-history"
Expand Down
28 changes: 3 additions & 25 deletions src/order-history/actions.js
Original file line number Diff line number Diff line change
@@ -1,26 +1,4 @@
import { AsyncActionType } from '../utils';
import { createRoutine } from 'redux-saga-routines';

export const FETCH_ORDERS = new AsyncActionType(
'ORDER_HISTORY',
'FETCH_ORDERS',
);

// FETCH ORDERS ACTIONS

export const fetchOrders = (pageToFetch) => ({
type: FETCH_ORDERS.BASE,
payload: { pageToFetch },
});

export const fetchOrdersBegin = () => ({
type: FETCH_ORDERS.BEGIN,
});

export const fetchOrdersSuccess = result => ({
type: FETCH_ORDERS.SUCCESS,
payload: result,
});

export const fetchOrdersReset = () => ({
type: FETCH_ORDERS.RESET,
});
// eslint-disable-next-line import/prefer-default-export
export const fetchOrders = createRoutine('FETCH_Orders');
19 changes: 11 additions & 8 deletions src/order-history/reducer.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { FETCH_ORDERS } from './actions';
import { fetchOrders } from './actions';

export const initialState = {
loading: false,
loadingError: null,
loadingError: false,
orders: [],
count: 0,
pageCount: 0,
Expand All @@ -13,13 +13,13 @@ export const initialState = {

const orderHistoryPage = (state = initialState, action = {}) => {
switch (action.type) {
case FETCH_ORDERS.BEGIN:
case fetchOrders.TRIGGER:
return {
...state,
loadingError: null,
loading: true,
loadingError: false,
};
case FETCH_ORDERS.SUCCESS:
case fetchOrders.SUCCESS:
return {
...state,
orders: action.payload.orders,
Expand All @@ -28,12 +28,15 @@ const orderHistoryPage = (state = initialState, action = {}) => {
previous: action.payload.previous,
pageCount: action.payload.pageCount,
currentPage: action.payload.currentPage,
loading: false,
};
case FETCH_ORDERS.RESET:
case fetchOrders.FAILURE:
return {
...state,
loadingError: true,
};
case fetchOrders.FULFILL:
return {
...state,
loadingError: null,
loading: false,
};
default:
Expand Down
24 changes: 6 additions & 18 deletions src/order-history/saga.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,12 @@
import { call, put, takeEvery } from 'redux-saga/effects';
import { takeEvery } from 'redux-saga/effects';

// Actions
import {
FETCH_ORDERS,
fetchOrdersBegin,
fetchOrdersSuccess,
fetchOrdersReset,
} from './actions';
import { createFetchHandler } from '../utils';

// Services
import * as OrdersApiService from './service';
import { fetchOrders } from './actions';
import { getOrders } from './service';

export function* handleFetchOrders(action) {
const { pageToFetch } = action.payload;
yield put(fetchOrdersBegin());
const result = yield call(OrdersApiService.getOrders, pageToFetch);
yield put(fetchOrdersSuccess(result));
yield put(fetchOrdersReset());
}
const handleFetchOrders = createFetchHandler(fetchOrders, getOrders);

export default function* orderHistorySaga() {
yield takeEvery(FETCH_ORDERS.BASE, handleFetchOrders);
yield takeEvery(fetchOrders.TRIGGER, handleFetchOrders);
}
Loading

0 comments on commit c2039c6

Please sign in to comment.