feat: Adds description field to `mongodbatlas_database_user` #3280

EspenAlbert · 2025-04-14T15:36:34Z

Description

Adds description field to mongodbatlas_database_user

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

Considerations

Choosing optional only attribute
- Implies extra logic to align config value with API value since null != "" in TPF. (Similar to recent resource_policy implementation)
- Also, there is no way of setting the description to null in the SDK. Once it is set, it will always be "" since we cannot dump null (to be discussed based on this example.)
Will handle test refactoring in a follow-up issue: https://jira.mongodb.org/browse/CLOUDP-312885
Choosing to keep model_database_user_test.go simple, all paths are covered in acceptance tests.
- state null plan null
- state some value plan null
- state null plan not-null
- state set and plan set

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

…se_user`

github-actions · 2025-04-16T09:41:58Z

APIx bot: a message has been sent to Docs Slack channel

internal/service/databaseuser/data_source_database_user_test.go

lantoli · 2025-04-16T09:51:36Z

internal/service/databaseuser/model_database_user.go

-		result.Password = dbUserModel.Password.ValueStringPointer()
+		result.Password = plan.Password.ValueStringPointer()
+	}
+	if plan.Description.IsNull() && !stateDescriptionValue.Equal(plan.Description) {


just to confirm, what happens if we don't have this condition?

wouldn't result.Description be "" in this case because of previous: Description: plan.Description.ValueStringPointer(), ?

No, ValueStringPointer would be nil and therefore not dumped.
Cannot use ValueString() since we don't want to set it to "" when it is not defined.

lantoli · 2025-04-16T09:53:17Z

internal/service/databaseuser/resource_database_user.go

@@ -204,28 +208,28 @@ func (r *databaseUserRS) Schema(ctx context.Context, req resource.SchemaRequest,
 }

 func (r *databaseUserRS) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {


nit: maybe in a follow-up PR we want to unify RS and DS tests in the same test file and tests

See PR description

lantoli · 2025-04-16T09:54:26Z

internal/service/databaseuser/resource_database_user.go

 	resp.Diagnostics.Append(diags...)
 	if resp.Diagnostics.HasError() {
 		return
 	}

-	dbUserReq, d := NewMongoDBDatabaseUser(ctx, types.StringNull(), databaseUserPlan)
+	dbUserReq, d := NewMongoDBDatabaseUser(ctx, types.StringNull(), types.StringNull(), plan)


nit: use diags or localDiags as in other places

Thanks. Changed in 27168d9

lantoli · 2025-04-16T09:56:04Z

internal/service/databaseuser/resource_database_user.go


 	// Use the ID only with the IMPORT operation
-	if databaseUserState.ID.ValueString() != "" && (username == "" || projectID == "" || authDatabaseName == "") {
-		projectID, username, authDatabaseName, err = SplitDatabaseUserImportID(databaseUserState.ID.ValueString())
+	if state.ID.ValueString() != "" && (username == "" || projectID == "" || authDatabaseName == "") {


I understand this change is not related to this PR, but wondering why we have a different Read logic when importing, instead of Import setting the attributes we need in Read so we don't need to do a special case in Read for import

Wow, yeah you are right. Moved to Import function in e860fa9

oarbusi · 2025-04-16T09:56:50Z

internal/service/databaseuser/model_database_user_test.go

@@ -65,6 +65,7 @@ var (
 		GroupId:      projectID,
 		DatabaseName: authDatabaseName,
 		Username:     username,
+		Description:  conversion.Pointer(""),


I would add a var emptyDescription = "" and then here use &emptyDescription

Not sure, I feel this is more readable. With an intermediate var you would need to check that var. Here it is all in one line

IMO it's more readable with the var but I don't have a strong opinion

oarbusi · 2025-04-16T10:00:19Z

internal/service/databaseuser/resource_database_user_test.go

 					resource.TestCheckResourceAttr(resourceName, "password", "test-acc-password"),
 					resource.TestCheckResourceAttr(resourceName, "auth_database_name", "admin"),
 					resource.TestCheckResourceAttr(resourceName, "labels.#", "2"),
 				),
 			},
 			{
-				Config: acc.ConfigDatabaseUserWithLabels(projectID, username, "read",
+				Config: acc.ConfigDatabaseUserWithLabels(projectID, username, "read", "",


oarbusi

LGTM

AgustinBettati

one doubt otherwise LGTM

internal/service/databaseuser/model_database_user.go

… is null

AgustinBettati

LGTM

AgustinBettati · 2025-04-16T16:31:16Z

internal/service/databaseuser/model_database_user.go

+	if inModel != nil && outModel.Description.Equal(types.StringValue("")) && inModel.Description.IsNull() {
 		// null != "" in TPF:  Error: Provider produced inconsistent result after apply. .description: was null, but now cty.StringVal("")
-		databaseUserModel.Description = model.Description
+		outModel.Description = types.StringNull()
 	}
-
-	return databaseUserModel, nil
+	return outModel, nil


nice way of keeping it readable 👍

kanchana-mongodb

LGTM

EspenAlbert added 7 commits April 14, 2025 16:03

feat: Adds description field to mongodbatlas_database_user

7ee2b09

test: Adds description test for database user

21482d4

test: Update unit tests

6bbb2f8

refactor: Support not sending empty description on create

8c907fa

update tests

6a21451

feat: Adds description field to data sources for `mongodbatlas_databa…

