-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[usage] Add listing of memberships by UserID #10547
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Tests look good, so good to merge if green. 🛹
{ | ||
ID: uuid.New(), | ||
TeamID: uuid.New(), | ||
UserID: uuid.New(), | ||
Role: db.TeamMembershipRole_Member, | ||
}, | ||
{ | ||
ID: uuid.New(), | ||
TeamID: uuid.New(), | ||
UserID: userWithTwoTeams, | ||
Role: db.TeamMembershipRole_Member, | ||
}, | ||
{ | ||
ID: uuid.New(), | ||
TeamID: uuid.New(), | ||
UserID: userWithTwoTeams, | ||
Role: db.TeamMembershipRole_Member, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this looks good, but I'm not sure why this works, given that the specified type for TeamMembership.SubscriptionID
is:
SubscriptionID uuid.UUID `gorm:"column:subscriptionId;type:char;size:36;" json:"subscriptionId"`
Does Go set this member to nil
by default? Also FYI, I think we'll want to change this type specification, because TeamMembership.SubscriptionID
can also be ""
(empty string -- its default value) [1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot. Will investigate what it stores the uninitialized values as. The nil value for a UUID is 0 as that's the nil value for a byte, it will depend on how the Value() interface for the DB is implemented. Will follow up on this.
Name: "no matching user IDs returns empty", | ||
UserIDs: []uuid.UUID{uuid.New()}, | ||
Expected: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Over-nit: This test has a non-zero chance of misfiring. 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UUIDs have 122 bits of entropy so the chance of two random UUIDs colliding is about 10^-37.
If you generate 2^46 UUIDs (approximately 1 petabyte of entropy) the chance of getting a collision is 1 in 50 billion.[1] You're 170 times more likely to win the jackpot in Powerball.[2]
ID: uuid.New(), | ||
TeamID: uuid.New(), | ||
UserID: userWithTwoTeams, | ||
Role: db.TeamMembershipRole_Member, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One role could also be owner, for some variety. 💭
Description
Adds implementation of listing memberships by User ID, needed to attribute workspace instances to a team.
This is used in sub-sequent PR
Related Issue(s)
Fixes #
How to test
Unit tests
Release Notes
Documentation
NONE