Add organization ID, and remove FKs and sequences #2896
Conversation
dnephin
requested review from
ssoroka,
BruceMacD,
jmorganca and
pdevine
as code owners
dnephin
changed the title
| member, ok := model.(orgMember) | ||
| if !ok { | ||
| return | ||
| } |
There was a problem hiding this comment.
When we are ready to, we can make SetOrganizationID a required method, and implement no-op implementations on the other tables. That gives us the same safeguards as putting it in the base model, without adding unused organization_id fields.
* feat: add migration for org ids * maintain: make the linter happy
dnephin
force-pushed
the
dnephin/add-org-id
branch
3 times, most recently
from
2dc7e90
to
3d4691f
Compare
| @@ -17,6 +18,7 @@ func CreatePasswordResetToken(db *gorm.DB, user *models.Identity) (*models.Passw | |||
| } | |||
|
|
|||
| prt := &models.PasswordResetToken{ | |||
| ID: uid.New(), | |||
There was a problem hiding this comment.
I forgot to say in the commit message, but this was to fix a test failure. On the org_collab branch we removed all the required not-null for primary keys, so this was not failing I guess? I did not include that part, so we need to set an primary key for this type before saving it. It does not use Model, so it doesn't get set that way.
dnephin
force-pushed
the
dnephin/add-org-id
branch
from
3d4691f
to
fe133e9
Compare
Co-Authored-By: Steven Soroka <steven@infrahq.com>
dnephin
force-pushed
the
dnephin/add-org-id
branch
from
fe133e9
to
a8e7ea6
Compare
|
|
||
| ALTER SEQUENCE settings_id_seq OWNED BY settings.id; | ||
|
|
||
| ALTER TABLE ONLY access_keys ALTER COLUMN id SET DEFAULT nextval('access_keys_id_seq'::regclass); |
There was a problem hiding this comment.
Should we still have these alter table statements? We manually set the ID by they seem ok to have.
There was a problem hiding this comment.
I'll let @ssoroka comment as well, because I think he removed them originally.
I like removing them because we don't use them, and I think having these sequences could hide bugs from us in testing. I think it's better off if the column is set to NOT NULL without a sequence to force us to set values.
Co-Authored-By: Daniel Nephin <dnephin@infrahq.com>
To indicate lack of org scope. Cherry-picked from org_collab branch. Co-Authored-By: Steven Soroka <steven@infrahq.com>
until we remove that field.
Move SaveSettings out of the common helper, and instead move it to the two tests that need it.
dnephin
force-pushed
the
dnephin/add-org-id
branch
from
a8e7ea6
to
d117853
Compare
Summary
This PR cherry-picks 501cd0d, the data parts of 02cd02c, and a bit of 0d1faf9 for merging into main. Best viewed by individual commit.
I made a few changes as well:
ByOrgIDinstead ofByOrgOrganizationMemberso that tables that don't use it don't receive an org_idI had to cherry-pick a couple of changes from #2862 so that AutoMigrate wouldn't create the wrong schema.