[Security Solution] Fix missing hash in sync to url (elastic#166847)

## Summary This PR fixes the root cause for elastic#166686 and elastic#166774 @angorayc @machadoum ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
watson · Sep 22, 2023 · d52c5a1 · d52c5a1
1 parent 3ff82f2
commit d52c5a1
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 10 deletions.
diff --git a/packages/kbn-url-state/index.test.ts b/packages/kbn-url-state/index.test.ts
@@ -23,6 +23,7 @@ describe('useSyncToUrl', () => {
     window.location = {
       ...originalLocation,
       search: '',
+      hash: '',
     };
     window.history = {
       ...originalHistory,
@@ -65,9 +66,30 @@ describe('useSyncToUrl', () => {
     );
   });
 
+  it('should should not alter the location hash', () => {
+    const key = 'testKey';
+    const valueToSerialize = { test: 'value' };
+    window.location.hash = '#should_be_there';
+
+    const { result } = renderHook(() => useSyncToUrl(key, jest.fn()));
+
+    act(() => {
+      result.current(valueToSerialize);
+    });
+
+    expect(window.history.replaceState).toHaveBeenCalledWith(
+      { path: expect.any(String) },
+      '',
+      '/#should_be_there?testKey=%28test%3Avalue%29'
+    );
+  });
+
   it('should clear the value from the query string on unmount', () => {
     const key = 'testKey';
 
+    // Location should have a key to clear
+    window.location.search = `?${key}=${encode({ test: 'value' })}`;
+
     const { unmount } = renderHook(() => useSyncToUrl(key, jest.fn()));
 
     act(() => {
@@ -85,17 +107,16 @@ describe('useSyncToUrl', () => {
     const key = 'testKey';
     const restore = jest.fn();
 
+    // Location should have a key to clear
+    window.location.search = `?${key}=${encode({ test: 'value' })}`;
+
     renderHook(() => useSyncToUrl(key, restore, true));
 
     act(() => {
       window.dispatchEvent(new Event('popstate'));
     });
 
     expect(window.history.replaceState).toHaveBeenCalledTimes(1);
-    expect(window.history.replaceState).toHaveBeenCalledWith(
-      { path: expect.any(String) },
-      '',
-      '/?'
-    );
+    expect(window.history.replaceState).toHaveBeenCalledWith({ path: expect.any(String) }, '', '/');
   });
 });
diff --git a/packages/kbn-url-state/use_sync_to_url.ts b/packages/kbn-url-state/use_sync_to_url.ts
@@ -55,10 +55,15 @@ export const useSyncToUrl = <TValueToSerialize>(
         searchParams.delete(key);
       }
 
-      const newSearch = searchParams.toString();
+      const stringifiedSearchParams = searchParams.toString();
+      const newSearch = stringifiedSearchParams.length > 0 ? `?${stringifiedSearchParams}` : '';
+
+      if (window.location.search === newSearch) {
+        return;
+      }
 
       // Update query string without unnecessary re-render
-      const newUrl = `${window.location.pathname}?${newSearch}`;
+      const newUrl = `${window.location.pathname}${window.location.hash}${newSearch}`;
       window.history.replaceState({ path: newUrl }, '', newUrl);
     },
     [key]

diff --git a/...ugins/security_solution/public/flyout/shared/hooks/url/use_sync_flyout_state_with_url.tsx b/...ugins/security_solution/public/flyout/shared/hooks/url/use_sync_flyout_state_with_url.tsx
@@ -22,9 +22,18 @@ type FlyoutState = Parameters<ExpandableFlyoutApi['openFlyout']>[0];
 export const useSyncFlyoutStateWithUrl = () => {
   const flyoutApi = useRef<ExpandableFlyoutApi>(null);
 
-  const syncStateToUrl = useSyncToUrl<FlyoutState>(FLYOUT_URL_PARAM, (data) => {
-    flyoutApi.current?.openFlyout(data);
-  });
+  const handleRestoreFlyout = useCallback(
+    (state?: FlyoutState) => {
+      if (!state) {
+        return;
+      }
+
+      flyoutApi.current?.openFlyout(state);
+    },
+    [flyoutApi]
+  );
+
+  const syncStateToUrl = useSyncToUrl<FlyoutState>(FLYOUT_URL_PARAM, handleRestoreFlyout);
 
   // This should be bound to flyout changed and closed events.
   // When flyout is closed, url state is cleared