Skip to content
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

Add Name to the Oauth Client Payload #466

Merged
merged 1 commit into from Jul 15, 2022

Conversation

barrettclark
Copy link
Contributor

@barrettclark barrettclark commented Jul 15, 2022

Description

The oauth-clients endpoint returns the name attribute of each OAuth Client, but the OAuthClient struct doesn't expose this attribute. This pull request adds the Name field to the OAuthClient struct.

Fixes #461

Testing plan

This is how you see the name in the Oauth Client payload:

  1. Make sure you have a GitHub personal access token
  2. Make sure you have a TF API token that has admin access
  3. Configure a GitHub Oauth linkage. If you give the Oauth linkage a name, you will see that name in the API response in the next step.
  4. Now you can fetch the list of OAuth Clients from the API

In order to run the integration test you also need to do the first two steps from above.

External links

Output from tests

Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.

$ GITHUB_TOKEN=$GITHUB_TOKEN TF_ACC=1 TFE_TOKEN=$TFE_TOKEN TFE_ADDRESS=$TFE_ADDRESS go test -v -run TestOAuthClientsCreate ./... --tags=integration

@barrettclark barrettclark force-pushed the barrettclark/add-name-to-oauth-client branch from 95c75bf to d1b630f Compare Jul 15, 2022
@barrettclark barrettclark requested a review from a team Jul 15, 2022
Copy link
Contributor

@sebasslash sebasslash left a comment

Really minor ⬇️ . Don't forget to include a changelog entry and given the OAuthClient interface has changed, you'll want to run ./generate_mocks.sh.

oauth_client.go Outdated
@@ -73,6 +73,7 @@ type OAuthClient struct {
HTTPURL string `jsonapi:"attr,http-url"`
Key string `jsonapi:"attr,key"`
RSAPublicKey string `jsonapi:"attr,rsa-public-key"`
Name string `jsonapi:"attr,name"`
Copy link
Contributor

@sebasslash sebasslash Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the default value of this field is null, we should make this a pointer.

@@ -109,6 +109,7 @@ func TestOAuthClientsCreate(t *testing.T) {
oc, err := client.OAuthClients.Create(ctx, orgTest.Name, options)
require.NoError(t, err)
assert.NotEmpty(t, oc.ID)
assert.Empty(t, oc.Name)
Copy link
Contributor

@sebasslash sebasslash Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert.Nil() 💅

Copy link
Contributor Author

@barrettclark barrettclark Jul 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - when I make Name a pointer it does actually come through as null rather than "".

@barrettclark barrettclark force-pushed the barrettclark/add-name-to-oauth-client branch from d1b630f to 1a4f8aa Compare Jul 15, 2022
Copy link
Contributor

@sebasslash sebasslash left a comment

Looks good to me 👍 -- I was incorrect though, you only need to regenerate mocks when a method signature changes.

@barrettclark barrettclark merged commit 954defe into main Jul 15, 2022
6 checks passed
@barrettclark barrettclark deleted the barrettclark/add-name-to-oauth-client branch Jul 15, 2022
@github-actions
Copy link

github-actions bot commented Jul 15, 2022

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants