fixes #25537 - Layout unwanted action calls

webpack/assets/javascripts/react_app/common/testHelpers.js

@@ -93,3 +93,12 @@ export const testReducerSnapshotWithFixtures = (reducer, fixtures) => {
   Object.entries(fixtures).forEach(([description, action]) =>
     it(description, () => expect(reduce(action)).toMatchSnapshot()));
 };
+
+/**
+ * Test selectors with fixtures and snapshots
+ * @param  {Object} fixtures  key=fixture description,
+ *                            value=selector runner function
+ */
+export const testSelectorsSnapshotWithFixtures = fixtures =>
+  Object.entries(fixtures).forEach(([description, selectorRunner]) =>
+    it(description, () => expect(selectorRunner()).toMatchSnapshot()));

webpack/assets/javascripts/react_app/components/Layout/Layout.fixtures.js

@@ -210,6 +210,7 @@ const serverUser = {
 
 export const layoutMock = {
   items: PFitems,
+  activeMenu: 'Monitor',
   data: {
     menu: [...hashItemsA, ...hashItemsB],
     locations,

webpack/assets/javascripts/react_app/components/Layout/Layout.js

@@ -4,17 +4,12 @@ import PropTypes from 'prop-types';
 import { VerticalNav } from 'patternfly-react';
 import { noop } from '../../common/helpers';
 
-import { getActiveOnBack, getCurrentPath } from './LayoutHelper';
+import { getActive, getCurrentPath, handleMenuClick } from './LayoutHelper';
 import TaxonomySwitcher from './components/TaxonomySwitcher';
 import UserDropdowns from './components/UserDropdowns';
 import './layout.scss';
 
 class Layout extends React.Component {
-  changeActiveOnBack() {
-    const { data, changeActiveMenu } = this.props;
-    changeActiveMenu(getActiveOnBack(data.menu, getCurrentPath()));
-  }
-
   componentDidMount() {
     const {
       items,
@@ -24,9 +19,16 @@ class Layout extends React.Component {
       currentLocation,
       changeOrganization,
       currentOrganization,
+      changeActiveMenu,
+      activeMenu,
     } = this.props;
     if (items.length === 0) fetchMenuItems(data);
 
+    const activeURLMenu = getActive(data.menu, getCurrentPath());
+    if (activeMenu !== activeURLMenu.title) {
+      changeActiveMenu(activeURLMenu);
+    }
+
     if (data.taxonomies.locations && !!data.locations.current_location
       && currentLocation !== data.locations.current_location) {
       const initialLocTitle = data.locations.current_location;
@@ -42,12 +44,6 @@ class Layout extends React.Component {
         .find(org => org.title === initialOrgTitle).id;
       changeOrganization({ title: initialOrgTitle, id: initialOrgId });
     }
-    // changeActive on Back navigation
-    window.addEventListener('popstate', () => this.changeActiveOnBack());
-  }
-
-  componentWillUnmount() {
-    window.removeEventListener('popstate', () => this.changeActiveOnBack());
   }
 
   render() {
@@ -60,12 +56,15 @@ class Layout extends React.Component {
       changeLocation,
       currentOrganization,
       currentLocation,
+      activeMenu,
     } = this.props;
+
     return (
       <VerticalNav
         hoverDelay={0}
         items={items}
-        onItemClick={changeActiveMenu}
+        onItemClick={primary => handleMenuClick(primary, activeMenu, changeActiveMenu)}
+        activePath={`/${activeMenu}/`}
         {...this.props}
       >
         <VerticalNav.Masthead>

webpack/assets/javascripts/react_app/components/Layout/LayoutHelper.js

@@ -1,13 +1,10 @@
 import { isEmpty } from 'lodash';
-import {
-  changeOrganization,
-  changeLocation,
-} from '../../../foreman_navigation';
+import { changeOrganization, changeLocation } from '../../../foreman_navigation';
 import { translate as __ } from '../../common/I18n';
 
 export const getCurrentPath = () => window.location.pathname;
 
-export const getActiveOnBack = (data, path) => {
+export const getActive = (data, path) => {
   let activeItem = '';
   data.forEach((item) => {
     item.children.forEach((child) => {
@@ -17,6 +14,10 @@ export const getActiveOnBack = (data, path) => {
   return { title: activeItem };
 };
 
+export const handleMenuClick = (primary, activeMenu, changeActive) => {
+  if (primary.title !== activeMenu) changeActive(primary);
+};
+
 export const combineMenuItems = (data) => {
   const items = [];
 
@@ -113,4 +114,3 @@ const createLocationItem = (locations) => {
   };
   return locItem;
 };
-

webpack/assets/javascripts/react_app/components/Layout/LayoutSelectors.js

@@ -1,41 +1,32 @@
 import { createSelector } from 'reselect';
-import { isEmpty, get } from 'lodash';
-import { changeActive } from '../../../foreman_navigation';
-import { getCurrentPath } from './LayoutHelper';
+import { get } from 'lodash';
 
 export const selectLayout = state => state.layout;
 
 export const selectMenuItems = state => selectLayout(state).items;
 export const selectActiveMenu = state => selectLayout(state).activeMenu;
+export const selectIsLoading = state => selectLayout(state).isLoading;
 export const selectCurrentLocation = state => get(selectLayout(state), 'currentLocation.title');
 export const selectCurrentOrganization = state => get(selectLayout(state), 'currentOrganization.title');
-const path = getCurrentPath();
 
 export const patternflyMenuItemsSelector = createSelector(
   selectMenuItems,
-  selectActiveMenu,
   selectCurrentLocation,
   selectCurrentOrganization,
-  (items, activeMenu, currentLocation, currentOrganization) =>
-    patternflyItems(items, path, activeMenu, currentLocation, currentOrganization),
+  (items, currentLocation, currentOrganization) =>
+    patternflyItems(items, currentLocation, currentOrganization),
 );
 
-const patternflyItems = (data, activePath, activeMenu, currentLocation, currentOrganization) => {
+const patternflyItems = (data, currentLocation, currentOrganization) => {
+  if (data.length === 0) return [];
   const items = [];
-  if (isEmpty(data)) return [];
 
   data.forEach((item) => {
-    let activeFlag = false;
     const childrenArray = [];
     item.children.forEach((child) => {
-      if (isEmpty(activeMenu) && child.url === activePath) { // activeMenu after Full page reload
-        activeFlag = true;
-        changeActive(item.name);
-      }
-
       const childObject = {
         title: child.name,
-        isDivider: child.type === 'divider' && !isEmpty(child.name),
+        isDivider: child.type === 'divider' && !!child.name,
         className: (child.name === currentLocation || child.name === currentOrganization) ? 'mobile-active' : '',
         href: child.url ? child.url : '#',
         preventHref: false,
@@ -45,7 +36,7 @@ const patternflyItems = (data, activePath, activeMenu, currentLocation, currentO
     });
     const itemObject = {
       title: item.name,
-      initialActive: activeFlag || item.active,
+      initialActive: item.active,
       iconClass: item.icon,
       subItems: childrenArray,
       className: item.className,

webpack/assets/javascripts/react_app/components/Layout/__tests__/LayoutHelper.test.js

@@ -1,13 +1,18 @@
-import { combineMenuItems, getActiveOnBack } from '../LayoutHelper';
+import { combineMenuItems, getActive, handleMenuClick } from '../LayoutHelper';
 import { layoutMock } from '../Layout.fixtures';
 
 describe('LayoutHelper', () => {
   it('should combineMenuItems', () => {
     const combined = combineMenuItems(layoutMock.data);
     expect(combined).toMatchSnapshot();
   });
-  it('should getActiveOnBack(Monitor)', () => {
-    const active = getActiveOnBack(layoutMock.data.menu, '/fact_values');
+  it('should getActive(Monitor)', () => {
+    const active = getActive(layoutMock.data.menu, '/fact_values');
     expect(active).toMatchSnapshot();
   });
+  it('should handleMenuClick', () => {
+    const change = jest.fn();
+    handleMenuClick({ title: 'Host' }, 'Infra', change);
+    expect(change).toHaveBeenCalled();
+  });
 });

webpack/assets/javascripts/react_app/components/Layout/__tests__/LayoutSelectors.test.js

@@ -1,27 +1,38 @@
-import { patternflyMenuItemsSelector } from '../LayoutSelectors';
+import { testSelectorsSnapshotWithFixtures } from '../../../common/testHelpers';
+import {
+  patternflyMenuItemsSelector,
+  selectIsLoading,
+  selectLayout,
+  selectCurrentLocation,
+  selectCurrentOrganization,
+} from '../LayoutSelectors';
 import { layoutMock } from '../Layout.fixtures';
 
 const state = {
   layout: {
     items: layoutMock.data.menu,
     activeMenu: 'Hosts',
     currentOrganization: { title: 'org1' },
+    currentLocation: { title: 'loc1' },
+    isLoading: true,
   },
 };
-describe('Layout Selectors', () => {
-  it('should return PF-React Compatible items', () => {
-    const output = patternflyMenuItemsSelector(state);
 
-    expect(output).toMatchSnapshot();
-  });
-  it('should return empty array', () => {
-    const emptyState = {
-      layout: {
-        items: [],
-      },
-    };
-    const output = patternflyMenuItemsSelector(emptyState);
+const emptyState = {
+  layout: {
+    items: [],
+  },
+};
+
+const fixtures = {
+  'should return Layout': () => selectLayout(state),
+  'should return PF-React Compatible items': () => patternflyMenuItemsSelector(state),
+  'should return empty array of items': () => patternflyMenuItemsSelector(emptyState),
+
+  'should return isLoading from selector': () => selectIsLoading(state),
+  'should return location from selector': () => selectCurrentLocation(state),
+  'should return organization from selector': () => selectCurrentOrganization(state),
+};
+
+describe('Layout selectors', () => testSelectorsSnapshotWithFixtures(fixtures));
 
-    expect(output).toMatchSnapshot();
-  });
-});

webpack/assets/javascripts/react_app/components/Layout/__tests__/__snapshots__/Layout.test.js.snap

@@ -2,6 +2,8 @@
 
 exports[`Layout rendering renders layout 1`] = `
 <VerticalNav
+  activeMenu="Monitor"
+  activePath="/Monitor/"
   blurDelay={700}
   blurDisabled={false}
   changeActiveMenu={[Function]}

webpack/assets/javascripts/react_app/components/Layout/__tests__/__snapshots__/LayoutHelper.test.js.snap

@@ -139,7 +139,7 @@ Array [
 ]
 `;
 
-exports[`LayoutHelper should getActiveOnBack(Monitor) 1`] = `
+exports[`LayoutHelper should getActive(Monitor) 1`] = `
 Object {
   "title": "Monitor",
 }

webpack/assets/javascripts/react_app/components/Layout/__tests__/__snapshots__/LayoutSelectors.test.js.snap

@@ -1,6 +1,93 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Layout Selectors should return PF-React Compatible items 1`] = `
+exports[`Layout selectors should return Layout 1`] = `
+Object {
+  "activeMenu": "Hosts",
+  "currentLocation": Object {
+    "title": "loc1",
+  },
+  "currentOrganization": Object {
+    "title": "org1",
+  },
+  "isLoading": true,
+  "items": Array [
+    Object {
+      "active": false,
+      "children": Array [
+        Object {
+          "name": "Dashboard",
+          "type": "item",
+          "url": "/",
+        },
+        Object {
+          "name": "Facts",
+          "type": "item",
+          "url": "/fact_values",
+        },
+      ],
+      "icon": "fa fa-tachometer",
+      "name": "Monitor",
+      "type": "sub_menu",
+    },
+    Object {
+      "active": false,
+      "children": Array [
+        Object {
+          "name": "All Hosts",
+          "type": "item",
+          "url": "/hosts/new",
+        },
+        Object {
+          "name": "Architectures",
+          "type": "item",
+          "url": "/architectures",
+        },
+      ],
+      "icon": "fa fa-server",
+      "name": "Hosts",
+      "type": "sub_menu",
+    },
+    Object {
+      "active": false,
+      "children": Array [
+        Object {
+          "name": "Environments",
+          "type": "item",
+          "url": "/environments",
+        },
+        Object {
+          "name": "Architectures",
+          "type": "item",
+          "url": "/architectures",
+        },
+      ],
+      "icon": "fa fa-wrench",
+      "name": "User",
+      "type": "sub_menu",
+    },
+    Object {
+      "active": false,
+      "children": Array [
+        Object {
+          "name": "Domains",
+          "type": "item",
+          "url": "/domains",
+        },
+        Object {
+          "name": "Realms",
+          "type": "item",
+          "url": "/realms",
+        },
+      ],
+      "icon": "pficon pficon-network",
+      "name": "Infrastructure",
+      "type": "sub_menu",
+    },
+  ],
+}
+`;
+
+exports[`Layout selectors should return PF-React Compatible items 1`] = `
 Array [
   Object {
     "className": undefined,
@@ -101,4 +188,10 @@ Array [
 ]
 `;
 
-exports[`Layout Selectors should return empty array 1`] = `Array []`;
+exports[`Layout selectors should return empty array of items 1`] = `Array []`;
+
+exports[`Layout selectors should return isLoading from selector 1`] = `true`;
+
+exports[`Layout selectors should return location from selector 1`] = `"loc1"`;
+
+exports[`Layout selectors should return organization from selector 1`] = `"org1"`;