Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding hashes to system tab URLs #5535

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions clients/admin-ui/cypress/e2e/systems-plus.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,4 +460,36 @@ describe("System management with Plus features", () => {
});
});
});

describe("tab navigation", () => {
it("updates URL hash when switching tabs", () => {
cy.visit(`${SYSTEM_ROUTE}/configure/demo_analytics_system#information`);
cy.location("hash").should("eq", "#information");

cy.getByTestId("tab-Data uses").click();
cy.location("hash").should("eq", "#data-uses");

cy.getByTestId("tab-Data flow").click();
cy.location("hash").should("eq", "#data-flow");

cy.getByTestId("tab-Integrations").click();
cy.location("hash").should("eq", "#integrations");

cy.getByTestId("tab-History").click();
cy.location("hash").should("eq", "#history");
});

it("loads correct tab directly based on URL hash", () => {
// Visit page with specific hash
cy.visit(`${SYSTEM_ROUTE}/configure/demo_analytics_system#data-uses`);

// Verify correct tab is active
cy.getByTestId("tab-Data uses").should(
"have.attr",
"aria-selected",
"true",
);
cy.location("hash").should("eq", "#data-uses");
});
});
});
91 changes: 86 additions & 5 deletions clients/admin-ui/src/features/system/hooks/useSystemFormTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ import {
} from "~/features/common/hooks/useIsAnyFormDirty";
import { useSystemOrDatamapRoute } from "~/features/common/hooks/useSystemOrDatamapRoute";
import { INTEGRATION_MANAGEMENT_ROUTE } from "~/features/common/nav/v2/routes";
import { DEFAULT_TOAST_PARAMS } from "~/features/common/toast";
import {
DEFAULT_TOAST_PARAMS,
errorToastParams,
successToastParams,
} from "~/features/common/toast";
import ToastLink from "~/features/common/ToastLink";
import ConnectionForm from "~/features/datastore-connections/system_portal_config/ConnectionForm";
import { ConsentAutomationForm } from "~/features/datastore-connections/system_portal_config/ConsentAutomationForm";
Expand All @@ -31,6 +35,38 @@ import {
import SystemInformationForm from "~/features/system/SystemInformationForm";
import { SystemResponse } from "~/types/api";

const SYSTEM_TABS = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formalizing this a bit so we don't have so many magic numbers and strings floating around

INFORMATION: {
index: 0,
hash: "#information",
},
DATA_USES: {
index: 1,
hash: "#data-uses",
},
DATA_FLOW: {
index: 2,
hash: "#data-flow",
},
INTEGRATIONS: {
index: 3,
hash: "#integrations",
},
HISTORY: {
index: 4,
hash: "#history",
},
} as const;

const getTabFromHash = (hash: string) => {
const normalizedHash = hash.startsWith("#") ? hash : `#${hash}`;
return Object.values(SYSTEM_TABS).find((tab) => tab.hash === normalizedHash);
};

const getTabFromIndex = (index: number) => {
return Object.values(SYSTEM_TABS).find((tab) => tab.index === index);
};

