[ui-storagebrowser] updates browser url while navigating (#3919)

* [ui-storagebrowser] updates browser url while navigating

* adds condition to change url only when they are different

Author: ramprasadagarwal

desktop/core/src/desktop/js/apps/storageBrowser/StorageBrowserTab/StorageBrowserTab.tsx

@@ -14,7 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import { Spin } from 'antd';
 
 import { i18nReact } from '../../../utils/i18nReact';
@@ -28,6 +28,7 @@ import useLoadData from '../../../utils/hooks/useLoadData';
 
 import './StorageBrowserTab.scss';
 import StorageFilePage from '../StorageFilePage/StorageFilePage';
+import changeURL from '../../../utils/url/changeURL';
 
 interface StorageBrowserTabProps {
   homeDir: string;
@@ -39,8 +40,9 @@ const defaultProps = {
 };
 
 const StorageBrowserTab = ({ homeDir, testId }: StorageBrowserTabProps): JSX.Element => {
-  const [filePath, setFilePath] = useState<string>(homeDir);
-  const fileName = filePath?.split('/')?.pop() ?? '';
+  const [urlPathname, urlFilePath] = decodeURIComponent(window.location.pathname).split('view=');
+  const [filePath, setFilePath] = useState<string>(urlFilePath || homeDir);
+  const fileName = filePath.split('/').pop() ?? '';
 
   const { t } = i18nReact.useTranslation();
 
@@ -51,6 +53,17 @@ const StorageBrowserTab = ({ homeDir, testId }: StorageBrowserTabProps): JSX.Ele
     skip: !filePath
   });
 
+  useEffect(() => {
+    const encodedPath = `${urlPathname}view=${encodeURIComponent(filePath)}`;
+    if (filePath && urlFilePath && filePath !== urlFilePath) {
+      changeURL(encodedPath);
+    }
+    // if url path is correct but not encoded properly
+    else if (encodedPath !== window.location.pathname) {
+      changeURL(encodedPath, {}, true);
+    }
+  }, [filePath, urlPathname, urlFilePath, window.location.pathname]);
+
   return (
     <Spin spinning={loading}>
       <div className="hue-storage-browser-tab-content" data-testid={testId}>

desktop/core/src/desktop/js/utils/url/changeURL.test.ts

@@ -15,12 +15,75 @@
 // limitations under the License.
 
 import changeURL from './changeURL';
+import { hueWindow } from 'types/types';
 
-describe('changeURL.ts', () => {
-  it('should change completely the URL', () => {
-    changeURL('/banana');
+describe('changeURL', () => {
+  const baseUrl = 'https://www.gethue.com/hue';
+  const mockPushState = jest.fn();
+  const mockReplaceState = jest.fn();
 
-    expect(window.location.pathname).toEqual('/banana');
-    changeURL('/jasmine');
+  beforeAll(() => {
+    global.window.history.pushState = mockPushState;
+    global.window.history.replaceState = mockReplaceState;
+    (global.window as hueWindow).HUE_BASE_URL = baseUrl;
+  });
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('should append query params to the URL with pushState', () => {
+    const newURL = '/new/path';
+    const params = { key1: 'value1', key2: 2 };
+
+    changeURL(newURL, params);
+
+    expect(mockReplaceState).not.toHaveBeenCalled();
+    expect(mockPushState).toHaveBeenCalledWith(null, '', `${baseUrl}/new/path?key1=value1&key2=2`);
+  });
+
+  it('should append hash to the URL with pushState', () => {
+    const newURL = '/new/path#section';
+    const params = { key1: 'value1' };
+
+    changeURL(newURL, params);
+
+    expect(mockReplaceState).not.toHaveBeenCalled();
+    expect(mockPushState).toHaveBeenCalledWith(null, '', `${baseUrl}/new/path?key1=value1#section`);
+  });
+
+  it('should handle replaceState when isReplace is true', () => {
+    const newURL = '/new/path';
+    const params = { key1: 'value1' };
+
+    changeURL(newURL, params, true);
+
+    expect(mockPushState).not.toHaveBeenCalled();
+    expect(mockReplaceState).toHaveBeenCalledWith(null, '', `${baseUrl}/new/path?key1=value1`);
+  });
+
+  it('should not add the base URL if the newURL already starts with the HUE_BASE_URL', () => {
+    const newURL = `${baseUrl}/new/path`;
+
+    changeURL(newURL, { key1: 'value1' });
+
+    expect(mockReplaceState).not.toHaveBeenCalled();
+    expect(mockPushState).toHaveBeenCalledWith(null, '', `${baseUrl}/new/path?key1=value1`);
+  });
+
+  it('should handle the absence of params correctly', () => {
+    const newURL = '/new/path#section';
+
+    changeURL(newURL);
+
+    expect(mockReplaceState).not.toHaveBeenCalled();
+    expect(mockPushState).toHaveBeenCalledWith(null, '', `${baseUrl}/new/path#section`);
+  });
+
+  it('should not change the URL if the new URL is the same as the current URL with hash', () => {
+    changeURL(baseUrl, {});
+
+    expect(mockReplaceState).not.toHaveBeenCalled();
+    expect(mockPushState).not.toHaveBeenCalled();
   });
 });

desktop/core/src/desktop/js/utils/url/changeURL.ts

@@ -16,36 +16,44 @@
 
 import { hueWindow } from 'types/types';
 
-const changeURL = (newURL: string, params?: Record<string, string | number | boolean>): void => {
+const changeURL = (
+  newURL: string,
+  params?: Record<string, string | number | boolean>,
+  isReplace?: boolean
+): void => {
   let extraSearch = '';
   if (params) {
-    const newSearchKeys = Object.keys(params);
-    if (newSearchKeys.length) {
-      while (newSearchKeys.length) {
-        const newKey = newSearchKeys.pop() || '';
-        extraSearch += newKey + '=' + params[newKey];
-        if (newSearchKeys.length) {
-          extraSearch += '&';
-        }
-      }
-    }
+    const paramsToString = Object.fromEntries(
+      Object.entries(params).map(([key, value]) => [key, String(value)])
+    );
+    extraSearch += new URLSearchParams(paramsToString).toString();
   }
 
   const hashSplit = newURL.split('#');
   const hueBaseUrl = (<hueWindow>window).HUE_BASE_URL;
   const base =
     hueBaseUrl && hashSplit[0].length && hashSplit[0].indexOf(hueBaseUrl) !== 0 ? hueBaseUrl : '';
-  let url = base + hashSplit[0];
+  let newUrl = base + hashSplit[0];
   if (extraSearch) {
-    url += (url.indexOf('?') === -1 ? '?' : '&') + extraSearch;
+    newUrl += (newUrl.indexOf('?') === -1 ? '?' : '&') + extraSearch;
   }
   if (hashSplit.length > 1) {
     //the foldername may contain # , so create substring ignoring first #
-    url += '#' + newURL.substring(newURL.indexOf('#') + 1);
+    newUrl += '#' + newURL.substring(newURL.indexOf('#') + 1);
   } else if (window.location.hash) {
-    url += window.location.hash;
+    newUrl += window.location.hash;
+  }
+
+  if (window.location.href === newUrl) {
+    // If the URLs are the same, do nothing
+    return;
+  }
+
+  if (isReplace) {
+    window.history.replaceState(null, '', newUrl);
+  } else {
+    window.history.pushState(null, '', newUrl);
   }
-  window.history.pushState(null, '', url);
 };
 
 export default changeURL;