DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource #2312

khelif96 · 2024-03-21T18:07:29Z

Description

This should not break any existing pages or urls and only serves to consolidate the slug names we use internally in the Spruce application.

Create Slugs for each existing hardcoded slug.
Update generic slugs so they correspond with the associated resource.
Update the usage of hardcoded slugs with their enum slug equivalent

Testing

Existing tests should suffice to catch any edge cases that could have been missed.

SupaJoon

I think we should keep the explicit typing in useParams without the use of slug variables. When a slug changes, it will be super easy to see which useParams are affected because of type errors. These kinds of changes wouldn't have been easily possible before slugs.

SupaJoon · 2024-03-22T15:19:10Z

src/hooks/useLegacyUIURL/index.ts

+ * @param matchedPathParams - Params object from react-router-dom's matchPath
+ * @returns string | undefined
+ */
+const slugToId = (matchedPathParams: Params): string | undefined => {


This code could potentially save tab, taskName, variantName as id based on the order of Object.keys(slugs). If you move forward with this abstraction you should create a separate set of slugs that represent ids. Since the URLs in the legacy UI are done coded and complete, I think this abstraction is overkill.

Good catch on this I updated it so that non id slugs are no longer included in the check. I preferred this approach over the previous implementation of just casing on every single slug.

minnakt · 2024-03-22T15:00:25Z

src/pages/Version.tsx

@@ -38,7 +38,7 @@ import { NameChangeModal } from "./version/NameChangeModal";
 // docs/decisions/2023-12-13_version_page_logic.md
 export const VersionPage: React.FC = () => {
  const spruceConfig = useSpruceConfig();
-  const { id } = useParams<{ id: string }>();
+  const { [slugs.versionId]: id } = useParams();


Suggested change

const { [slugs.versionId]: id } = useParams();

const { [slugs.versionId]: versionId } = useParams();

minnakt · 2024-03-22T15:07:01Z

src/pages/host/index.tsx

@@ -33,7 +34,7 @@ const { getLimitFromSearch, getPageFromSearch } = url;

 const Host: React.FC = () => {
  const dispatchToast = useToastContext();
-  const { id } = useParams<{ id: string }>();
+  const { [slugs.hostId]: id } = useParams();


Suggested change

const { [slugs.hostId]: id } = useParams();

const { [slugs.hostId]: hostId } = useParams();

minnakt · 2024-03-22T15:30:34Z

src/pages/projectSettings/index.tsx

-  }>();
+  const { [slugs.projectIdentifier]: identifier, [slugs.tab]: tab } =
+    useParams<{
+      [slugs.projectIdentifier]: string | null;


just curious if | null is required here:

Suggested change

[slugs.projectIdentifier]: string | null;

[slugs.projectIdentifier]: string;

minnakt · 2024-03-22T15:32:43Z

src/pages/taskQueue/index.tsx

@@ -27,7 +27,7 @@ import TaskQueueTable from "./TaskQueueTable";
 const TaskQueue = () => {
  const taskQueueAnalytics = useTaskQueueAnalytics();

-  const { distro } = useParams<{ distro: string }>();
+  const { [slugs.distro]: distro } = useParams();


do you think we should just make TaskQueue use the distroId slug?

Good idea we can consolidate them

src/pages/version/VersionTabs.tsx

minnakt · 2024-03-22T15:40:44Z

src/pages/preferences/index.tsx

 import { usePageTitle } from "hooks";
 import { PreferencesTabs } from "pages/preferences/PreferencesTabs";

 const Preferences: React.FC = () => {
  usePageTitle("Preferences");
-  const { tab } = useParams<{ tab: string }>();
+  const { [slugs.tab]: tab } = useParams();


same comment

minnakt · 2024-03-22T15:41:42Z

src/pages/spawn/index.tsx

 import { SpawnHost } from "./SpawnHost";
 import { SpawnVolume } from "./SpawnVolume";

 const Spawn: React.FC = () => {
-  const { tab } = useParams<{ tab: string }>();
+  const { [slugs.tab]: tab } = useParams();


can add more type specificity with <{ tab: SpawnTab }>

minnakt · 2024-03-22T15:45:41Z

src/pages/version/VersionTabs.tsx

@@ -109,9 +109,9 @@ export const VersionTabs: React.FC<Props> = ({
      taskCount,
      childPatches,
      numFailedChildPatches,
-      patchId: id,
+      patchId: versionId,


changing this parameter to be called versionId might make this a bit less confusing to read

minnakt · 2024-03-22T15:48:58Z

src/constants/routes.ts

  hosts: paths.hosts,
-  jobLogs: `${paths.jobLogs}/:buildId`,
+  jobLogs: `${paths.jobLogs}/:${slugs.buildId}`,
+  jobLogsGroup: `${paths.jobLogs}/:${slugs.taskId}`,


where does this one come from? is taskId the correct parameter rather than something like groupId?

Good catch this is from my job logs pr I must have mixed up the commits

minnakt · 2024-03-22T15:49:21Z

src/constants/routes.ts

  login: paths.login,
  myPatches: `${paths.user}/${PageNames.Patches}`,
-  patch: `${paths.patch}/:id`,
+  patch: `${paths.patch}/:${slugs.versionId}`,


should it be patchId? (not sure, just guessing since it seems odd)

Woah this page should not exist it looks like we have 2 version pages with 2 urls

(i saw you tried deleting it but it didn't work, just curious what you found out this page is for)

This reverts commit 4c20326.

SupaJoon · 2024-03-27T15:16:52Z

src/hooks/useConfigurePatch.ts

@@ -152,7 +152,7 @@ export const useConfigurePatch = (
 ): HookResult => {
  const navigate = useNavigate();
  const location = useLocation();
-  const { tab } = useParams<{ tab: PatchTab | null }>();
+  const { [slugs.tab]: tab } = useParams<{ [slugs.tab]: PatchTab | null }>();


tab will be undefined if the query param doesn't exist

minnakt · 2024-03-27T17:47:57Z

src/pages/configurePatch/configurePatchCore/useConfigurePatch/index.ts

@@ -125,7 +125,7 @@ interface HookResult extends ConfigurePatchState {
 const useConfigurePatch = (patch: ConfigurePatchQuery["patch"]): HookResult => {
  const navigate = useNavigate();
  const location = useLocation();
-  const { tab } = useParams<{ tab: PatchTab | null }>();
+  const { [slugs.tab]: tab } = useParams();


minnakt · 2024-03-27T17:49:04Z

src/constants/routes.ts

-  taskQueue: paths.taskQueue,
+  task: `${paths.task}/:${slugs.taskId}`,
+  taskHistory: `${paths.taskHistory}/:${slugs.projectIdentifier}/:${slugs.taskName}`,
+  taskQueue: `${paths.taskQueue}`,


Suggested change

taskQueue: `${paths.taskQueue}`,

taskQueue: paths.taskQueue,

minnakt · 2024-03-27T17:49:40Z

src/constants/routes.ts

  login: paths.login,
  myPatches: `${paths.user}/${PageNames.Patches}`,
-  patch: `${paths.patch}/:id`,
+  patch: `${paths.patch}/:${slugs.versionId}`,


(i saw you tried deleting it but it didn't work, just curious what you found out this page is for)

minnakt · 2024-03-28T13:22:12Z

Closing this PR since it has already been moved to the UI repo

khelif96 added 2 commits March 21, 2024 13:59

Create slugs for all routes

c2ee82d

Replace all existing slugs with thier corresponding enum

7c020ab

khelif96 changed the title ~~DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate resource name to corresponding slug~~ DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource Mar 21, 2024

khelif96 added 17 commits March 21, 2024 14:46

Add new slugs

703f4d1

Merge branch 'main' into DEVPROD-5762

535ff63

Type using enum

9b3356e

Missed a spot

e01b777

Migrate configure page

56532a7

Migrate container page

24d5390

Migrate hosts page

ab1c7a3

Migrate version pages

0187d26

Migrate task pages

d762676

Migrate userpatches

d488084

Fix podid references

3f72dcb

Update useLegacyUIURL to rely on slugs map

fe5ef97

Update build variant card param usage

8a20313

Use correct slug for user patches redirect

9dc5125

Remove references to id slug

18f81b9

Add useLegacyUIURL hook and its test to the same folder

4ea1dd6

Fix legacy url logic for commits page

13d045e

khelif96 marked this pull request as ready for review March 22, 2024 14:09

khelif96 requested a review from a team March 22, 2024 14:22

SupaJoon reviewed Mar 22, 2024

View reviewed changes

Seperate out slugs that are ids

d0e7615

minnakt suggested changes Mar 22, 2024

View reviewed changes

khelif96 added 5 commits March 25, 2024 10:55

Merge branch 'main' into DEVPROD-5762

1a039b8

Bump types

2ab63bb

Rename id to versionId

101a404

Refactor hostId variable in Host page

a29d437

Update distro slug to distroid

b4f3712

khelif96 added 5 commits March 25, 2024 13:49

Revert change on tabs for id field

dcb0498

Type tab field in useParams

0d59763

Patch id to version id

7145f6f

Remove not yet necessary job logs page route

69f77e9

Remove unused duplicate patch page

4c20326

khelif96 requested review from SupaJoon and minnakt March 25, 2024 21:36

khelif96 added 3 commits March 26, 2024 15:16

Update slugs in content tree

3a2f1d8

Revert "Remove unused duplicate patch page"

8cb069c

This reverts commit 4c20326.

Update tab

8501eb2

SupaJoon approved these changes Mar 27, 2024

View reviewed changes

minnakt approved these changes Mar 27, 2024

View reviewed changes

khelif96 added 2 commits March 27, 2024 14:51

Fix string template

3b1b449

Type tabs

6539d9a

khelif96 mentioned this pull request Mar 27, 2024

DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource evergreen-ci/ui#18

Merged

minnakt closed this Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource #2312

DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource #2312

khelif96 commented Mar 21, 2024 •

edited

Loading

SupaJoon left a comment

SupaJoon Mar 22, 2024

khelif96 Mar 25, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

khelif96 Mar 25, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

minnakt Mar 22, 2024

khelif96 Mar 25, 2024

minnakt Mar 22, 2024

khelif96 Mar 25, 2024

minnakt Mar 27, 2024

SupaJoon Mar 27, 2024

minnakt Mar 27, 2024

minnakt Mar 27, 2024 •

edited

Loading

minnakt Mar 27, 2024

minnakt commented Mar 28, 2024

	const { [slugs.versionId]: id } = useParams();
	const { [slugs.versionId]: versionId } = useParams();

	const { [slugs.hostId]: id } = useParams();
	const { [slugs.hostId]: hostId } = useParams();

	[slugs.projectIdentifier]: string \| null;
	[slugs.projectIdentifier]: string;

DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource #2312

DEVPROD-5762 Replace all hardcoded slugs with an enum slug and consolidate slug name to associated resource #2312

Conversation

khelif96 commented Mar 21, 2024 • edited Loading

Description

Testing

SupaJoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minnakt Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minnakt commented Mar 28, 2024

khelif96 commented Mar 21, 2024 •

edited

Loading

minnakt Mar 27, 2024 •

edited

Loading