feat(auth): Add ability to link a federated id with the UpdateUser() method. #344
Conversation
And inline the implementation.
This reverts commit ae498f7.
* chore: Added Actions-based release workflow * Set GOPATH * Fixed working directory for tests * Decrypting credentials into the testdata directory * Added preflight and post check scripts
|
Note the diffbase. I'll merge+retarget to dev once the bulk_get stuff lands. |
auth/user_mgt.go
Outdated
| if _, ok := req["email"]; ok { | ||
| // We could relax this to only return an error if the email addrs don't | ||
| // match. But for now, we'll be extra picky. | ||
| return nil, fmt.Errorf( |
There was a problem hiding this comment.
You can use errors.New() when the error message doesn't contain any formatting directives.
auth/user_mgt.go
Outdated
| // We could relax this to only return an error if the email addrs don't | ||
| // match. But for now, we'll be extra picky. | ||
| return nil, fmt.Errorf( | ||
| "Both UserToUpdate.Email and UserToUpdate.ProviderToLink.ProviderID='email' " + |
There was a problem hiding this comment.
These error messages do not exactly follow the Go convention: https://github.com/golang/go/wiki/Errors (i.e. start with a lowercase letter etc). But we also don't strictly adhere to those conventions. So I'd leave it to your judgement.
There was a problem hiding this comment.
I've fixed it. (Lower case, remove ending punctuation.) Hopefully throughout.
| // exists. | ||
| // TODO(rsgowman): This function was ported from node.js port; a number of tests | ||
| // there use this, but haven't been ported to go yet. Do so. | ||
| func deletePhoneNumberUser(t *testing.T, phoneNumber string) { |
There was a problem hiding this comment.
We really ought to stop passing t to helper functions. Instead return an error from this function, and handle it at the callsite.
There was a problem hiding this comment.
This pattern comes direct from go/go-style/best-practices#test-helper-error-handling.
I like it, since it reduces clutter at the call site for what's essentially a failed precondition. But I can change it to return an error if you feel strongly.
There was a problem hiding this comment.
Oh, that's good to know. I guess I'm always a bit weirded out by code that passes t around. But I suppose that's considered normal in Go.
| @@ -0,0 +1,105 @@ | |||
|
|
|||
There was a problem hiding this comment.
Please remove any changes to Actions workflow configs.
There was a problem hiding this comment.
Done. (Must've been a bad merge)
| // exists. | ||
| // TODO(rsgowman): This function was ported from node.js port; a number of tests | ||
| // there use this, but haven't been ported to go yet. Do so. | ||
| func deletePhoneNumberUser(t *testing.T, phoneNumber string) { |
There was a problem hiding this comment.
Oh, that's good to know. I guess I'm always a bit weirded out by code that passes t around. But I suppose that's considered normal in Go.
integration/auth/user_mgt_test.go
Outdated
| t.Fatal(err) | ||
| } | ||
|
|
||
| err = client.DeleteUser(context.Background(), userRecord.UID) |
There was a problem hiding this comment.
Error check can be inlined here.
hiranya911
changed the title
| return nil, err | ||
| } | ||
|
|
||
| // Although we don't really advertise it, we want to also handle linking of |
There was a problem hiding this comment.
Assuming that none of this comment gets parsed into the generated docs.
There was a problem hiding this comment.
No, this shouldn't make it into the public facing docs.
No description provided.