Add dropdown to instance breadcrumb #2392
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
The CI failure was a flake, but both Chrome and Safari flaked on the same test, which has to do with instance states, so it most likely was caused by #2360. There's probably a timeout I can tweak that will fix it. I'll take a look. |
|
Ok, this is amazing: that CI test failure is real, it's dead reproducible locally. The only thing this PR changes is it adds another query watching the |
| onDelete: () => { | ||
| apiQueryClient.invalidateQueries('instanceList') | ||
| navigate(pb.instances(instanceSelector)) | ||
| }, |
There was a problem hiding this comment.
This was a legit bug, and I know why this change turned it up. The failure was that the expected disappearance of you-fail from the instance list was not happening.
What happens in this delete from instance detail test is:
- go to the instance detail page for
you-fail - delete it
- let it automatically land us on the list view
- confirm the instance is gone
Before this PR, there was nothing on the instance detail page "observing" the instanceList query, so when we nav to instance list on delete success, that query will always be fetched fresh because it is "new" from RQ's point of view. But with this PR, there is now the breadcrumb that uses the list of instances, so the query is "active" on the instance detail page.
Here is the funny bit: when you do what the test does by hand, you inevitably sit on the you-fail page longer than the 2 seconds required for the query to become stale. That means it will automatically refetch when the loader fires for the instance list page on navigation. But the test is faster than you, so it lands on the instance detail and hits delete in under 2 seconds, so the query is not stale by the time we land on the instance list and therefore does not refetch. I confirmed this theory by adding a sleep to the test so it sits on the page for 2 seconds before clicking delete. That makes the test pass! But of course the real fix is to specifically invalidate the instance list in the delete success callback.
There was a problem hiding this comment.
Oh, wow. Good sleuthing on this.
| export function InstancesPage() { | ||
| const { project } = useProjectSelector() | ||
|
|
||
| const queryClient = useApiQueryClient() | ||
| const refetchInstances = () => queryClient.invalidateQueries('instanceList') | ||
|
|
There was a problem hiding this comment.
The changes in this file are irrelevant to the fix, but I did this while debugging and I like it better.
oxidecomputer/console@33b7a50...8dcddce * [8dcddcef](oxidecomputer/console@8dcddcef) oxidecomputer/console#2392 * [f53bb383](oxidecomputer/console@f53bb383) oxidecomputer/console#2387 * [6a005a0b](oxidecomputer/console@6a005a0b) oxidecomputer/console#2389 * [2f83a1fb](oxidecomputer/console@2f83a1fb) tweak firewall rule copy (closes oxidecomputer/console#2348) * [22bdac78](oxidecomputer/console@22bdac78) oxidecomputer/console#2386 * [5353a6ea](oxidecomputer/console@5353a6ea) oxidecomputer/console#2385 * [7cf1ed54](oxidecomputer/console@7cf1ed54) oxidecomputer/console#2383 * [c52cc37b](oxidecomputer/console@c52cc37b) oxidecomputer/console#2382 * [e442f6bd](oxidecomputer/console@e442f6bd) oxidecomputer/console#2378
oxidecomputer/console@33b7a50...8dcddce * [8dcddcef](oxidecomputer/console@8dcddcef) oxidecomputer/console#2392 * [f53bb383](oxidecomputer/console@f53bb383) oxidecomputer/console#2387 * [6a005a0b](oxidecomputer/console@6a005a0b) oxidecomputer/console#2389 * [2f83a1fb](oxidecomputer/console@2f83a1fb) tweak firewall rule copy (closes oxidecomputer/console#2348) * [22bdac78](oxidecomputer/console@22bdac78) oxidecomputer/console#2386 * [5353a6ea](oxidecomputer/console@5353a6ea) oxidecomputer/console#2385 * [7cf1ed54](oxidecomputer/console@7cf1ed54) oxidecomputer/console#2383 * [c52cc37b](oxidecomputer/console@c52cc37b) oxidecomputer/console#2382 * [e442f6bd](oxidecomputer/console@e442f6bd) oxidecomputer/console#2378
Closes #2380
We'll want to revisit the overall breadcrumb designs (see #2380 for a few possible directions / links to more in Figma), but in the interim, we can add the dropdown to the instance item in the breadcrumb nav.