diff --git a/config/test.js b/config/test.js index b97533b9fbc..fc136c9adcb 100644 --- a/config/test.js +++ b/config/test.js @@ -2,4 +2,5 @@ module.exports = { apiHost: 'https://localhost', allowErrorSimulation: true, + enableClientConsole: true, }; diff --git a/src/amo/actions/reviews.js b/src/amo/actions/reviews.js index 128be521a0a..c87de96a5d9 100644 --- a/src/amo/actions/reviews.js +++ b/src/amo/actions/reviews.js @@ -1,5 +1,7 @@ /* @flow */ -import { SET_ADDON_REVIEWS, SET_REVIEW } from 'amo/constants'; +import { + FETCH_REVIEWS, SET_ADDON_REVIEWS, SET_REVIEW, +} from 'amo/constants'; import type { ApiReviewType } from 'amo/api'; export type UserReviewType = {| @@ -47,6 +49,36 @@ export const setReview = (review: ApiReviewType): SetReviewAction => { return { type: SET_REVIEW, payload: denormalizeReview(review) }; }; +type FetchReviewsParams = {| + addonSlug: string, + errorHandlerId: string, + page?: number, +|}; + +export type FetchReviewsAction = {| + type: string, + payload: {| + addonSlug: string, + errorHandlerId: string, + page: number, + |}, +|}; + +export function fetchReviews( + { addonSlug, errorHandlerId, page = 1 }: FetchReviewsParams +): FetchReviewsAction { + if (!errorHandlerId) { + throw new Error('errorHandlerId cannot be empty'); + } + if (!addonSlug) { + throw new Error('addonSlug cannot be empty'); + } + return { + type: FETCH_REVIEWS, + payload: { addonSlug, errorHandlerId, page }, + }; +} + export const setDenormalizedReview = ( review: UserReviewType ): SetReviewAction => { diff --git a/src/amo/api/index.js b/src/amo/api/index.js index 1dd95a246e2..817a734ab57 100644 --- a/src/amo/api/index.js +++ b/src/amo/api/index.js @@ -87,7 +87,8 @@ export function submitReview({ } type GetReviewsParams = {| - addon?: number, + // This is the addon ID, slug, or guid. + addon?: number | string, apiState?: ApiStateType, filter?: string, page?: number, diff --git a/src/amo/components/Addon/index.js b/src/amo/components/Addon/index.js index 8b584d453aa..61d4bc77087 100644 --- a/src/amo/components/Addon/index.js +++ b/src/amo/components/Addon/index.js @@ -207,11 +207,11 @@ export class AddonBase extends React.Component { renderShowMoreCard() { const { addon, i18n } = this.props; const addonType = addon ? addon.type : ADDON_TYPE_EXTENSION; - let description; const descriptionProps = {}; if (addon) { - description = addon.description ? addon.description : addon.summary; + const description = + addon.description ? addon.description : addon.summary; if (!description || !description.length) { return null; } diff --git a/src/amo/components/AddonReviewList.js b/src/amo/components/AddonReviewList.js index c5296f458bc..85ef7f146b6 100644 --- a/src/amo/components/AddonReviewList.js +++ b/src/amo/components/AddonReviewList.js @@ -1,29 +1,29 @@ /* @flow */ -/* eslint-disable react/no-unused-prop-types */ +/* eslint-disable react/sort-comp, react/no-unused-prop-types */ import React from 'react'; import { connect } from 'react-redux'; import { compose } from 'redux'; import Rating from 'ui/components/Rating'; -import { setAddonReviews } from 'amo/actions/reviews'; -import { getReviews } from 'amo/api'; +import { fetchReviews } from 'amo/actions/reviews'; +import { setViewContext } from 'amo/actions/viewContext'; import fallbackIcon from 'amo/img/icons/default-64.png'; +import { fetchAddon } from 'core/actions/addons'; import Paginate from 'core/components/Paginate'; +import { withErrorHandler } from 'core/errorHandler'; import translate from 'core/i18n/translate'; -import { - isAllowedOrigin, - findAddon, - loadAddonIfNeeded, - safeAsyncConnect, -} from 'core/utils'; +import { isAllowedOrigin, findAddon } from 'core/utils'; +import log from 'core/logger'; import { parsePage } from 'core/searchUtils'; import Link from 'amo/components/Link'; import CardList from 'ui/components/CardList'; +import type { ErrorHandlerType } from 'core/errorHandler'; import type { UserReviewType } from 'amo/actions/reviews'; import type { ReviewState } from 'amo/reducers/reviews'; import type { AddonType } from 'core/types/addons'; -import type { DispatchFunc, ReduxStore } from 'core/types/redux'; +import type { DispatchFunc } from 'core/types/redux'; import type { ReactRouterLocation } from 'core/types/router'; +import LoadingText from 'ui/components/LoadingText'; import 'amo/css/AddonReviewList.scss'; @@ -34,15 +34,67 @@ type AddonReviewListRouteParams = {| type AddonReviewListProps = {| i18n: Object, addon?: AddonType, + dispatch: DispatchFunc, + errorHandler: ErrorHandlerType, location: ReactRouterLocation, params: AddonReviewListRouteParams, reviewCount?: number, reviews?: Array, |}; +type RenderReviewParams = {| + review?: UserReviewType, + key: string, +|}; + export class AddonReviewListBase extends React.Component { props: AddonReviewListProps; + componentWillMount() { + this.loadDataIfNeeded(); + } + + componentWillReceiveProps(nextProps: AddonReviewListProps) { + this.loadDataIfNeeded(nextProps); + } + + loadDataIfNeeded(nextProps?: AddonReviewListProps) { + const { + addon, dispatch, errorHandler, params, reviews, + } = { + ...this.props, + ...nextProps, + }; + + if (errorHandler.hasError()) { + log.warn('Not loading data because of an error'); + return; + } + + if (!addon) { + dispatch(fetchAddon({ slug: params.addonSlug, errorHandler })); + } else { + dispatch(setViewContext(addon.type)); + } + + let location = this.props.location; + let locationChanged = false; + if (nextProps && nextProps.location) { + if (nextProps.location !== location) { + locationChanged = true; + } + location = nextProps.location; + } + + if (!reviews || locationChanged) { + dispatch(fetchReviews({ + addonSlug: params.addonSlug, + errorHandlerId: errorHandler.id, + page: parsePage(location.query.page), + })); + } + } + addonURL() { const { addon } = this.props; if (!addon) { @@ -55,118 +107,101 @@ export class AddonReviewListBase extends React.Component { return `${this.addonURL()}reviews/`; } - renderReview(review: UserReviewType) { + renderReview({ review, key }: RenderReviewParams) { const { i18n } = this.props; - const timestamp = i18n.moment(review.created).fromNow(); + + let byLine; + if (review) { + const timestamp = i18n.moment(review.created).fromNow(); + // L10n: Example: "from Jose, last week" + byLine = i18n.sprintf( + i18n.gettext('from %(authorName)s, %(timestamp)s'), + { authorName: review.userName, timestamp }); + } else { + byLine = ; + } + return ( -
  • -

    {review.title}

    -

    {review.body}

    +
  • +

    {review ? review.title : }

    +

    {review ? review.body : }

    - - {/* L10n: Example: "from Jose, last week" */} - {i18n.sprintf(i18n.gettext('from %(authorName)s, %(timestamp)s'), - { authorName: review.userName, timestamp })} + {review ? + + : null + } + {byLine}
    • ); } render() { - const { addon, location, params, i18n, reviewCount, reviews } = this.props; + const { + addon, errorHandler, location, params, i18n, reviewCount, reviews, + } = this.props; if (!params.addonSlug) { throw new Error('params.addonSlug cannot be falsey'); } - if (!reviews || !addon) { - // TODO: add a spinner - return
    {i18n.gettext('Loading...')}
    ; + + // When reviews have not loaded yet, make a list of 4 empty reviews + // as a placeholder. + const allReviews = reviews || Array(4).fill(null); + const iconUrl = addon && isAllowedOrigin(addon.icon_url) ? + addon.icon_url : fallbackIcon; + const iconImage = ( + {i18n.gettext('Add-on + ); + + let header; + if (addon) { + header = i18n.sprintf( + i18n.gettext('Reviews for %(addonName)s'), { addonName: addon.name }); + } else { + header = ; } - const allReviews = reviews || []; - const iconUrl = isAllowedOrigin(addon.icon_url) ? addon.icon_url : - fallbackIcon; + let addonName; + if (addon) { + addonName = {addon.name}; + } else { + addonName = ; + } return (
    + {errorHandler.hasError() ? errorHandler.renderError() : null}
    - - {i18n.gettext('Add-on - + {addon ? {iconImage} : iconImage}
    -

    - {i18n.sprintf(i18n.gettext('Reviews for %(addonName)s'), - { addonName: addon.name })} -

    +

    {header}

    {i18n.gettext('All written reviews')}

    -

    {addon.name}

    +

    {addonName}

      - {allReviews.map((review) => this.renderReview(review))} + {allReviews.map((review, index) => { + return this.renderReview({ review, key: String(index) }); + })}
    - + {addon && reviewCount ? + + : null + }
    ); } } -export function loadAddonReviews( - { - addonId, addonSlug, dispatch, page = 1, - }: {| - addonId: number, - addonSlug: string, - dispatch: DispatchFunc, - page?: number, - |} -) { - return getReviews({ addon: addonId, page }) - .then((response) => { - const allReviews = response.results; - // Ignore reviews with null bodies as those are incomplete. - // For example, the user selected a star rating but hasn't submitted - // review text yet. - const reviews = allReviews.filter((review) => Boolean(review.body)); - dispatch(setAddonReviews({ - addonSlug, reviews, reviewCount: response.count, - })); - }); -} - -export function loadInitialData( - { - location, params, store, - }: {| - location: ReactRouterLocation, - params: AddonReviewListRouteParams, - store: ReduxStore, - |} -) { - const { addonSlug } = params; - if (!addonSlug) { - return Promise.reject(new Error('missing URL param addonSlug')); - } - let page; - return new Promise((resolve) => { - page = parsePage(location.query.page); - return resolve(); - }) - .then(() => loadAddonIfNeeded({ store, params: { slug: addonSlug } })) - .then(() => findAddon(store.getState(), addonSlug)) - .then((addon) => loadAddonReviews({ - addonId: addon.id, addonSlug, dispatch: store.dispatch, page, - })); -} - export function mapStateToProps( state: {| reviews: ReviewState |}, ownProps: AddonReviewListProps, ) { @@ -183,10 +218,7 @@ export function mapStateToProps( } export default compose( - safeAsyncConnect([{ - key: 'AddonReviewList', - promise: loadInitialData, - }]), connect(mapStateToProps), translate({ withRef: true }), + withErrorHandler({ name: 'AddonReviewList' }), )(AddonReviewListBase); diff --git a/src/amo/constants.js b/src/amo/constants.js index 2e28769dcbd..08b8678e97e 100644 --- a/src/amo/constants.js +++ b/src/amo/constants.js @@ -1,4 +1,5 @@ // Action types. +export const FETCH_REVIEWS = 'FETCH_REVIEWS'; export const SET_ADDON_REVIEWS = 'SET_ADDON_REVIEWS'; export const SET_REVIEW = 'SET_REVIEW'; diff --git a/src/amo/sagas/index.js b/src/amo/sagas/index.js index 10bdec470fd..841deff4037 100644 --- a/src/amo/sagas/index.js +++ b/src/amo/sagas/index.js @@ -3,6 +3,7 @@ import { fork } from 'redux-saga/effects'; import addons from 'core/sagas/addons'; import categories from './categories'; +import reviews from './reviews'; // Export all sagas for this app so runSaga can consume them. @@ -10,5 +11,6 @@ export default function* rootSaga() { yield [ fork(addons), fork(categories), + fork(reviews), ]; } diff --git a/src/amo/sagas/reviews.js b/src/amo/sagas/reviews.js new file mode 100644 index 00000000000..7c754b24e0d --- /dev/null +++ b/src/amo/sagas/reviews.js @@ -0,0 +1,40 @@ +/* @flow */ +/* global Generator */ +import { hideLoading, showLoading } from 'react-redux-loading-bar'; +import { call, put, select, takeEvery } from 'redux-saga/effects'; + +import { getReviews } from 'amo/api'; +import { setAddonReviews } from 'amo/actions/reviews'; +import { FETCH_REVIEWS } from 'amo/constants'; +import log from 'core/logger'; +import { createErrorHandler, getApi } from 'core/sagas/utils'; +import type { FetchReviewsAction } from 'amo/actions/reviews'; + + +export function* fetchReviews( + { + payload: { errorHandlerId, addonSlug, page }, + }: FetchReviewsAction +): Generator { + const errorHandler = createErrorHandler(errorHandlerId); + try { + yield put(showLoading()); + const api = yield select(getApi); + const response = yield call(getReviews, { + // Hide star-only ratings (reviews that do not have a body). + api, addon: addonSlug, page, filter: 'without_empty_body', + }); + yield put(setAddonReviews({ + addonSlug, reviews: response.results, reviewCount: response.count, + })); + } catch (error) { + log.warn(`Failed to load reviews for add-on slug ${addonSlug}: ${error}`); + yield put(errorHandler.createErrorAction(error)); + } finally { + yield put(hideLoading()); + } +} + +export default function* reviewsSaga(): Generator { + yield takeEvery(FETCH_REVIEWS, fetchReviews); +} diff --git a/src/core/sagas/addons.js b/src/core/sagas/addons.js index 762a4c5d457..77dfeb99df6 100644 --- a/src/core/sagas/addons.js +++ b/src/core/sagas/addons.js @@ -6,21 +6,16 @@ import { call, put, select, takeEvery } from 'redux-saga/effects'; import { loadEntities } from 'core/actions'; import { fetchAddon as fetchAddonFromApi } from 'core/api'; import { FETCH_ADDON } from 'core/constants'; -import { ErrorHandler } from 'core/errorHandler'; import log from 'core/logger'; import type { FetchAddonAction } from 'core/actions/addons'; -import { getApi } from './utils'; +import { createErrorHandler, getApi } from './utils'; export function* fetchAddon( { payload: { errorHandlerId, slug } }: FetchAddonAction ): Generator { - const errorHandler = new ErrorHandler({ - id: errorHandlerId, - dispatch: () => log.error( - 'ErrorHandler cannot dispatch from a saga'), - }); + const errorHandler = createErrorHandler(errorHandlerId); yield put(errorHandler.createClearingAction()); try { yield put(showLoading()); diff --git a/src/core/sagas/utils.js b/src/core/sagas/utils.js index 534b0623d93..5f9fda0b59f 100644 --- a/src/core/sagas/utils.js +++ b/src/core/sagas/utils.js @@ -1,2 +1,19 @@ +/* @flow */ +import { ErrorHandler } from 'core/errorHandler'; +import log from 'core/logger'; +import type { ApiStateType } from 'core/reducers/api'; + +export function createErrorHandler(id: string): typeof ErrorHandler { + return new ErrorHandler({ + id, + // Make sure the dispatch() method can't be used. A saga will yield + // put(action) instead so this shouldn't cause a problem. + dispatch: () => log.error( + 'ErrorHandler cannot dispatch from a saga'), + }); +} + // Convenience function to extract API info. -export const getApi = (state) => state.api; +export function getApi(state: {| api: ApiStateType |}): ApiStateType { + return state.api; +} diff --git a/tests/unit/amo/actions/testReviews.js b/tests/unit/amo/actions/testReviews.js index 487f328d537..7cb9463abd2 100644 --- a/tests/unit/amo/actions/testReviews.js +++ b/tests/unit/amo/actions/testReviews.js @@ -1,5 +1,6 @@ import { denormalizeReview, + fetchReviews, setDenormalizedReview, setReview, setAddonReviews, @@ -9,6 +10,34 @@ import { fakeAddon, fakeReview } from 'tests/unit/amo/helpers'; // See reducer tests for more coverage of review actions. describe('amo.actions.reviews', () => { + describe('fetchReviews', () => { + const defaultParams = Object.freeze({ + addonSlug: fakeAddon.slug, + errorHandlerId: 'some-error-handler-id', + page: 1, + }); + + it('requires a truthy add-on slug', () => { + const params = { ...defaultParams }; + delete params.addonSlug; + expect(() => fetchReviews(params)) + .toThrowError(/addonSlug cannot be empty/); + }); + + it('requires a truthy error handler ID', () => { + const params = { ...defaultParams }; + delete params.errorHandlerId; + expect(() => fetchReviews(params)) + .toThrowError(/errorHandlerId cannot be empty/); + }); + + it('defaults to page 1', () => { + const params = { ...defaultParams }; + delete params.page; + expect(fetchReviews(params)).toMatchObject({ payload: { page: 1 } }); + }); + }); + describe('setReview', () => { it('requires a truthy review', () => { expect(() => setReview()).toThrowError(/review cannot be empty/); diff --git a/tests/unit/amo/components/TestAddonReviewList.js b/tests/unit/amo/components/TestAddonReviewList.js index 73c853d5c80..a503c702eb2 100644 --- a/tests/unit/amo/components/TestAddonReviewList.js +++ b/tests/unit/amo/components/TestAddonReviewList.js @@ -1,35 +1,25 @@ +import { shallow } from 'enzyme'; import React from 'react'; -import { - findRenderedComponentWithType, - renderIntoDocument, - scryRenderedComponentsWithType, -} from 'react-addons-test-utils'; -import { Provider } from 'react-redux'; -import { findDOMNode } from 'react-dom'; - -import * as amoApi from 'amo/api'; + import fallbackIcon from 'amo/img/icons/default-64.png'; import createStore from 'amo/store'; -import { setAddonReviews } from 'amo/actions/reviews'; +import { fetchReviews, setAddonReviews } from 'amo/actions/reviews'; +import { setViewContext } from 'amo/actions/viewContext'; import { AddonReviewListBase, - loadAddonReviews, - loadInitialData, mapStateToProps, } from 'amo/components/AddonReviewList'; import Link from 'amo/components/Link'; import Paginate from 'core/components/Paginate'; -import translate from 'core/i18n/translate'; import { loadEntities } from 'core/actions'; -import * as coreApi from 'core/api'; +import { fetchAddon } from 'core/actions/addons'; +import { ErrorHandler } from 'core/errorHandler'; import { denormalizeAddon } from 'core/reducers/addons'; -import { initialApiState } from 'core/reducers/api'; -import I18nProvider from 'core/i18n/Provider'; +import ErrorList from 'ui/components/ErrorList'; import Rating from 'ui/components/Rating'; import { fakeAddon, fakeReview } from 'tests/unit/amo/helpers'; -import { - apiResponsePage, createFetchAddonResult, getFakeI18nInst, unexpectedSuccess, -} from 'tests/unit/helpers'; +import { createFetchAddonResult, getFakeI18nInst } from 'tests/unit/helpers'; +import LoadingText from 'ui/components/LoadingText'; function getLoadedReviews({ addonSlug = fakeAddon.slug, reviews = [fakeReview], reviewCount = 1 } = {}, @@ -43,6 +33,11 @@ describe('amo/components/AddonReviewList', () => { describe('', () => { function render({ addon = fakeAddon, + dispatch = sinon.stub(), + errorHandler = new ErrorHandler({ + id: 'some-error-handler', + dispatch: sinon.stub(), + }), params = { addonSlug: fakeAddon.slug, }, @@ -50,9 +45,11 @@ describe('amo/components/AddonReviewList', () => { ...customProps } = {}) { const loadedReviews = reviews ? getLoadedReviews({ reviews }) : null; - const { store } = createStore(); const props = { addon: addon && denormalizeAddon(addon), + dispatch, + errorHandler, + i18n: getFakeI18nInst(), location: { query: {} }, params, reviewCount: loadedReviews && loadedReviews.length, @@ -60,20 +57,7 @@ describe('amo/components/AddonReviewList', () => { ...customProps, }; - const AddonReviewList = translate({ withRef: true })(AddonReviewListBase); - const tree = renderIntoDocument( - - - - - - ); - - return tree; - } - - function renderToDOM(...args) { - return findDOMNode(render(...args)); + return shallow(); } it('requires an addonSlug property', () => { @@ -81,9 +65,195 @@ describe('amo/components/AddonReviewList', () => { .toThrowError(/addonSlug cannot be falsey/); }); - it('waits for reviews to load', () => { - const root = renderToDOM({ reviews: null }); - expect(root.textContent).toEqual('Loading...'); + it('waits for an addon and reviews to load', () => { + const root = render({ addon: null, reviews: null }); + expect(root.find('.AddonReviewList-header-icon img').prop('src')) + .toContain('default'); + expect(root.find('.AddonReviewList-header-text').find(LoadingText)) + .toHaveLength(2); + + // Make sure four review placeholders were rendered. + expect(root.find('.AddonReviewList-li')).toHaveLength(4); + // Do a sanity check on the first placeholder; + expect(root.find('.AddonReviewList-li h3').at(0).find(LoadingText)) + .toHaveLength(1); + expect(root.find('.AddonReviewList-li p').at(0).find(LoadingText)) + .toHaveLength(1); + expect(root.find('.AddonReviewList-by-line').at(0).find(LoadingText)) + .toHaveLength(1); + }); + + it('does not paginate before reviews have loaded', () => { + const root = render({ addon: fakeAddon, reviews: null }); + + expect(root.find(Paginate)).toHaveLength(0); + }); + + it('fetches an addon if needed', () => { + const addonSlug = 'some-addon-slug'; + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + id: 'fetch-error-handler', + dispatch: sinon.stub(), + }); + + render({ + addon: null, errorHandler, params: { addonSlug }, dispatch, + }); + + sinon.assert.calledWith(dispatch, fetchAddon({ + slug: addonSlug, errorHandler, + })); + }); + + it('fetches reviews if needed', () => { + const addon = { ...fakeAddon, slug: 'some-other-slug' }; + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + id: 'fetch-reviews-error-handler', + dispatch: sinon.stub(), + }); + + render({ + addon, + reviews: null, + errorHandler, + params: { addonSlug: addon.slug }, + dispatch, + }); + + sinon.assert.calledWith(dispatch, fetchReviews({ + addonSlug: addon.slug, + errorHandlerId: errorHandler.id, + })); + }); + + it('fetches reviews if needed during an update', () => { + const addon = { ...fakeAddon, slug: 'some-other-slug' }; + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + id: 'fetch-reviews-error-handler', + dispatch: sinon.stub(), + }); + + const root = render({ + addon: null, + reviews: null, + errorHandler, + params: { addonSlug: addon.slug }, + dispatch, + }); + dispatch.reset(); + // Simulate how a redux state change will introduce an addon. + root.setProps({ addon }); + + sinon.assert.calledWith(dispatch, fetchReviews({ + addonSlug: addon.slug, + errorHandlerId: errorHandler.id, + })); + }); + + it('fetches reviews by page', () => { + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + id: 'fetch-reviews-error-handler', + dispatch: sinon.stub(), + }); + const addonSlug = fakeAddon.slug; + const page = 2; + + render({ + reviews: null, + errorHandler, + location: { query: { page } }, + params: { addonSlug }, + dispatch, + }); + + sinon.assert.calledWith(dispatch, fetchReviews({ + addonSlug, + errorHandlerId: errorHandler.id, + page, + })); + }); + + it('fetches reviews when the page changes', () => { + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + id: 'fetch-reviews-error-handler', + dispatch: sinon.stub(), + }); + const addonSlug = fakeAddon.slug; + + const root = render({ + errorHandler, + location: { query: { page: 2 } }, + params: { addonSlug }, + dispatch, + }); + dispatch.reset(); + root.setProps({ location: { query: { page: 3 } } }); + + sinon.assert.calledWith(dispatch, fetchReviews({ + addonSlug, + errorHandlerId: errorHandler.id, + page: 3, + })); + }); + + it('does not fetch an addon if there is an error', () => { + const addon = { ...fakeAddon, slug: 'some-other-slug' }; + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + capturedError: new Error('some error'), + id: 'fetch-addon-error-handler', + dispatch: sinon.stub(), + }); + + render({ + addon: null, + errorHandler, + params: { addonSlug: addon.slug }, + dispatch, + }); + + sinon.assert.notCalled(dispatch); + }); + + it('does not fetch reviews if there is an error', () => { + const dispatch = sinon.stub(); + const errorHandler = new ErrorHandler({ + capturedError: new Error('some error'), + id: 'fetch-reviews-error-handler', + dispatch: sinon.stub(), + }); + + render({ + reviews: null, + errorHandler, + dispatch, + }); + + sinon.assert.notCalled(dispatch); + }); + + it('dispatches a view context for the add-on', () => { + const dispatch = sinon.stub(); + render({ addon: fakeAddon, dispatch }); + + sinon.assert.calledWith( + dispatch, setViewContext(fakeAddon.type)); + }); + + it('renders an error', () => { + const errorHandler = new ErrorHandler({ + capturedError: new Error('some error'), + id: 'custom-error-handler', + dispatch: sinon.stub(), + }); + + const root = render({ errorHandler }); + expect(root.find(ErrorList)).toHaveLength(1); }); it('renders a list of reviews with ratings', () => { @@ -92,157 +262,91 @@ describe('amo/components/AddonReviewList', () => { { ...fakeReview, rating: 2 }, ]; const tree = render({ reviews }); - const ratings = scryRenderedComponentsWithType(tree, Rating); - expect(ratings.length).toEqual(2); + const ratings = tree.find(Rating); + expect(ratings).toHaveLength(2); - expect(ratings[0].props.rating).toEqual(1); - expect(ratings[0].props.readOnly).toEqual(true); - expect(ratings[1].props.rating).toEqual(2); - expect(ratings[1].props.readOnly).toEqual(true); + expect(ratings.at(0)).toHaveProp('rating', 1); + expect(ratings.at(0)).toHaveProp('readOnly', true); + expect(ratings.at(1)).toHaveProp('rating', 2); + expect(ratings.at(1)).toHaveProp('readOnly', true); }); it('renders a review', () => { - const root = renderToDOM({ reviews: [fakeReview] }); + const root = render({ reviews: [fakeReview] }); - const title = root.querySelector('.AddonReviewList-li h3'); - expect(title.textContent).toEqual(fakeReview.title); + expect(root.find('.AddonReviewList-li h3')) + .toHaveText(fakeReview.title); - const body = root.querySelector('.AddonReviewList-li p'); - expect(body.textContent).toEqual(fakeReview.body); + expect(root.find('.AddonReviewList-li p')) + .toHaveText(fakeReview.body); - const byLine = - root.querySelector('.AddonReviewList-by-line').textContent; - expect(byLine).toContain(fakeReview.user.name); + expect(root.find('.AddonReviewList-by-line')) + .toIncludeText(fakeReview.user.name); }); it("renders the add-on's icon in the header", () => { - const root = renderToDOM({ addon: fakeAddon }); - const img = root.querySelector('.AddonReviewList-header-icon img'); - expect(img.src).toEqual(fakeAddon.icon_url); + const root = render({ addon: fakeAddon }); + const img = root.find('.AddonReviewList-header-icon img'); + expect(img).toHaveProp('src', fakeAddon.icon_url); }); it('renders the fallback icon if the origin is not allowed', () => { - const root = renderToDOM({ + const root = render({ addon: { ...fakeAddon, icon_url: 'http://foo.com/hax.png', }, }); - const img = root.querySelector('.AddonReviewList-header-icon img'); - expect(img.src).toEqual(fallbackIcon); + const img = root.find('.AddonReviewList-header-icon img'); + expect(img).toHaveProp('src', fallbackIcon); }); it('renders a hidden h1 for SEO', () => { - const root = renderToDOM({ addon: fakeAddon }); - const h1 = root.querySelector('.AddonReviewList-header h1'); - expect(h1.className).toEqual('visually-hidden'); - expect(h1.textContent).toEqual(`Reviews for ${fakeAddon.name}`); + const root = render({ addon: fakeAddon }); + const h1 = root.find('.AddonReviewList-header h1'); + expect(h1).toHaveClassName('visually-hidden'); + expect(h1).toHaveText(`Reviews for ${fakeAddon.name}`); }); it('produces an addon URL', () => { - const root = findRenderedComponentWithType( - render(), AddonReviewListBase); - expect(root.addonURL()).toEqual(`/addon/${fakeAddon.slug}/`); + expect(render().instance().addonURL()) + .toEqual(`/addon/${fakeAddon.slug}/`); }); it('produces a URL to itself', () => { - const root = findRenderedComponentWithType( - render(), AddonReviewListBase); - expect(root.url()).toEqual(`/addon/${fakeAddon.slug}/reviews/`); + expect(render().instance().url()) + .toEqual(`/addon/${fakeAddon.slug}/reviews/`); }); it('requires an addon prop to produce a URL', () => { - const root = findRenderedComponentWithType(render({ - addon: null, - }), AddonReviewListBase); - expect(() => root.addonURL()).toThrowError(/cannot access addonURL/); + expect(() => render({ addon: null }).instance().addonURL()) + .toThrowError(/cannot access addonURL/); }); it('configures a paginator with the right URL', () => { - const tree = render(); - const root = findRenderedComponentWithType(tree, AddonReviewListBase); - const paginator = findRenderedComponentWithType(tree, Paginate); - - expect(paginator.props.pathname).toEqual(root.url()); + const root = render(); + expect(root.find(Paginate)) + .toHaveProp('pathname', root.instance().url()); }); it('configures a paginator with the right Link', () => { - const paginator = findRenderedComponentWithType(render(), Paginate); - expect(paginator.props.LinkComponent).toEqual(Link); + expect(render().find(Paginate)).toHaveProp('LinkComponent', Link); }); it('configures a paginator with the right review count', () => { - const paginator = findRenderedComponentWithType( - render({ reviewCount: 500 }), Paginate); - expect(paginator.props.count).toEqual(500); + const root = render({ reviewCount: 500 }); + expect(root.find(Paginate)).toHaveProp('count', 500); }); it('sets the paginator to page 1 without a query', () => { - const paginator = findRenderedComponentWithType( - // Render with an empty query string. - render({ location: { query: {} } }), Paginate); - expect(paginator.props.currentPage).toEqual(1); + // Render with an empty query string. + const root = render({ location: { query: {} } }); + expect(root.find(Paginate)).toHaveProp('currentPage', 1); }); it('sets the paginator to the query string page', () => { - const paginator = findRenderedComponentWithType( - render({ location: { query: { page: 3 } } }), Paginate); - expect(paginator.props.currentPage).toEqual(3); - }); - }); - - describe('loadAddonReviews', () => { - let mockAmoApi; - - beforeEach(() => { - mockAmoApi = sinon.mock(amoApi); - }); - - it('loads all add-on reviews', () => { - const addonSlug = fakeAddon.slug; - const dispatch = sinon.stub(); - const page = 2; - const reviews = [fakeReview]; - - mockAmoApi - .expects('getReviews') - .once() - .withArgs({ addon: fakeAddon.id, page }) - .returns(apiResponsePage({ results: reviews })); - - return loadAddonReviews({ - addonId: fakeAddon.id, addonSlug, dispatch, page, - }) - .then(() => { - mockAmoApi.verify(); - - expect(dispatch.called).toBeTruthy(); - const expectedAction = setAddonReviews({ - addonSlug, reviewCount: reviews.length, reviews, - }); - expect(dispatch.firstCall.args[0]).toEqual(expectedAction); - }); - }); - - it('ignores incomplete reviews', () => { - const addonSlug = fakeAddon.slug; - const dispatch = sinon.stub(); - const reviews = [fakeReview, { ...fakeReview, body: null }]; - - mockAmoApi - .expects('getReviews') - .returns(apiResponsePage({ results: reviews })); - - return loadAddonReviews({ addonId: fakeAddon.id, addonSlug, dispatch }) - .then(() => { - // Expect an action with a filtered review array. - // However, the count will not take into account the filtering. - const expectedAction = setAddonReviews({ - addonSlug, reviews: [fakeReview], reviewCount: 2, - }); - expect(dispatch.called).toBeTruthy(); - expect(dispatch.firstCall.args[0]).toEqual(expectedAction); - }); + const root = render({ location: { query: { page: 3 } } }); + expect(root.find(Paginate)).toHaveProp('currentPage', 3); }); }); @@ -307,56 +411,4 @@ describe('amo/components/AddonReviewList', () => { expect(props.reviewCount).toEqual(1); }); }); - - describe('loadInitialData', () => { - let mockAmoApi; - let mockCoreApi; - - beforeEach(() => { - mockAmoApi = sinon.mock(amoApi); - mockCoreApi = sinon.mock(coreApi); - }); - - it('gets initial data from the API', () => { - const { store } = createStore(); - const addonSlug = fakeAddon.slug; - const page = 2; - const reviews = [fakeReview]; - - mockCoreApi - .expects('fetchAddon') - .once() - .withArgs({ slug: addonSlug, api: { ...initialApiState } }) - .returns(Promise.resolve(createFetchAddonResult(fakeAddon))); - - mockAmoApi - .expects('getReviews') - .once() - .withArgs({ addon: fakeAddon.id, page }) - .returns(apiResponsePage({ results: reviews })); - - return loadInitialData({ - location: { query: { page } }, store, params: { addonSlug }, - }) - .then(() => { - mockAmoApi.verify(); - mockCoreApi.verify(); - - const props = mapStateToProps(store.getState(), - { params: { addonSlug } }); - expect(props.addon).toEqual(denormalizeAddon(fakeAddon)); - expect(props.reviews).toEqual(getLoadedReviews({ reviews })); - }); - }); - - it('requires a slug param', () => { - const { store } = createStore(); - return loadInitialData({ - location: { query: {} }, store, params: { addonSlug: null }, - }) - .then(unexpectedSuccess, (error) => { - expect(error.message).toMatch(/missing URL param addonSlug/); - }); - }); - }); }); diff --git a/tests/unit/amo/sagas/testReviews.js b/tests/unit/amo/sagas/testReviews.js new file mode 100644 index 00000000000..121083a3014 --- /dev/null +++ b/tests/unit/amo/sagas/testReviews.js @@ -0,0 +1,87 @@ +import { hideLoading, showLoading } from 'react-redux-loading-bar'; +import SagaTester from 'redux-saga-tester'; + +import * as amoApi from 'amo/api'; +import { fetchReviews, setAddonReviews } from 'amo/actions/reviews'; +import { SET_ADDON_REVIEWS } from 'amo/constants'; +import reviewsReducer from 'amo/reducers/reviews'; +import reviewsSaga from 'amo/sagas/reviews'; +import { ErrorHandler } from 'core/errorHandler'; +import apiReducer from 'core/reducers/api'; +import { + dispatchSignInActions, fakeAddon, fakeReview, +} from 'tests/unit/amo/helpers'; +import { apiResponsePage } from 'tests/unit/helpers'; + +describe('amo/sagas/reviews', () => { + describe('fetchReviews', () => { + let apiState; + let errorHandler; + let mockAmoApi; + let sagaTester; + + beforeEach(() => { + errorHandler = new ErrorHandler({ + id: 'some-addon-handler', + dispatch: sinon.stub(), + }); + mockAmoApi = sinon.mock(amoApi); + + const { state } = dispatchSignInActions(); + apiState = state.api; + sagaTester = new SagaTester({ + initialState: state, + reducers: { reviews: reviewsReducer, api: apiReducer }, + }); + + sagaTester.start(reviewsSaga); + }); + + function _fetchReviews(params = {}) { + sagaTester.dispatch(fetchReviews({ + errorHandlerId: errorHandler.id, + addonSlug: fakeAddon.slug, + ...params, + })); + } + + it('fetches reviews from the API', async () => { + const reviews = [fakeReview]; + mockAmoApi + .expects('getReviews') + .once() + .withArgs({ + addon: fakeAddon.slug, + page: 1, + api: apiState, + filter: 'without_empty_body', + }) + .returns(apiResponsePage({ results: reviews })); + + _fetchReviews(); + + await sagaTester.waitFor(SET_ADDON_REVIEWS); + mockAmoApi.verify(); + + const calledActions = sagaTester.getCalledActions(); + expect(calledActions[1]).toEqual(showLoading()); + expect(calledActions[2]).toEqual(setAddonReviews({ + addonSlug: fakeAddon.slug, reviews, reviewCount: 1, + })); + expect(calledActions[3]).toEqual(hideLoading()); + }); + + it('dispatches an error', async () => { + const error = new Error('some API error maybe'); + mockAmoApi.expects('getReviews').returns(Promise.reject(error)); + + _fetchReviews(); + + const errorAction = errorHandler.createErrorAction(error); + await sagaTester.waitFor(errorAction.type); + const calledActions = sagaTester.getCalledActions(); + expect(calledActions[2]).toEqual(errorAction); + expect(calledActions[3]).toEqual(hideLoading()); + }); + }); +}); diff --git a/tests/unit/core/sagas/testAddons.js b/tests/unit/core/sagas/testAddons.js index abd9ca07399..8b560ae8d66 100644 --- a/tests/unit/core/sagas/testAddons.js +++ b/tests/unit/core/sagas/testAddons.js @@ -81,9 +81,7 @@ describe('core/sagas/addons', () => { it('dispatches an error', async () => { const error = new Error('some API error maybe'); - mockApi - .expects('fetchAddon') - .returns(Promise.reject(error)); + mockApi.expects('fetchAddon').returns(Promise.reject(error)); _fetchAddon();