Skip to content

Commit

Permalink
fix(productViewMissing): ent-3913 apply platform appNavClick (#695)
Browse files Browse the repository at this point in the history
* testing, mock appNavClick, hook
* productViewMissing, useRouter, setAppNav
* platformActions, expose setAppNav
* platformServices, expose appNavClick, replace navigation
* useRouter, pass useHistory, proxy useHistory push, setAppNav
  • Loading branch information
cdcabrera committed Jul 13, 2021
1 parent c0927de commit 086c71e
Show file tree
Hide file tree
Showing 17 changed files with 251 additions and 79 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"plugin:jsdoc/recommended"
],
"globals": {
"mockHook": "readonly",
"mountHookComponent": "readonly",
"mockWindowLocation": "readonly"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`ProductViewMissing Component should redirect when there are limited product cards: redirect 1`] = `"Redirected towards /rhel"`;
exports[`ProductViewMissing Component should redirect when there are limited product cards: redirect action 1`] = `
Array [
Array [
Object {
"meta": Object {
"appName": undefined,
"id": "rhel",
"secondaryNav": undefined,
},
"payload": Promise {},
"type": "PLATFORM_SET_NAV",
},
],
]
`;

exports[`ProductViewMissing Component should render a non-connected component: non-connected 1`] = `
<PageLayout
Expand Down
21 changes: 15 additions & 6 deletions src/components/productView/__tests__/productViewMissing.test.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import React from 'react';
import { shallow } from 'enzyme';
import * as reactRedux from 'react-redux';
import { ProductViewMissing } from '../productViewMissing';
import { Redirect } from '../../router';

describe('ProductViewMissing Component', () => {
const useDispatchMock = jest.spyOn(reactRedux, 'useDispatch');

afterEach(() => {
useDispatchMock.mockClear();
});

it('should render a non-connected component', () => {
useDispatchMock.mockReturnValue(jest.fn());

const props = {
availableProductsRedirect: 1
};
Expand All @@ -28,12 +36,13 @@ describe('ProductViewMissing Component', () => {
);
});

it('should redirect when there are limited product cards', () => {
it('should redirect when there are limited product cards', async () => {
const mockDispatch = jest.fn();
useDispatchMock.mockReturnValue(action => action(mockDispatch));

const props = {};

mockWindowLocation(() => {
const component = shallow(<ProductViewMissing {...props} />);
expect(component.find(Redirect).html()).toMatchSnapshot('redirect');
});
await mountHookComponent(<ProductViewMissing {...props} />);
expect(mockDispatch.mock.calls).toMatchSnapshot('redirect action');
});
});
41 changes: 20 additions & 21 deletions src/components/productView/productViewMissing.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,22 @@ import React from 'react';
import PropTypes from 'prop-types';
import { Button, Card, CardBody, CardFooter, CardTitle, Gallery, Title, PageSection } from '@patternfly/react-core';
import { ArrowRightIcon } from '@patternfly/react-icons';
import { useHistory } from 'react-router-dom';
import { useMount } from 'react-use';
import { PageLayout, PageHeader } from '../pageLayout/pageLayout';
import { Redirect, routerHelpers } from '../router';
import { routerHelpers } from '../router';
import { helpers } from '../../common';
import { translate } from '../i18n/i18n';
import { useHistory } from '../../hooks/useRouter';

/**
* Return a list of available products.
*
* @returns {Array}
*/
const filterAvailableProducts = () => {
const { configs, allConfigs } = routerHelpers.getRouteConfigByPath();
return (configs.length && configs) || allConfigs.filter(({ isSearchable }) => isSearchable === true);
};

/**
* Render a missing product view.
Expand All @@ -19,16 +30,13 @@ import { translate } from '../i18n/i18n';
*/
const ProductViewMissing = ({ availableProductsRedirect, t }) => {
const history = useHistory();
const availableProducts = filterAvailableProducts();

/**
* Return a list of available products.
*
* @returns {Array}
*/
const filterAvailableProducts = () => {
const { configs, allConfigs } = routerHelpers.getRouteConfigByPath();
return (configs.length && configs) || allConfigs.filter(({ isSearchable }) => isSearchable === true);
};
useMount(() => {
if (availableProducts.length <= availableProductsRedirect) {
history.push(availableProducts[0].path);
}
});

/**
* On click, update history.
Expand All @@ -37,16 +45,7 @@ const ProductViewMissing = ({ availableProductsRedirect, t }) => {
* @param {string} id
* @returns {void}
*/
const onNavigate = id => {
const { routeHref } = routerHelpers.getRouteConfig({ id });
history.push(routeHref);
};

const availableProducts = filterAvailableProducts();

if (availableProducts.length <= availableProductsRedirect) {
return <Redirect isForced route={availableProducts[0].path} />;
}
const onNavigate = id => history.push(id);

return (
<PageLayout className="curiosity-missing-view">
Expand Down
35 changes: 35 additions & 0 deletions src/hooks/__tests__/__snapshots__/useRouter.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`useRouter should apply a hook for useHistory: push, config route 1`] = `
Array [
Array [
Object {
"meta": Object {
"appName": undefined,
"id": "rhel",
"secondaryNav": undefined,
},
"payload": Promise {},
"type": "PLATFORM_SET_NAV",
},
],
]
`;

exports[`useRouter should apply a hook for useHistory: push, unique route 1`] = `
Array [
Array [
"/lorem/ipsum",
undefined,
],
]
`;

exports[`useRouter should return specific properties: specific properties 1`] = `
Object {
"useHistory": [Function],
"useLocation": [Function],
"useParams": [Function],
"useRouteMatch": [Function],
}
`;
24 changes: 24 additions & 0 deletions src/hooks/__tests__/useRouter.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { routerHooks } from '../useRouter';

describe('useRouter', () => {
it('should return specific properties', () => {
expect(routerHooks).toMatchSnapshot('specific properties');
});

it('should apply a hook for useHistory', () => {
const mockDispatch = jest.fn();
const mockHistoryPush = jest.fn();
const mockUseHistory = mockHook(() =>
routerHooks.useHistory({
useDispatch: () => action => action(mockDispatch),
useHistory: () => ({ push: mockHistoryPush })
})
);

mockUseHistory.push('rhel');
expect(mockDispatch.mock.calls).toMatchSnapshot('push, config route');

mockUseHistory.push('/lorem/ipsum');
expect(mockHistoryPush.mock.calls).toMatchSnapshot('push, unique route');
});
});
49 changes: 49 additions & 0 deletions src/hooks/useRouter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { useHistory as useHistoryRRD, useLocation, useParams, useRouteMatch } from 'react-router-dom';
import { routerHelpers } from '../components/router/routerHelpers';
import { reduxActions, useDispatch } from '../redux';
import { helpers } from '../common/helpers';

/**
* ToDo: reevaluate this alternative pattern of passing library hooks as options
* We did this as a test to see if its more convenient for unit testing instead of
* having to spy or mock entire resources.
*/
/**
* Pass useHistory methods. Proxy useHistory push with Platform specific navigation update.
*
* @param {object} hooks
* @param {Function} hooks.useHistory
* @param {Function} hooks.useDispatch
* @returns {object<history>}
*/
const useHistory = ({
useHistory: useAliasHistory = useHistoryRRD,
useDispatch: useAliasDispatch = useDispatch
} = {}) => {
const history = useAliasHistory();
const dispatch = useAliasDispatch();

return {
...history,
push: (pathLocation, historyState) => {
const pathName = (typeof pathLocation === 'string' && pathLocation) || pathLocation?.pathname;
const { productParameter, id, routeHref } = routerHelpers.getRouteConfig({ pathName, id: pathName });
const { hash, search } = window.location;

if (!helpers.DEV_MODE && productParameter) {
return dispatch(reduxActions.platform.setAppNav(id));
}

return history.push(routeHref || (pathName && `${pathName}${search}${hash}`) || pathLocation, historyState);
}
};
};

const routerHooks = {
useHistory,
useLocation,
useParams,
useRouteMatch
};

export { routerHooks as default, routerHooks, useHistory, useLocation, useParams, useRouteMatch };
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,16 @@ exports[`PlatformActions Should return a function for the onNavigation method: e

