refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

abailly-akamai · 2024-10-24T14:19:01Z

Description 📝

This PR implements our new routing to /volumes 🎉

Beyond replicating functionalities, it also switches to using params for dialogs (modals & drawers) for the whole feature.
While being a tad more involved in terms of development, the benefits are quite great:

shareable URLs: routed state allows to encode the state of the UI in the URL, making it easy to share specific views or states via a link (ex: link to an detail drawer from another page without complicated state management)
browser History: changes in routed state are reflected in the browser's history, allowing users to use the back and forward buttons to navigate through different states.
deep Linking: users can bookmark or directly navigate to a specific state of the application, which is useful for returning to a specific view.
persistence: the state is preserved across page reloads, as it is stored in the URL.

An overview of how this works

Changes 🔄

Implement new routing for any route under (and including) /volumes
deprecate state based drawer/modal display in favor of routes
make V2 versions of usePagination and userOrder to use the new router methods
Unit tests for all the above

How to test 🧪

Verification steps

note: you can use prod-test-029 to test with an account with many volumes

Try to break things!

test everything in /volumes
- pagination: preferences, reload page on existing and new params etc
  - page size
  - page number
- query volumes via search
- navigate to all actions and confirm all is working
- navigate back & forth with browser button
- try loading a drawer or modal with an invalid ID

As an Author I have considered 🤔

Check all that apply

abailly-akamai · 2024-10-31T20:51:07Z

packages/manager/src/features/Volumes/EditVolumeDrawer.tsx

