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
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
20 changes: 11 additions & 9 deletions app/forms/ip-pool-edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,21 @@ export function EditIpPoolSideModalForm() {
const { data: pool } = usePrefetchedApiQuery('ipPoolView', { path: poolSelector })

const form = useForm({ defaultValues: pool })
const onDismiss = () => navigate(pb.ipPool({ pool: poolSelector.pool }))

const editPool = useApiMutation('ipPoolUpdate', {
onSuccess(_pool) {
onSuccess(updatedPool) {
queryClient.invalidateQueries('ipPoolList')
if (pool.name !== _pool.name) {
// as the pool's name has changed, we need to navigate to an updated URL
navigate(pb.ipPool({ pool: _pool.name }))
} else {
navigate(pb.ipPool({ pool: updatedPool.name }))
addToast({ content: 'Your IP pool has been updated' })

// Only invalidate if we're staying on the same page. If the name
// _has_ changed, invalidating ipPoolView causes an error page to flash
// 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.

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.

addToast({ content: 'Your IP pool has been updated' })
},
})

Expand All @@ -56,7 +58,7 @@ export function EditIpPoolSideModalForm() {
form={form}
formType="edit"
resourceName="IP pool"
onDismiss={onDismiss}
onDismiss={() => navigate(pb.ipPool({ pool: poolSelector.pool }))}
onSubmit={({ name, description }) => {
editPool.mutate({ path: poolSelector, body: { name, description } })
}}
Expand Down
Loading