// The toast doesn't seem to handle next links well, so use buttons with onClick
// handlers instead
const ToastMessage = ({
Expand All @@ -56,16 +92,25 @@ const ToastMessage = ({

const useSystemFormTabs = ({
isCreate,
initialTabIndex,
}: {
initialTabIndex?: number;
/** If true, then some editing features will not be enabled */
isCreate?: boolean;
}) => {
const [tabIndex, setTabIndex] = useState(initialTabIndex);
const router = useRouter();

// Get initial tab index based on URL hash
const getInitialTabIndex = (): number => {
const hash: string = router.asPath.split("#")[1];
return hash
? (getTabFromHash(hash)?.index ?? SYSTEM_TABS.INFORMATION.index)
: SYSTEM_TABS.INFORMATION.index;
};

const [tabIndex, setTabIndex] = useState(getInitialTabIndex());

const [showSaveMessage, setShowSaveMessage] = useState(false);
const { systemOrDatamapRoute } = useSystemOrDatamapRoute();
const router = useRouter();
const toast = useToast();
const dispatch = useAppDispatch();
const activeSystem = useAppSelector(selectActiveSystem) as SystemResponse;
Expand Down Expand Up @@ -145,13 +190,49 @@ const useSystemFormTabs = ({
(index: number) => {
attemptAction().then((modalConfirmed: boolean) => {
if (modalConfirmed) {
const { status } = router.query;
if (status) {
if (status === "succeeded") {
toast(successToastParams(`Integration successfully authorized.`));
} else {
toast(errorToastParams(`Failed to authorize integration.`));
}
}

// Update local state first
setTabIndex(index);

// Update URL if router is ready
if (router.isReady) {
const tab = getTabFromIndex(index);
if (tab) {
const newQuery = { ...router.query };
delete newQuery.status;

router.replace(
{
pathname: router.pathname,
query: newQuery,
hash: tab.hash,
},
undefined,
{ shallow: true },
);
}
}
}
});
},
[attemptAction],
[attemptAction, router, toast],
);

useEffect(() => {
const { status } = router.query;
if (status) {
onTabChange(SYSTEM_TABS.INTEGRATIONS.index);
}
}, [router.query, onTabChange]);

const showNewIntegrationNotice = hasPlus && dataDiscoveryAndDetection;

const tabData: TabData[] = [
Expand Down
34 changes: 2 additions & 32 deletions clients/admin-ui/src/pages/systems/configure/[id].tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AntButton as Button, Box, Text, useToast, VStack } from "fidesui";
import { AntButton as Button, Box, Text, VStack } from "fidesui";
import type { NextPage } from "next";
import { useRouter } from "next/router";
import { useEffect, useState } from "react";
import { useEffect } from "react";

import { useAppDispatch, useAppSelector } from "~/app/hooks";
import DataTabsContent from "~/features/common/DataTabsContent";
Expand All @@ -16,7 +16,6 @@ import {
SYSTEM_ROUTE,
} from "~/features/common/nav/v2/routes";
import PageHeader from "~/features/common/PageHeader";
import { errorToastParams, successToastParams } from "~/features/common/toast";
import { useGetAllDictionaryEntriesQuery } from "~/features/plus/plus.slice";
import {
setActiveSystem,
Expand All @@ -29,13 +28,9 @@ import {
import GVLNotice from "~/features/system/GVLNotice";
import useSystemFormTabs from "~/features/system/hooks/useSystemFormTabs";

const INTEGRATION_TAB_INDEX = 3; // this needs to be updated if the order of the tabs changes

const ConfigureSystem: NextPage = () => {
const toast = useToast();
const router = useRouter();
const dispatch = useAppDispatch();
const [initialTabIndex, setInitialTabIndex] = useState(0);

let systemId = "";
if (router.query.id) {
Expand Down Expand Up @@ -70,33 +65,8 @@ const ConfigureSystem: NextPage = () => {
}
}, [system, dispatch, isTCFEnabled]);

useEffect(() => {
const { status } = router.query;

if (status) {
if (status === "succeeded") {
toast(successToastParams(`Integration successfully authorized.`));
} else {
toast(errorToastParams(`Failed to authorize integration.`));
}
// create a new url without the status query param
const newQuery = { ...router.query };
delete newQuery.status;
const newUrl = {
pathname: router.pathname,
query: newQuery,
};

// replace the current history entry
router.replace(newUrl, undefined, { shallow: true });

setInitialTabIndex(INTEGRATION_TAB_INDEX);
}
}, [router, toast]);

const { tabData, tabIndex, onTabChange } = useSystemFormTabs({
isCreate: false,
initialTabIndex,
});

if ((isLoading || isDictionaryLoading) && !dictionaryError) {
Expand Down
1 change: 1 addition & 0 deletions clients/admin-ui/src/pages/systems/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const Systems: NextPage = () => {
query: {
id: system.fides_key,
},
hash: "#information",
});
},
[dispatch, router],
Expand Down
Loading