exports[`PlatformActions Should return a function for the onNavigation method: function 1`] = `[Function]`;

exports[`PlatformActions Should return a function for the setNavigation method: expected process 1`] = `
Array [
Object {
"active": false,
exports[`PlatformActions Should return a function for the setAppNav method: expected process 1`] = `
Object {
"meta": Object {
"appName": undefined,
"id": "lorem",
"secondaryNav": undefined,
},
Object {
"active": false,
"id": "ipsum",
},
]
"payload": Promise {},
"type": "PLATFORM_SET_NAV",
}
`;

exports[`PlatformActions Should return a function for the setNavigation method: function 1`] = `[Function]`;
exports[`PlatformActions Should return a function for the setAppNav method: function 1`] = `[Function]`;
8 changes: 3 additions & 5 deletions src/redux/actions/__tests__/platformActions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ describe('PlatformActions', () => {
expect(platformActions.setAppName()).toMatchSnapshot('dispatch object');
});

it('Should return a function for the setNavigation method', () => {
expect(platformActions.setNavigation()).toMatchSnapshot('function');
it('Should return a function for the setAppNav method', () => {
expect(platformActions.setAppNav()).toMatchSnapshot('function');

window.insights.chrome.navigation = jest.fn().mockImplementation(value => value);
const dispatch = obj => obj;
expect(platformActions.setNavigation([{ id: 'lorem' }, { id: 'ipsum' }])(dispatch)).toMatchSnapshot(
'expected process'
);
expect(platformActions.setAppNav('lorem')(dispatch)).toMatchSnapshot('expected process');
});
});
23 changes: 15 additions & 8 deletions src/redux/actions/platformActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,24 @@ const setAppName = name => ({
});

/**
* Apply platform method for handling the left-nav navigation active item.
* Apply platform method for changing routes via the left-nav navigation.
*
* @param {object} data
* @param {string} id
* @param {object} options
* @param {string} options.appName
* @param {boolean} options.secondaryNav
* @returns {Function}
*/
const setNavigation = data => dispatch => {
const setAppNav = (id, { appName, secondaryNav } = {}) => dispatch =>
dispatch({
type: platformTypes.PLATFORM_SET_NAV
type: platformTypes.PLATFORM_SET_NAV,
payload: platformServices.setAppNav(id, { appName, secondaryNav }),
meta: {
id,
appName,
secondaryNav
}
});
return platformServices.setNavigation(data);
};

const platformActions = {
addNotification,
Expand All @@ -98,7 +105,7 @@ const platformActions = {
initializeChrome,
onNavigation,
setAppName,
setNavigation
setAppNav
};

export {
Expand All @@ -111,5 +118,5 @@ export {
initializeChrome,
onNavigation,
setAppName,
setNavigation
setAppNav
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ exports[`PlatformServices should return a failed initializeChrome: failed initia

