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.

Sign up for GitHub

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

Jump to bottom

Search Organization tags by name #705

Closed
wants to merge 10 commits into from
Closed

Search Organization tags by name #705

wants to merge 10 commits into from

Conversation

sharathrnair87
Copy link
Contributor

@sharathrnair87 sharathrnair87 commented May 15, 2023 •
edited

Description

Added support for the query parameter q described here
https://developer.hashicorp.com/terraform/cloud-docs/api-docs/organization-tags#query-parameters

Testing plan

  1. code snippet
  for {
    log.Debugf("Processing page %d.\n", currentPage)
    options := &tfe.OrganizationTagsListOptions{
      ListOptions: tfe.ListOptions{
        PageNumber: currentPage,
        PageSize:   50,
      },
      Query: tagName, // Name of the tag as a search query string
    }

Output from tests

$ go test -run TestOrganizationTagsList -v ./...


=== RUN   TestOrganizationTagsList
=== RUN   TestOrganizationTagsList/with_no_query_params
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#01
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#02
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#03
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#04
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#05
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#06
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#07
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#08
=== RUN   TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#09
=== RUN   TestOrganizationTagsList/with_query_param_Filter
=== RUN   TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded
=== RUN   TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#01
=== RUN   TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#02
=== RUN   TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#03
=== RUN   TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#04
=== RUN   TestOrganizationTagsList/with_query_param
=== RUN   TestOrganizationTagsList/with_query_param/ensure_org_relation_is_properly_decoded
--- PASS: TestOrganizationTagsList (5.98s)
    --- PASS: TestOrganizationTagsList/with_no_query_params (0.31s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#01 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#02 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#03 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#04 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#05 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#06 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#07 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#08 (0.00s)
        --- PASS: TestOrganizationTagsList/with_no_query_params/ensure_org_relation_is_properly_decoded#09 (0.00s)
    --- PASS: TestOrganizationTagsList/with_query_param_Filter (0.30s)
        --- PASS: TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded (0.00s)
        --- PASS: TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#01 (0.00s)
        --- PASS: TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#02 (0.00s)
        --- PASS: TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#03 (0.00s)
        --- PASS: TestOrganizationTagsList/with_query_param_Filter/ensure_org_relation_is_properly_decoded#04 (0.00s)
    --- PASS: TestOrganizationTagsList/with_query_param_Quey (0.30s)
        --- PASS: TestOrganizationTagsList/with_query_param_Query/ensure_org_relation_is_properly_decoded (0.00s)
PASS
ok      github.com/hashicorp/go-tfe     5.985s
?       github.com/hashicorp/go-tfe/examples/configuration_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/organizations      [no test files]
?       github.com/hashicorp/go-tfe/examples/registry_modules   [no test files]
?       github.com/hashicorp/go-tfe/examples/users      [no test files]
?       github.com/hashicorp/go-tfe/examples/workspaces [no test files]
?       github.com/hashicorp/go-tfe/mocks       [no test files]

@sharathrnair87 sharathrnair87 requested a review from a team as a code owner May 15, 2023 08:41
@hashicorp-cla
Copy link

hashicorp-cla commented May 15, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@sharathrnair87 sharathrnair87 changed the title Feature/org tags search param Search Organization tags by name May 15, 2023
sebasslash
sebasslash requested changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@sebasslash sebasslash left a comment •
edited

Choose a reason for hiding this comment

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

👋 Thanks so much for your contribution! I apologize for having taken so long to review these changes -- Some minor feedback below. I'll go ahead and rebase your PR onto a local branch so the changes can run against CI once the feedback is addressed. 👍

organization_tags_integration_test.go Outdated
Comment on lines 99 to 107
assert.Equal(t, 1, len(tags.Items))

for _, tag := range tags.Items {
assert.Equal(t, tag.Name, testTagName)

t.Run("ensure org relation is properly decoded", func(t *testing.T) {
assert.NotNil(t, tag.Organization)
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use require.Len(t, tags.Items, 1) as there is no point in continuing the rest of this test if that is not true. For context, require will error and stop the execution of a test, whereas assert will not.

The second item here is the for loop is unnecessary as there should only be one item in the list. You also don't need a subtest for ensuring the Organization relation is not nil (but I can leave that at your discretion). You can simplify to:

assert.Equal(t, tags.Items[0].Name, testTagName)
assert.NotNil(t, tags.Items[0].Organization)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sebasslash, Thank you for taking the time to review this PR.
I have updated the tests with your feedback, please let me know if this looks good! Cheers!

@sebasslash
Copy link
Contributor

These changes are 👍 to merge 🚀 . Please see #720 which is the PR that will be merged, I will close this one.

@sebasslash sebasslash closed this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebasslash sebasslash Awaiting requested review from sebasslash

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sharathrnair87 @hashicorp-cla @sebasslash