Merge pull request #13869 from keithjgrant/persistent-filter-race-condition

Rework PersistentFilter to avoid double API call

awx/ui/src/components/PersistentFilters/PersistentFilters.js

@@ -1,25 +1,9 @@
 import { useEffect } from 'react';
-import { useLocation, useHistory } from 'react-router';
+import { useLocation } from 'react-router';
 import { PERSISTENT_FILTER_KEY } from '../../constants';
 
 export default function PersistentFilters({ pageKey, children }) {
   const location = useLocation();
-  const history = useHistory();
-
-  useEffect(() => {
-    if (!location.search.includes('restoreFilters=true')) {
-      return;
-    }
-
-    const filterString = sessionStorage.getItem(PERSISTENT_FILTER_KEY);
-    const filter = filterString ? JSON.parse(filterString) : { qs: '' };
-
-    if (filter.pageKey === pageKey) {
-      history.replace(`${location.pathname}${filter.qs}`);
-    } else {
-      history.replace(location.pathname);
-    }
-  }, [history, location, pageKey]);
 
   useEffect(() => {
     const filter = {
@@ -31,3 +15,13 @@ export default function PersistentFilters({ pageKey, children }) {
 
   return children;
 }
+
+export function getPersistentFilters(key) {
+  const filterString = sessionStorage.getItem(PERSISTENT_FILTER_KEY);
+  const filter = filterString ? JSON.parse(filterString) : { qs: '' };
+
+  if (filter.pageKey === key) {
+    return filter.qs;
+  }
+  return '';
+}

awx/ui/src/components/PersistentFilters/PersistentFilters.test.js

@@ -24,28 +24,6 @@ describe('PersistentFilters', () => {
     });
   });
 
-  test('should restore filters from sessionStorage', () => {
-    expect(
-      sessionStorage.setItem(
-        KEY,
-        JSON.stringify({
-          pageKey: 'templates',
-          qs: '?page=2&name=foo',
-        })
-      )
-    );
-    const history = createMemoryHistory({
-      initialEntries: ['/templates?restoreFilters=true'],
-    });
-    render(
-      <Router history={history}>
-        <PersistentFilters pageKey="templates">test</PersistentFilters>
-      </Router>
-    );
-
-    expect(history.location.search).toEqual('?page=2&name=foo');
-  });
-
   test('should not restore filters without restoreFilters query param', () => {
     expect(
       sessionStorage.setItem(
@@ -68,28 +46,6 @@ describe('PersistentFilters', () => {
     expect(history.location.search).toEqual('');
   });
 
-  test("should not restore filters if page key doesn't match", () => {
-    expect(
-      sessionStorage.setItem(
-        KEY,
-        JSON.stringify({
-          pageKey: 'projects',
-          qs: '?page=2&name=foo',
-        })
-      )
-    );
-    const history = createMemoryHistory({
-      initialEntries: ['/templates?restoreFilters=true'],
-    });
-    render(
-      <Router history={history}>
-        <PersistentFilters pageKey="templates">test</PersistentFilters>
-      </Router>
-    );
-
-    expect(history.location.search).toEqual('');
-  });
-
   test('should update stored filters when qs changes', async () => {
     const history = createMemoryHistory({
       initialEntries: ['/templates'],

awx/ui/src/components/PersistentFilters/index.js

@@ -1 +1,2 @@
 export { default } from './PersistentFilters';
+export { getPersistentFilters } from './PersistentFilters';

awx/ui/src/components/RoutedTabs/RoutedTabs.js

@@ -7,6 +7,7 @@ import {
 } from '@patternfly/react-core';
 import { useHistory, useLocation } from 'react-router-dom';
 import styled from 'styled-components';
+import { getPersistentFilters } from 'components/PersistentFilters';
 
 const Tabs = styled(PFTabs)`
   & > ul {
@@ -40,8 +41,8 @@ function RoutedTabs({ tabsArray }) {
     const match = tabsArray.find((tab) => tab.id === eventKey);
     if (match) {
       event.preventDefault();
-      const link = match.isBackButton
+      const link = match.persistentFilterKey
-        ? `${match.link}?restoreFilters=true`
+        ? `${match.link}${getPersistentFilters(match.persistentFilterKey)}`
         : match.link;
       history.push(link);
     }

awx/ui/src/screens/Application/Application/Application.js

@@ -74,7 +74,7 @@ function Application({ setBreadcrumb }) {
       ),
       link: '/applications',
       id: 0,
-      isBackButton: true,
+      persistentFilterKey: 'applications',
     },
     { name: t`Details`, link: `/applications/${id}/details`, id: 1 },
     { name: t`Tokens`, link: `/applications/${id}/tokens`, id: 2 },

awx/ui/src/screens/Credential/Credential.js

@@ -82,7 +82,7 @@ function Credential({ setBreadcrumb }) {
       ),
       link: `/credentials`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'credentials',
     },
     { name: t`Details`, link: `/credentials/${id}/details`, id: 0 },
     {

awx/ui/src/screens/CredentialType/CredentialType.js

@@ -57,7 +57,7 @@ function CredentialType({ setBreadcrumb }) {
       ),
       link: '/credential_types',
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'credentialTypes',
     },
     {
       name: t`Details`,

awx/ui/src/screens/ExecutionEnvironment/ExecutionEnvironment.js

@@ -59,7 +59,7 @@ function ExecutionEnvironment({ setBreadcrumb }) {
       ),
       link: '/execution_environments',
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'executionEnvironments',
     },
     {
       name: t`Details`,

awx/ui/src/screens/Host/Host.js

@@ -52,7 +52,7 @@ function Host({ setBreadcrumb }) {
       ),
       link: `/hosts`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'hosts',
     },
     {
       name: t`Details`,

awx/ui/src/screens/InstanceGroup/InstanceGroup.js

@@ -63,7 +63,7 @@ function InstanceGroup({ setBreadcrumb }) {
       ),
       link: '/instance_groups',
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'instanceGroups',
     },
     {
       name: t`Details`,

awx/ui/src/screens/Instances/Instance.js

@@ -24,7 +24,7 @@ function Instance({ setBreadcrumb }) {
       ),
       link: `/instances`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'instances',
     },
     { name: t`Details`, link: `${match.url}/details`, id: 0 },
   ];

awx/ui/src/screens/Inventory/Inventory.js

@@ -60,7 +60,7 @@ function Inventory({ setBreadcrumb }) {
       ),
       link: `/inventories`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'inventories',
     },
     { name: t`Details`, link: `${match.url}/details`, id: 0 },
     { name: t`Access`, link: `${match.url}/access`, id: 1 },

awx/ui/src/screens/Job/Job.js

@@ -124,7 +124,7 @@ function Job({ setBreadcrumb }) {
         </>
       ),
       link: `/jobs`,
-      isBackButton: true,
+      persistentFilterKey: 'jobs',
       id: 99,
     },
     { name: t`Details`, link: `${match.url}/details`, id: 0 },

awx/ui/src/screens/ManagementJob/ManagementJob.js

@@ -98,7 +98,7 @@ function ManagementJob({ setBreadcrumb }) {
           {t`Back to management jobs`}
         </>
       ),
-      isBackButton: true,
+      persistentFilterKey: 'managementJobs',
     },
   ];
 

awx/ui/src/screens/NotificationTemplate/NotificationTemplate.js

@@ -78,7 +78,7 @@ function NotificationTemplate({ setBreadcrumb }) {
       ),
       link: `/notification_templates`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'notificationTemplates',
     },
     {
       name: t`Details`,

awx/ui/src/screens/Organization/Organization.js

@@ -118,7 +118,7 @@ function Organization({ setBreadcrumb, me }) {
       ),
       link: `/organizations`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'organizations',
     },
     { name: t`Details`, link: `${match.url}/details`, id: 0 },
     { name: t`Access`, link: `${match.url}/access`, id: 1 },

awx/ui/src/screens/Project/Project.js

@@ -99,7 +99,7 @@ function Project({ setBreadcrumb }) {
       ),
       link: `/projects`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'projects',
     },
     { name: t`Details`, link: `/projects/${id}/details` },
     { name: t`Access`, link: `/projects/${id}/access` },

awx/ui/src/screens/Team/Team.js

@@ -52,7 +52,7 @@ function Team({ setBreadcrumb }) {
       ),
       link: `/teams`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'teams',
     },
     { name: t`Details`, link: `/teams/${id}/details`, id: 0 },
     { name: t`Access`, link: `/teams/${id}/access`, id: 1 },

awx/ui/src/screens/Template/Template.js

@@ -129,7 +129,7 @@ function Template({ setBreadcrumb }) {
         </>
       ),
       link: `/templates`,
-      isBackButton: true,
+      persistentFilterKey: 'templates',
       id: 99,
     },
     { name: t`Details`, link: `${match.url}/details` },

awx/ui/src/screens/Template/WorkflowJobTemplate.js

@@ -111,7 +111,7 @@ function WorkflowJobTemplate({ setBreadcrumb }) {
         </>
       ),
       link: `/templates`,
-      isBackButton: true,
+      persistentFilterKey: 'templates',
       id: 99,
     },
     { name: t`Details`, link: `${match.url}/details` },

awx/ui/src/screens/User/User.js

@@ -59,7 +59,7 @@ function User({ setBreadcrumb, me }) {
       ),
       link: `/users`,
       id: 99,
-      isBackButton: true,
+      persistentFilterKey: 'users',
     },
     { name: t`Details`, link: `${match.url}/details`, id: 0 },
     {

awx/ui/src/screens/WorkflowApproval/WorkflowApproval.js

@@ -70,7 +70,7 @@ function WorkflowApproval({ setBreadcrumb }) {
         </>
       ),
       link: `/workflow_approvals`,
-      isBackButton: true,
+      persistentFilterKey: 'workflowApprovals',
       id: 99,
     },
     {