From 20df1c9611f1feb393ec4bb284ad316fb9e01f42 Mon Sep 17 00:00:00 2001
From: Krithika Sundararajan <krithika369@live.com>
Date: Wed, 26 Oct 2022 11:15:13 +0800
Subject: [PATCH] Bugfix: Navigation with react-router-dom (#48)

* Navigation bugfixes

* Do not set homepage in Header

Co-authored-by: Krithika Sundararajan <krithika.sundararajan@go-jek.com>
---
 ui/src/AppRoutes.js                             | 16 +++++++++++-----
 ui/src/Home.js                                  | 11 +++++++----
 ui/src/PrivateLayout.js                         |  8 ++++----
 ui/src/experiments/ExperimentsLandingPage.js    |  2 +-
 ui/src/experiments/list/ListExperimentsTable.js |  6 +++++-
 5 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/ui/src/AppRoutes.js b/ui/src/AppRoutes.js
index 53f04430..c1739727 100644
--- a/ui/src/AppRoutes.js
+++ b/ui/src/AppRoutes.js
@@ -7,13 +7,19 @@ import Home from "Home";
 
 const App = () => {
   const { appConfig } = useConfig();
+
   return (
     <Routes>
-      <Route path={appConfig.homepage} />
-      <Route index element={<Home />} />
-      <Route path="projects/:projectId">
-        <Route index element={<Navigate to="experiments" replace={true} />} />
-        <Route path="experiments/*" element={<ExperimentsLandingPage />} />
+      {/* We need this redirection because XP is not a recognized MLP app and so PrivateLayout 
+      should not redirect to /xp directly on project change. */}
+      <Route path="projects/:projectId/*" element={<Navigate to={appConfig.homepage} replace={true} />} />
+      {/* ALL ROUTES */}
+      <Route path={appConfig.homepage}>
+        <Route index element={<Home />} />
+        <Route path="projects/:projectId">
+          <Route index element={<Navigate to="experiments" replace={true} />} />
+          <Route path="experiments/*" element={<ExperimentsLandingPage />} />
+        </Route>
       </Route>
       {/* DEFAULT */}
       <Route path="*" element={<Navigate to="/pages/404" replace={true} />} />
diff --git a/ui/src/Home.js b/ui/src/Home.js
index 052eed5c..889144e9 100644
--- a/ui/src/Home.js
+++ b/ui/src/Home.js
@@ -1,13 +1,14 @@
-import React, { Fragment } from "react";
-
+import React, { Fragment, useContext } from "react";
 import { EuiPageTemplate } from "@elastic/eui";
-
+import { ProjectsContext } from "@gojek/mlp-ui";
+import { Navigate } from "react-router-dom";
 import { useConfig } from "config";
 
 const Home = () => {
   const { appConfig } = useConfig();
+  const { currentProject } = useContext(ProjectsContext);
 
-  return (
+  return !currentProject ? (
     <EuiPageTemplate>
       <EuiPageTemplate.EmptyPrompt
         iconType={appConfig.appIcon}
@@ -19,6 +20,8 @@ const Home = () => {
         }
       />
     </EuiPageTemplate>
+  ) : (
+    <Navigate to={`projects/${currentProject.id}`} replace={true} />
   );
 };
 
diff --git a/ui/src/PrivateLayout.js b/ui/src/PrivateLayout.js
index 467aac61..ed5c4abb 100644
--- a/ui/src/PrivateLayout.js
+++ b/ui/src/PrivateLayout.js
@@ -22,14 +22,14 @@ export const PrivateLayout = () => {
       <ApplicationsContextProvider>
         <ProjectsContextProvider>
           <ApplicationsContext.Consumer>
-            {({ currentApp }) => (
+            {() => (
               <Header
-                homepage={appConfig.homepage}
                 onProjectSelect={pId =>
-                  navigate(urlJoin(currentApp?.href, "projects", pId, "experiments"))
+                  navigate(urlJoin("projects", pId))
                 }
                 docLinks={appConfig.docsUrl}
-              />)}
+              />
+            )}
           </ApplicationsContext.Consumer>
           <Outlet />
         </ProjectsContextProvider>
diff --git a/ui/src/experiments/ExperimentsLandingPage.js b/ui/src/experiments/ExperimentsLandingPage.js
index f2dd3d90..673a9a86 100644
--- a/ui/src/experiments/ExperimentsLandingPage.js
+++ b/ui/src/experiments/ExperimentsLandingPage.js
@@ -68,7 +68,7 @@ const ExperimentsLandingPage = () => {
         which results in incorrect /experiments/experiments prefix.
          */}
           <Route
-            path="experiments"
+            path="experiments/*"
             element={<Navigate to={location.pathname.replace("/experiments/experiments", "/experiments")}
               replace={true} />} />
         </Routes>
diff --git a/ui/src/experiments/list/ListExperimentsTable.js b/ui/src/experiments/list/ListExperimentsTable.js
index ee47d02d..461626c1 100644
--- a/ui/src/experiments/list/ListExperimentsTable.js
+++ b/ui/src/experiments/list/ListExperimentsTable.js
@@ -1,6 +1,8 @@
 import React from "react";
 
 import { EuiHealth, EuiLink, EuiText } from "@elastic/eui";
+import { useLocation } from "react-router-dom";
+import urlJoin from "proper-url-join";
 
 import { useConfig } from "config";
 import { BasicTable } from "components/table/BasicTable";
@@ -17,6 +19,7 @@ const ListExperimentsTable = ({
   onRowClick,
 }) => {
   const { appConfig } = useConfig();
+  const location = useLocation();
   const tableColumns = [
     {
       field: "id",
@@ -88,11 +91,12 @@ const ListExperimentsTable = ({
       width: "5%",
       render: (item) => {
         return (
+          // We need to do urlJoin to correctly handle trailing slash when used as a remote component
           <EuiLink
             onClick={(e) => {
               e.stopPropagation();
             }}
-            href={item.id}
+            href={urlJoin(location.pathname, item.id)}
             target="_blank"
           />
         );