- Notifications
You must be signed in to change notification settings - Fork928
feat: Allow using username in user queries#1221
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedApr 29, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #1221 +/- ##==========================================+ Coverage 65.60% 65.70% +0.10%========================================== Files 269 269 Lines 17337 17365 +28 Branches 162 162 ==========================================+ Hits 11374 11410 +36+ Misses 4772 4767 -5+ Partials 1191 1188 -3
Continue to review full report at Codecov.
|
| }) | ||
| return | ||
| } | ||
| } else if userID, err := uuid.Parse(userQuery); err == nil { |
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.
It might reduce complexity if we just disallow getting by UUID entirely. I'm not sure it provides a ton of value anyways, because you need to get the user to get the ID. You could of course list users to do that, but then just use theusername.
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.
I'm not inclined to drop the functionality because usernamesmight be able to be changed. UUIDs never will.
It's really nice to have a ID, as you can assume that will be constant no matter what.
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.
Fair enough fair enough
| }) | ||
| return | ||
| } | ||
| } else if userID, err := uuid.Parse(userQuery); err == nil { |
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.
Fair enough fair enough
* feat: Allow using username in user queries* Test needs a username/email to not match empty string
Uh oh!
There was an error while loading.Please reload this page.
The
codersdkstill acceptsuuidfor all user params. I added aUserByUsernameontop onUser()func.I was thinking we should either use generics, or an interface for a
UserIdentifierand accept"me",uuid.UUID, orUsernameI was writing the generic version, but you can't add Generics to methods on structs. 😢