Commit `4fa1930038`

4fa1930038e594bcad209d872374fbb4a968aa46

parent: 6bf4591bd7

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-28 08:32 UTC

issue create: resolve milestone and assignees first, set fields through issue label/milestone/assign

A typo creates nothing, and the assignee gets the assigned-you notice
and the labeled, milestoned and assigned events are recorded.

Ref #268

Layout: unified · split

internal/control/issue.go +79 −47

 			return c.fail(protocol.ExitDenied, "permission denied on %s; ask its owner for access", path)
 		}
 	}
+	// Resolve everything that can be refused before the issue exists, so
+	// a typo in a milestone or an assignee creates nothing.
+	var milestone store.Milestone
+	if m := f.Value("--milestone"); m != "" {
+		if milestone, err = c.Store.MilestoneByTitle(repo, m); err != nil {
+			return milestoneErr(c, repo, m, err)
+		}
+	}
+	assignees, code := resolveUsers(c, f.List("--assignee"))
+	if code >= 0 {
+		return code
+	}
 	b, err := bodyFrom(c, body, file)
 	if err != nil {
 		return c.failInput(err)
 		return c.fail(protocol.ExitFailure, "%v", err)
 	}
 	notifyMentions(c, repo, issueThread, issue.ID, n, title, b)
-	for _, l := range f.List("--label") {
+	if labels := f.List("--label"); len(labels) > 0 {
-		if err := c.Store.SetIssueLabel(repo, issue.ID, l, true); err != nil {
+		if _, code := labelIssue(c, repo, issue, labels, nil); code >= 0 {
-			return c.fail(protocol.ExitFailure, "%v", err)
+			return code
 		}
 	}
-	if m := f.Value("--milestone"); m != "" {
+	if milestone.ID != 0 {
-		ms, err := c.Store.MilestoneByTitle(repo, m)
+		if err := recordItemMilestone(c, repo, "issue", n, milestone.ID, milestone.Title, func(id int64) error {
-		if err != nil {
+			return c.Store.SetIssueMilestone(issue.ID, id)
-			return milestoneErr(c, repo, m, err)
+		}); err != nil {
-		}
-		if err := c.Store.SetIssueMilestone(issue.ID, ms.ID); err != nil {
 			return c.fail(protocol.ExitFailure, "%v", err)
 		}
 	}
-	for _, name := range f.List("--assignee") {
+	if len(assignees) > 0 {
-		u, err := c.Store.UserByUsername(name)
+		if _, code := assignIssue(c, repo, issue, assignees, nil); code >= 0 {
-		if errors.Is(err, store.ErrNotFound) {
+			return code
-			return c.fail(protocol.ExitNotFound, "no such user %q", name)
-		}
-		if err != nil {
-			return c.fail(protocol.ExitFailure, "%v", err)
-		}
-		if err := c.Store.SetIssueAssignee(issue.ID, u.ID, true); err != nil {
-			return c.fail(protocol.ExitFailure, "%v", err)
 		}
 	}
 	return c.emit(Created{Number: n}, func(w io.Writer) {
 	if code := refuseArchived(c, repo); code >= 0 {
 		return code
 	}
+	labels, code := labelIssue(c, repo, issue, adds, removes)
+	if code >= 0 {
+		return code
+	}
+	return c.emit(map[string]any{"number": issue.Number, "labels": labels}, func(w io.Writer) {
+		fmt.Fprintf(w, "labels on %s#%d: %s\n", repo.Path(), issue.Number, strings.Join(labels, ", "))
+	})
+}
+// labelIssue adds and removes labels on issue and records the
+// issue.labeled event, returning the labels it carries afterwards. It
+// backs issue label and issue create --label.
+func labelIssue(c *Ctx, repo store.Repo, issue store.Issue, adds, removes []string) ([]string, int) {
 	for _, l := range adds {
 		if err := c.Store.SetIssueLabel(repo, issue.ID, l, true); err != nil {
-			return c.fail(protocol.ExitFailure, "%v", err)
+			return nil, c.fail(protocol.ExitFailure, "%v", err)
 		}
 	}
 	for _, l := range removes {
 		if err := c.Store.SetIssueLabel(repo, issue.ID, l, false); err != nil {
 			if errors.Is(err, store.ErrNotFound) {
-				return c.fail(protocol.ExitNotFound, "%v", err)
+				return nil, c.fail(protocol.ExitNotFound, "%v", err)
 			}
-			return c.fail(protocol.ExitFailure, "%v", err)
+			return nil, c.fail(protocol.ExitFailure, "%v", err)
 		}
 	}
 	updated, err := c.Store.IssueByNumber(repo.ID, issue.Number)
 	if err != nil {
-		return c.fail(protocol.ExitFailure, "%v", err)
+		return nil, c.fail(protocol.ExitFailure, "%v", err)
 	}
 	c.Store.RecordEvent(repo.ID, c.User.ID, "issue.labeled",
 		fmt.Sprintf(`{"number":%d,"labels":%s}`, issue.Number, jsonStrings(updated.Labels)))
-	return c.emit(map[string]any{"number": issue.Number, "labels": updated.Labels}, func(w io.Writer) {
+	return updated.Labels, -1
-		fmt.Fprintf(w, "labels on %s#%d: %s\n", repo.Path(), issue.Number, strings.Join(updated.Labels, ", "))
-	})
 }
 func runIssueAssign(c *Ctx, args []string) int {
 	if code := refuseArchived(c, repo); code >= 0 {
 		return code
 	}
-	resolve := func(name string) (store.User, int) {
+	add, code := resolveUsers(c, adds)
+	if code >= 0 {
+		return code
+	}
+	remove, code := resolveUsers(c, removes)
+	if code >= 0 {
+		return code
+	}
+	assignees, code := assignIssue(c, repo, issue, add, remove)
+	if code >= 0 {
+		return code
+	}
+	return c.emit(map[string]any{"number": issue.Number, "assignees": assignees}, func(w io.Writer) {
+		fmt.Fprintf(w, "assignees on %s#%d: %s\n", repo.Path(), issue.Number, strings.Join(assignees, ", "))
+	})
+}
+// resolveUsers looks up every name, failing on the first that does not
+// exist, so a typo changes nothing.
+func resolveUsers(c *Ctx, names []string) ([]store.User, int) {
+	users := make([]store.User, 0, len(names))
+	for _, name := range names {
 		u, err := c.Store.UserByUsername(name)
 		if errors.Is(err, store.ErrNotFound) {
-			return u, c.fail(protocol.ExitNotFound, "no such user %q", name)
+			return nil, c.fail(protocol.ExitNotFound, "no such user %q", name)
 		}
 		if err != nil {
-			return u, c.fail(protocol.ExitFailure, "%v", err)
+			return nil, c.fail(protocol.ExitFailure, "%v", err)
 		}
-		return u, -1
+		users = append(users, u)
 	}
+	return users, -1
+}
+// assignIssue adds and removes assignees on issue, records the
+// issue.assigned event and tells each newly added account, returning the
+// assignees afterwards. It backs issue assign and issue create --assignee.
+func assignIssue(c *Ctx, repo store.Repo, issue store.Issue, adds, removes []store.User) ([]string, int) {
 	// issue is the read from before the update, so its Assignees are who
 	// was already on it. SetIssueAssignee inserts ON CONFLICT DO NOTHING
 	// and returns nil whether or not it inserted, and the notice below is
 		assigned[name] = true
 	}
 	var added []int64
-	for _, name := range adds {
+	for _, u := range adds {
-		u, code := resolve(name)
-		if code >= 0 {
-			return code
-		}
 		if err := c.Store.SetIssueAssignee(issue.ID, u.ID, true); err != nil {
-			return c.fail(protocol.ExitFailure, "%v", err)
+			return nil, c.fail(protocol.ExitFailure, "%v", err)
 		}
 		if assigned[u.Username] {
 			continue
 		assigned[u.Username] = true
 		added = append(added, u.ID)
 	}
-	for _, name := range removes {
+	for _, u := range removes {
-		u, code := resolve(name)
-		if code >= 0 {
-			return code
-		}
 		if err := c.Store.SetIssueAssignee(issue.ID, u.ID, false); err != nil {
 			if errors.Is(err, store.ErrNotFound) {
-				return c.fail(protocol.ExitNotFound, "%s is not assigned", name)
+				return nil, c.fail(protocol.ExitNotFound, "%s is not assigned", u.Username)
 			}
-			return c.fail(protocol.ExitFailure, "%v", err)
+			return nil, c.fail(protocol.ExitFailure, "%v", err)
 		}
 	}
 	updated, err := c.Store.IssueByNumber(repo.ID, issue.Number)
 	if err != nil {
-		return c.fail(protocol.ExitFailure, "%v", err)
+		return nil, c.fail(protocol.ExitFailure, "%v", err)
 	}
 	c.Store.RecordEvent(repo.ID, c.User.ID, "issue.assigned",
 		fmt.Sprintf(`{"number":%d,"assignees":%s}`, issue.Number, jsonStrings(updated.Assignees)))
 			action:  fmt.Sprintf("assigned you to #%d", issue.Number),
 			path:    fmt.Sprintf("%s/issues/%d", repo.Path(), issue.Number)})
 	}
-	return c.emit(map[string]any{"number": issue.Number, "assignees": updated.Assignees}, func(w io.Writer) {
+	return updated.Assignees, -1
-		fmt.Fprintf(w, "assignees on %s#%d: %s\n", repo.Path(), issue.Number, strings.Join(updated.Assignees, ", "))
-	})
 }

internal/control/issue_test.go +44 −1

	@@ -3,6 +3,7 @@ package control
3	import (	3	import (
4	"bytes"	4	"bytes"
5	"errors"	5	"errors"
		6	"slices"
6	"testing"	7	"testing"
7		8
8	"gitbay.org/gitbay/internal/protocol"	9	"gitbay.org/gitbay/internal/protocol"
	@@ -58,7 +59,8 @@ func TestIssueCreateSetsLabelsMilestoneAndAssignee(t *testing.T) {
58	if _, err := c.Store.CreateMilestone(repo, "m1", "", ""); err != nil {	59	if _, err := c.Store.CreateMilestone(repo, "m1", "", ""); err != nil {
59	t.Fatal(err)	60	t.Fatal(err)
60	}	61	}
61	if _, err := c.Store.CreateUser("bob", false); err != nil {	62	bob, err := c.Store.CreateUser("bob", false)
		63	if err != nil {
62	t.Fatal(err)	64	t.Fatal(err)
63	}	65	}
64		66
	@@ -66,6 +68,24 @@ func TestIssueCreateSetsLabelsMilestoneAndAssignee(t *testing.T) {
66	"--label", "bug", "--milestone", "m1", "--assignee", "bob"}); code != 0 {	68	"--label", "bug", "--milestone", "m1", "--assignee", "bob"}); code != 0 {
67	t.Fatalf("exit %d", code)	69	t.Fatalf("exit %d", code)
68	}	70	}
		71	rows, _ := c.Store.Inbox(bob, false, 20, 0)
		72	if len(rows) != 1 \|\| rows[0].Summary != "assigned you to #1" {
		73	t.Errorf("bob's inbox = %+v, want one assigned-you notice", rows)
		74	}
		75	var kinds []string
		76	ev, err := c.Store.DB.Query("SELECT kind FROM events WHERE repo_id = ? ORDER BY id", repo.ID)
		77	if err != nil {
		78	t.Fatal(err)
		79	}
		80	for ev.Next() {
		81	var k string
		82	ev.Scan(&k)
		83	kinds = append(kinds, k)
		84	}
		85	ev.Close()
		86	if want := []string{"issue.created", "issue.labeled", "issue.milestoned", "issue.assigned"}; !slices.Equal(kinds, want) {
		87	t.Errorf("events = %v, want %v", kinds, want)
		88	}
69	issue, err := c.Store.IssueByNumber(repo.ID, 1)	89	issue, err := c.Store.IssueByNumber(repo.ID, 1)
70	if err != nil {	90	if err != nil {
71	t.Fatal(err)	91	t.Fatal(err)
	@@ -81,6 +101,29 @@ func TestIssueCreateSetsLabelsMilestoneAndAssignee(t *testing.T) {
81	}	101	}
82	}	102	}
83		103
		104	// A milestone or assignee that does not resolve is refused before the
		105	// issue exists, so a typo creates nothing.
		106	func TestIssueCreateRefusesUnknownMilestoneOrAssigneeFirst(t *testing.T) {
		107	c := notifTestCtx(t, "alice")
		108	repoID, err := c.Store.CreateRepo("user", c.User.ID, "app", "public")
		109	if err != nil {
		110	t.Fatal(err)
		111	}
		112	repo, err := c.Store.RepoByID(repoID)
		113	if err != nil {
		114	t.Fatal(err)
		115	}
		116	for _, extra := range [][]string{{"--milestone", "nope"}, {"--assignee", "nobody"}} {
		117	args := append([]string{repo.Path(), "--title", "t", "--body", "b", "--label", "bug"}, extra...)
		118	if code := runIssueCreate(c, args); code != protocol.ExitNotFound {
		119	t.Errorf("%v: exit %d, want %d", extra, code, protocol.ExitNotFound)
		120	}
		121	if _, err := c.Store.IssueByNumber(repo.ID, 1); !errors.Is(err, store.ErrNotFound) {
		122	t.Fatalf("%v: an issue was created: %v", extra, err)
		123	}
		124	}
		125	}
		126
84	func TestIssueAssignNotifiesTheAssignee(t *testing.T) {	127	func TestIssueAssignNotifiesTheAssignee(t *testing.T) {
85	c, repo, bob := testRepoWithWatcher(t)	128	c, repo, bob := testRepoWithWatcher(t)
86		129

internal/control/milestone.go +14 −5

 	})
 }
+// recordItemMilestone sets milestone id (0 clears it) through set and
+// records the milestoned event naming title ("" when cleared).
+func recordItemMilestone(c *Ctx, repo store.Repo, noun string, number, id int64, title string, set func(int64) error) error {
+	if err := set(id); err != nil {
+		return err
+	}
+	c.Store.RecordEvent(repo.ID, c.User.ID, noun+".milestoned",
+		fmt.Sprintf(`{"number":%d,"milestone":%q}`, number, title))
+	return nil
+}
 // noun and number name what the milestone was set on, for the event.
 func setItemMilestone(c *Ctx, repo store.Repo, noun string, number int64, title string, set func(int64) error) int {
 	var id int64
 		}
 		id = m.ID
 	}
-	if err := set(id); err != nil {
-		return c.fail(protocol.ExitFailure, "%v", err)
-	}
 	cleared := title
 	if cleared == "none" {
 		cleared = ""
 	}
-	c.Store.RecordEvent(repo.ID, c.User.ID, noun+".milestoned",
+	if err := recordItemMilestone(c, repo, noun, number, id, cleared, set); err != nil {
-		fmt.Sprintf(`{"number":%d,"milestone":%q}`, number, cleared))
+		return c.fail(protocol.ExitFailure, "%v", err)
+	}
 	if title == "none" {
 		return c.emit(map[string]string{"milestone": ""}, func(w io.Writer) {
 			fmt.Fprintln(w, "milestone cleared")

Commit 4fa1930038

Commit `4fa1930038`