control: issues and merge requests share their thread code !209

merged merged by cmc on 2026-09-04 01:17 UTC · krz/gitbay:issue-mr-dedup into main

3 files changed, +140 −137

Layout: unified · split

internal/control/issue.go +11 −66
@@ -49,17 +49,10 @@ func init() {
4949
5050// issueArgs parses "<owner/name> <n>" plus flags handled by the caller.
5151func issueRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.Issue, int) {
52 if len(args) < 2 {
53 return store.Repo{}, store.Issue{}, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
54 }
55 repo, code := resolveRepo(c, args[0], perm)
52 repo, n, code := refArgs(c, args, perm, "issue")
5653 if code >= 0 {
5754 return repo, store.Issue{}, code
5855 }
59 n, err := strconv.ParseInt(args[1], 10, 64)
60 if err != nil {
61 return repo, store.Issue{}, c.fail(protocol.ExitUsage, "bad issue number %q", args[1])
62 }
6356 issue, err := c.Store.IssueByNumber(repo.ID, n)
6457 if errors.Is(err, store.ErrNotFound) {
6558 return repo, issue, c.fail(protocol.ExitNotFound, "issue #%d not found in %s", n, repo.Path())
@@ -219,12 +212,6 @@ func runIssueShow(c *Ctx, args []string) int {
219212 if err != nil {
220213 return c.fail(protocol.ExitFailure, "%v", err)
221214 }
222 type commentOut struct {
223 Author string `json:"author"`
224 Body string `json:"body"`
225 BodyFormat string `json:"body_format,omitempty"`
226 CreatedAt string `json:"created_at"`
227 }
228215 var cs []commentOut
229216 for _, cm := range comments {
230217 cs = append(cs, commentOut{cm.Author, cm.Body, cm.BodyFormat, cm.CreatedAt})
@@ -252,45 +239,12 @@ func runIssueShow(c *Ctx, args []string) int {
252239}
253240
254241func runIssueComment(c *Ctx, args []string) int {
255 f, err := parseFlags(args, flagSpec{Values: []string{"--format", "--message", "--file"}, MaxPos: -1,
256 Usage: "issue comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"})
257 if err != nil {
258 return c.fail(protocol.ExitUsage, "%v", err)
259 }
260 rest := f.Pos
261 message, file, format := f.Value("--message"), f.Value("--file"), f.Value("--format")
262 fmtName, err := markupFormat(format)
263 if err != nil {
264 return c.failErr(err)
265 }
266 if fmtName == "" {
267 fmtName = "md"
268 }
269 repo, issue, code := issueRef(c, rest, policy.CanRead)
270 if code >= 0 {
271 return code
272 }
273 if code := refuseArchived(c, repo); code >= 0 {
274 return code
275 }
276 body, err := bodyFrom(c, message, file)
277 if err != nil {
278 return c.failErr(err)
279 }
280 if strings.TrimSpace(body) == "" {
281 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
282 }
283 if err := c.Store.AddIssueComment(issue.ID, c.User.ID, body, fmtName); err != nil {
284 return c.fail(protocol.ExitFailure, "%v", err)
285 }
286 c.Store.RecordEvent(repo.ID, c.User.ID, "issue.commented", fmt.Sprintf(`{"number":%d}`, issue.Number))
287 if parts, err := c.Store.IssueParticipants(issue.ID); err == nil {
288 notifyUsers(c, parts, issueSubject(repo, issue.Number, issue.Title),
289 notifyBody(c, fmt.Sprintf("commented on #%d", issue.Number), body, fmt.Sprintf("%s/issues/%d", repo.Path(), issue.Number)))
290 }
291 return c.emit(map[string]any{"number": issue.Number}, func(w io.Writer) {
292 fmt.Fprintf(w, "commented on %s#%d\n", repo.Path(), issue.Number)
293 })
242 return runComment(c, args, issueThread, "issue",
243 func(rest []string) (store.Repo, int64, int64, string, int) {
244 repo, issue, code := issueRef(c, rest, policy.CanRead)
245 return repo, issue.ID, issue.Number, issue.Title, code
246 },
247 c.Store.AddIssueComment, c.Store.IssueParticipants)
294248}
295249
296250func setIssueState(c *Ctx, args []string, state string) int {
@@ -305,13 +259,8 @@ func setIssueState(c *Ctx, args []string, state string) int {
305259 if len(args) != 2 {
306260 return c.fail(protocol.ExitUsage, "usage: issue %s <owner/name> <n>", state)
307261 }
308 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
309 if err != nil {
310 return c.fail(protocol.ExitFailure, "%v", err)
311 }
312 if issue.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
313 return c.fail(protocol.ExitDenied, "only the author or users with write access can %s this issue",
314 map[string]string{"open": "reopen", "closed": "close"}[state])
262 if code := authorOrWrite(c, repo, issue.Author, map[string]string{"open": "reopen", "closed": "close"}[state]+" this issue"); code >= 0 {
263 return code
315264 }
316265 if issue.State == state {
317266 return c.fail(protocol.ExitUsage, "issue #%d is already %s", issue.Number, state)
@@ -382,12 +331,8 @@ func runIssueEdit(c *Ctx, args []string) int {
382331 if code := refuseArchived(c, repo); code >= 0 {
383332 return code
384333 }
385 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
386 if err != nil {
387 return c.fail(protocol.ExitFailure, "%v", err)
388 }
389 if issue.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
390 return c.fail(protocol.ExitDenied, "only the author or users with write access can edit this issue")
334 if code := authorOrWrite(c, repo, issue.Author, "edit this issue"); code >= 0 {
335 return code
391336 }
392337 if err := c.Store.UpdateIssueText(issue.ID, title, body, format); err != nil {
393338 return c.fail(protocol.ExitFailure, "%v", err)
internal/control/mr.go +13 −71
@@ -196,17 +196,10 @@ func runRequireSigned(c *Ctx, args []string) int {
196196
197197// mrRef parses "<owner/name> <n>" and loads the MR.
198198func mrRef(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool) (store.Repo, store.MR, int) {
199 if len(args) < 2 {
200 return store.Repo{}, store.MR{}, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
201 }
202 repo, code := resolveRepo(c, args[0], perm)
199 repo, n, code := refArgs(c, args, perm, "MR")
203200 if code >= 0 {
204201 return repo, store.MR{}, code
205202 }
206 n, err := strconv.ParseInt(args[1], 10, 64)
207 if err != nil {
208 return repo, store.MR{}, c.fail(protocol.ExitUsage, "bad MR number %q", args[1])
209 }
210203 mr, err := c.Store.MRByNumber(repo.ID, n)
211204 if errors.Is(err, store.ErrNotFound) {
212205 return repo, mr, c.fail(protocol.ExitNotFound, "MR !%d not found in %s", n, repo.Path())
@@ -456,12 +449,6 @@ func runMRShow(c *Ctx, args []string) int {
456449 if err != nil {
457450 return c.fail(protocol.ExitFailure, "%v", err)
458451 }
459 type commentOut struct {
460 Author string `json:"author"`
461 Body string `json:"body"`
462 BodyFormat string `json:"body_format,omitempty"`
463 CreatedAt string `json:"created_at"`
464 }
465452 type reviewOut struct {
466453 Reviewer string `json:"reviewer"`
467454 Verdict string `json:"verdict"`
@@ -607,12 +594,8 @@ func runMREdit(c *Ctx, args []string) int {
607594 if code := refuseArchived(c, repo); code >= 0 {
608595 return code
609596 }
610 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
611 if err != nil {
612 return c.fail(protocol.ExitFailure, "%v", err)
613 }
614 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
615 return c.fail(protocol.ExitDenied, "only the author or users with write access can edit this merge request")
597 if code := authorOrWrite(c, repo, mr.Author, "edit this merge request"); code >= 0 {
598 return code
616599 }
617600 if err := c.Store.UpdateMRText(mr.ID, title, body, format); err != nil {
618601 return c.fail(protocol.ExitFailure, "%v", err)
@@ -635,12 +618,8 @@ func runMRRetarget(c *Ctx, args []string) int {
635618 if code := refuseArchived(c, repo); code >= 0 {
636619 return code
637620 }
638 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
639 if err != nil {
640 return c.fail(protocol.ExitFailure, "%v", err)
641 }
642 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
643 return c.fail(protocol.ExitDenied, "only the author or users with write access can retarget this merge request")
621 if code := authorOrWrite(c, repo, mr.Author, "retarget this merge request"); code >= 0 {
622 return code
644623 }
645624 if mr.State == "merged" || mr.State == "closed" {
646625 return c.fail(protocol.ExitUsage, "!%d is %s; only an open merge request can be retargeted", mr.Number, mr.State)
@@ -680,45 +659,12 @@ func runMRRetarget(c *Ctx, args []string) int {
680659}
681660
682661func runMRComment(c *Ctx, args []string) int {
683 f, err := parseFlags(args, flagSpec{Values: []string{"--message", "--file", "--format"}, MaxPos: -1,
684 Usage: "mr comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"})
685 if err != nil {
686 return c.fail(protocol.ExitUsage, "%v", err)
687 }
688 rest := f.Pos
689 message, file, format := f.Value("--message"), f.Value("--file"), f.Value("--format")
690 fmtName, err := markupFormat(format)
691 if err != nil {
692 return c.failErr(err)
693 }
694 if fmtName == "" {
695 fmtName = "md"
696 }
697 repo, mr, code := mrRef(c, rest, policy.CanRead)
698 if code >= 0 {
699 return code
700 }
701 if code := refuseArchived(c, repo); code >= 0 {
702 return code
703 }
704 body, err := bodyFrom(c, message, file)
705 if err != nil {
706 return c.failErr(err)
707 }
708 if strings.TrimSpace(body) == "" {
709 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
710 }
711 if err := c.Store.AddMRComment(mr.ID, c.User.ID, body, fmtName); err != nil {
712 return c.fail(protocol.ExitFailure, "%v", err)
713 }
714 c.Store.RecordEvent(repo.ID, c.User.ID, "mr.commented", fmt.Sprintf(`{"number":%d}`, mr.Number))
715 if parts, err := c.Store.MRParticipants(mr.ID); err == nil {
716 notifyUsers(c, parts, mrSubject(repo, mr.Number, mr.Title),
717 notifyBody(c, fmt.Sprintf("commented on !%d", mr.Number), body, fmt.Sprintf("%s/mrs/%d", repo.Path(), mr.Number)))
718 }
719 return c.emit(map[string]any{"number": mr.Number}, func(w io.Writer) {
720 fmt.Fprintf(w, "commented on %s!%d\n", repo.Path(), mr.Number)
721 })
662 return runComment(c, args, mrThread, "mr",
663 func(rest []string) (store.Repo, int64, int64, string, int) {
664 repo, mr, code := mrRef(c, rest, policy.CanRead)
665 return repo, mr.ID, mr.Number, mr.Title, code
666 },
667 c.Store.AddMRComment, c.Store.MRParticipants)
722668}
723669
724670func runMRReview(c *Ctx, args []string) int {
@@ -1194,12 +1140,8 @@ func runMRClose(c *Ctx, args []string) int {
11941140 if len(args) != 2 {
11951141 return c.fail(protocol.ExitUsage, "usage: mr close <owner/name> <n>")
11961142 }
1197 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
1198 if err != nil {
1199 return c.fail(protocol.ExitFailure, "%v", err)
1200 }
1201 if mr.Author != c.User.Username && !policy.CanWrite(c.User, repo, grant) {
1202 return c.fail(protocol.ExitDenied, "only the author or users with write access can close this MR")
1143 if code := authorOrWrite(c, repo, mr.Author, "close this merge request"); code >= 0 {
1144 return code
12031145 }
12041146 if mr.State == "merged" || mr.State == "closed" {
12051147 return c.fail(protocol.ExitUsage, "MR !%d is already %s", mr.Number, mr.State)
internal/control/thread.go added +116
@@ -0,0 +1,116 @@
1package control
2
3import (
4 "fmt"
5 "io"
6 "strconv"
7 "strings"
8
9 "gitbay.org/gitbay/internal/policy"
10 "gitbay.org/gitbay/internal/protocol"
11 "gitbay.org/gitbay/internal/store"
12)
13
14// Issues and merge requests share their shape: a numbered thread in a
15// repository with an author, comments and participants. What used to be
16// two copies of the same code, one per noun, lives here once (#110).
17
18// refArgs resolves "<owner/name> <n>" to the repository and the number.
19func refArgs(c *Ctx, args []string, perm func(store.User, store.Repo, string) bool, noun string) (store.Repo, int64, int) {
20 if len(args) < 2 {
21 return store.Repo{}, 0, c.fail(protocol.ExitUsage, "expected <owner/name> <number>")
22 }
23 repo, code := resolveRepo(c, args[0], perm)
24 if code >= 0 {
25 return repo, 0, code
26 }
27 n, err := strconv.ParseInt(args[1], 10, 64)
28 if err != nil {
29 return repo, 0, c.fail(protocol.ExitUsage, "bad %s number %q", noun, args[1])
30 }
31 return repo, n, -1
32}
33
34// authorOrWrite admits the thread's author and anyone with write access,
35// which is who may change what a thread says or whether it is open.
36// -1 means proceed; what names the action in the refusal.
37func authorOrWrite(c *Ctx, repo store.Repo, author, what string) int {
38 if author == c.User.Username {
39 return -1
40 }
41 grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
42 if err != nil {
43 return c.fail(protocol.ExitFailure, "%v", err)
44 }
45 if !policy.CanWrite(c.User, repo, grant) {
46 return c.fail(protocol.ExitDenied, "only the author or users with write access can %s", what)
47 }
48 return -1
49}
50
51// commentOut is one comment as show emits it, for both nouns.
52type commentOut struct {
53 Author string `json:"author"`
54 Body string `json:"body"`
55 BodyFormat string `json:"body_format,omitempty"`
56 CreatedAt string `json:"created_at"`
57}
58
59// thread is what a comment command needs to know about its noun.
60type thread struct {
61 symbol string // "#" or "!"
62 segment string // "issues" or "mrs"
63 event string // issue.commented or mr.commented
64}
65
66var (
67 issueThread = thread{"#", "issues", "issue.commented"}
68 mrThread = thread{"!", "mrs", "mr.commented"}
69)
70
71// runComment is issue comment and mr comment: the noun's resolver hands
72// back the thread's id, number and title, and the rest is the same.
73func runComment(c *Ctx, args []string, t thread, noun string,
74 resolve func(rest []string) (repo store.Repo, id, number int64, title string, code int),
75 add func(id, userID int64, body, format string) error,
76 participants func(id int64) ([]int64, error),
77) int {
78 f, err := parseFlags(args, flagSpec{Values: []string{"--message", "--file", "--format"}, MaxPos: -1,
79 Usage: noun + " comment <owner/name> <n> [--message <m> | --file -] [--format md|org]"})
80 if err != nil {
81 return c.fail(protocol.ExitUsage, "%v", err)
82 }
83 fmtName, err := markupFormat(f.Value("--format"))
84 if err != nil {
85 return c.failErr(err)
86 }
87 if fmtName == "" {
88 fmtName = "md"
89 }
90 repo, id, number, title, code := resolve(f.Pos)
91 if code >= 0 {
92 return code
93 }
94 if code := refuseArchived(c, repo); code >= 0 {
95 return code
96 }
97 body, err := bodyFrom(c, f.Value("--message"), f.Value("--file"))
98 if err != nil {
99 return c.failErr(err)
100 }
101 if strings.TrimSpace(body) == "" {
102 return c.fail(protocol.ExitUsage, "empty comment; use --message or --file -")
103 }
104 if err := add(id, c.User.ID, body, fmtName); err != nil {
105 return c.fail(protocol.ExitFailure, "%v", err)
106 }
107 c.Store.RecordEvent(repo.ID, c.User.ID, t.event, fmt.Sprintf(`{"number":%d}`, number))
108 if parts, err := participants(id); err == nil {
109 subject := fmt.Sprintf("[%s] %s%d: %s", repo.Path(), t.symbol, number, title)
110 notifyUsers(c, parts, subject,
111 notifyBody(c, fmt.Sprintf("commented on %s%d", t.symbol, number), body, fmt.Sprintf("%s/%s/%d", repo.Path(), t.segment, number)))
112 }
113 return c.emit(map[string]any{"number": number}, func(w io.Writer) {
114 fmt.Fprintf(w, "commented on %s%s%d\n", repo.Path(), t.symbol, number)
115 })
116}