tech-story: [M3-8784] - Tanstack routing for Domains #11418

abailly-akamai · 2024-12-13T18:21:50Z

Description 📝

Pretty simple PR (relative to the complex work that needed to be done for the /volume routing refactoring) to handle the Tanstack router migration for /domains

Nothing out of the ordinary for this one, just follows the now established routing patterns (soon to be documented)

Changes 🔄

-/domains routing setup

Lazy routes consolidation
useOrderV2 & usePaginationV2
Drawer routing

⚠️ note: domain details (adding records) drawers also should be routed, but since DomainRecords is a very large class component I decided to come back and do it once it's been broken down and converted to a functional component (M3-9015 & M3-9016)

Preview 📷

No visual difference to be expected (except for loading state in drawers)

How to test 🧪

Verification steps

Confirm navigation & routed drawers at /domains
- Confirm pagination
- Confirm sorting
Confirm navigation & drawers at /domains/{id}

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

All unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

abailly-akamai · 2024-12-16T14:05:45Z

packages/manager/src/features/Betas/BetaSignup.tsx

@@ -15,7 +15,7 @@ import { useCreateAccountBetaMutation } from 'src/queries/account/betas';
 import { useBetaQuery } from 'src/queries/betas';

 export const BetaSignup = () => {
-  const betaAgreement = `# Early Adopter Testing Program
+  const betaAgreement = `### Early Adopter Testing Program


This was caught by my route test once I made it more resilient. It caught two H1 headers on the page 🎉

Nice, solid test!

abailly-akamai · 2024-12-16T14:07:46Z

packages/manager/src/hooks/useDialogData.ts

-//   paramKey: 'volumeId',
-//   queryHook: useVolumeQuery,
-//   redirectToOnNotFound: '/volumes',
-// });


left over code i had forgotten to cleanup

abailly-akamai · 2024-12-16T14:09:36Z

packages/manager/src/mocks/serverHandlers.ts

+          ended: DateTime.now().plus({ days: 5 }).toISO(),
+          enrolled: undefined,
+          started: DateTime.now().minus({ days: 30 }).toISO(),
+        }),


unrelated to PR - just bettering the mocking experience in /betas to have an active non-enrolled one featured in the list

abailly-akamai · 2024-12-16T14:10:02Z

packages/manager/src/mocks/serverHandlers.ts

+  http.get('*/domains/*/records', () => {
+    const records = domainRecordFactory.buildList(1);
+    return HttpResponse.json(makeResourcePage(records));
+  }),


We were missing those, resulting in test failures with the route testing

abailly-akamai · 2024-12-16T14:10:27Z

packages/manager/src/queries/domains.ts

+  useQuery<Domain, APIError[]>({
+    ...domainQueries.domain(id),
+    enabled,
+  });


Just adding an enabled toggle

abailly-akamai · 2024-12-16T14:11:24Z

packages/manager/src/routes/domains/domainsLazyRoutes.ts

+
+export const domainDetailLazyRoute = createLazyRoute('/domains/$domainId')({
+  component: DomainDetail,
+});


Reminder we have decided to consolidate lazy routes in their own file (there is no particular need to have them within the component itself, which could be problematic for HMR)

abailly-akamai · 2024-12-16T14:12:33Z

packages/manager/src/routes/routes.test.tsx

+        };
+      }),
+  };
+});


This addition allows to bypass any loading state from react-query so we can always assert for a header

github-actions · 2024-12-16T14:38:32Z

Coverage Report: ❌
Base Coverage: 86.97%
Current Coverage: 86.96%

dwiley-akamai

Tests & checks pass ✅
Navigation & routed drawers at /domains ✅
Pagination ✅
Sorting ✅
Navigation & drawers at /domains/{id} ✅

⚠️ note: domain details (adding records) drawers also should be routed [...]

Another thing to include in the next ticket is having the routing update when "Delete Domain" is clicked from the domain detail page (clicking "Delete" on the domain landing page updates the route)

dwiley-akamai · 2024-12-17T15:59:43Z

packages/manager/src/features/Domains/CreateDomain/CreateDomain.tsx

