chore: add database test fixture to insert non-unique linked_ids #12111
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| @@ -0,0 +1,19 @@ | |||
| -- This is a deleted user that shares the same username and linked_id as the existing user below. | |||
There was a problem hiding this comment.
@coadler Is this ok to name this fixture different then the last migration?
Should I make this fixture to an earlier migration number?
There was a problem hiding this comment.
I wonder if you could also just add it in the base fixture 🤔 or wherever the linked_ids were introduced
There was a problem hiding this comment.
I was wondering that too. Was not sure what the protocol is on the immutability of test fixtures.
There was a problem hiding this comment.
I would probably add it as the same migration id that user_links was added, but I'm not super familiar with these. Maybe @mafredri can weight in
There was a problem hiding this comment.
I'll move it to an older one. I guess just migrations are immutable.
There was a problem hiding this comment.
Yeah, since the fixtures should never make their way into a customer deployment I think this should be fine.
There was a problem hiding this comment.
I moved it to 000048_userdelete.up.sql. That is when deleting users was supported. Which is what this really is, adding a deleted user.
There was a problem hiding this comment.
Yeah definitely, always feel free to go wild with the fixtures, and use them to place in wrong-use traps for the code. 😄
2883815 to
78e90d6
Compare
78e90d6 to
90e7164
Compare
Added to the migration that added the delete field to users
| VALUES ('a0061a8e-7db7-4585-838c-3116a003dd21', 'githubuser@coder.com', 'githubuser', '\x', '2022-11-02 13:05:21.445455+02', '2022-11-02 13:05:21.445455+02', 'active', '{}', true) ON CONFLICT DO NOTHING; | ||
| INSERT INTO public.organization_members VALUES ('a0061a8e-7db7-4585-838c-3116a003dd21', 'bb640d07-ca8a-4869-b6bc-ae61ebb2fda1', '2022-11-02 13:05:21.447595+02', '2022-11-02 13:05:21.447595+02', '{}') ON CONFLICT DO NOTHING; | ||
| INSERT INTO public.user_links(user_id, login_type, linked_id, oauth_access_token) | ||
| VALUES('a0061a8e-7db7-4585-838c-3116a003dd21', 'github', '100', ''); |
There was a problem hiding this comment.
Will this trigger potential issues even with an empty token?
There was a problem hiding this comment.
Nah. If it does, that would be quite interesting.
It's in a fixture, so this never touches anything in a production environment.
Related to #11861
I am working on a solution to the issue above. Adding test fixtures to ensure any added constraints handle these cases.
Before I even do that, I want to make sure our test fixtures accurately represent real database possibilities.