From 862c464f1428e1e63b06ddd4267f87157d75cd2d Mon Sep 17 00:00:00 2001 From: colin-stuart Date: Wed, 30 Jul 2025 16:29:34 -0500 Subject: [PATCH 1/4] if multiple organization users found, loop through results and return exact match on 'login' --- .../grafana/data_source_organization_user.go | 69 ++++++++--- .../data_source_organization_user_test.go | 111 ++++++++++++++++++ 2 files changed, 164 insertions(+), 16 deletions(-) diff --git a/internal/resources/grafana/data_source_organization_user.go b/internal/resources/grafana/data_source_organization_user.go index e05499429..ebcd2c60d 100644 --- a/internal/resources/grafana/data_source_organization_user.go +++ b/internal/resources/grafana/data_source_organization_user.go @@ -48,31 +48,68 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, GetPayload() []*models.UserLookupDTO } - emailOrLogin := d.Get("email").(string) - if emailOrLogin == "" { - emailOrLogin = d.Get("login").(string) - } - if emailOrLogin == "" { + email := d.Get("email").(string) + login := d.Get("login").(string) + + if email == "" && login == "" { return diag.Errorf("must specify one of email or login") } - params := org.NewGetOrgUsersForCurrentOrgLookupParams().WithQuery(&emailOrLogin) + // Use email if provided, otherwise use login + query := email + if query == "" { + query = login + } + + params := org.NewGetOrgUsersForCurrentOrgLookupParams().WithQuery(&query) resp, err := client.Org.GetOrgUsersForCurrentOrgLookup(params) if err != nil { return diag.FromErr(err) } - // Make sure that exactly 1 user was returned - if len(resp.GetPayload()) > 1 { - return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", emailOrLogin) - } else if len(resp.GetPayload()) == 0 { - return diag.Errorf("organization user not found with query: %q", emailOrLogin) + users := resp.GetPayload() + + // If no users found, return error + if len(users) == 0 { + return diag.Errorf("organization user not found with query: %q", query) } - user := resp.GetPayload()[0] - d.Set("user_id", user.UserID) - d.Set("login", user.Login) + // If exactly one user found, use it + if len(users) == 1 { + user := users[0] + d.Set("user_id", user.UserID) + d.Set("login", user.Login) + d.SetId(MakeOrgResourceID(orgID, user.UserID)) + return nil + } + + // Multiple users found - try to find exact match + var exactMatch *models.UserLookupDTO + + if login != "" { + // Look for exact login match + for _, user := range users { + if user.Login == login { + if exactMatch != nil { + // Multiple exact matches found (shouldn't happen with login) + return diag.Errorf("ambiguous query when reading organization user, multiple users with exact login match: %q", login) + } + exactMatch = user + } + } + } else if email != "" { + // For email queries, we can't do exact matching since UserLookupDTO doesn't have Email field + // Return the ambiguous error as before + return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) + } + + if exactMatch != nil { + d.Set("user_id", exactMatch.UserID) + d.Set("login", exactMatch.Login) + d.SetId(MakeOrgResourceID(orgID, exactMatch.UserID)) + return nil + } - d.SetId(MakeOrgResourceID(orgID, user.UserID)) - return nil + // No exact match found, return ambiguous error + return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) } diff --git a/internal/resources/grafana/data_source_organization_user_test.go b/internal/resources/grafana/data_source_organization_user_test.go index f2bd64bff..f0fc4b2b6 100644 --- a/internal/resources/grafana/data_source_organization_user_test.go +++ b/internal/resources/grafana/data_source_organization_user_test.go @@ -38,3 +38,114 @@ func TestAccDatasourceOrganizationUser_basic(t *testing.T) { }, }) } + +func TestAccDatasourceOrganizationUser_exactMatch(t *testing.T) { + testutils.CheckOSSTestsEnabled(t) + + var user1, user2 models.UserProfileDTO + checks := []resource.TestCheckFunc{ + userCheckExists.exists("grafana_user.test1", &user1), + userCheckExists.exists("grafana_user.test2", &user2), + // Test that exact login match works when multiple users are returned + resource.TestCheckResourceAttr( + "data.grafana_organization_user.exact_match", "login", "test-exact-match", + ), + resource.TestMatchResourceAttr( + "data.grafana_organization_user.exact_match", "user_id", common.IDRegexp, + ), + } + + resource.ParallelTest(t, resource.TestCase{ + ProtoV5ProviderFactories: testutils.ProtoV5ProviderFactories, + CheckDestroy: userCheckExists.destroyed(&user1, nil), + Steps: []resource.TestStep{ + { + Config: testAccDatasourceOrganizationUserExactMatch, + Check: resource.ComposeTestCheckFunc(checks...), + }, + }, + }) +} + +// TestDataSourceOrganizationUserExactMatchLogic tests the exact matching logic without requiring a Grafana instance +func TestDataSourceOrganizationUserExactMatchLogic(t *testing.T) { + // Test case 2: Multiple users returned, exact login match exists + usersMultiple := []*models.UserLookupDTO{ + { + UserID: 1, + Login: "test-exact-match", + }, + { + UserID: 2, + Login: "test-exact-match-other", + }, + } + + // Test case 3: Multiple users returned, no exact login match + usersNoExact := []*models.UserLookupDTO{ + { + UserID: 1, + Login: "test-exact-match-other1", + }, + { + UserID: 2, + Login: "test-exact-match-other2", + }, + } + + // These test cases verify the logic we implemented + // In a real scenario, the API would return these results and our code would process them + + // Test that we can identify exact matches + var exactMatch *models.UserLookupDTO + login := "test-exact-match" + + for _, user := range usersMultiple { + if user.Login == login { + if exactMatch != nil { + t.Fatal("Multiple exact matches found when there should only be one") + } + exactMatch = user + } + } + + if exactMatch == nil { + t.Fatal("Expected to find exact match but didn't") + } + + if exactMatch.UserID != 1 { + t.Fatalf("Expected UserID 1, got %d", exactMatch.UserID) + } + + // Test that we don't find exact matches when they don't exist + exactMatch = nil + for _, user := range usersNoExact { + if user.Login == login { + exactMatch = user + } + } + + if exactMatch != nil { + t.Fatal("Expected no exact match but found one") + } +} + +const testAccDatasourceOrganizationUserExactMatch = ` +resource "grafana_user" "test1" { + email = "test1.exact@example.com" + name = "Test Exact Match 1" + login = "test-exact-match" + password = "my-password" +} + +resource "grafana_user" "test2" { + email = "test2.exact@example.com" + name = "Test Exact Match 2" + login = "test-exact-match-other" + password = "my-password" +} + +data "grafana_organization_user" "exact_match" { + login = grafana_user.test1.login +} +` From a79d4b5c5fc9f0b969088d5c0d81af87a57f8ca5 Mon Sep 17 00:00:00 2001 From: colin-stuart Date: Wed, 30 Jul 2025 16:37:16 -0500 Subject: [PATCH 2/4] update tests --- internal/resources/grafana/data_source_organization_user.go | 5 ++--- .../resources/grafana/data_source_organization_user_test.go | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/resources/grafana/data_source_organization_user.go b/internal/resources/grafana/data_source_organization_user.go index ebcd2c60d..4467b0885 100644 --- a/internal/resources/grafana/data_source_organization_user.go +++ b/internal/resources/grafana/data_source_organization_user.go @@ -98,8 +98,7 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, } } } else if email != "" { - // For email queries, we can't do exact matching since UserLookupDTO doesn't have Email field - // Return the ambiguous error as before + // For email queries, we can't do exact matching since UserLookupDTO doesn't have Email field, return error return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) } @@ -110,6 +109,6 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, return nil } - // No exact match found, return ambiguous error + // No exact match found, return error return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) } diff --git a/internal/resources/grafana/data_source_organization_user_test.go b/internal/resources/grafana/data_source_organization_user_test.go index f0fc4b2b6..d4312075b 100644 --- a/internal/resources/grafana/data_source_organization_user_test.go +++ b/internal/resources/grafana/data_source_organization_user_test.go @@ -93,9 +93,6 @@ func TestDataSourceOrganizationUserExactMatchLogic(t *testing.T) { }, } - // These test cases verify the logic we implemented - // In a real scenario, the API would return these results and our code would process them - // Test that we can identify exact matches var exactMatch *models.UserLookupDTO login := "test-exact-match" From 8171106c6b601a5db4b661d3a3fce6bcc30a22e5 Mon Sep 17 00:00:00 2001 From: Victor Cinaglia Date: Wed, 6 Aug 2025 16:45:34 -0300 Subject: [PATCH 3/4] fetch users via GetOrgUsersForCurrentOrg which returns email addresses --- .../grafana/data_source_organization_user.go | 78 +++++-------- .../data_source_organization_user_test.go | 108 +++++------------- 2 files changed, 53 insertions(+), 133 deletions(-) diff --git a/internal/resources/grafana/data_source_organization_user.go b/internal/resources/grafana/data_source_organization_user.go index 4467b0885..6227c04c7 100644 --- a/internal/resources/grafana/data_source_organization_user.go +++ b/internal/resources/grafana/data_source_organization_user.go @@ -45,70 +45,46 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, client, orgID := OAPIClientFromNewOrgResource(meta, d) var resp interface { - GetPayload() []*models.UserLookupDTO + GetPayload() []*models.OrgUserDTO } - email := d.Get("email").(string) - login := d.Get("login").(string) - - if email == "" && login == "" { - return diag.Errorf("must specify one of email or login") + matchBy := matchByEmail + emailOrLogin := d.Get("email").(string) + if emailOrLogin == "" { + emailOrLogin = d.Get("login").(string) + matchBy = matchByLogin } - - // Use email if provided, otherwise use login - query := email - if query == "" { - query = login + if emailOrLogin == "" { + return diag.Errorf("must specify one of email or login") } - params := org.NewGetOrgUsersForCurrentOrgLookupParams().WithQuery(&query) - resp, err := client.Org.GetOrgUsersForCurrentOrgLookup(params) + params := org.NewGetOrgUsersForCurrentOrgParams().WithQuery(&emailOrLogin) + resp, err := client.Org.GetOrgUsersForCurrentOrg(params) if err != nil { return diag.FromErr(err) } - users := resp.GetPayload() - - // If no users found, return error - if len(users) == 0 { - return diag.Errorf("organization user not found with query: %q", query) - } - - // If exactly one user found, use it - if len(users) == 1 { - user := users[0] - d.Set("user_id", user.UserID) - d.Set("login", user.Login) - d.SetId(MakeOrgResourceID(orgID, user.UserID)) - return nil + if len(resp.GetPayload()) == 0 { + return diag.Errorf("organization user not found with query: %q", emailOrLogin) } - // Multiple users found - try to find exact match - var exactMatch *models.UserLookupDTO - - if login != "" { - // Look for exact login match - for _, user := range users { - if user.Login == login { - if exactMatch != nil { - // Multiple exact matches found (shouldn't happen with login) - return diag.Errorf("ambiguous query when reading organization user, multiple users with exact login match: %q", login) - } - exactMatch = user - } + for _, user := range resp.GetPayload() { + if matchBy(user, emailOrLogin) { + d.Set("user_id", user.UserID) + d.Set("login", user.Login) + d.Set("email", user.Email) + d.SetId(MakeOrgResourceID(orgID, user.UserID)) + return nil } - } else if email != "" { - // For email queries, we can't do exact matching since UserLookupDTO doesn't have Email field, return error - return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) } - if exactMatch != nil { - d.Set("user_id", exactMatch.UserID) - d.Set("login", exactMatch.Login) - d.SetId(MakeOrgResourceID(orgID, exactMatch.UserID)) - return nil - } + return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", emailOrLogin) +} + +func matchByEmail(user *models.OrgUserDTO, email string) bool { + return user.Email == email +} - // No exact match found, return error - return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", query) +func matchByLogin(user *models.OrgUserDTO, login string) bool { + return user.Login == login } diff --git a/internal/resources/grafana/data_source_organization_user_test.go b/internal/resources/grafana/data_source_organization_user_test.go index d4312075b..6b5467a4a 100644 --- a/internal/resources/grafana/data_source_organization_user_test.go +++ b/internal/resources/grafana/data_source_organization_user_test.go @@ -39,110 +39,54 @@ func TestAccDatasourceOrganizationUser_basic(t *testing.T) { }) } -func TestAccDatasourceOrganizationUser_exactMatch(t *testing.T) { +func TestAccDatasourceOrganizationUser_disambiguation(t *testing.T) { testutils.CheckOSSTestsEnabled(t) var user1, user2 models.UserProfileDTO checks := []resource.TestCheckFunc{ - userCheckExists.exists("grafana_user.test1", &user1), - userCheckExists.exists("grafana_user.test2", &user2), - // Test that exact login match works when multiple users are returned - resource.TestCheckResourceAttr( - "data.grafana_organization_user.exact_match", "login", "test-exact-match", - ), - resource.TestMatchResourceAttr( - "data.grafana_organization_user.exact_match", "user_id", common.IDRegexp, - ), + userCheckExists.exists("grafana_user.user1", &user1), + userCheckExists.exists("grafana_user.user2", &user2), + resource.TestCheckResourceAttr("data.grafana_organization_user.from_email", "login", "login1"), + resource.TestCheckResourceAttr("data.grafana_organization_user.from_email", "email", "test@example.com"), + resource.TestCheckResourceAttr("data.grafana_organization_user.from_login", "login", "log"), + resource.TestCheckResourceAttr("data.grafana_organization_user.from_login", "email", "test@example.com~"), } resource.ParallelTest(t, resource.TestCase{ ProtoV5ProviderFactories: testutils.ProtoV5ProviderFactories, - CheckDestroy: userCheckExists.destroyed(&user1, nil), + CheckDestroy: resource.ComposeTestCheckFunc( + userCheckExists.destroyed(&user1, nil), + userCheckExists.destroyed(&user2, nil), + ), Steps: []resource.TestStep{ { - Config: testAccDatasourceOrganizationUserExactMatch, + Config: testAccDatasourceOrganizationUserDisambiguation, Check: resource.ComposeTestCheckFunc(checks...), }, }, }) } -// TestDataSourceOrganizationUserExactMatchLogic tests the exact matching logic without requiring a Grafana instance -func TestDataSourceOrganizationUserExactMatchLogic(t *testing.T) { - // Test case 2: Multiple users returned, exact login match exists - usersMultiple := []*models.UserLookupDTO{ - { - UserID: 1, - Login: "test-exact-match", - }, - { - UserID: 2, - Login: "test-exact-match-other", - }, - } - - // Test case 3: Multiple users returned, no exact login match - usersNoExact := []*models.UserLookupDTO{ - { - UserID: 1, - Login: "test-exact-match-other1", - }, - { - UserID: 2, - Login: "test-exact-match-other2", - }, - } - - // Test that we can identify exact matches - var exactMatch *models.UserLookupDTO - login := "test-exact-match" - - for _, user := range usersMultiple { - if user.Login == login { - if exactMatch != nil { - t.Fatal("Multiple exact matches found when there should only be one") - } - exactMatch = user - } - } - - if exactMatch == nil { - t.Fatal("Expected to find exact match but didn't") - } - - if exactMatch.UserID != 1 { - t.Fatalf("Expected UserID 1, got %d", exactMatch.UserID) - } - - // Test that we don't find exact matches when they don't exist - exactMatch = nil - for _, user := range usersNoExact { - if user.Login == login { - exactMatch = user - } - } - - if exactMatch != nil { - t.Fatal("Expected no exact match but found one") - } +var testAccDatasourceOrganizationUserDisambiguation = ` +resource "grafana_user" "user1" { + email = "test@example.com" + name = "Test User 1" + login = "login1" + password = "my-password" } -const testAccDatasourceOrganizationUserExactMatch = ` -resource "grafana_user" "test1" { - email = "test1.exact@example.com" - name = "Test Exact Match 1" - login = "test-exact-match" +resource "grafana_user" "user2" { + email = "test@example.com~" + name = "Test User 1a" + login = "log" password = "my-password" } -resource "grafana_user" "test2" { - email = "test2.exact@example.com" - name = "Test Exact Match 2" - login = "test-exact-match-other" - password = "my-password" +data "grafana_organization_user" "from_email" { + email = grafana_user.user1.email } -data "grafana_organization_user" "exact_match" { - login = grafana_user.test1.login +data "grafana_organization_user" "from_login" { + login = grafana_user.user2.login } ` From 08570fd4622afb81b7469352dfe07097de13e8f4 Mon Sep 17 00:00:00 2001 From: Victor Cinaglia Date: Thu, 7 Aug 2025 10:50:09 -0300 Subject: [PATCH 4/4] feedback: tweak error message when users are not found --- internal/resources/grafana/data_source_organization_user.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/resources/grafana/data_source_organization_user.go b/internal/resources/grafana/data_source_organization_user.go index 6227c04c7..cd8f11a06 100644 --- a/internal/resources/grafana/data_source_organization_user.go +++ b/internal/resources/grafana/data_source_organization_user.go @@ -50,9 +50,11 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, matchBy := matchByEmail emailOrLogin := d.Get("email").(string) + searchType := "email" if emailOrLogin == "" { emailOrLogin = d.Get("login").(string) matchBy = matchByLogin + searchType = "login" } if emailOrLogin == "" { return diag.Errorf("must specify one of email or login") @@ -78,7 +80,7 @@ func dataSourceOrganizationUserRead(ctx context.Context, d *schema.ResourceData, } } - return diag.Errorf("ambiguous query when reading organization user, multiple users returned by query: %q", emailOrLogin) + return diag.Errorf("no organization user found with %s: %q (users returned: %d)", searchType, emailOrLogin, len(resp.GetPayload())) } func matchByEmail(user *models.OrgUserDTO, email string) bool {