Add --org-id filter to project delete cmd #350
Conversation
WalkthroughThe changes add a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant API
User->>CLI: project list --org-id=<orgId>
CLI->>API: Fetch all projects
API-->>CLI: Return project list
CLI->>CLI: Filter projects by orgId (if provided)
CLI-->>User: Display filtered list
User->>CLI: project delete --org-id=<orgId>
CLI->>API: Fetch all projects
API-->>CLI: Return project list
CLI->>CLI: Filter projects by orgId (if provided)
CLI->>API: Delete filtered projects
CLI-->>User: Confirm deletion
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
cmd/project.go (3)
733-734: Handle potentialGetStringflag errors instead of discarding themAlthough
cmd.Flags().GetString()is unlikely to fail at runtime (the flag should exist), the call still returns an error value that is currently ignored. Propagating or at least logging this error will make unexpected flag-table issues far easier to diagnose.-orgId, _ := cmd.Flags().GetString("org-id") +orgId, err := cmd.Flags().GetString("org-id") +if err != nil { + logger.Fatalf("unable to read --org-id flag: %v", err) +}
741-749: Factor out and validate the organisation-filtering logic
- Efficiency / DRY – The slice-filter loop is fine, but we are already allocating a new slice and doing the copy for the happy path (
orgId == "").
A concise pattern avoids duplication:filtered := make([]project.ProjectListData, 0, len(unfilteredProjects)) for _, p := range unfilteredProjects { if orgId == "" || p.OrgId == orgId { filtered = append(filtered, p) } } projects = filtered
- Better UX for invalid IDs – If the user supplies an
--org-idthat does not belong to them (or is mistyped),projectsends up empty and we just show “no projects found”. Consider surfacing a clearer message:if orgId != "" && len(projects) == 0 { tui.ShowWarning("no projects found for organization %s", orgId) return }This prevents silent confusion when a typo is made.
897-897: Flag added only to delete – confirm if list should also accept--org-idThe new flag is registered for
project delete, but theproject listcommand does not currently expose or honour the same flag, even though that might be equally useful. If this is intentional, please disregard; otherwise, consider adding:projectListCmd.Flags().String("org-id", "", "Only list the projects in the specified organization")…and apply filtering similar to the delete path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/project.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/project.go (1)
901-906: Consider moving the hidden flag configuration closer to related flags.The hidden flag configuration for
projectImportCmdis positioned between thelistanddeleteflag configurations. For better code organization, consider grouping all flag configurations for each command together.+ // hidden because they must be all passed together and we havent documented that + projectImportCmd.Flags().MarkHidden("name") + projectImportCmd.Flags().MarkHidden("description") + projectImportCmd.Flags().MarkHidden("org-id") + projectDeleteCmd.Flags().String("org-id", "", "Only delete the projects in the specified organization") - - // hidden because they must be all passed together and we havent documented that - projectImportCmd.Flags().MarkHidden("name") - projectImportCmd.Flags().MarkHidden("description") - projectImportCmd.Flags().MarkHidden("org-id")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/project.go(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
🔇 Additional comments (4)
cmd/project.go (4)
613-628: LGTM! Well-designed helper function with proper error handling.The
listProjectsfunction effectively centralizes the filtering logic and provides a clean abstraction. The implementation correctly handles the case where noorgIdfilter is applied by returning all projects, and properly filters when anorgIdis specified.
654-658: Good refactoring to use the centralized filtering logic.The
listcommand now properly retrieves theorg-idflag and delegates to the newlistProjectshelper function, which ensures consistent filtering behavior across commands.
747-751: Excellent implementation of organization filtering in delete command.This change successfully addresses the PR objective by adding
--org-idfilter support to the project delete command. The implementation mirrors the pattern used in the list command, ensuring consistency.
888-888: Appropriate flag registration for the list command.The
org-idflag is properly registered with a clear description that explains its filtering purpose.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/project.go (1)
613-628: Good implementation of the filtering helper function.The
listProjectsfunction properly centralizes the organization filtering logic and handles the emptyorgIdcase correctly. The error handling is appropriate.However, consider validating the provided
orgIdto give users better feedback when they specify an invalid organization ID.Consider adding orgId validation to provide better user experience:
func listProjects(ctx context.Context, logger logger.Logger, apiUrl string, apikey string, orgId string) []project.ProjectListData { unfilteredProjects, err := project.ListProjects(ctx, logger, apiUrl, apikey) if err != nil { errsystem.New(errsystem.ErrApiRequest, err, errsystem.WithContextMessage("Failed to list projects")).ShowErrorAndExit() } if orgId == "" { return unfilteredProjects } + + // Check if any projects exist for the specified orgId + hasMatchingOrg := false + for _, project := range unfilteredProjects { + if project.OrgId == orgId { + hasMatchingOrg = true + break + } + } + var projects []project.ProjectListData for _, project := range unfilteredProjects { if project.OrgId == orgId { projects = append(projects, project) } } + + // If no projects found but we have projects in other orgs, suggest the orgId might be invalid + if len(projects) == 0 && len(unfilteredProjects) > 0 && !hasMatchingOrg { + logger.Warn("No projects found for organization '%s'. Use 'agentuity organization list' to see available organizations.", orgId) + } + return projects }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cmd/project.go(7 hunks)
🔇 Additional comments (5)
cmd/project.go (5)
630-638: Clean enhancement to support organization-specific messaging.The modification to
showNoProjectsfunction properly handles both filtered and unfiltered cases, providing appropriate messaging based on whether an organization filter is applied.
658-658: Proper integration of organization filtering in list command.The changes correctly integrate the new
listProjectsfunction and pass theorgIdparameter consistently throughout the command execution.Also applies to: 662-662, 671-671
751-752: Consistent implementation of organization filtering in delete command.The delete command properly adopts the same pattern as the list command, ensuring consistent behavior across project management operations.
Also applies to: 755-755, 771-771
892-892: Appropriate flag registration for organization filtering.The
--org-idflag is properly registered for bothlistanddeletecommands with clear descriptions that match their functionality.Also applies to: 910-910
905-909:Details
✅ Verification successful
Verify the necessity of marking org-id as hidden for import command.
The
org-idflag is marked as hidden for theimportcommand along with other flags. Based on the comment, this is because all these flags must be passed together and aren't documented for individual use.Verify that the
org-idflag behavior is intentional for the import command:
🏁 Script executed:
#!/bin/bash # Description: Check if org-id flag is used in import command logic and verify its hidden status is appropriate # Search for org-id usage in import command ast-grep --pattern $'func (cmd *cobra.Command, args []string) { $$$ orgId, _ := cmd.Flags().GetString("org-id") $$$ }' # Look for other hidden flags in import command for consistency rg -A 10 -B 2 "MarkHidden.*org-id"Length of output: 15030
Confirmed intentional hiding of
org-idfor headless importThe
org-idflag incmd/project.gois only read in the headless‐import branch (whenname,description,api-keyandorg-idare all provided together). Hiding it alongsidenameanddescriptionprevents users from seeing undocumented, multi‐flag only options in help output. No changes required.• cmd/project.go –
projectImportCmd.Flags().MarkHidden("org-id")(with"name"and"description") aligns with the headless import logic.
Summary by CodeRabbit