Skip to content

chore: Update mongodbatlas_cloud_user_team_assignment resource to use new query param user_id #3574

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

Merged
merged 11 commits into from
Aug 11, 2025

Conversation

csanx
Copy link
Collaborator

@csanx csanx commented Aug 8, 2025

Description

  • Updated the logic in the Read operation in mongodbatlas_cloud_user_team_assignment resource to use new queryParam user_id to filter results in the "Return All MongoDB Cloud Users Assigned to One Team" endpoint.

Additionally,

  • Updated checkDestroy in resource_test to use user_id filter param.
  • Eliminated trivial test TestAccCloudUserTeamAssignmentDS_error which checked if a required attribute was missing.

Link to any related issue(s): CLOUDP-334100

Type of change:

  • Bug fix (non-breaking change which fixes an issue). Please, add the "bug" label to the PR.
  • New feature (non-breaking change which adds functionality). Please, add the "enhancement" label to the PR. A migration guide must be created or updated if the new feature will go in a major version.
  • Breaking change (fix or feature that would cause existing functionality to not work as expected). Please, add the "breaking change" label to the PR. A migration guide must be created or updated.
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contributing guides
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • If changes include deprecations or removals I have added appropriate changelog entries.
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Further comments

Fixed issue with test TestAccCloudUserTeamAssignment_basic. The import check was failing when running this test in parallel due to a diff in the number of teams assigned to the user between step 1 and 2. This issue only occurred in parallel execution with other tests and was not reproducible locally. Changed the test to run sequentially.

This replaces PR #3572 , which targeted the wrong branch.
All feedback from that PR has been addressed here.

@csanx csanx marked this pull request as ready for review August 8, 2025 09:26
@Copilot Copilot AI review requested due to automatic review settings August 8, 2025 09:26
@csanx csanx requested a review from a team as a code owner August 8, 2025 09:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Updates the mongodbatlas_cloud_user_team_assignment resource to utilize the new user_id query parameter for filtering in the MongoDB Cloud API endpoint. This change improves the efficiency of user lookups when fetching team assignments.

  • Refactored the Read operation to use query parameters instead of fetching all users and filtering client-side
  • Updated test utilities to use the new query parameter approach
  • Removed a trivial test case that checked for missing required attributes

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/service/clouduserteamassignment/resource.go Extracted common fetch logic into fetchTeamUser function using query parameters
internal/service/clouduserteamassignment/data_source.go Updated to use the new fetchTeamUser function instead of custom filtering logic
internal/service/clouduserteamassignment/resource_test.go Updated checkDestroy to use query parameters and removed trivial test case

}

userResp = &(userListResp.GetResults())[0]
userResp, found, err := fetchTeamUser(ctx, connV2, orgID, teamID, &userID, &username)
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we check for nulls here?

Copy link
Collaborator Author

@csanx csanx Aug 8, 2025

Choose a reason for hiding this comment

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

The previous instruction uses ValueString() which returns a known string value. If String is null or unknown, it return an empty string "". Therefore, these values won’t be null.

Comment on lines 132 to 135
userListUserID, _, err := conn.MongoDBCloudUsersApi.ListTeamUsersWithParams(ctx, userIDParams).Execute()
if userListUserID.HasResults() {
return fmt.Errorf("cloud user team assignment for user (%s) in team (%s) still exists %s", userID, teamID, err)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this and the code below be written only once like you did in fetchTeamUser ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In fact, the username part is not necessary since user_id is required and always available. I’m removing it.

OrgId: orgID,
TeamId: teamID,
}
} else if username != nil && *username != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I might have missed this, but just curious: can you explain why we try to do also the call with username and we don't just do it only with userID?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because, during the discussion of the posible migration paths, we decided to continue supporting both import keys: org_id/team_id/user_id and org_id/team_id/username.

Copy link
Member

@AgustinBettati AgustinBettati left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -73,73 +71,61 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou
resp.Diagnostics.Append(resp.State.Set(ctx, newUserTeamAssignmentModel)...)
}

func fetchTeamUser(ctx context.Context, connV2 *admin.APIClient, orgID, teamID string, userID, username *string) (*admin.OrgUserResponse, bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

q: any reason for having the bool found result? Can see it is redundant as same information is available with *admin.OrgUserResponse either being defined or nil.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right. I refactored that function 5bc1e3d

@csanx csanx merged commit ba78a8b into CLOUDP-320243-dev-2.0.0 Aug 11, 2025
43 checks passed
@csanx csanx deleted the CLOUDP-334100 branch August 11, 2025 06:22
oarbusi pushed a commit that referenced this pull request Aug 11, 2025
…se new query param `user_id` (#3574)

* Updated SDK

* Revert "Updated SDK"

This reverts commit f8d2d0a.

* Added userId as param to filter

* Changed to sequential test

* Fix

* Refactored Read method in resource and datasource

* Fixed CheckDestroy

* Removed redundant `found`parameter

* Fixed checkDestroy

---------

Co-authored-by: Cristina Sánchez Sánchez <cristina.sanchez@mongodb.com>
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.

4 participants