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

Transform addons reducer/actions/action creators into a ducks module #3025

Merged
merged 1 commit into from
Aug 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/amo/components/Addon/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import NotFound from 'amo/components/ErrorPage/NotFound';
import DefaultRatingManager from 'amo/components/RatingManager';
import ScreenShots from 'amo/components/ScreenShots';
import Link from 'amo/components/Link';
import { fetchAddon } from 'core/actions/addons';
import { fetchAddon } from 'core/reducers/addons';
import { withErrorHandler } from 'core/errorHandler';
import InstallButton from 'core/components/InstallButton';
import {
Expand Down
2 changes: 1 addition & 1 deletion src/amo/components/AddonReviewList.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { compose } from 'redux';
import Rating from 'ui/components/Rating';
import { fetchReviews } from 'amo/actions/reviews';
import { setViewContext } from 'amo/actions/viewContext';
import { fetchAddon } from 'core/actions/addons';
import { fetchAddon } from 'core/reducers/addons';
import Paginate from 'core/components/Paginate';
import { withErrorHandler } from 'core/errorHandler';
import translate from 'core/i18n/translate';
Expand Down
29 changes: 0 additions & 29 deletions src/core/actions/addons.js

This file was deleted.

1 change: 0 additions & 1 deletion src/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export const SEARCH_SORT_POPULAR = 'hotness';
export const SEARCH_SORT_TOP_RATED = 'rating';

// Action types.
export const FETCH_ADDON = 'FETCH_ADDON';
export const CATEGORIES_FETCH = 'CATEGORIES_FETCH';
export const CATEGORIES_LOAD = 'CATEGORIES_LOAD';
export const CLEAR_ERROR = 'CLEAR_ERROR';
Expand Down
45 changes: 42 additions & 3 deletions src/core/reducers/addons.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,43 @@
/* @flow */
import { ADDON_TYPE_THEME } from 'core/constants';
import type { ErrorHandlerType } from 'core/errorHandler';
import type { AddonType } from 'core/types/addons';


export const FETCH_ADDON = 'FETCH_ADDON';
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we have a naming pattern for action types btw? poke @tofumatt

Copy link
Contributor

Choose a reason for hiding this comment

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

We have been inconsistent but we want VERB_NOUN so FETCH_ADDON, LOAD_ADDON, etc. makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

FETCH_ADDON to trigger the API call, and LOAD_ADDON to update the state with the API results, is that correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, that's what we want. And when starting things like autocomplete or a search it would probably be START_SEARCH, START_AUTOCOMPLETE

Copy link
Member Author

Choose a reason for hiding this comment

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

wonderful!


const initialState = {};

export function getGuid(result) {
type Action = Object;
type AddonState = Object;

type FetchAddonParams = {|
errorHandler: ErrorHandlerType,
slug: string,
|};

export type FetchAddonAction = {|
type: string,
payload: {|
errorHandlerId: string,
slug: string,
|},
|};

export function fetchAddon({ errorHandler, slug }: FetchAddonParams): FetchAddonAction {
if (!errorHandler) {
throw new Error('errorHandler cannot be empty');
}
if (!slug) {
throw new Error('slug cannot be empty');
}
return {
type: FETCH_ADDON,
payload: { errorHandlerId: errorHandler.id, slug },
};
}

export function getGuid(result: AddonType) {
if (result.type === ADDON_TYPE_THEME) {
// This mimics how Firefox appends @personas.mozilla.org internally.
// It's needed to look up themes in mozAddonManager.
Expand All @@ -11,7 +46,7 @@ export function getGuid(result) {
return result.guid;
}

export function denormalizeAddon(apiAddon) {
export function denormalizeAddon(apiAddon: AddonType) {
if (apiAddon.icon_url) {
return {
...apiAddon,
Expand All @@ -22,8 +57,12 @@ export function denormalizeAddon(apiAddon) {
return apiAddon;
}

export default function addon(state = initialState, action) {
export default function addon(
state: AddonState = initialState,
action: Action = {}
) {
const { payload } = action;

if (payload && payload.entities && payload.entities.addons) {
const newState = { ...state };
Object.keys(payload.entities.addons).forEach((key) => {
Expand Down
4 changes: 2 additions & 2 deletions src/core/sagas/addons.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ 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 { FETCH_ADDON } from 'core/reducers/addons';
import log from 'core/logger';
import type { FetchAddonAction } from 'core/actions/addons';
import type { FetchAddonAction } from 'core/reducers/addons';

import { createErrorHandler, getState } from './utils';

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/amo/components/TestAddon.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import RatingManager, {
} from 'amo/components/RatingManager';
import createStore from 'amo/store';
import { loadEntities } from 'core/actions';
import { fetchAddon as fetchAddonAction } from 'core/actions/addons';
import { fetchAddon as fetchAddonAction } from 'core/reducers/addons';
import { setError } from 'core/actions/errors';
import { setInstallState } from 'core/actions/installations';
import { createApiError } from 'core/api/index';
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/amo/components/TestAddonReviewList.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ import {
import Link from 'amo/components/Link';
import Paginate from 'core/components/Paginate';
import { loadEntities } from 'core/actions';
import { fetchAddon } from 'core/actions/addons';
import { denormalizeAddon } from 'core/reducers/addons';
import { denormalizeAddon, fetchAddon } from 'core/reducers/addons';
import ErrorList from 'ui/components/ErrorList';
import Rating from 'ui/components/Rating';
import { fakeAddon, fakeReview } from 'tests/unit/amo/helpers';
Expand Down
25 changes: 0 additions & 25 deletions tests/unit/core/actions/test_addons.js

This file was deleted.

30 changes: 27 additions & 3 deletions tests/unit/core/reducers/test_addons.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { loadEntities } from 'core/actions';
import { ADDON_TYPE_THEME } from 'core/constants';
import addons, { denormalizeAddon } from 'core/reducers/addons';
import { createFetchAddonResult } from 'tests/unit/helpers';
import addons, { denormalizeAddon, fetchAddon } from 'core/reducers/addons';
import {
createFetchAddonResult,
createStubErrorHandler,
} from 'tests/unit/helpers';
import { createFakeAddon, fakeAddon } from 'tests/unit/amo/helpers';


describe('addon reducer', () => {
describe(__filename, () => {
let addonsState;

beforeEach(() => {
Expand Down Expand Up @@ -180,4 +183,25 @@ describe('addon reducer', () => {
}),
});
});

describe('fetchAddon', () => {
const defaultParams = Object.freeze({
slug: 'addon-slug',
errorHandler: createStubErrorHandler(),
});

it('requires an error handler', () => {
const params = { ...defaultParams };
delete params.errorHandler;
expect(() => fetchAddon(params))
.toThrowError(/errorHandler cannot be empty/);
});

it('requires a slug', () => {
const params = { ...defaultParams };
delete params.slug;
expect(() => fetchAddon(params))
.toThrowError(/slug cannot be empty/);
});
});
});
3 changes: 1 addition & 2 deletions tests/unit/core/sagas/testAddons.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { hideLoading, showLoading } from 'react-redux-loading-bar';
import SagaTester from 'redux-saga-tester';

import { fetchAddon } from 'core/actions/addons';
import * as api from 'core/api';
import { ENTITIES_LOADED } from 'core/constants';
import addonsReducer from 'core/reducers/addons';
import addonsReducer, { fetchAddon } from 'core/reducers/addons';
import apiReducer from 'core/reducers/api';
import addonsSaga from 'core/sagas/addons';
import { dispatchSignInActions, fakeAddon } from 'tests/unit/amo/helpers';
Expand Down