chore: move default everyone group to a migration #12435
Conversation
|
|
||
| _, err = q.InsertAllUsersGroup(context.Background(), defaultOrg.ID) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to create default group: %w", err)) | ||
| } | ||
|
|
There was a problem hiding this comment.
dbmem doesn't have the concept of "migrations". I wonder if we should have something similar rather than chucking them here.
There was a problem hiding this comment.
I don't think "migrations" is the right concept, since we don't ever change an old dbmem into a new one, but it might make sense to have an abstraction that populates the initial contents of the database, which is what this does...
|
|
||
| _, err = q.InsertAllUsersGroup(context.Background(), defaultOrg.ID) | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to create default group: %w", err)) | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't think "migrations" is the right concept, since we don't ever change an old dbmem into a new one, but it might make sense to have an abstraction that populates the initial contents of the database, which is what this does...
| 'Everyone', | ||
| -- Org ID and group ID must match. | ||
| (SELECT id FROM organizations WHERE is_default = true LIMIT 1), | ||
| (SELECT id FROM organizations WHERE is_default = true LIMIT 1) |
There was a problem hiding this comment.
Is LIMIT 1 required for SQL to correctly cast a rowset into a scaler? The unique index on is_default should ensure there is exactly one organization where it is true.
There was a problem hiding this comment.
It is not required, the unique index should protect this.
I added the LIMIT 1 for defensive programming in our migrations since they can prevent boot of Coderd.
No description provided.