Skip to content

Fix issue with sourceType overload on project vs silo images and disks - #2097

Merged
david-crespo merged 9 commits into
existing-boot-diskfrom
fix-form-sourceType-bug
Mar 27, 2024
Merged

david-crespo merged 9 commits into
existing-boot-diskfrom
fix-form-sourceType-bug

Conversation

@charliepark

@charliepark charliepark commented Mar 26, 2024 •

Copy link
Copy Markdown
Contributor

This proposal addresses an issue we're seeing in the instance create form.

Currently, because the create-instance form has a default image selected when the form is initialized, the form will use that image as the foundation for the instance, even if the user clicks to a different image source tab, if they don't select a new image. So they might select the "Project images" tab but end up creating an instance using the default Silo image. This is also a problem when the user selects the new "existing disk" tab.

This PR addresses the issue by setting up each of the tabs as their own piece of the form, and setting up a bootDiskSourceType that keeps track of the desired boot disk source.

This also allows us to maintain state across tab changes, so if the user has selected a source image and then clicks to a different tab, then clicks back, they don't lose their work.

@vercel

vercel Bot commented Mar 26, 2024 •

Copy link
Copy Markdown

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

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Mar 27, 2024 10:41pm

Comment thread app/forms/instance-create.tsx Outdated
@charliepark charliepark changed the title Fix issue with sourceType collision on project vs silo images and disks Fix issue with sourceType overload on project vs silo images and disks Mar 26, 2024
Comment thread app/forms/instance-create.tsx Outdated
Comment thread app/forms/instance-create.tsx Outdated
Comment thread app/forms/instance-create.tsx Outdated
@david-crespo

Copy link
Copy Markdown
Collaborator

This is looking really good. It fixes the bug. As we talked about in chat, I think it would be slightly better if your selection on each tab was preserved as you switch between tabs, so you could check what's on another tab without losing what you've done. This would require splitting bootDiskSource into a separate value for each tab and then tweaking what we do in onSubmit to construct the request body. I'm not sure if it makes validation harder, hopefully it just works.

@charliepark
charliepark marked this pull request as ready for review March 27, 2024 20:00
@charliepark charliepark mentioned this pull request Mar 27, 2024
2 tasks done
Comment thread test/e2e/instance-create.e2e.ts
Comment thread app/forms/instance-create.tsx
Comment thread app/forms/instance-create.tsx Outdated

@david-crespo david-crespo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sweet. let’s do final review in #2076

@david-crespo
david-crespo merged commit 6976890 into existing-boot-disk Mar 27, 2024
@david-crespo
david-crespo deleted the fix-form-sourceType-bug branch March 27, 2024 22:59
david-crespo added a commit that referenced this pull request Apr 3, 2024
* Create instance from existing boot disk

* Remove setting undefined on tab change (for now)

* Improve descriptions

* copy tweaks

* tweak empty states copy, fix diskList prefetch

* Fix issue with sourceType overload on project vs silo images and disks (#2097)

* Fix issue with sourceType collision on project vs silo images and disks

* refactor

* Refactoring, and updating types

* Maintain state of fields across tab navigation (#2100)

* Simplify test; make it work for Safari

* Clean up and add comments

* verify in test that correct image used

* clean up comment

* imageSize -> imageSizeGiB

---------

Co-authored-by: David Crespo <david.crespo@oxidecomputer.com>
Co-authored-by: Charlie Park <charlie@oxidecomputer.com>

This branch was successfully deployed

1 active deployment
Preview — a078205e Deployed Mar 27, 2024 by vercel[bot]
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