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

Mock API: error if parent selectors present when resource is fetched by ID #2396

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

david-crespo
Copy link
Collaborator

@david-crespo david-crespo commented Aug 26, 2024

Trying to avoid the bug fixed in #2394, where we accidentally include parent selectors. At first I thought we should figure out how to do this automatically, hence oxidecomputer/oxide.ts#264, but as noted there, it's hard to do automatically, and I remembered we already have this whole manually-implemented mock DB lookup apparatus in the console. We have run into this category of bug once before, and I fixed it in precisely this way in #2203, but only for one resource, which is silly.

The mechanical stuff here was pretty easy. The annoying part was going through the real API and code fixing a couple of spots where we currently deviate.

Copy link

vercel bot commented Aug 26, 2024

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

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Aug 27, 2024 7:12pm

if (isUuid(id)) {
ensureNoParentSelectors('subnet', vpcSelector)
return lookupById(db.vpcSubnets, id)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's a small downside to this implementation for double-nested resources like subnet: if you were to try to select a subnet by (subnet name, VPC ID, project name), the thing that would blow up would be the VPC lookup, and the error would be "when VPC is specified by ID, project should not be specified." When I put it that way, it's not that confusing. But if you only look at the error message and not what endpoint the error is coming from, you can get thrown off.

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.

Thank you for fixing all of these

@david-crespo david-crespo merged commit 681355c into main Aug 29, 2024
8 checks passed
@david-crespo david-crespo deleted the mock-api-selectors branch August 29, 2024 21:55
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