diff --git a/docs/source/issue/update.md b/docs/source/issue/update.md index 66bfcbfe68a83501e7b0dfebb37051024a6732a6..5082f3c8bb95b6643e3938a468d2588bb9afb4de 100644 --- a/docs/source/issue/update.md +++ b/docs/source/issue/update.md @@ -22,6 +22,10 @@ glab issue update [flags] ```console $ glab issue update 42 --label ui,ux $ glab issue update 42 --unlabel working +$ glab issue update 42 --linked-issues 10,15 --link-type is_blocked_by +$ glab issue update 42 --unlink-issues 10,15 +$ glab issue update 42 --epic 12345 +$ glab issue update 42 --epic 0 ``` @@ -32,13 +36,17 @@ $ glab issue update 42 --unlabel working -c, --confidential Make issue confidential -d, --description string Issue description. Set to "-" to open an editor. --due-date string A date in 'YYYY-MM-DD' format. + --epic int ID of the epic to assign this issue to. Set to 0 to remove from epic. -l, --label strings Add labels. + --link-type string Type for the issue link (relates_to, blocks, is_blocked_by). (default "relates_to") + --linked-issues ints The IIDs of issues to link to this issue. --lock-discussion Lock discussion on issue. -m, --milestone string Title of the milestone to assign Set to "" or 0 to unassign. -p, --public Make issue public. -t, --title string Title of issue. --unassign Unassign all users. -u, --unlabel strings Remove labels. + --unlink-issues ints The IIDs of issues to unlink from this issue. --unlock-discussion Unlock discussion on issue. -w, --weight int Set weight of the issue. ``` diff --git a/go.mod b/go.mod index 102e39e2fc525ec96f3e72b899fdb756d992f30b..2877313d3bcfd623e32f97cc7592c4bd6355f4e5 100644 --- a/go.mod +++ b/go.mod @@ -108,6 +108,7 @@ require ( github.com/sourcegraph/conc v0.3.0 // indirect github.com/spf13/afero v1.12.0 // indirect github.com/spf13/cast v1.7.1 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/x448/float16 v0.8.4 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect diff --git a/internal/commands/issue/update/issue_update.go b/internal/commands/issue/update/issue_update.go index ca5449eb381b489de04fbf4b637b563f0872dca4..5b0f7627d6bb35b581189ebada4054b3298c3eec 100644 --- a/internal/commands/issue/update/issue_update.go +++ b/internal/commands/issue/update/issue_update.go @@ -3,11 +3,14 @@ package update import ( "errors" "fmt" + "io" + "strconv" "strings" "gitlab.com/gitlab-org/cli/internal/api" "gitlab.com/gitlab-org/cli/internal/cmdutils" "gitlab.com/gitlab-org/cli/internal/commands/issue/issueutils" + "gitlab.com/gitlab-org/cli/internal/glrepo" "github.com/MakeNowJust/heredoc/v2" "github.com/spf13/cobra" @@ -22,6 +25,10 @@ func NewCmdUpdate(f cmdutils.Factory) *cobra.Command { Example: heredoc.Doc(` $ glab issue update 42 --label ui,ux $ glab issue update 42 --unlabel working + $ glab issue update 42 --linked-issues 10,15 --link-type is_blocked_by + $ glab issue update 42 --unlink-issues 10,15 + $ glab issue update 42 --epic 12345 + $ glab issue update 42 --epic 0 `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -179,13 +186,47 @@ func NewCmdUpdate(f cmdutils.Factory) *cobra.Command { l.DueDate = gitlab.Ptr(dueDate) } - fmt.Fprintf(out, "- Updating issue #%d\n", issue.IID) + // Handle epic assignment + if cmd.Flags().Changed("epic") { + epicID, err := cmd.Flags().GetInt("epic") + if err != nil { + return err + } - issue, err = api.UpdateIssue(client, repo.FullName(), issue.IID, l) + if epicID == 0 { + // Remove from epic + actions = append(actions, "removed from epic") + l.EpicID = gitlab.Ptr(0) + } else { + // Assign to epic + actions = append(actions, fmt.Sprintf("assigned to epic #%d", epicID)) + l.EpicID = gitlab.Ptr(epicID) + } + } + + // Only call UpdateIssue API if there are actual issue property changes + if len(actions) > 0 { + fmt.Fprintf(out, "- Updating issue #%d\n", issue.IID) + + issue, err = api.UpdateIssue(client, repo.FullName(), issue.IID, l) + if err != nil { + return err + } + } + + // Handle issue linking after the main update + linkActions, err := handleIssueLinks(cmd, client, repo, issue, out) if err != nil { return err } + // Show "Updating issue" message if we only have linking operations + if len(actions) == 0 && len(linkActions) > 0 { + fmt.Fprintf(out, "- Updating issue #%d\n", issue.IID) + } + + actions = append(actions, linkActions...) + for _, s := range actions { fmt.Fprintln(out, c.GreenCheck(), s) } @@ -209,6 +250,100 @@ func NewCmdUpdate(f cmdutils.Factory) *cobra.Command { issueUpdateCmd.Flags().Bool("unassign", false, "Unassign all users.") issueUpdateCmd.Flags().IntP("weight", "w", 0, "Set weight of the issue.") issueUpdateCmd.Flags().StringP("due-date", "", "", "A date in 'YYYY-MM-DD' format.") + issueUpdateCmd.Flags().IntSlice("linked-issues", []int{}, "The IIDs of issues to link to this issue.") + issueUpdateCmd.Flags().String("link-type", "relates_to", "Type for the issue link (relates_to, blocks, is_blocked_by).") + issueUpdateCmd.Flags().IntSlice("unlink-issues", []int{}, "The IIDs of issues to unlink from this issue.") + issueUpdateCmd.Flags().Int("epic", 0, "ID of the epic to assign this issue to. Set to 0 to remove from epic.") return issueUpdateCmd } + +// handleIssueLinks manages linking and unlinking issues +func handleIssueLinks(cmd *cobra.Command, client *gitlab.Client, repo glrepo.Interface, issue *gitlab.Issue, out io.Writer) ([]string, error) { + var actions []string + + // Handle linking new issues + if cmd.Flags().Changed("linked-issues") { + linkedIssues, err := cmd.Flags().GetIntSlice("linked-issues") + if err != nil { + return nil, err + } + + linkType, err := cmd.Flags().GetString("link-type") + if err != nil { + return nil, err + } + + // Validate link type + validLinkTypes := map[string]bool{ + "relates_to": true, + "blocks": true, + "is_blocked_by": true, + } + if !validLinkTypes[linkType] { + return nil, fmt.Errorf("invalid link type %q. Valid types are: relates_to, blocks, is_blocked_by", linkType) + } + + for _, targetIssueIID := range linkedIssues { + // Validate that we're not trying to link to ourselves + if targetIssueIID == issue.IID { + return nil, fmt.Errorf("cannot link issue to itself (#%d)", targetIssueIID) + } + + fmt.Fprintf(out, "- Linking to issue #%d\n", targetIssueIID) + _, _, err := client.IssueLinks.CreateIssueLink(repo.FullName(), issue.IID, &gitlab.CreateIssueLinkOptions{ + TargetIssueIID: gitlab.Ptr(strconv.Itoa(targetIssueIID)), + LinkType: gitlab.Ptr(linkType), + }) + if err != nil { + return nil, fmt.Errorf("failed to link issue #%d: %w", targetIssueIID, err) + } + actions = append(actions, fmt.Sprintf("linked to issue #%d (%s)", targetIssueIID, linkType)) + } + } + + // Handle unlinking issues + if cmd.Flags().Changed("unlink-issues") { + unlinkIssues, err := cmd.Flags().GetIntSlice("unlink-issues") + if err != nil { + return nil, err + } + + if len(unlinkIssues) > 0 { + // First, get all existing relations for this issue + relations, _, err := client.IssueLinks.ListIssueRelations(repo.FullName(), issue.IID) + if err != nil { + return nil, fmt.Errorf("failed to get existing issue relations: %w", err) + } + + // Create a map of target IID to link ID for easy lookup + linkMap := make(map[int]int) + for _, relation := range relations { + // Map the target issue IID (the "other" issue in the relation) + targetIID := relation.IID + if relation.IID == issue.IID { + // If this relation's IID is our current issue, the target is the other field + // This logic may need adjustment based on the actual API response structure + continue // Skip self-references or handle appropriately + } + linkMap[targetIID] = relation.IssueLinkID + } + + for _, targetIssueIID := range unlinkIssues { + linkID, exists := linkMap[targetIssueIID] + if !exists { + return nil, fmt.Errorf("no link found to issue #%d", targetIssueIID) + } + + fmt.Fprintf(out, "- Unlinking from issue #%d\n", targetIssueIID) + _, _, err := client.IssueLinks.DeleteIssueLink(repo.FullName(), issue.IID, linkID) + if err != nil { + return nil, fmt.Errorf("failed to unlink issue #%d: %w", targetIssueIID, err) + } + actions = append(actions, fmt.Sprintf("unlinked from issue #%d", targetIssueIID)) + } + } + } + + return actions, nil +} diff --git a/internal/commands/issue/update/issue_update_test.go b/internal/commands/issue/update/issue_update_test.go new file mode 100644 index 0000000000000000000000000000000000000000..61583bdeabda63b6f6fd6e635eb9099f24c3790c --- /dev/null +++ b/internal/commands/issue/update/issue_update_test.go @@ -0,0 +1,320 @@ +package update + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "gitlab.com/gitlab-org/cli/internal/api" + "gitlab.com/gitlab-org/cli/internal/cmdutils" + "gitlab.com/gitlab-org/cli/internal/config" + "gitlab.com/gitlab-org/cli/internal/testing/cmdtest" +) + +func TestNewCmdUpdate_Flags(t *testing.T) { + cfg, err := config.Init() + assert.NoError(t, err) + + ios, _, _, _ := cmdtest.TestIOStreams() + f := cmdutils.NewFactory(ios, false, cfg, api.BuildInfo{}) + + cmd := NewCmdUpdate(f) + + // Test that new flags are available + linkedIssuesFlag := cmd.Flags().Lookup("linked-issues") + assert.NotNil(t, linkedIssuesFlag) + + linkTypeFlag := cmd.Flags().Lookup("link-type") + assert.NotNil(t, linkTypeFlag) + + unlinkIssuesFlag := cmd.Flags().Lookup("unlink-issues") + assert.NotNil(t, unlinkIssuesFlag) + + epicFlag := cmd.Flags().Lookup("epic") + assert.NotNil(t, epicFlag) + + // Test default values + linkType, err := cmd.Flags().GetString("link-type") + assert.NoError(t, err) + assert.Equal(t, "relates_to", linkType) + + linkedIssues, err := cmd.Flags().GetIntSlice("linked-issues") + assert.NoError(t, err) + assert.Empty(t, linkedIssues) + + unlinkIssues, err := cmd.Flags().GetIntSlice("unlink-issues") + assert.NoError(t, err) + assert.Empty(t, unlinkIssues) + + epicID, err := cmd.Flags().GetInt("epic") + assert.NoError(t, err) + assert.Equal(t, 0, epicID) +} + +func TestLinkTypeFlag_SetAndGet(t *testing.T) { + cfg, err := config.Init() + assert.NoError(t, err) + + ios, _, _, _ := cmdtest.TestIOStreams() + f := cmdutils.NewFactory(ios, false, cfg, api.BuildInfo{}) + + cmd := NewCmdUpdate(f) + + // Test valid link types + validTypes := []string{"relates_to", "blocks", "is_blocked_by"} + for _, linkType := range validTypes { + err := cmd.Flags().Set("link-type", linkType) + assert.NoError(t, err) + + value, err := cmd.Flags().GetString("link-type") + assert.NoError(t, err) + assert.Equal(t, linkType, value) + } +} + +func TestLinkTypeValidation(t *testing.T) { + tests := []struct { + name string + linkType string + expectValid bool + }{ + { + name: "valid relates_to", + linkType: "relates_to", + expectValid: true, + }, + { + name: "valid blocks", + linkType: "blocks", + expectValid: true, + }, + { + name: "valid is_blocked_by", + linkType: "is_blocked_by", + expectValid: true, + }, + { + name: "invalid link type", + linkType: "invalid_type", + expectValid: false, + }, + { + name: "empty link type", + linkType: "", + expectValid: false, + }, + { + name: "case sensitive validation", + linkType: "RELATES_TO", + expectValid: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the validation logic directly + validLinkTypes := map[string]bool{ + "relates_to": true, + "blocks": true, + "is_blocked_by": true, + } + + isValid := validLinkTypes[tt.linkType] + assert.Equal(t, tt.expectValid, isValid, "Link type %q validation failed", tt.linkType) + }) + } +} + +func TestSelfReferenceValidation(t *testing.T) { + tests := []struct { + name string + issueIID int + linkedIssueIID int + expectError bool + }{ + { + name: "different issues - valid", + issueIID: 1, + linkedIssueIID: 2, + expectError: false, + }, + { + name: "same issue - invalid", + issueIID: 42, + linkedIssueIID: 42, + expectError: true, + }, + { + name: "zero IID edge case", + issueIID: 0, + linkedIssueIID: 0, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the self-reference validation logic + isSelfReference := tt.issueIID == tt.linkedIssueIID + assert.Equal(t, tt.expectError, isSelfReference, "Self-reference validation failed") + }) + } +} + +func TestRelationMappingLogic(t *testing.T) { + tests := []struct { + name string + currentIssueIID int + relations []mockRelation + expectedMap map[int]int + }{ + { + name: "empty relations", + currentIssueIID: 1, + relations: []mockRelation{}, + expectedMap: map[int]int{}, + }, + { + name: "relations with self-reference", + currentIssueIID: 1, + relations: []mockRelation{ + {IID: 1, IssueLinkID: 100}, // Self-reference, should be skipped + {IID: 2, IssueLinkID: 200}, // Valid relation + }, + expectedMap: map[int]int{2: 200}, + }, + { + name: "multiple valid relations", + currentIssueIID: 1, + relations: []mockRelation{ + {IID: 10, IssueLinkID: 100}, + {IID: 15, IssueLinkID: 150}, + {IID: 20, IssueLinkID: 200}, + }, + expectedMap: map[int]int{10: 100, 15: 150, 20: 200}, + }, + { + name: "only self-references", + currentIssueIID: 5, + relations: []mockRelation{ + {IID: 5, IssueLinkID: 100}, + {IID: 5, IssueLinkID: 200}, + }, + expectedMap: map[int]int{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Simulate the relation mapping logic from handleIssueLinks + linkMap := make(map[int]int) + for _, relation := range tt.relations { + targetIID := relation.IID + if relation.IID == tt.currentIssueIID { + // Skip self-references + continue + } + linkMap[targetIID] = relation.IssueLinkID + } + + assert.Equal(t, tt.expectedMap, linkMap, "Relation mapping failed") + }) + } +} + +func TestStringConversionLogic(t *testing.T) { + tests := []struct { + name string + input int + expected string + }{ + { + name: "positive integer", + input: 42, + expected: "42", + }, + { + name: "zero", + input: 0, + expected: "0", + }, + { + name: "large number", + input: 999999, + expected: "999999", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the string conversion logic used in TargetIssueIID + result := fmt.Sprintf("%d", tt.input) + assert.Equal(t, tt.expected, result, "String conversion failed") + }) + } +} + +func TestEpicFlag_SetAndGet(t *testing.T) { + cfg, err := config.Init() + assert.NoError(t, err) + + ios, _, _, _ := cmdtest.TestIOStreams() + f := cmdutils.NewFactory(ios, false, cfg, api.BuildInfo{}) + + cmd := NewCmdUpdate(f) + + // Test setting epic values + testValues := []int{0, 12345, 999999} + for _, epicID := range testValues { + err := cmd.Flags().Set("epic", fmt.Sprintf("%d", epicID)) + assert.NoError(t, err) + + value, err := cmd.Flags().GetInt("epic") + assert.NoError(t, err) + assert.Equal(t, epicID, value) + } +} + +func TestEpicAssignmentLogic(t *testing.T) { + tests := []struct { + name string + epicID int + expectedAction string + }{ + { + name: "assign to epic", + epicID: 12345, + expectedAction: "assigned to epic #12345", + }, + { + name: "remove from epic", + epicID: 0, + expectedAction: "removed from epic", + }, + { + name: "assign to different epic", + epicID: 99999, + expectedAction: "assigned to epic #99999", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Test the epic assignment action message logic + var action string + if tt.epicID == 0 { + action = "removed from epic" + } else { + action = fmt.Sprintf("assigned to epic #%d", tt.epicID) + } + + assert.Equal(t, tt.expectedAction, action, "Epic action message failed") + }) + } +} + +// Helper type for testing relation mapping logic +type mockRelation struct { + IID int + IssueLinkID int +}