exports[`PlatformServices should return a failed setAppName: failed setAppName 1`] = `"{ identifyApp } = insights.chrome, insights.chrome.identifyApp is not a function"`;

exports[`PlatformServices should return a failed setAppNav: failed setAppNav 1`] = `[Error: { appNavClick } = insights.chrome, insights.chrome.appNavClick is not a function]`;

exports[`PlatformServices should return a successful getUser with a specific response: specific success for authorized user 1`] = `"lorem ipsum"`;

exports[`PlatformServices should return a successful getUser: success authorized user 1`] = `
Expand Down
12 changes: 6 additions & 6 deletions src/services/__tests__/platformServices.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('PlatformServices', () => {
expect(platformServices.initializeChrome).toBeDefined();
expect(platformServices.onNavigation).toBeDefined();
expect(platformServices.setAppName).toBeDefined();
expect(platformServices.setNavigation).toBeDefined();
expect(platformServices.setAppNav).toBeDefined();
});

/**
Expand Down Expand Up @@ -95,10 +95,10 @@ describe('PlatformServices', () => {
expect(response).toMatchSnapshot('failed setAppName');
});

it('should return a failed setNavigation', () => {
window.insights.chrome.navigation = undefined;
expect(platformServices.setNavigation).toThrowError(
'{ navigation } = insights.chrome, insights.chrome.navigation is not a function'
);
it('should return a failed setAppNav', async () => {
window.insights.chrome.appNavClick = undefined;
const response = await returnPromiseAsync(platformServices.setAppNav);

expect(response).toMatchSnapshot('failed setAppNav');
});
});
Loading

0 comments on commit 086c71e

Please sign in to comment.