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

Tweak IP pool edit onSuccess logic #2409

Merged
merged 1 commit into from
Sep 3, 2024
Merged

Tweak IP pool edit onSuccess logic #2409

merged 1 commit into from
Sep 3, 2024

Conversation

david-crespo
Copy link
Collaborator

Followup for #2405 (comment), which I posted seconds after merge. I made the conditional narrower and added a comment.

Copy link

vercel bot commented Sep 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Sep 3, 2024 7:45pm

queryClient.invalidateQueries('ipPoolView')
onDismiss()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I found the if-else confusing because the onDismiss and navigate do exactly the same thing

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, good point.

// while the loader for the target page is running because the current
// page's pool gets cleared out while we're still on the page. If we're
// navigating to a different page, its query will fetch anew regardless.
if (pool.name === updatedPool.name) {
Copy link
Collaborator Author

@david-crespo david-crespo Sep 3, 2024

Choose a reason for hiding this comment

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

Another way of doing this that's worth considering would be to add the ability to invalidateQueries to invalidate more narrowly than an entire endpoint.

console/app/api/hooks.ts

Lines 243 to 253 in c648c44

/**
* Note that we only take a single argument, `method`, rather than allowing
* the full query key `[query, params]` to be specified. This is to avoid
* accidentally overspecifying and therefore failing to match the desired
* query. The params argument can be added back in if we ever have a use case
* for it.
*
* Passing no arguments will invalidate all queries.
*/
invalidateQueries: <M extends keyof A>(method?: M, filters?: InvalidateQueryFilters) =>
queryClient.invalidateQueries(method ? { queryKey: [method], ...filters } : undefined),

So in this case we'd be able to avoid the conditional by invalidating the fetch under the old name, but not the new name. Working on a quick PR showing that option. I thought it was kind of dicey but it's not.

#2410

Copy link
Contributor

Choose a reason for hiding this comment

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

As I mentioned in the other PR, I prefer (w/o strong feelings on it) the way you have it in this PR. But open to either direction.

Copy link
Contributor

@charliepark charliepark left a comment

Choose a reason for hiding this comment

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

Cleaner. Also agree that updatedPool is clearer.

@david-crespo david-crespo merged commit f042ad4 into main Sep 3, 2024
8 checks passed
@david-crespo david-crespo deleted the pool-edit-success branch September 3, 2024 20:25
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Sep 16, 2024
oxidecomputer/console@7712765...ce242e7

* [ce242e77](oxidecomputer/console@ce242e77) bump API to latest, only doc comment changes
* [3fda42e3](oxidecomputer/console@3fda42e3) oxidecomputer/console#2446
* [a5c23616](oxidecomputer/console@a5c23616) oxidecomputer/console#2436
* [ca272336](oxidecomputer/console@ca272336) oxidecomputer/console#2445
* [22582e3c](oxidecomputer/console@22582e3c) oxidecomputer/console#2444
* [f9f0d157](oxidecomputer/console@f9f0d157) oxidecomputer/console#2443
* [be84c196](oxidecomputer/console@be84c196) oxidecomputer/console#2441
* [53e1da78](oxidecomputer/console@53e1da78) chore: just skip the flaky test in safari
* [3a66ca77](oxidecomputer/console@3a66ca77) bump a couple of dev deps I missed
* [ac67a4cf](oxidecomputer/console@ac67a4cf) bump omicron version to latest
* [63c604b4](oxidecomputer/console@63c604b4) oxidecomputer/console#2440
* [b4e2626b](oxidecomputer/console@b4e2626b) oxidecomputer/console#2439
* [cdecf4e6](oxidecomputer/console@cdecf4e6) oxidecomputer/console#2435
* [01d71c07](oxidecomputer/console@01d71c07) oxidecomputer/console#2432
* [353b98d6](oxidecomputer/console@353b98d6) oxidecomputer/console#2404
* [49d6d7d8](oxidecomputer/console@49d6d7d8) oxidecomputer/console#2433
* [9c532ce9](oxidecomputer/console@9c532ce9) chore: fix react duplicate key warning  on vpcs page
* [1892fc14](oxidecomputer/console@1892fc14) npm audit fix, bump msw
* [efc43be0](oxidecomputer/console@efc43be0) oxidecomputer/console#2422
* [f2cc79ee](oxidecomputer/console@f2cc79ee) oxidecomputer/console#2418
* [a0c29a53](oxidecomputer/console@a0c29a53) oxidecomputer/console#2427
* [a8fcdab9](oxidecomputer/console@a8fcdab9) oxidecomputer/console#2426
* [8ecb36ad](oxidecomputer/console@8ecb36ad) oxidecomputer/console#2425
* [93bcef9d](oxidecomputer/console@93bcef9d) oxidecomputer/console#2296
* [af42e704](oxidecomputer/console@af42e704) oxidecomputer/console#2421
* [4126f000](oxidecomputer/console@4126f000) oxidecomputer/console#2420
* [880cb8c4](oxidecomputer/console@880cb8c4) oxidecomputer/console#2415
* [c2909e7a](oxidecomputer/console@c2909e7a) oxidecomputer/console#2414
* [12fc862b](oxidecomputer/console@12fc862b) oxidecomputer/console#2412
* [169edeed](oxidecomputer/console@169edeed) remove unreachable sign in button in user dropdown
* [f042ad4b](oxidecomputer/console@f042ad4b) oxidecomputer/console#2409
* [c648c44c](oxidecomputer/console@c648c44c) oxidecomputer/console#2408
* [e8175d30](oxidecomputer/console@e8175d30) oxidecomputer/console#2405
* [bdb55b86](oxidecomputer/console@bdb55b86) chore: bump ts client generator version in api-diff
* [681355cd](oxidecomputer/console@681355cd) oxidecomputer/console#2396
* [286b8d28](oxidecomputer/console@286b8d28) oxidecomputer/console#2401
* [b2373359](oxidecomputer/console@b2373359) oxidecomputer/console#2400
* [267b9359](oxidecomputer/console@267b9359) oxidecomputer/console#2399
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Sep 16, 2024
oxidecomputer/console@7712765...ce242e7

* [ce242e77](oxidecomputer/console@ce242e77)
bump API to latest, only doc comment changes
* [3fda42e3](oxidecomputer/console@3fda42e3)
oxidecomputer/console#2446
* [a5c23616](oxidecomputer/console@a5c23616)
oxidecomputer/console#2436
* [ca272336](oxidecomputer/console@ca272336)
oxidecomputer/console#2445
* [22582e3c](oxidecomputer/console@22582e3c)
oxidecomputer/console#2444
* [f9f0d157](oxidecomputer/console@f9f0d157)
oxidecomputer/console#2443
* [be84c196](oxidecomputer/console@be84c196)
oxidecomputer/console#2441
* [53e1da78](oxidecomputer/console@53e1da78)
chore: just skip the flaky test in safari
* [3a66ca77](oxidecomputer/console@3a66ca77)
bump a couple of dev deps I missed
* [ac67a4cf](oxidecomputer/console@ac67a4cf)
bump omicron version to latest
* [63c604b4](oxidecomputer/console@63c604b4)
oxidecomputer/console#2440
* [b4e2626b](oxidecomputer/console@b4e2626b)
oxidecomputer/console#2439
* [cdecf4e6](oxidecomputer/console@cdecf4e6)
oxidecomputer/console#2435
* [01d71c07](oxidecomputer/console@01d71c07)
oxidecomputer/console#2432
* [353b98d6](oxidecomputer/console@353b98d6)
oxidecomputer/console#2404
* [49d6d7d8](oxidecomputer/console@49d6d7d8)
oxidecomputer/console#2433
* [9c532ce9](oxidecomputer/console@9c532ce9)
chore: fix react duplicate key warning on vpcs page
* [1892fc14](oxidecomputer/console@1892fc14)
npm audit fix, bump msw
* [efc43be0](oxidecomputer/console@efc43be0)
oxidecomputer/console#2422
* [f2cc79ee](oxidecomputer/console@f2cc79ee)
oxidecomputer/console#2418
* [a0c29a53](oxidecomputer/console@a0c29a53)
oxidecomputer/console#2427
* [a8fcdab9](oxidecomputer/console@a8fcdab9)
oxidecomputer/console#2426
* [8ecb36ad](oxidecomputer/console@8ecb36ad)
oxidecomputer/console#2425
* [93bcef9d](oxidecomputer/console@93bcef9d)
oxidecomputer/console#2296
* [af42e704](oxidecomputer/console@af42e704)
oxidecomputer/console#2421
* [4126f000](oxidecomputer/console@4126f000)
oxidecomputer/console#2420
* [880cb8c4](oxidecomputer/console@880cb8c4)
oxidecomputer/console#2415
* [c2909e7a](oxidecomputer/console@c2909e7a)
oxidecomputer/console#2414
* [12fc862b](oxidecomputer/console@12fc862b)
oxidecomputer/console#2412
* [169edeed](oxidecomputer/console@169edeed)
remove unreachable sign in button in user dropdown
* [f042ad4b](oxidecomputer/console@f042ad4b)
oxidecomputer/console#2409
* [c648c44c](oxidecomputer/console@c648c44c)
oxidecomputer/console#2408
* [e8175d30](oxidecomputer/console@e8175d30)
oxidecomputer/console#2405
* [bdb55b86](oxidecomputer/console@bdb55b86)
chore: bump ts client generator version in api-diff
* [681355cd](oxidecomputer/console@681355cd)
oxidecomputer/console#2396
* [286b8d28](oxidecomputer/console@286b8d28)
oxidecomputer/console#2401
* [b2373359](oxidecomputer/console@b2373359)
oxidecomputer/console#2400
* [267b9359](oxidecomputer/console@267b9359)
oxidecomputer/console#2399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants