describe diff better

broadinstitute · Sep 30, 2024 · 4f887c4 · 4f887c4
1 parent fe2e029
commit 4f887c4
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 5 deletions.
diff --git a/sherlock/internal/role_propagation/propagation_engines/azure_invited_account.go b/sherlock/internal/role_propagation/propagation_engines/azure_invited_account.go
@@ -309,8 +309,12 @@ func (a *AzureInvitedAccountEngine) Update(ctx context.Context, _ bool, identifi
 }
 
 func (a *AzureInvitedAccountEngine) describeDiff(oldFields AzureInvitedAccountFields, newFields AzureInvitedAccountFields) string {
-	if oldFields.Email != newFields.Email || oldFields.MailNickname != newFields.MailNickname || !reflect.DeepEqual(oldFields.OtherMails, newFields.OtherMails) {
-		return "update account email info" // This is really, *really* unlikely to happen but we'll at least handle it
+	if oldFields.Email != newFields.Email {
+		return fmt.Sprintf("update email from `%s` to `%s`", oldFields.Email, newFields.Email)
+	} else if oldFields.MailNickname != newFields.MailNickname {
+		return fmt.Sprintf("update mail nickname from `%s` to `%s`", oldFields.MailNickname, newFields.MailNickname)
+	} else if !reflect.DeepEqual(oldFields.OtherMails, newFields.OtherMails) {
+		return fmt.Sprintf("update other emails from `%v` to `%v`", oldFields.OtherMails, newFields.OtherMails)
 	} else if oldFields.DisplayName != newFields.DisplayName {
 		return fmt.Sprintf("update display name from `%s` to `%s`", oldFields.DisplayName, newFields.DisplayName)
 	} else {

diff --git a/sherlock/internal/role_propagation/propagation_engines/azure_invited_account_test.go b/sherlock/internal/role_propagation/propagation_engines/azure_invited_account_test.go
@@ -346,7 +346,7 @@ func TestAzureInvitedAccountEngine_describeDiff(t *testing.T) {
 				MailNickname: "baz",
 				OtherMails:   []string{"qux"},
 			},
-			want: "update account email info",
+			want: "update email from `foo` to `bar`",
 		},
 		{
 			name: "mail nickname",
@@ -362,7 +362,7 @@ func TestAzureInvitedAccountEngine_describeDiff(t *testing.T) {
 				MailNickname: "bar",
 				OtherMails:   []string{"qux"},
 			},
-			want: "update account email info",
+			want: "update mail nickname from `foo` to `bar`",
 		},
 		{
 			name: "other mails",
@@ -378,7 +378,7 @@ func TestAzureInvitedAccountEngine_describeDiff(t *testing.T) {
 				MailNickname: "baz",
 				OtherMails:   []string{"bar"},
 			},
-			want: "update account email info",
+			want: "update other emails from `[foo]` to `[bar]`",
 		},
 		{
 			name: "name change",