From 60936cc08f994484fa9ff594425698a4d6ef29c6 Mon Sep 17 00:00:00 2001 From: Tim Schindler Date: Thu, 19 Feb 2026 18:12:24 +0100 Subject: [PATCH] fix: resolve directory names for groups in favorites add favorites add was calling ListGroupsEligibility directly, bypassing the directory name enrichment that fetchGroupsEligibility provides. Groups displayed as "Group: X (azure)" instead of "Directory: Y / Group: X (azure)". - Replace raw ListGroupsEligibility calls with fetchGroupsEligibility in both the unified interactive path and the --type groups path - Add eligLister param to addGroupFavorite for buildDirectoryNameMap - Add nil guard to buildDirectoryNameMap for graceful degradation - Add tests verifying directory name enrichment and nil eligLister fallback --- CHANGELOG.md | 1 + cmd/favorites.go | 26 +++++++++++-------------- cmd/favorites_test.go | 45 ++++++++++++++++++++++++++++++++++++++++++- cmd/helpers.go | 3 +++ 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8a6508..53c365e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ All notable changes to this project will be documented in this file. - Group favorites now verify DirectoryID, preventing wrong-group elevation when multiple directories have identically-named groups - `grant status` now resolves directory names for group sessions via `buildDirectoryNameMap` - `buildDirectoryNameMap` now handles nil eligibility response gracefully +- `grant favorites add` now resolves directory names for groups, matching root command display (`Directory: X / Group: Y`) ### Removed diff --git a/cmd/favorites.go b/cmd/favorites.go index b3ac916..65ec6e8 100644 --- a/cmd/favorites.go +++ b/cmd/favorites.go @@ -7,7 +7,6 @@ import ( survey "github.com/Iilun/survey/v2" "github.com/aaearon/grant-cli/internal/config" - scamodels "github.com/aaearon/grant-cli/internal/sca/models" "github.com/spf13/cobra" ) @@ -219,7 +218,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi // Groups flow if favType == config.FavoriteTypeGroups { - return addGroupFavorite(cmd, name, group, cfg, cfgPath, groupsElig, sel, prompter) + return addGroupFavorite(cmd, name, group, cfg, cfgPath, groupsElig, eligLister, sel, prompter) } // Cloud flow @@ -247,12 +246,12 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi items = append(items, selectionItem{kind: selectionCloud, cloud: &allTargets[i]}) } - // Fetch groups eligibility (best-effort) + // Fetch groups eligibility (best-effort, enriched with directory names) if groupsElig != nil { - eligResp, gErr := groupsElig.ListGroupsEligibility(ctx, scamodels.CSPAzure) - if gErr == nil && len(eligResp.Response) > 0 { - for i := range eligResp.Response { - items = append(items, selectionItem{kind: selectionGroup, group: &eligResp.Response[i]}) + groups, gErr := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr()) + if gErr == nil { + for i := range groups { + items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) } } } @@ -310,7 +309,7 @@ func runFavoritesAddWithDeps(cmd *cobra.Command, args []string, eligLister eligi } // addGroupFavorite handles the --type groups flow for favorites add. -func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config, cfgPath string, groupsElig groupsEligibilityLister, sel unifiedSelector, prompter namePrompter) error { +func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config, cfgPath string, groupsElig groupsEligibilityLister, eligLister eligibilityLister, sel unifiedSelector, prompter namePrompter) error { var fav config.Favorite fav.Type = config.FavoriteTypeGroups fav.Provider = "azure" @@ -323,17 +322,14 @@ func addGroupFavorite(cmd *cobra.Command, name, group string, cfg *config.Config ctx, cancel := context.WithTimeout(context.Background(), apiTimeout) defer cancel() - eligResp, err := groupsElig.ListGroupsEligibility(ctx, scamodels.CSPAzure) + groups, err := fetchGroupsEligibility(ctx, groupsElig, eligLister, cmd.ErrOrStderr()) if err != nil { - return fmt.Errorf("failed to fetch eligible groups: %w", err) - } - if len(eligResp.Response) == 0 { - return fmt.Errorf("no eligible groups found") + return err } var items []selectionItem - for i := range eligResp.Response { - items = append(items, selectionItem{kind: selectionGroup, group: &eligResp.Response[i]}) + for i := range groups { + items = append(items, selectionItem{kind: selectionGroup, group: &groups[i]}) } selected, err := sel.SelectItem(items) diff --git a/cmd/favorites_test.go b/cmd/favorites_test.go index eb1b443..a36d1a9 100644 --- a/cmd/favorites_test.go +++ b/cmd/favorites_test.go @@ -733,6 +733,7 @@ func TestFavoritesAddGroupFavorite(t *testing.T) { tests := []struct { name string setupConfig func(string) + eligLister eligibilityLister groupsElig groupsEligibilityLister selector unifiedSelector namePrompter namePrompter @@ -796,6 +797,17 @@ func TestFavoritesAddGroupFavorite(t *testing.T) { cfg := config.DefaultConfig() _ = config.Save(cfg, path) }, + eligLister: &mockEligibilityLister{ + response: &models.EligibilityResponse{ + Response: []models.EligibleTarget{ + { + WorkspaceID: "dir-1", + WorkspaceName: "CyberIAM Tech Labs", + WorkspaceType: models.WorkspaceTypeDirectory, + }, + }, + }, + }, groupsElig: &mockGroupsEligibilityLister{ response: &models.GroupsEligibilityResponse{ Response: []models.GroupsEligibleTarget{ @@ -812,6 +824,37 @@ func TestFavoritesAddGroupFavorite(t *testing.T) { return nil, errors.New("expected only group items for --type groups") } } + // Verify directory name was enriched + if items[0].group.DirectoryName != "CyberIAM Tech Labs" { + return nil, errors.New("expected DirectoryName to be enriched") + } + return &items[0], nil + }, + }, + args: []string{"my-grp", "--type", "groups"}, + wantContain: []string{"Added favorite", "my-grp", "groups/Engineering"}, + wantErr: false, + }, + { + name: "interactive - works without eligLister (no directory enrichment)", + setupConfig: func(path string) { + cfg := config.DefaultConfig() + _ = config.Save(cfg, path) + }, + groupsElig: &mockGroupsEligibilityLister{ + response: &models.GroupsEligibilityResponse{ + Response: []models.GroupsEligibleTarget{ + {DirectoryID: "dir-1", GroupID: "grp-1", GroupName: "Engineering"}, + }, + Total: 1, + }, + }, + selector: &mockUnifiedSelector{ + selectFunc: func(items []selectionItem) (*selectionItem, error) { + // Directory name should be empty when no eligLister + if items[0].group.DirectoryName != "" { + return nil, errors.New("expected empty DirectoryName without eligLister") + } return &items[0], nil }, }, @@ -840,7 +883,7 @@ func TestFavoritesAddGroupFavorite(t *testing.T) { tt.setupConfig(configPath) rootCmd := newTestRootCommand() - favCmd := NewFavoritesCommandWithAllDeps(nil, tt.selector, tt.namePrompter, tt.groupsElig) + favCmd := NewFavoritesCommandWithAllDeps(tt.eligLister, tt.selector, tt.namePrompter, tt.groupsElig) rootCmd.AddCommand(favCmd) cmdArgs := append([]string{"favorites", "add"}, tt.args...) diff --git a/cmd/helpers.go b/cmd/helpers.go index 085309d..70c30c8 100644 --- a/cmd/helpers.go +++ b/cmd/helpers.go @@ -102,6 +102,9 @@ func fetchStatusData( // Errors are silently ignored (graceful degradation — groups display without directory context). func buildDirectoryNameMap(ctx context.Context, eligLister eligibilityLister, errWriter io.Writer) map[string]string { nameMap := make(map[string]string) + if eligLister == nil { + return nameMap + } resp, err := eligLister.ListEligibility(ctx, scamodels.CSPAzure) if err != nil || resp == nil {