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.

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

Product Out of Stock Message #1229

Merged
merged 11 commits into from
Jun 6, 2019
5 changes: 5 additions & 0 deletions packages/venia-concept/src/RootComponents/Product/Product.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { string, func } from 'prop-types';

import { connect, Query } from 'src/drivers';
import { addItemToCart } from 'src/actions/cart';
import ErrorView from 'src/components/ErrorView';
import { loadingIndicator } from 'src/components/LoadingIndicator';
import ProductFullDetail from 'src/components/ProductFullDetail';
import getUrlKey from 'src/util/getUrlKey';
Expand Down Expand Up @@ -52,6 +53,10 @@ class Product extends Component {

const product = data.productDetail.items[0];

if (!product) {
return <ErrorView outOfStock={true} />;
}

return (
<ProductFullDetail
product={this.mapProduct(product)}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`it renders the correct tree when loading 1`] = `
<h1>
<Classify(LoadingIndicator)>
<span>
Fetching Data...
</span>
</Classify(LoadingIndicator)>
</h1>
`;

exports[`it renders the correct tree when out of stock 1`] = `
<h1>
This Product is currently out of stock. Please try again later.
</h1>
`;

exports[`it renders the correct tree when page not found 1`] = `
<h1>
That page could not be found. Please try again.
</h1>
`;

exports[`it renders the internal error tree otherwise 1`] = `
<h1>
Something went wrong. Please try again.
</h1>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import React from 'react';
import ShallowRenderer from 'react-test-renderer/shallow';

import ErrorView from '../errorView';

const renderer = new ShallowRenderer();

test('it renders the correct tree when loading', () => {
const tree = renderer.render(<ErrorView loading={true} />);

expect(tree).toMatchSnapshot();
});

test('it renders the correct tree when page not found', () => {
const tree = renderer.render(<ErrorView notFound={true} />);

expect(tree).toMatchSnapshot();
});

test('it renders the correct tree when out of stock', () => {
const tree = renderer.render(<ErrorView outOfStock={true} />);

expect(tree).toMatchSnapshot();
});

test('it renders the internal error tree otherwise', () => {
const tree = renderer.render(<ErrorView />);

expect(tree).toMatchSnapshot();
});
10 changes: 8 additions & 2 deletions packages/venia-concept/src/components/ErrorView/errorView.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@ import { loadingIndicator } from 'src/components/LoadingIndicator';
const messages = new Map()
.set('loading', loadingIndicator)
.set('notFound', 'That page could not be found. Please try again.')
.set('internalError', 'Something went wrong. Please try again.');
.set('internalError', 'Something went wrong. Please try again.')
.set(
'outOfStock',
'This Product is currently out of stock. Please try again later.'
);

class ErrorView extends Component {
render() {
const { loading, notFound } = this.props;
const { loading, notFound, outOfStock } = this.props;
const message = loading
? messages.get('loading')
: notFound
? messages.get('notFound')
: outOfStock
? messages.get('outOfStock')
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want to allow an error string rather than using this prop. This will allow us to separate the error messages and types from the ErrorView implementation.

export const ERROR_TYPES = {
  'LOADING': 'loading',
  'OUT_OF_STOCK': 'outOfStock',
  ...
};
export const errorMap = new Map({
  'loading': loadingIndicator,
  'outOfStock': 'This Product is currently out of stock. Please try again later.',
  'default': 'Something went wrong. Please try again.'
  ...
});

class ErrorView extends Component {
  render() {
    const { type } = this.props;
    const key = ERROR_TYPES[type] || 'default';
    return <h1>{errorMap[key]}</h1>;
  }
} 

Copy link
Contributor

@supernova-at supernova-at Jun 4, 2019

Choose a reason for hiding this comment

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

Yeah the ErrorView certainly could use a refactor. I already felt like adding outOfStock was a little scope creep here and I felt like refactoring all of ErrorView would be solidly out of scope for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can create a follow up ticket to refactor ErrorView for sure

Copy link
Contributor

Choose a reason for hiding this comment

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

That's cool -- though the component is super small and I feel like the effort to refactor now is less than the effort to create, groom and work on it later. But w/e you want to do is cool, you're the one doing this work now :D

Copy link
Contributor

Choose a reason for hiding this comment

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

A fair point, lol.

I created #1308 just to keep things moving though.

: messages.get('internalError');

return <h1>{message}</h1>;
Expand Down