09be40c

…se_user`

chore: Add changelog entry

a666009

github-actions bot added the enhancement label Apr 14, 2025

EspenAlbert added 2 commits April 14, 2025 16:42

doc: Document new description field

474922b

refactor: rename TfDatabaseUserModel variables

2d9a991

EspenAlbert marked this pull request as ready for review April 16, 2025 09:41

EspenAlbert requested review from a team as code owners April 16, 2025 09:41

oarbusi reviewed Apr 16, 2025

View reviewed changes

internal/service/databaseuser/data_source_database_user_test.go Show resolved Hide resolved

lantoli reviewed Apr 16, 2025

View reviewed changes

lantoli approved these changes Apr 16, 2025

View reviewed changes

oarbusi reviewed Apr 16, 2025

View reviewed changes

oarbusi approved these changes Apr 16, 2025

View reviewed changes

marcosuma approved these changes Apr 16, 2025

View reviewed changes

EspenAlbert added 2 commits April 16, 2025 15:02

rename to local diags

27168d9

move importID split to ImportState function

e860fa9

AgustinBettati reviewed Apr 16, 2025

View reviewed changes

internal/service/databaseuser/model_database_user.go Outdated Show resolved Hide resolved

fix: only switch back to state value when response is empty and state…

6a6b44f

… is null

AgustinBettati approved these changes Apr 16, 2025

View reviewed changes

kanchana-mongodb approved these changes Apr 21, 2025

View reviewed changes

EspenAlbert merged commit f7d100e into master Apr 21, 2025
40 checks passed

EspenAlbert deleted the CLOUDP-288713_db_user_description branch April 21, 2025 14:36

svc-apix-Bot added a commit that referenced this pull request Apr 21, 2025

chore: Updates CHANGELOG.md for #3280

77af139

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds description field to `mongodbatlas_database_user` #3280

feat: Adds description field to `mongodbatlas_database_user` #3280

EspenAlbert commented Apr 14, 2025 •

edited

Loading

github-actions bot commented Apr 16, 2025

lantoli Apr 16, 2025

EspenAlbert Apr 16, 2025

lantoli Apr 16, 2025

lantoli Apr 16, 2025

EspenAlbert Apr 16, 2025

lantoli Apr 16, 2025

lantoli Apr 16, 2025

EspenAlbert Apr 16, 2025

lantoli Apr 16, 2025

EspenAlbert Apr 16, 2025

oarbusi Apr 16, 2025

EspenAlbert Apr 16, 2025 •

edited

Loading

oarbusi Apr 16, 2025

oarbusi Apr 16, 2025

oarbusi left a comment

AgustinBettati left a comment

AgustinBettati left a comment

AgustinBettati Apr 16, 2025

kanchana-mongodb left a comment

		@@ -204,28 +208,28 @@ func (r *databaseUserRS) Schema(ctx context.Context, req resource.SchemaRequest,
		}

		func (r databaseUserRS) Create(ctx context.Context, req resource.CreateRequest, resp resource.CreateResponse) {

feat: Adds description field to mongodbatlas_database_user #3280

feat: Adds description field to mongodbatlas_database_user #3280

Conversation

EspenAlbert commented Apr 14, 2025 • edited Loading

Description

Considerations

Type of change:

Required Checklist:

Further comments

github-actions bot commented Apr 16, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EspenAlbert Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oarbusi left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

AgustinBettati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanchana-mongodb left a comment

Choose a reason for hiding this comment

feat: Adds description field to `mongodbatlas_database_user` #3280

feat: Adds description field to `mongodbatlas_database_user` #3280

EspenAlbert commented Apr 14, 2025 •

edited

Loading

EspenAlbert Apr 16, 2025 •

edited

Loading