feat: checkbox to make pool default when linking to silo#3267
feat: checkbox to make pool default when linking to silo#3267sudomateo wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Looks good, only thing to change might be having the message on default IP pool reflect the type of the pool, since you can have a default for each IP version. "default IPv4 pool for silo" etc. In theory we could also say "unicast" but since you can have a default each for unicast and multicast too, but it's borderline. Also the checkbox should be tested in a playwright test. |
sudomateo
force-pushed
the
sudomateo/nzywoxnyvurz
branch
from
15d66cc to
fbb52ee
Compare
|
Added a checkbox to the IP pool and subnet pool linking modals to make the pool the default pool for the silo. Closes oxidecomputer/customer-support#413. Amp-Thread: https://ampcode.com/threads/T-019f1129-f325-73e9-96da-94a8a24fa617
sudomateo
force-pushed
the
sudomateo/nzywoxnyvurz
branch
from
fbb52ee to
4b2a483
Compare
|
I went way overboard but the result is really good. You can see it in the commit list, but here is what happened: The checkbox was only added to the link IP pool modal on the silo IP pools page, but was missing from the IP pool silos page, so I added it to the latter. Then I noticed the mock server had some out of date logic around what happens when you try to link an IP pool and set it default at the same time. What happens is: if there is already a default, it will error if you try to link and set default at the same time. You are supposed to first link it, and then set it as default. This is to make it a little harder to overwrite the existing default in a single API call. I brought the mock server up to date. In doing that, I realized the user of the web console would be very annoyed if they checked the box and we errored out, telling them to do something we could have just done for them. There is no reason the web console has to make a single call for every action! By checking the box, the user has indicated that they want to make the thing default, and we (being a Web Application) can actually inform them right there of the consequences of that action. So the result of all that is this. I like it! 
When the user checks the box, we link the thing as non-default and then we make a second call to make it the default. The only thing I don't really like about it is that we're doing all the error handling through toasts: if the initial link fails (rare), we keep the modal open and there's a toast saying it failed (this is how it already worked before this change). If the link succeeds and the make default fails, we have already closed the modal but the toast tells them to try again through the row action. It's not that bad. Maybe in an ideal world we'd keep the modal open in both cases and show the errors inline. 2026-06-30-ip-pool-link-default.mp42026-06-30-ip-pool-link-silo-default.mp4 |
| } | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
We don't do very much of this kind of action sequencing in the console but it's really fine, especially since we're relying on the toasts for error handling rather than returning the error state from the hook.
|
Muahahah, I see my nerdsnipe worked. Thanks for getting this into a shippable state! |
|
The test failure is a bug that only happens during the first hour of the month UTC lmao. Pushing a fix as a separate PR. |
2 participants
Added a checkbox to the IP pool and subnet pool linking modals to make the pool the default pool for the silo.
Closes https://github.com/oxidecomputer/customer-support/issues/413.
Amp-Thread: https://ampcode.com/threads/T-019f1129-f325-73e9-96da-94a8a24fa617