@@ -127,9 +126,12 @@ export const CreateDomain = () => {
  const isCreatingPrimaryDomain = values.type === 'master';
  const isCreatingSecondaryDomain = values.type === 'slave';

-  const redirect = (id: '' | number, state?: Record<string, string>) => {
+  const redirect = (id: '' | number, _state?: Record<string, string>) => {


_state doesn't get used in the function?

Thanks for pointing this out: I handled this more clearly in e784402

bit of a weird edge case but worth keeping parity over indeed 👍

mjac0bs

Navigation to the different routes, ordering, and pagination generally worked as expected. ✅

I left a couple of observations.

packages/manager/src/features/Domains/CloneDomainDrawer.tsx

packages/manager/src/features/Domains/DomainsLanding.tsx

mjac0bs · 2024-12-18T06:02:18Z

packages/manager/src/features/Betas/BetaSignup.tsx

@@ -15,7 +15,7 @@ import { useCreateAccountBetaMutation } from 'src/queries/account/betas';
 import { useBetaQuery } from 'src/queries/betas';

 export const BetaSignup = () => {
-  const betaAgreement = `# Early Adopter Testing Program
+  const betaAgreement = `### Early Adopter Testing Program


Nice, solid test!

abailly-akamai · 2024-12-18T14:26:22Z

Solid review @mjac0bs - thanks for the fix and pointers.

Fixed the clone issue, created ticket for others!

mjac0bs

🚢

linode-gh-bot · 2024-12-18T17:14:06Z

Cloud Manager UI test results

🎉 470 passing tests on test run #12 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`0 Failing`	`470 Passing`	`2 Skipped`	`97m 37s`

cypress · 2024-12-18T19:06:38Z

Cloud Manager E2E Run #6995

Run Properties: Passed #6995 • 26adcaca8f: tech-story: [M3-8784] - Tanstack routing for Domains (#11418)

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Passed #6995`
Run duration	`26m 58s`
Commit	`26adcaca8f: tech-story: [M3-8784] - Tanstack routing for Domains (#11418)`
Committer	`Alban Bailly`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`2`
Pending	`2`
Skipped	`0`
Passing	`470`
View all changes introduced in this branch ↗︎

@dwiley-akamai

* save progress * SURGERY * Implement basic routing * Wrap up routing * fix unit * routes test update * better handling in test * fix smoke test * Added changeset: Refactor Domains Feature Routing * MSW fix * edit changeset * mock path consistency * feedback @dwiley-akamai * feedback @mjac0bs * fix e2e

abailly-akamai added the Routing Refactor label Dec 13, 2024

abailly-akamai self-assigned this Dec 13, 2024

abailly-akamai marked this pull request as ready for review December 16, 2024 14:01

abailly-akamai requested a review from a team as a code owner December 16, 2024 14:01

abailly-akamai requested review from dwiley-akamai and mjac0bs and removed request for a team December 16, 2024 14:01

abailly-akamai commented Dec 16, 2024

View reviewed changes

dwiley-akamai approved these changes Dec 17, 2024

View reviewed changes

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Dec 17, 2024

mjac0bs reviewed Dec 18, 2024

View reviewed changes

abailly-akamai added 14 commits December 18, 2024 09:25

save progress

aac13fd

SURGERY

2d71ba1

Implement basic routing

bc4c53f

Wrap up routing

44032ee

fix unit

1d472e3

routes test update

8b6fa6a

better handling in test

a0c578b

fix smoke test

3086e5f

Added changeset: Refactor Domains Feature Routing

9467f2d

MSW fix

77f673c

edit changeset

474e217

mock path consistency

eab0a6f

feedback @dwiley-akamai

644da0d

feedback @mjac0bs

808364b

abailly-akamai force-pushed the M3-8784 branch from f726f3b to 808364b Compare December 18, 2024 14:25

abailly-akamai requested a review from mjac0bs December 18, 2024 14:26

mjac0bs approved these changes Dec 18, 2024

View reviewed changes

mjac0bs added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Dec 18, 2024

fix e2e

5dc1760

abailly-akamai requested a review from a team as a code owner December 18, 2024 16:31

abailly-akamai requested review from AzureLatte and removed request for a team December 18, 2024 16:31

AzureLatte approved these changes Dec 18, 2024

View reviewed changes

abailly-akamai merged commit 26adcac into linode:develop Dec 18, 2024
23 checks passed

This was referenced Jan 8, 2025

Release v1.134.0 - release → staging #11494

Closed

Release v1.134.0 - release → staging #11497

Merged

hana-akamai mentioned this pull request Jan 9, 2025

Release v1.134.0 - release → staging #11503

Merged

jaalah-akamai mentioned this pull request Jan 14, 2025

Release v1.134.0 - staging → master #11519

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tech-story: [M3-8784] - Tanstack routing for Domains #11418

tech-story: [M3-8784] - Tanstack routing for Domains #11418

abailly-akamai commented Dec 13, 2024 •

edited

Loading

abailly-akamai Dec 16, 2024

mjac0bs Dec 18, 2024

abailly-akamai Dec 16, 2024

abailly-akamai Dec 16, 2024

abailly-akamai Dec 16, 2024

abailly-akamai Dec 16, 2024

abailly-akamai Dec 16, 2024

abailly-akamai Dec 16, 2024

github-actions bot commented Dec 16, 2024 •

edited

Loading

dwiley-akamai left a comment

dwiley-akamai Dec 17, 2024

abailly-akamai Dec 17, 2024

mjac0bs left a comment

mjac0bs Dec 18, 2024

abailly-akamai commented Dec 18, 2024

mjac0bs left a comment

linode-gh-bot commented Dec 18, 2024

cypress bot commented Dec 18, 2024

tech-story: [M3-8784] - Tanstack routing for Domains #11418

tech-story: [M3-8784] - Tanstack routing for Domains #11418

Conversation

abailly-akamai commented Dec 13, 2024 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Verification steps

As an Author, to speed up the review process, I considered 🤔

As an Author, before moving this PR from Draft to Open, I confirmed ✅

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

github-actions bot commented Dec 16, 2024 • edited Loading

dwiley-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjac0bs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai commented Dec 18, 2024

mjac0bs left a comment

Choose a reason for hiding this comment

linode-gh-bot commented Dec 18, 2024

Cloud Manager UI test results

🎉 470 passing tests on test run #12 ↗︎

cypress bot commented Dec 18, 2024

abailly-akamai commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 16, 2024 •

edited

Loading