From e35b279566f185b12f0566a43b2424cc59bacd0d Mon Sep 17 00:00:00 2001
From: Luke G <11671118+lgestc@users.noreply.github.com>
Date: Fri, 22 Sep 2023 10:23:50 +0200
Subject: [PATCH] [Security Solution] Fix missing hash in sync to url (#166847)

## Summary

This PR fixes the root cause for
https://github.com/elastic/kibana/issues/166686 and
https://github.com/elastic/kibana/issues/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

(cherry picked from commit d52c5a15fdd9e3c8502fbb75378295c5c7649cc6)
---
 packages/kbn-url-state/index.test.ts          | 31 ++++++++++++++++---
 packages/kbn-url-state/use_sync_to_url.ts     |  9 ++++--
 .../url/use_sync_flyout_state_with_url.tsx    | 15 +++++++--
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/packages/kbn-url-state/index.test.ts b/packages/kbn-url-state/index.test.ts
index 33dc285e100e54..e2a85e58902f1c 100644
--- 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,6 +107,9 @@ 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(() => {
@@ -92,10 +117,6 @@ describe('useSyncToUrl', () => {
     });
 
     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
index e38f9bc688a8b1..e6f1531980f752 100644
--- 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/x-pack/plugins/security_solution/public/flyout/shared/hooks/url/use_sync_flyout_state_with_url.tsx b/x-pack/plugins/security_solution/public/flyout/shared/hooks/url/use_sync_flyout_state_with_url.tsx
index 554cf1d417994b..c78aecf44d84e9 100644
--- a/x-pack/plugins/security_solution/public/flyout/shared/hooks/url/use_sync_flyout_state_with_url.tsx
+++ b/x-pack/plugins/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