feat: fetch user by email #263
Conversation
| return | ||
| } | ||
| createAPITokenRequest.ExpiresAt = &expire | ||
| createAPITokenRequest.SetExpiresAt(expire) |
There was a problem hiding this comment.
What happened here? Did our API change? Is this now optional?
There was a problem hiding this comment.
v0.6.0 of the generated API client provides setter helpers for optional fields like expiresAt, and using the helper preserves the behaviour of the previous pointer assignment: Unleash/unleash-server-api-go@87392b0#diff-4f4e55d02e361836eb5485af7002e491e934b9742cfea3df8bcce513bf3c81d9R87-R89
| } | ||
| user = apiUser | ||
|
|
||
| if emailProvided { |
There was a problem hiding this comment.
Struggling a bit with the nesting here. Not sure but I think flattening this if statement may help to make this monster a bit more readble. Tempted to say the conditions for the if statement can just be a single well named function because I also can't spot at a glance what this case represents
There was a problem hiding this comment.
f9f3dee should flatten this a bit :D
|
|
||
| // Read refreshes the Terraform state with the latest data. | ||
| func (d *userDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { | ||
| tflog.Debug(ctx, "Preparing to read user data source") |
There was a problem hiding this comment.
This could really benefit from being broken up into smaller functions, this beast is over a hundred lines long now. At a quick glance I see 3 responsibilities here:
- An initial quick check of the config to check for an early exit
- Fetching a user by whatever property
- Converting that data into a terraform response
There was a problem hiding this comment.
Agree: 435f130 should make it more readable
| idProvided := !config.Id.IsNull() && config.Id.ValueString() != "" | ||
| emailProvided := !config.Email.IsNull() && config.Email.ValueString() != "" | ||
|
|
||
| if !idProvided && !emailProvided { |
There was a problem hiding this comment.
It's a shame to do this check at runtime, a config validator might let us push this to plan time. Something like this?
func (d *userDataSource) ConfigValidators(_ context.Context) []datasource.ConfigValidator {
return []datasource.ConfigValidator{
datasourcevalidator.AtLeastOneOf(
path.MatchRoot("id"),
path.MatchRoot("email"),
),
}
}
No idea if this will work on our version of tf, it's based off a quick search
There was a problem hiding this comment.
Interesting! Let me dig into it
There was a problem hiding this comment.
Ok, this requires terraform-plugin-framework-validators dependency... I'm validating the approach
There was a problem hiding this comment.
@sighphyre wdyt of this: #265
github-project-automation
bot
moved this from In Progress
to Approved PRs
in Issues and PRs
2 participants
About the changes
Adds the ability to search for a user by email when using the user data source. This simplifies referring to a user when their ID is unknown.
Fixes #253