@@ -56,7 +56,7 @@ export const EditVolumeDrawer = (props: Props) => {
    values,
  } = useFormik({
    enableReinitialize: true,
-    initialValues: { label: volume?.label, tags: volume?.tags },
+    initialValues: { label: volume?.label ?? '', tags: volume?.tags ?? [] },


avoids console error (uncontrolled to controlled input)

abailly-akamai · 2024-10-31T20:55:14Z

packages/manager/src/features/Volumes/VolumeTableRow.tsx

+      //   params: { linodeId: volume.linode_id },
+      //   search: { upgrade: 'true' },
+      //   to: '/linodes/$linodeId/storage',
+      // });


Important
We can't use Tanstack router's navigate() to route to a feature that has not been migrated to the new routing. This will result in a browser crash. In this case we will keep the react-router-dom history.push till /linodes/* routes are served by the new router

abailly-akamai · 2024-10-31T20:56:04Z

packages/manager/src/features/Volumes/VolumesLandingEmptyStateData.ts

@@ -26,7 +26,8 @@ export const gettingStartedGuides: ResourcesLinkSection = {
    },
    {
      text: 'Create and Manage Block Storage Volumes',
-      to: 'https://techdocs.akamai.com/cloud-computing/docs/block-storage',
+      to:
+        'https://techdocs.akamai.com/cloud-computing/docs/manage-block-storage-volumes',


unrelated, doc link was just wrong

abailly-akamai · 2024-10-31T20:56:46Z

packages/manager/src/mocks/utilities/response.ts

@@ -108,7 +108,7 @@ export const makePaginatedResponse = <T extends JsonBodyType>({
              typeof item === 'object' &&
              item !== null &&
              key in item &&
-              String(item).toLowerCase().includes(searchValue)
+              String(item[key]).toLowerCase().includes(searchValue)


a fix to make the filtering work when using CRUD MSW (volumes are supported)

abailly-akamai · 2024-10-31T20:58:27Z

packages/manager/src/routes/types.ts

@@ -8,4 +9,12 @@ export type RouterContext = {
  isACLPEnabled?: boolean;
  isDatabasesEnabled?: boolean;
  isPlacementGroupsEnabled?: boolean;
+  queryClient: QueryClient;


We need this new react-query context to be able to inspect the cache to validate the entity the router is navigating to.

Note: it is not used in this PR but will be used in features that can use prefetching at the route level

packages/manager/src/routes/volumes/index.ts

github-actions · 2024-11-01T19:17:41Z

Coverage Report: ✅
Base Coverage: 86.84%
Current Coverage: 86.9%

jaalah-akamai · 2024-11-04T13:49:26Z

When a user requests a page number that exceeds our available pages (e.g., ?page=100), should we:

Automatically redirect them to the last available page, or
Maintain current UX where the table displays 'No volume found'?

jaalah-akamai

Thanks for tackling this, looking really good.

Nit: After clearing the search, the URL still includes an empty query parameter /volumes?query= We should look into cleaning this up since it gets appended for subsequent routes, IE: /volumes/1320163/details?query=

packages/manager/src/routes/volumes/index.ts

packages/manager/src/hooks/usePaginationV2.ts

hkhalil-akamai

Leaving a couple comments and minor suggestions. Will leave a true review once this PR is in its final state.

packages/api-v4/src/types.ts

packages/manager/cypress/e2e/core/volumes/create-volume.smoke.spec.ts

packages/manager/src/Router.tsx

packages/manager/src/components/ActionMenu/ActionMenu.tsx

packages/manager/src/components/InlineMenuAction/InlineMenuAction.tsx

packages/manager/src/features/Volumes/VolumesActionMenu.tsx

packages/manager/src/features/Volumes/VolumesLanding.tsx

packages/manager/src/hooks/useOrderV2.test.tsx

packages/manager/src/routes/utils/buildXFilters.ts

packages/manager/src/routes/volumes/index.ts

abailly-akamai · 2024-11-06T23:58:40Z

Converting this to draft in order to avoid too much feedback based on things being in flux because of reworking parts of the dialog routing from early comments

hkhalil-akamai · 2024-11-07T21:48:19Z

Converting this to draft in order to avoid too much feedback based on things being in flux because of reworking parts of the dialog routing from early comments

@abailly-akamai for future reference, we should make it clear as early as possible when a PR is in an incomplete state. Reviewing 1000+ line PRs is a time-intensive effort and I didn't realize I was reviewing non-finalized changes until I was most of the way through my review.

abailly-akamai · 2024-11-07T21:57:17Z

@hkhalil-akamai Apologies, you are right, should had done it earlier - addressing feedback and this PR is still in shape so it's tricky - It must be noted your feedback was very valuable

linode-gh-bot · 2024-12-10T13:38:36Z

Cloud Manager UI test results

🎉 467 passing tests on test run #60 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`0 Failing`	`467 Passing`	`2 Skipped`	`96m 16s`

coliu-akamai

✅ back and forth for order by working
✅ confirmed behavior of other items - pagination, search, actions, etc

thanks Alban!

cypress · 2024-12-10T15:58:51Z

Cloud Manager E2E Run #6944

Run Properties: Failed #6944 • ac15267b6e: refactor: [M3-8783] - Migrate /volumes to Tanstack Router (#11154)

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Failed #6944`
Run duration	`32m 18s`
Commit	`ac15267b6e: refactor: [M3-8783] - Migrate /volumes to Tanstack Router (#11154)`
Committer	`Alban Bailly`
View all properties for this run ↗︎

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

Tests for review

linodes/rebuild-linode.spec.ts • 2 failed tests

View Output Video

Test		Artifacts
rebuild linode > rebuilds a linode from Community StackScript		`Screenshots` `Video`
rebuild linode > rebuilds a linode from Account StackScript		`Screenshots` `Video`

images/search-images.spec.ts • 1 failed test

View Output Video

Test		Artifacts
Search Images > creates two images and make sure they show up in the table and are searchable		`Screenshots` `Video`

cypress/e2e/core/linodes/linode-config.spec.ts • 1 flaky test

View Output Video

Test		Artifacts
Linode Config management > End-to-End > Clones a config		`Screenshots` `Video`

abailly-akamai self-assigned this Oct 24, 2024

abailly-akamai force-pushed the M3-8783 branch 4 times, most recently from 870fa9c to b51f8da Compare October 29, 2024 18:04

abailly-akamai added the Routing Refactor label Oct 29, 2024

abailly-akamai force-pushed the M3-8783 branch 2 times, most recently from 1ba4b04 to 071f12f Compare October 31, 2024 13:12

abailly-akamai commented Oct 31, 2024

View reviewed changes

abailly-akamai marked this pull request as ready for review October 31, 2024 21:10

abailly-akamai requested a review from a team as a code owner October 31, 2024 21:10

abailly-akamai requested review from hkhalil-akamai, coliu-akamai, bnussman-akamai and jaalah-akamai and removed request for a team October 31, 2024 21:10

abailly-akamai force-pushed the M3-8783 branch from aa049fb to 2f88aae Compare November 1, 2024 18:43

abailly-akamai force-pushed the M3-8783 branch from 2f88aae to 7572630 Compare November 4, 2024 15:06

jaalah-akamai reviewed Nov 4, 2024

View reviewed changes

packages/manager/src/routes/volumes/index.ts Outdated Show resolved Hide resolved

packages/manager/src/hooks/usePaginationV2.ts Outdated Show resolved Hide resolved

abailly-akamai requested a review from a team as a code owner November 5, 2024 22:58

abailly-akamai requested review from cliu-akamai and removed request for a team November 5, 2024 22:58

abailly-akamai force-pushed the M3-8783 branch from 94d206f to b365c6c Compare November 6, 2024 20:29

hkhalil-akamai reviewed Nov 6, 2024

View reviewed changes

abailly-akamai marked this pull request as draft November 6, 2024 23:55

abailly-akamai added 24 commits December 10, 2024 07:59

route level error handling

5656d01

xFilter util

9062419

xFilter util improvements

403ea72

post rebase fix

49f296e

testing xQuery builder

55f909a

moar testing and cleanup

748eb0d

Added changeset: Migrate /volumes to Tanstack router

8696686

Save progress

5337929

fix one test

30938ae

fix remaining test

22ca472

feedback @jaalah-akamai

3ed5e99

more work on preloading

ac19483

save progress

6c5c570

Walking back and using a more progressive approach

8af2e9a

cleanup

92ace27

entity not found logic

a3de9ca

post rebase fix

4a39156

post rebase fix

6927d90

update loading state

cc2198e

fix the smoke tests

2d228e2

Feedback @bnussman-akamai

83103bd

JsDocs for Tanstack link components

0c09de5

Improve new useOrder hook

0b0d678

feedback @coliu-akamai @hkhalil-akamai

afaaa1d

abailly-akamai force-pushed the M3-8783 branch from d7524d0 to afaaa1d Compare December 10, 2024 12:59

coliu-akamai approved these changes Dec 10, 2024

View reviewed changes

coliu-akamai added the Approved Multiple approvals and ready to merge! label Dec 10, 2024

abailly-akamai merged commit ac15267 into linode:develop Dec 10, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

abailly-akamai commented Oct 24, 2024 •

edited

Loading

abailly-akamai Oct 31, 2024

abailly-akamai Oct 31, 2024

abailly-akamai Oct 31, 2024

abailly-akamai Oct 31, 2024

abailly-akamai Oct 31, 2024

abailly-akamai Nov 8, 2024

github-actions bot commented Nov 1, 2024 •

edited

Loading

jaalah-akamai commented Nov 4, 2024

jaalah-akamai left a comment

hkhalil-akamai left a comment

abailly-akamai commented Nov 6, 2024

hkhalil-akamai commented Nov 7, 2024

abailly-akamai commented Nov 7, 2024

linode-gh-bot commented Dec 10, 2024

coliu-akamai left a comment

cypress bot commented Dec 10, 2024

refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

refactor: [M3-8783] - Migrate /volumes to Tanstack Router #11154

Conversation

abailly-akamai commented Oct 24, 2024 • edited Loading

Description 📝

Changes 🔄

How to test 🧪

Verification steps

As an Author I have considered 🤔

abailly-akamai Oct 31, 2024

Choose a reason for hiding this comment

abailly-akamai Oct 31, 2024

Choose a reason for hiding this comment

abailly-akamai Oct 31, 2024

Choose a reason for hiding this comment

abailly-akamai Oct 31, 2024

Choose a reason for hiding this comment

abailly-akamai Oct 31, 2024

Choose a reason for hiding this comment

abailly-akamai Nov 8, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 1, 2024 • edited Loading

jaalah-akamai commented Nov 4, 2024

jaalah-akamai left a comment

Choose a reason for hiding this comment

hkhalil-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai commented Nov 6, 2024

hkhalil-akamai commented Nov 7, 2024

abailly-akamai commented Nov 7, 2024

linode-gh-bot commented Dec 10, 2024

Cloud Manager UI test results

🎉 467 passing tests on test run #60 ↗︎

coliu-akamai left a comment

Choose a reason for hiding this comment

cypress bot commented Dec 10, 2024

Cloud Manager E2E Run #6944

Tests for review

abailly-akamai commented Oct 24, 2024 •

edited

Loading

github-actions bot commented Nov 1, 2024 •

edited

Loading