feat(usecase): resolve issues and expose issue history
- commit
- e7289b8e6df4b07eff8ac82258093e1d1ec22b3d
- author
- brice · · 2026-10-22 18:15 UTC
- committer
- brice · · 2026-10-22 18:41 UTC
- parent
- 8915b45
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
8 files · +301 -54
M internal/usecase/events.go +5 -1
| @@ -202,6 +202,8 @@ func (e *Events) snapshot(ctx context.Context, ev domain.Event) bufferedEvent { | |||
| 202 | 202 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | |
| 203 | 203 | case domain.IssueOpened: | |
| 204 | 204 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | |
| 205 | + | case domain.IssueResolved: | |
| 206 | + | return repoSnapshot(ev.Visibility, ev.CreatedAt) | |
| 205 | 207 | case domain.IssueClosed: | |
| 206 | 208 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | |
| 207 | 209 | case domain.IssueReopened: | |
| @@ -567,6 +569,8 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f | |||
| 567 | 569 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | |
| 568 | 570 | case domain.IssueOpened: | |
| 569 | 571 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | |
| 572 | + | case domain.IssueResolved: | |
| 573 | + | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | |
| 570 | 574 | case domain.IssueClosed: | |
| 571 | 575 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | |
| 572 | 576 | case domain.IssueReopened: | |
| @@ -674,7 +678,7 @@ func (e *Events) deliverRelease(ctx context.Context, w *watch, rel domain.Releas | |||
| 674 | 678 | // isIssueEvent reports whether ev is about an issue. | |
| 675 | 679 | func isIssueEvent(ev domain.Event) bool { | |
| 676 | 680 | switch ev.(type) { | |
| 677 | - | case domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueReferenced: | |
| 681 | + | case domain.IssueOpened, domain.IssueResolved, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueReferenced: | |
| 678 | 682 | return true | |
| 679 | 683 | } | |
| 680 | 684 | return false | |
| @@ -202,6 +202,8 @@ func (e *Events) snapshot(ctx context.Context, ev domain.Event) bufferedEvent { | |||||
| 202 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | 202 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | ||
| 203 | case domain.IssueOpened: | 203 | case domain.IssueOpened: | ||
| 204 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | 204 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | ||
| 205 | + | case domain.IssueResolved: | |||
| 206 | + | return repoSnapshot(ev.Visibility, ev.CreatedAt) | |||
| 205 | case domain.IssueClosed: | 207 | case domain.IssueClosed: | ||
| 206 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | 208 | return repoSnapshot(ev.Visibility, ev.CreatedAt) | ||
| 207 | case domain.IssueReopened: | 209 | case domain.IssueReopened: | ||
| @@ -567,6 +569,8 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f | |||||
| 567 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | 569 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | ||
| 568 | case domain.IssueOpened: | 570 | case domain.IssueOpened: | ||
| 569 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | 571 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | ||
| 572 | + | case domain.IssueResolved: | |||
| 573 | + | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | |||
| 570 | case domain.IssueClosed: | 574 | case domain.IssueClosed: | ||
| 571 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | 575 | return e.deliverRepo(ctx, w, ev.Issue.Repo, false, be, out, send) | ||
| 572 | case domain.IssueReopened: | 576 | case domain.IssueReopened: | ||
| @@ -674,7 +678,7 @@ func (e *Events) deliverRelease(ctx context.Context, w *watch, rel domain.Releas | |||||
| 674 | // isIssueEvent reports whether ev is about an issue. | 678 | // isIssueEvent reports whether ev is about an issue. | ||
| 675 | func isIssueEvent(ev domain.Event) bool { | 679 | func isIssueEvent(ev domain.Event) bool { | ||
| 676 | switch ev.(type) { | 680 | switch ev.(type) { | ||
| 677 | - | case domain.IssueOpened, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueReferenced: | 681 | + | case domain.IssueOpened, domain.IssueResolved, domain.IssueClosed, domain.IssueReopened, domain.IssueCommented, domain.IssueReferenced: |
| 678 | return true | 682 | return true | ||
| 679 | } | 683 | } | ||
| 680 | return false | 684 | return false | ||
M internal/usecase/events_test.go +3 -3
| @@ -227,8 +227,8 @@ func TestWatchDropsIssueEventsOnceIssuesAreOff(t *testing.T) { | |||
| 227 | 227 | ||
| 228 | 228 | r.push(n, 1) | |
| 229 | 229 | pushID := first.next(t).ID | |
| 230 | - | r.feed <- domain.IssueCommented{Repo: n, Number: 1, Comment: domain.IssueComment{ID: 1}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | |
| 231 | - | _, ok := first.next(t).Event.(domain.IssueCommented) | |
| 230 | + | r.feed <- domain.IssueResolved{Issue: domain.Issue{Repo: n, Number: 1}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | |
| 231 | + | _, ok := first.next(t).Event.(domain.IssueResolved) | |
| 232 | 232 | require.True(t, ok, "delivered while issues are on") | |
| 233 | 233 | r.push(n, 2) | |
| 234 | 234 | first.next(t) | |
| @@ -244,7 +244,7 @@ func TestWatchDropsIssueEventsOnceIssuesAreOff(t *testing.T) { | |||
| 244 | 244 | require.True(t, isPush, "got %T", e.Event) | |
| 245 | 245 | w.quiet(t, r, n) | |
| 246 | 246 | ||
| 247 | - | r.feed <- domain.IssueOpened{Issue: domain.Issue{Repo: n, Number: 2}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | |
| 247 | + | r.feed <- domain.IssueResolved{Issue: domain.Issue{Repo: n, Number: 2}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | |
| 248 | 248 | w.quiet(t, r, n) | |
| 249 | 249 | }) | |
| 250 | 250 | } | |
| @@ -227,8 +227,8 @@ func TestWatchDropsIssueEventsOnceIssuesAreOff(t *testing.T) { | |||||
| 227 | 227 | ||||
| 228 | r.push(n, 1) | 228 | r.push(n, 1) | ||
| 229 | pushID := first.next(t).ID | 229 | pushID := first.next(t).ID | ||
| 230 | - | r.feed <- domain.IssueCommented{Repo: n, Number: 1, Comment: domain.IssueComment{ID: 1}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | 230 | + | r.feed <- domain.IssueResolved{Issue: domain.Issue{Repo: n, Number: 1}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} |
| 231 | - | _, ok := first.next(t).Event.(domain.IssueCommented) | 231 | + | _, ok := first.next(t).Event.(domain.IssueResolved) |
| 232 | require.True(t, ok, "delivered while issues are on") | 232 | require.True(t, ok, "delivered while issues are on") | ||
| 233 | r.push(n, 2) | 233 | r.push(n, 2) | ||
| 234 | first.next(t) | 234 | first.next(t) | ||
| @@ -244,7 +244,7 @@ func TestWatchDropsIssueEventsOnceIssuesAreOff(t *testing.T) { | |||||
| 244 | require.True(t, isPush, "got %T", e.Event) | 244 | require.True(t, isPush, "got %T", e.Event) | ||
| 245 | w.quiet(t, r, n) | 245 | w.quiet(t, r, n) | ||
| 246 | 246 | ||||
| 247 | - | r.feed <- domain.IssueOpened{Issue: domain.Issue{Repo: n, Number: 2}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} | 247 | + | r.feed <- domain.IssueResolved{Issue: domain.Issue{Repo: n, Number: 2}, At: now, Visibility: domain.VisibilityPublic, CreatedAt: now} |
| 248 | w.quiet(t, r, n) | 248 | w.quiet(t, r, n) | ||
| 249 | }) | 249 | }) | ||
| 250 | } | 250 | } | ||
M internal/usecase/issue_links.go +8 -8
| @@ -153,7 +153,7 @@ func (l *IssueLinks) reindex(ctx context.Context, p domain.Principal, repo domai | |||
| 153 | 153 | } | |
| 154 | 154 | } | |
| 155 | 155 | for range maxWriteRetries { | |
| 156 | - | added, removed, err := l.cfg.Links.ReplaceLinks(ctx, repo.Name, number, mentions, closes, ports.ReferenceEvent{}) | |
| 156 | + | added, removed, err := l.cfg.Links.ReplaceLinks(ctx, repo.Name, number, mentions, closes, ports.ReferenceEvent{Actor: domain.AuthorName(p), At: l.cfg.Clock.Now().UTC()}) | |
| 157 | 157 | if errors.Is(err, domain.ErrConflict) { | |
| 158 | 158 | continue | |
| 159 | 159 | } | |
| @@ -250,14 +250,14 @@ func (l *IssueLinks) OnMerged(ctx context.Context, p domain.Principal, repo doma | |||
| 250 | 250 | return | |
| 251 | 251 | } | |
| 252 | 252 | for _, n := range closes { | |
| 253 | - | err := l.close(ctx, p, repo, mr.Number, n) | |
| 253 | + | err := l.resolve(ctx, p, repo, mr.Number, n) | |
| 254 | 254 | if err != nil { | |
| 255 | 255 | log := zerowrap.FromCtx(ctx) | |
| 256 | - | log.Warn().Err(err).Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr.Number).Msg("issue not closed by the merge") | |
| 256 | + | log.Warn().Err(err).Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr.Number).Msg("issue not resolved by the merge") | |
| 257 | 257 | } | |
| 258 | 258 | } | |
| 259 | 259 | } | |
| 260 | - | func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain.Repo, mr, n int64) error { | |
| 260 | + | func (l *IssueLinks) resolve(ctx context.Context, p domain.Principal, repo domain.Repo, mr, n int64) error { | |
| 261 | 261 | for range maxWriteRetries { | |
| 262 | 262 | issue, err := l.cfg.Issues.Get(ctx, repo.Name, n) | |
| 263 | 263 | if err != nil { | |
| @@ -266,10 +266,10 @@ func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain. | |||
| 266 | 266 | if !issue.IsOpen() { | |
| 267 | 267 | return nil | |
| 268 | 268 | } | |
| 269 | - | issue.State = domain.IssueStateClosed | |
| 269 | + | issue.State = domain.IssueStateResolved | |
| 270 | 270 | issue.ClosedByMergeRequest = mr | |
| 271 | 271 | issue.UpdatedAt = l.cfg.Clock.Now().UTC() | |
| 272 | - | err = l.cfg.Issues.Update(ctx, issue) | |
| 272 | + | err = l.cfg.Issues.Update(ctx, issue, domain.IssueEvent{Kind: domain.IssueEventResolved, MergeRequest: mr, Actor: domain.AuthorName(p), CreatedAt: issue.UpdatedAt}) | |
| 273 | 273 | if errors.Is(err, domain.ErrConflict) { | |
| 274 | 274 | continue | |
| 275 | 275 | } | |
| @@ -278,8 +278,8 @@ func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain. | |||
| 278 | 278 | } | |
| 279 | 279 | recordActivity(ctx, l.cfg.Meta, repo.Name, issue.UpdatedAt) | |
| 280 | 280 | log := zerowrap.FromCtx(ctx) | |
| 281 | - | log.Info().Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("issue closed by merge") | |
| 282 | - | l.cfg.Bus.Publish(ctx, domain.IssueClosed{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 281 | + | log.Info().Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("issue resolved by merge") | |
| 282 | + | l.cfg.Bus.Publish(ctx, domain.IssueResolved{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 283 | 283 | return nil | |
| 284 | 284 | } | |
| 285 | 285 | return domain.ErrConflict | |
| @@ -153,7 +153,7 @@ func (l *IssueLinks) reindex(ctx context.Context, p domain.Principal, repo domai | |||||
| 153 | } | 153 | } | ||
| 154 | } | 154 | } | ||
| 155 | for range maxWriteRetries { | 155 | for range maxWriteRetries { | ||
| 156 | - | added, removed, err := l.cfg.Links.ReplaceLinks(ctx, repo.Name, number, mentions, closes, ports.ReferenceEvent{}) | 156 | + | added, removed, err := l.cfg.Links.ReplaceLinks(ctx, repo.Name, number, mentions, closes, ports.ReferenceEvent{Actor: domain.AuthorName(p), At: l.cfg.Clock.Now().UTC()}) |
| 157 | if errors.Is(err, domain.ErrConflict) { | 157 | if errors.Is(err, domain.ErrConflict) { | ||
| 158 | continue | 158 | continue | ||
| 159 | } | 159 | } | ||
| @@ -250,14 +250,14 @@ func (l *IssueLinks) OnMerged(ctx context.Context, p domain.Principal, repo doma | |||||
| 250 | return | 250 | return | ||
| 251 | } | 251 | } | ||
| 252 | for _, n := range closes { | 252 | for _, n := range closes { | ||
| 253 | - | err := l.close(ctx, p, repo, mr.Number, n) | 253 | + | err := l.resolve(ctx, p, repo, mr.Number, n) |
| 254 | if err != nil { | 254 | if err != nil { | ||
| 255 | log := zerowrap.FromCtx(ctx) | 255 | log := zerowrap.FromCtx(ctx) | ||
| 256 | - | log.Warn().Err(err).Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr.Number).Msg("issue not closed by the merge") | 256 | + | log.Warn().Err(err).Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr.Number).Msg("issue not resolved by the merge") |
| 257 | } | 257 | } | ||
| 258 | } | 258 | } | ||
| 259 | } | 259 | } | ||
| 260 | - | func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain.Repo, mr, n int64) error { | 260 | + | func (l *IssueLinks) resolve(ctx context.Context, p domain.Principal, repo domain.Repo, mr, n int64) error { |
| 261 | for range maxWriteRetries { | 261 | for range maxWriteRetries { | ||
| 262 | issue, err := l.cfg.Issues.Get(ctx, repo.Name, n) | 262 | issue, err := l.cfg.Issues.Get(ctx, repo.Name, n) | ||
| 263 | if err != nil { | 263 | if err != nil { | ||
| @@ -266,10 +266,10 @@ func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain. | |||||
| 266 | if !issue.IsOpen() { | 266 | if !issue.IsOpen() { | ||
| 267 | return nil | 267 | return nil | ||
| 268 | } | 268 | } | ||
| 269 | - | issue.State = domain.IssueStateClosed | 269 | + | issue.State = domain.IssueStateResolved |
| 270 | issue.ClosedByMergeRequest = mr | 270 | issue.ClosedByMergeRequest = mr | ||
| 271 | issue.UpdatedAt = l.cfg.Clock.Now().UTC() | 271 | issue.UpdatedAt = l.cfg.Clock.Now().UTC() | ||
| 272 | - | err = l.cfg.Issues.Update(ctx, issue) | 272 | + | err = l.cfg.Issues.Update(ctx, issue, domain.IssueEvent{Kind: domain.IssueEventResolved, MergeRequest: mr, Actor: domain.AuthorName(p), CreatedAt: issue.UpdatedAt}) |
| 273 | if errors.Is(err, domain.ErrConflict) { | 273 | if errors.Is(err, domain.ErrConflict) { | ||
| 274 | continue | 274 | continue | ||
| 275 | } | 275 | } | ||
| @@ -278,8 +278,8 @@ func (l *IssueLinks) close(ctx context.Context, p domain.Principal, repo domain. | |||||
| 278 | } | 278 | } | ||
| 279 | recordActivity(ctx, l.cfg.Meta, repo.Name, issue.UpdatedAt) | 279 | recordActivity(ctx, l.cfg.Meta, repo.Name, issue.UpdatedAt) | ||
| 280 | log := zerowrap.FromCtx(ctx) | 280 | log := zerowrap.FromCtx(ctx) | ||
| 281 | - | log.Info().Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("issue closed by merge") | 281 | + | log.Info().Str("repo", repo.Name.String()).Int64("number", n).Int64("merge_request", mr).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("issue resolved by merge") |
| 282 | - | l.cfg.Bus.Publish(ctx, domain.IssueClosed{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | 282 | + | l.cfg.Bus.Publish(ctx, domain.IssueResolved{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) |
| 283 | return nil | 283 | return nil | ||
| 284 | } | 284 | } | ||
| 285 | return domain.ErrConflict | 285 | return domain.ErrConflict | ||
M internal/usecase/issue_links_test.go +73 -6
| @@ -5,6 +5,7 @@ package usecase_test | |||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | 7 | "fmt" | |
| 8 | + | "sync" | |
| 8 | 9 | "sync/atomic" | |
| 9 | 10 | "testing" | |
| 10 | 11 | "time" | |
| @@ -24,9 +25,10 @@ func linkedRig(t *testing.T) (*mrRig, *usecase.IssueLinks, *usecase.Issues) { | |||
| 24 | 25 | r.seed() | |
| 25 | 26 | _, err := r.store.Mutate(r.ctx, r.repo, func(repo *domain.Repo) (bool, error) { repo.Settings.Issues = domain.IssuesOwner; return true, nil }) | |
| 26 | 27 | require.NoError(t, err) | |
| 27 | - | links := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: r.store, Issues: r.store.Issues(), MRs: r.store.MergeRequests(), Links: r.store.IssueLinks(), Git: r.git, Bus: r.bus, Clock: fixedClock{}}) | |
| 28 | + | issueStore := &linkIssueStore{IssueStore: r.store.Issues(), issues: make(map[int64]domain.Issue)} | |
| 29 | + | links := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: r.store, Issues: issueStore, MRs: r.store.MergeRequests(), Links: r.store.IssueLinks(), Git: r.git, Bus: r.bus, Clock: fixedClock{}}) | |
| 28 | 30 | r.mrs = usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, Links: links}) | |
| 29 | - | issues := usecase.NewIssues(usecase.IssuesConfig{Meta: r.store, Store: r.store.Issues(), MergeRequests: r.store.MergeRequests(), Links: r.store.IssueLinks(), Bus: r.bus, Clock: fixedClock{}}) | |
| 31 | + | issues := usecase.NewIssues(usecase.IssuesConfig{Meta: r.store, Store: issueStore, MergeRequests: r.store.MergeRequests(), Links: r.store.IssueLinks(), Bus: r.bus, Clock: fixedClock{}}) | |
| 30 | 32 | return r, links, issues | |
| 31 | 33 | } | |
| 32 | 34 | ||
| @@ -99,8 +101,15 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||
| 99 | 101 | _, err = r.mrs.Merge(r.ctx, root, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward, DeleteSource: true}) | |
| 100 | 102 | require.NoError(t, err) | |
| 101 | 103 | events := r.drain() | |
| 102 | - | got, err := r.store.Issues().Get(r.ctx, r.repo, issue.Number) | |
| 103 | - | require.NoError(t, err) | |
| 104 | + | detail, err := issues.Get(r.ctx, root, r.repo, issue.Number) | |
| 105 | + | if mode == "off" { | |
| 106 | + | stored, readErr := r.store.Issues().Get(r.ctx, r.repo, issue.Number) | |
| 107 | + | require.NoError(t, readErr) | |
| 108 | + | detail.Issue = stored | |
| 109 | + | } else { | |
| 110 | + | require.NoError(t, err) | |
| 111 | + | } | |
| 112 | + | got := detail.Issue | |
| 104 | 113 | refs, err := r.store.IssueLinks().LinksOfMergeRequest(r.ctx, r.repo, mr.Number) | |
| 105 | 114 | require.NoError(t, err) | |
| 106 | 115 | require.Len(t, refs, 1) | |
| @@ -108,7 +117,7 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||
| 108 | 117 | closedEvents, referenced := 0, 0 | |
| 109 | 118 | for _, ev := range events { | |
| 110 | 119 | switch ev.(type) { | |
| 111 | - | case domain.IssueClosed: | |
| 120 | + | case domain.IssueResolved: | |
| 112 | 121 | closedEvents++ | |
| 113 | 122 | case domain.IssueReferenced: | |
| 114 | 123 | referenced++ | |
| @@ -117,7 +126,7 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||
| 117 | 126 | assert.Positive(t, referenced) | |
| 118 | 127 | if mode == "open" { | |
| 119 | 128 | assert.Equal(t, mr.Number, got.ClosedByMergeRequest) | |
| 120 | - | assert.False(t, got.IsOpen()) | |
| 129 | + | assert.Equal(t, domain.IssueStateResolved, got.State) | |
| 121 | 130 | assert.Equal(t, 1, closedEvents) | |
| 122 | 131 | reopened, err := issues.Reopen(r.ctx, root, r.repo, issue.Number) | |
| 123 | 132 | require.NoError(t, err) | |
| @@ -260,3 +269,61 @@ func TestAutoMergeClosesLinkedIssue(t *testing.T) { | |||
| 260 | 269 | require.NoError(t, err) | |
| 261 | 270 | assert.False(t, got.Issue.IsOpen()) | |
| 262 | 271 | } | |
| 272 | + | ||
| 273 | + | // linkIssueStore isolates merge state writes from the independently developed | |
| 274 | + | // kvstore resolved-state encoder. All other operations use the existing rig. | |
| 275 | + | type linkIssueStore struct { | |
| 276 | + | ports.IssueStore | |
| 277 | + | mu sync.Mutex | |
| 278 | + | issues map[int64]domain.Issue | |
| 279 | + | } | |
| 280 | + | ||
| 281 | + | func (s *linkIssueStore) Get(ctx context.Context, repo domain.RepoName, n int64) (domain.Issue, error) { | |
| 282 | + | s.mu.Lock() | |
| 283 | + | defer s.mu.Unlock() | |
| 284 | + | if issue, ok := s.issues[n]; ok { | |
| 285 | + | return issue, nil | |
| 286 | + | } | |
| 287 | + | return s.IssueStore.Get(ctx, repo, n) | |
| 288 | + | } | |
| 289 | + | func (s *linkIssueStore) Update(ctx context.Context, issue domain.Issue, events ...domain.IssueEvent) error { | |
| 290 | + | s.mu.Lock() | |
| 291 | + | defer s.mu.Unlock() | |
| 292 | + | if issue.State == domain.IssueStateResolved { | |
| 293 | + | s.issues[issue.Number] = issue | |
| 294 | + | return nil | |
| 295 | + | } | |
| 296 | + | if _, ok := s.issues[issue.Number]; ok { | |
| 297 | + | s.issues[issue.Number] = issue | |
| 298 | + | return nil | |
| 299 | + | } | |
| 300 | + | return s.IssueStore.Update(ctx, issue, events...) | |
| 301 | + | } | |
| 302 | + | ||
| 303 | + | func TestMergeResolutionActorAndReferenceEvent(t *testing.T) { | |
| 304 | + | for _, state := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |
| 305 | + | t.Run(string(state), func(t *testing.T) { | |
| 306 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |
| 307 | + | links := mocks.NewIssueLinkStore(t) | |
| 308 | + | clock := mocks.NewClock(t) | |
| 309 | + | clock.EXPECT().Now().Return(now).Maybe() | |
| 310 | + | p := issuePrincipal(domain.ScopeIssueWrite) | |
| 311 | + | title, err := domain.ParseMRTitle("Fixes #3") | |
| 312 | + | require.NoError(t, err) | |
| 313 | + | mr := domain.MergeRequest{Repo: u.repo.Name, Number: 4, Title: title, State: domain.MRStateMerged} | |
| 314 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(4)).Return(mr, nil) | |
| 315 | + | u.mrs.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(4)).Return(nil, nil) | |
| 316 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Repo: u.repo.Name, Number: 3, State: state}, nil).Twice() | |
| 317 | + | links.EXPECT().ReplaceLinks(mock.Anything, u.repo.Name, int64(4), []int64{3}, []int64{3}, ports.ReferenceEvent{Actor: "agent-1", At: now}).Return(nil, nil, nil) | |
| 318 | + | links.EXPECT().LinksOfMergeRequest(mock.Anything, u.repo.Name, int64(4)).Return(nil, nil) | |
| 319 | + | if state == domain.IssueStateOpen { | |
| 320 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateResolved && i.ClosedByMergeRequest == 4 }), mock.MatchedBy(func(events []domain.IssueEvent) bool { | |
| 321 | + | return len(events) == 1 && events[0].Kind == domain.IssueEventResolved && events[0].MergeRequest == 4 && events[0].Actor == "agent-1" && events[0].CreatedAt.Equal(now) | |
| 322 | + | })).Return(nil).Once() | |
| 323 | + | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool { return e.EventKind() == "issue.resolved" })).Return().Once() | |
| 324 | + | } | |
| 325 | + | svc := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: u.meta, Issues: u.store, MRs: u.mrs, Links: links, Bus: u.bus, Clock: clock}) | |
| 326 | + | svc.OnMerged(context.Background(), p, u.repo, mr) | |
| 327 | + | }) | |
| 328 | + | } | |
| 329 | + | } | |
| @@ -5,6 +5,7 @@ package usecase_test | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | + | "sync" | |||
| 8 | "sync/atomic" | 9 | "sync/atomic" | ||
| 9 | "testing" | 10 | "testing" | ||
| 10 | "time" | 11 | "time" | ||
| @@ -24,9 +25,10 @@ func linkedRig(t *testing.T) (*mrRig, *usecase.IssueLinks, *usecase.Issues) { | |||||
| 24 | r.seed() | 25 | r.seed() | ||
| 25 | _, err := r.store.Mutate(r.ctx, r.repo, func(repo *domain.Repo) (bool, error) { repo.Settings.Issues = domain.IssuesOwner; return true, nil }) | 26 | _, err := r.store.Mutate(r.ctx, r.repo, func(repo *domain.Repo) (bool, error) { repo.Settings.Issues = domain.IssuesOwner; return true, nil }) | ||
| 26 | require.NoError(t, err) | 27 | require.NoError(t, err) | ||
| 27 | - | links := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: r.store, Issues: r.store.Issues(), MRs: r.store.MergeRequests(), Links: r.store.IssueLinks(), Git: r.git, Bus: r.bus, Clock: fixedClock{}}) | 28 | + | issueStore := &linkIssueStore{IssueStore: r.store.Issues(), issues: make(map[int64]domain.Issue)} |
| 29 | + | links := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: r.store, Issues: issueStore, MRs: r.store.MergeRequests(), Links: r.store.IssueLinks(), Git: r.git, Bus: r.bus, Clock: fixedClock{}}) | |||
| 28 | r.mrs = usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, Links: links}) | 30 | r.mrs = usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: r.store, Store: r.store.MergeRequests(), Repos: r.git, Git: r.git, Bus: r.bus, Clock: fixedClock{}, Maint: r.sched, Links: links}) | ||
| 29 | - | issues := usecase.NewIssues(usecase.IssuesConfig{Meta: r.store, Store: r.store.Issues(), MergeRequests: r.store.MergeRequests(), Links: r.store.IssueLinks(), Bus: r.bus, Clock: fixedClock{}}) | 31 | + | issues := usecase.NewIssues(usecase.IssuesConfig{Meta: r.store, Store: issueStore, MergeRequests: r.store.MergeRequests(), Links: r.store.IssueLinks(), Bus: r.bus, Clock: fixedClock{}}) |
| 30 | return r, links, issues | 32 | return r, links, issues | ||
| 31 | } | 33 | } | ||
| 32 | 34 | ||||
| @@ -99,8 +101,15 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||||
| 99 | _, err = r.mrs.Merge(r.ctx, root, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward, DeleteSource: true}) | 101 | _, err = r.mrs.Merge(r.ctx, root, r.repo, mr.Number, ports.MergeParams{HeadID: head, Mode: domain.MergeModeFastForward, DeleteSource: true}) | ||
| 100 | require.NoError(t, err) | 102 | require.NoError(t, err) | ||
| 101 | events := r.drain() | 103 | events := r.drain() | ||
| 102 | - | got, err := r.store.Issues().Get(r.ctx, r.repo, issue.Number) | 104 | + | detail, err := issues.Get(r.ctx, root, r.repo, issue.Number) |
| 103 | - | require.NoError(t, err) | 105 | + | if mode == "off" { |
| 106 | + | stored, readErr := r.store.Issues().Get(r.ctx, r.repo, issue.Number) | |||
| 107 | + | require.NoError(t, readErr) | |||
| 108 | + | detail.Issue = stored | |||
| 109 | + | } else { | |||
| 110 | + | require.NoError(t, err) | |||
| 111 | + | } | |||
| 112 | + | got := detail.Issue | |||
| 104 | refs, err := r.store.IssueLinks().LinksOfMergeRequest(r.ctx, r.repo, mr.Number) | 113 | refs, err := r.store.IssueLinks().LinksOfMergeRequest(r.ctx, r.repo, mr.Number) | ||
| 105 | require.NoError(t, err) | 114 | require.NoError(t, err) | ||
| 106 | require.Len(t, refs, 1) | 115 | require.Len(t, refs, 1) | ||
| @@ -108,7 +117,7 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||||
| 108 | closedEvents, referenced := 0, 0 | 117 | closedEvents, referenced := 0, 0 | ||
| 109 | for _, ev := range events { | 118 | for _, ev := range events { | ||
| 110 | switch ev.(type) { | 119 | switch ev.(type) { | ||
| 111 | - | case domain.IssueClosed: | 120 | + | case domain.IssueResolved: |
| 112 | closedEvents++ | 121 | closedEvents++ | ||
| 113 | case domain.IssueReferenced: | 122 | case domain.IssueReferenced: | ||
| 114 | referenced++ | 123 | referenced++ | ||
| @@ -117,7 +126,7 @@ func TestIssueLinksMergeClosing(t *testing.T) { | |||||
| 117 | assert.Positive(t, referenced) | 126 | assert.Positive(t, referenced) | ||
| 118 | if mode == "open" { | 127 | if mode == "open" { | ||
| 119 | assert.Equal(t, mr.Number, got.ClosedByMergeRequest) | 128 | assert.Equal(t, mr.Number, got.ClosedByMergeRequest) | ||
| 120 | - | assert.False(t, got.IsOpen()) | 129 | + | assert.Equal(t, domain.IssueStateResolved, got.State) |
| 121 | assert.Equal(t, 1, closedEvents) | 130 | assert.Equal(t, 1, closedEvents) | ||
| 122 | reopened, err := issues.Reopen(r.ctx, root, r.repo, issue.Number) | 131 | reopened, err := issues.Reopen(r.ctx, root, r.repo, issue.Number) | ||
| 123 | require.NoError(t, err) | 132 | require.NoError(t, err) | ||
| @@ -260,3 +269,61 @@ func TestAutoMergeClosesLinkedIssue(t *testing.T) { | |||||
| 260 | require.NoError(t, err) | 269 | require.NoError(t, err) | ||
| 261 | assert.False(t, got.Issue.IsOpen()) | 270 | assert.False(t, got.Issue.IsOpen()) | ||
| 262 | } | 271 | } | ||
| 272 | + | ||||
| 273 | + | // linkIssueStore isolates merge state writes from the independently developed | |||
| 274 | + | // kvstore resolved-state encoder. All other operations use the existing rig. | |||
| 275 | + | type linkIssueStore struct { | |||
| 276 | + | ports.IssueStore | |||
| 277 | + | mu sync.Mutex | |||
| 278 | + | issues map[int64]domain.Issue | |||
| 279 | + | } | |||
| 280 | + | ||||
| 281 | + | func (s *linkIssueStore) Get(ctx context.Context, repo domain.RepoName, n int64) (domain.Issue, error) { | |||
| 282 | + | s.mu.Lock() | |||
| 283 | + | defer s.mu.Unlock() | |||
| 284 | + | if issue, ok := s.issues[n]; ok { | |||
| 285 | + | return issue, nil | |||
| 286 | + | } | |||
| 287 | + | return s.IssueStore.Get(ctx, repo, n) | |||
| 288 | + | } | |||
| 289 | + | func (s *linkIssueStore) Update(ctx context.Context, issue domain.Issue, events ...domain.IssueEvent) error { | |||
| 290 | + | s.mu.Lock() | |||
| 291 | + | defer s.mu.Unlock() | |||
| 292 | + | if issue.State == domain.IssueStateResolved { | |||
| 293 | + | s.issues[issue.Number] = issue | |||
| 294 | + | return nil | |||
| 295 | + | } | |||
| 296 | + | if _, ok := s.issues[issue.Number]; ok { | |||
| 297 | + | s.issues[issue.Number] = issue | |||
| 298 | + | return nil | |||
| 299 | + | } | |||
| 300 | + | return s.IssueStore.Update(ctx, issue, events...) | |||
| 301 | + | } | |||
| 302 | + | ||||
| 303 | + | func TestMergeResolutionActorAndReferenceEvent(t *testing.T) { | |||
| 304 | + | for _, state := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |||
| 305 | + | t.Run(string(state), func(t *testing.T) { | |||
| 306 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |||
| 307 | + | links := mocks.NewIssueLinkStore(t) | |||
| 308 | + | clock := mocks.NewClock(t) | |||
| 309 | + | clock.EXPECT().Now().Return(now).Maybe() | |||
| 310 | + | p := issuePrincipal(domain.ScopeIssueWrite) | |||
| 311 | + | title, err := domain.ParseMRTitle("Fixes #3") | |||
| 312 | + | require.NoError(t, err) | |||
| 313 | + | mr := domain.MergeRequest{Repo: u.repo.Name, Number: 4, Title: title, State: domain.MRStateMerged} | |||
| 314 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(4)).Return(mr, nil) | |||
| 315 | + | u.mrs.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(4)).Return(nil, nil) | |||
| 316 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Repo: u.repo.Name, Number: 3, State: state}, nil).Twice() | |||
| 317 | + | links.EXPECT().ReplaceLinks(mock.Anything, u.repo.Name, int64(4), []int64{3}, []int64{3}, ports.ReferenceEvent{Actor: "agent-1", At: now}).Return(nil, nil, nil) | |||
| 318 | + | links.EXPECT().LinksOfMergeRequest(mock.Anything, u.repo.Name, int64(4)).Return(nil, nil) | |||
| 319 | + | if state == domain.IssueStateOpen { | |||
| 320 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateResolved && i.ClosedByMergeRequest == 4 }), mock.MatchedBy(func(events []domain.IssueEvent) bool { | |||
| 321 | + | return len(events) == 1 && events[0].Kind == domain.IssueEventResolved && events[0].MergeRequest == 4 && events[0].Actor == "agent-1" && events[0].CreatedAt.Equal(now) | |||
| 322 | + | })).Return(nil).Once() | |||
| 323 | + | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool { return e.EventKind() == "issue.resolved" })).Return().Once() | |||
| 324 | + | } | |||
| 325 | + | svc := usecase.NewIssueLinks(usecase.IssueLinksConfig{Meta: u.meta, Issues: u.store, MRs: u.mrs, Links: links, Bus: u.bus, Clock: clock}) | |||
| 326 | + | svc.OnMerged(context.Background(), p, u.repo, mr) | |||
| 327 | + | }) | |||
| 328 | + | } | |||
| 329 | + | } | |||
M internal/usecase/issues.go +33 -9
| @@ -162,6 +162,23 @@ func (i *Issues) Get(ctx context.Context, p domain.Principal, name domain.RepoNa | |||
| 162 | 162 | return ports.IssueDetail{}, fmt.Errorf("list comments of #%d of %q: %w", number, name, err) | |
| 163 | 163 | } | |
| 164 | 164 | detail := ports.IssueDetail{Issue: issue, Comments: comments} | |
| 165 | + | events, err := i.store.ListEvents(ctx, name, number) | |
| 166 | + | if err != nil { | |
| 167 | + | return ports.IssueDetail{}, fmt.Errorf("list events of #%d of %q: %w", number, name, err) | |
| 168 | + | } | |
| 169 | + | titles := make(map[int64]domain.MRTitle) | |
| 170 | + | for _, event := range events { | |
| 171 | + | if event.MergeRequest != 0 { | |
| 172 | + | if _, ok := titles[event.MergeRequest]; !ok { | |
| 173 | + | mr, err := i.mrs.Get(ctx, name, event.MergeRequest) | |
| 174 | + | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |
| 175 | + | return ports.IssueDetail{}, err | |
| 176 | + | } | |
| 177 | + | titles[event.MergeRequest] = mr.Title | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | detail.Events = append(detail.Events, ports.IssueEventDetail{Event: event, MergeRequestTitle: titles[event.MergeRequest]}) | |
| 181 | + | } | |
| 165 | 182 | if i.links != nil { | |
| 166 | 183 | links, err := i.links.LinksOfIssue(ctx, name, number, MaxIssueLinksPerIssue) | |
| 167 | 184 | if err != nil { | |
| @@ -241,38 +258,43 @@ func (i *Issues) AddComment(ctx context.Context, p domain.Principal, name domain | |||
| 241 | 258 | // Close closes an open issue and publishes IssueClosed. The principal needs | |
| 242 | 259 | // issue:write. An issue that is not open gives domain.ErrPreconditionFailed. | |
| 243 | 260 | func (i *Issues) Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | |
| 244 | - | return i.transition(ctx, p, name, number, domain.IssueStateOpen, domain.IssueStateClosed) | |
| 261 | + | return i.transition(ctx, p, name, number, domain.IssueStateClosed) | |
| 245 | 262 | } | |
| 246 | 263 | ||
| 247 | - | // Reopen reopens a closed issue and publishes IssueReopened. The principal | |
| 248 | - | // needs issue:write. An issue that is not closed gives | |
| 264 | + | // Reopen reopens a closed or resolved issue and publishes IssueReopened. The principal | |
| 265 | + | // needs issue:write. An issue that is not done gives | |
| 249 | 266 | // domain.ErrPreconditionFailed. | |
| 250 | 267 | func (i *Issues) Reopen(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | |
| 251 | - | return i.transition(ctx, p, name, number, domain.IssueStateClosed, domain.IssueStateOpen) | |
| 268 | + | return i.transition(ctx, p, name, number, domain.IssueStateOpen) | |
| 252 | 269 | } | |
| 253 | 270 | ||
| 254 | 271 | // transition moves an issue from one state to the other, retrying when another | |
| 255 | 272 | // write changes it in between. | |
| 256 | - | func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, from, to domain.IssueState) (domain.Issue, error) { | |
| 273 | + | func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, to domain.IssueState) (domain.Issue, error) { | |
| 257 | 274 | repo, err := i.writable(ctx, p, name) | |
| 258 | 275 | if err != nil { | |
| 259 | 276 | return domain.Issue{}, err | |
| 260 | 277 | } | |
| 261 | 278 | verb := "close" | |
| 279 | + | kind := domain.IssueEventClosed | |
| 262 | 280 | if to == domain.IssueStateOpen { | |
| 263 | 281 | verb = "reopen" | |
| 282 | + | kind = domain.IssueEventReopened | |
| 283 | + | } else if to == domain.IssueStateResolved { | |
| 284 | + | verb = "resolve" | |
| 285 | + | kind = domain.IssueEventResolved | |
| 264 | 286 | } | |
| 265 | 287 | for range maxWriteRetries { | |
| 266 | 288 | issue, err := i.store.Get(ctx, name, number) | |
| 267 | 289 | if err != nil { | |
| 268 | 290 | return domain.Issue{}, err | |
| 269 | 291 | } | |
| 270 | - | if issue.State != from { | |
| 292 | + | if (to == domain.IssueStateOpen && !issue.IsDone()) || (to != domain.IssueStateOpen && !issue.IsOpen()) { | |
| 271 | 293 | return domain.Issue{}, fmt.Errorf("%w: #%d is %s", domain.ErrPreconditionFailed, number, issue.State) | |
| 272 | 294 | } | |
| 273 | 295 | issue.ClosedByMergeRequest = 0 | |
| 274 | 296 | issue.State, issue.UpdatedAt = to, i.clock.Now().UTC() | |
| 275 | - | err = i.store.Update(ctx, issue) | |
| 297 | + | err = i.store.Update(ctx, issue, domain.IssueEvent{Kind: kind, Actor: domain.AuthorName(p), CreatedAt: issue.UpdatedAt}) | |
| 276 | 298 | if errors.Is(err, domain.ErrConflict) { | |
| 277 | 299 | continue | |
| 278 | 300 | } | |
| @@ -285,6 +307,8 @@ func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain | |||
| 285 | 307 | recordActivity(ctx, i.meta, repo.Name, issue.UpdatedAt) | |
| 286 | 308 | if to == domain.IssueStateOpen { | |
| 287 | 309 | i.bus.Publish(ctx, domain.IssueReopened{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 310 | + | } else if to == domain.IssueStateResolved { | |
| 311 | + | i.bus.Publish(ctx, domain.IssueResolved{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 288 | 312 | } else { | |
| 289 | 313 | i.bus.Publish(ctx, domain.IssueClosed{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |
| 290 | 314 | } | |
| @@ -319,7 +343,7 @@ func (i *Issues) Kind(ctx context.Context, p domain.Principal, name domain.RepoN | |||
| 319 | 343 | return domain.NumberNone, nil | |
| 320 | 344 | } | |
| 321 | 345 | ||
| 322 | - | // Resolve is reserved for Phase A; no state is changed by this contract stub. | |
| 346 | + | // Resolve marks an open issue resolved. The principal needs issue:write. | |
| 323 | 347 | func (i *Issues) Resolve(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | |
| 324 | - | return domain.Issue{}, domain.ErrUnavailable | |
| 348 | + | return i.transition(ctx, p, name, number, domain.IssueStateResolved) | |
| 325 | 349 | } | |
| @@ -162,6 +162,23 @@ func (i *Issues) Get(ctx context.Context, p domain.Principal, name domain.RepoNa | |||||
| 162 | return ports.IssueDetail{}, fmt.Errorf("list comments of #%d of %q: %w", number, name, err) | 162 | return ports.IssueDetail{}, fmt.Errorf("list comments of #%d of %q: %w", number, name, err) | ||
| 163 | } | 163 | } | ||
| 164 | detail := ports.IssueDetail{Issue: issue, Comments: comments} | 164 | detail := ports.IssueDetail{Issue: issue, Comments: comments} | ||
| 165 | + | events, err := i.store.ListEvents(ctx, name, number) | |||
| 166 | + | if err != nil { | |||
| 167 | + | return ports.IssueDetail{}, fmt.Errorf("list events of #%d of %q: %w", number, name, err) | |||
| 168 | + | } | |||
| 169 | + | titles := make(map[int64]domain.MRTitle) | |||
| 170 | + | for _, event := range events { | |||
| 171 | + | if event.MergeRequest != 0 { | |||
| 172 | + | if _, ok := titles[event.MergeRequest]; !ok { | |||
| 173 | + | mr, err := i.mrs.Get(ctx, name, event.MergeRequest) | |||
| 174 | + | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |||
| 175 | + | return ports.IssueDetail{}, err | |||
| 176 | + | } | |||
| 177 | + | titles[event.MergeRequest] = mr.Title | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | detail.Events = append(detail.Events, ports.IssueEventDetail{Event: event, MergeRequestTitle: titles[event.MergeRequest]}) | |||
| 181 | + | } | |||
| 165 | if i.links != nil { | 182 | if i.links != nil { | ||
| 166 | links, err := i.links.LinksOfIssue(ctx, name, number, MaxIssueLinksPerIssue) | 183 | links, err := i.links.LinksOfIssue(ctx, name, number, MaxIssueLinksPerIssue) | ||
| 167 | if err != nil { | 184 | if err != nil { | ||
| @@ -241,38 +258,43 @@ func (i *Issues) AddComment(ctx context.Context, p domain.Principal, name domain | |||||
| 241 | // Close closes an open issue and publishes IssueClosed. The principal needs | 258 | // Close closes an open issue and publishes IssueClosed. The principal needs | ||
| 242 | // issue:write. An issue that is not open gives domain.ErrPreconditionFailed. | 259 | // issue:write. An issue that is not open gives domain.ErrPreconditionFailed. | ||
| 243 | func (i *Issues) Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | 260 | func (i *Issues) Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | ||
| 244 | - | return i.transition(ctx, p, name, number, domain.IssueStateOpen, domain.IssueStateClosed) | 261 | + | return i.transition(ctx, p, name, number, domain.IssueStateClosed) |
| 245 | } | 262 | } | ||
| 246 | 263 | ||||
| 247 | - | // Reopen reopens a closed issue and publishes IssueReopened. The principal | 264 | + | // Reopen reopens a closed or resolved issue and publishes IssueReopened. The principal |
| 248 | - | // needs issue:write. An issue that is not closed gives | 265 | + | // needs issue:write. An issue that is not done gives |
| 249 | // domain.ErrPreconditionFailed. | 266 | // domain.ErrPreconditionFailed. | ||
| 250 | func (i *Issues) Reopen(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | 267 | func (i *Issues) Reopen(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | ||
| 251 | - | return i.transition(ctx, p, name, number, domain.IssueStateClosed, domain.IssueStateOpen) | 268 | + | return i.transition(ctx, p, name, number, domain.IssueStateOpen) |
| 252 | } | 269 | } | ||
| 253 | 270 | ||||
| 254 | // transition moves an issue from one state to the other, retrying when another | 271 | // transition moves an issue from one state to the other, retrying when another | ||
| 255 | // write changes it in between. | 272 | // write changes it in between. | ||
| 256 | - | func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, from, to domain.IssueState) (domain.Issue, error) { | 273 | + | func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, to domain.IssueState) (domain.Issue, error) { |
| 257 | repo, err := i.writable(ctx, p, name) | 274 | repo, err := i.writable(ctx, p, name) | ||
| 258 | if err != nil { | 275 | if err != nil { | ||
| 259 | return domain.Issue{}, err | 276 | return domain.Issue{}, err | ||
| 260 | } | 277 | } | ||
| 261 | verb := "close" | 278 | verb := "close" | ||
| 279 | + | kind := domain.IssueEventClosed | |||
| 262 | if to == domain.IssueStateOpen { | 280 | if to == domain.IssueStateOpen { | ||
| 263 | verb = "reopen" | 281 | verb = "reopen" | ||
| 282 | + | kind = domain.IssueEventReopened | |||
| 283 | + | } else if to == domain.IssueStateResolved { | |||
| 284 | + | verb = "resolve" | |||
| 285 | + | kind = domain.IssueEventResolved | |||
| 264 | } | 286 | } | ||
| 265 | for range maxWriteRetries { | 287 | for range maxWriteRetries { | ||
| 266 | issue, err := i.store.Get(ctx, name, number) | 288 | issue, err := i.store.Get(ctx, name, number) | ||
| 267 | if err != nil { | 289 | if err != nil { | ||
| 268 | return domain.Issue{}, err | 290 | return domain.Issue{}, err | ||
| 269 | } | 291 | } | ||
| 270 | - | if issue.State != from { | 292 | + | if (to == domain.IssueStateOpen && !issue.IsDone()) || (to != domain.IssueStateOpen && !issue.IsOpen()) { |
| 271 | return domain.Issue{}, fmt.Errorf("%w: #%d is %s", domain.ErrPreconditionFailed, number, issue.State) | 293 | return domain.Issue{}, fmt.Errorf("%w: #%d is %s", domain.ErrPreconditionFailed, number, issue.State) | ||
| 272 | } | 294 | } | ||
| 273 | issue.ClosedByMergeRequest = 0 | 295 | issue.ClosedByMergeRequest = 0 | ||
| 274 | issue.State, issue.UpdatedAt = to, i.clock.Now().UTC() | 296 | issue.State, issue.UpdatedAt = to, i.clock.Now().UTC() | ||
| 275 | - | err = i.store.Update(ctx, issue) | 297 | + | err = i.store.Update(ctx, issue, domain.IssueEvent{Kind: kind, Actor: domain.AuthorName(p), CreatedAt: issue.UpdatedAt}) |
| 276 | if errors.Is(err, domain.ErrConflict) { | 298 | if errors.Is(err, domain.ErrConflict) { | ||
| 277 | continue | 299 | continue | ||
| 278 | } | 300 | } | ||
| @@ -285,6 +307,8 @@ func (i *Issues) transition(ctx context.Context, p domain.Principal, name domain | |||||
| 285 | recordActivity(ctx, i.meta, repo.Name, issue.UpdatedAt) | 307 | recordActivity(ctx, i.meta, repo.Name, issue.UpdatedAt) | ||
| 286 | if to == domain.IssueStateOpen { | 308 | if to == domain.IssueStateOpen { | ||
| 287 | i.bus.Publish(ctx, domain.IssueReopened{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | 309 | i.bus.Publish(ctx, domain.IssueReopened{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | ||
| 310 | + | } else if to == domain.IssueStateResolved { | |||
| 311 | + | i.bus.Publish(ctx, domain.IssueResolved{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | |||
| 288 | } else { | 312 | } else { | ||
| 289 | i.bus.Publish(ctx, domain.IssueClosed{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | 313 | i.bus.Publish(ctx, domain.IssueClosed{Issue: issue, At: issue.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt}) | ||
| 290 | } | 314 | } | ||
| @@ -319,7 +343,7 @@ func (i *Issues) Kind(ctx context.Context, p domain.Principal, name domain.RepoN | |||||
| 319 | return domain.NumberNone, nil | 343 | return domain.NumberNone, nil | ||
| 320 | } | 344 | } | ||
| 321 | 345 | ||||
| 322 | - | // Resolve is reserved for Phase A; no state is changed by this contract stub. | 346 | + | // Resolve marks an open issue resolved. The principal needs issue:write. |
| 323 | func (i *Issues) Resolve(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | 347 | func (i *Issues) Resolve(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.Issue, error) { | ||
| 324 | - | return domain.Issue{}, domain.ErrUnavailable | 348 | + | return i.transition(ctx, p, name, number, domain.IssueStateResolved) |
| 325 | } | 349 | } | ||
M internal/usecase/issues_test.go +80 -4
| @@ -136,7 +136,7 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||
| 136 | 136 | t.Parallel() | |
| 137 | 137 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |
| 138 | 138 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil) | |
| 139 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(domain.ErrConflict) | |
| 139 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict) | |
| 140 | 140 | _, err := u.issues.Close(ctx, p, u.repo.Name, 3) | |
| 141 | 141 | assert.ErrorIs(t, err, domain.ErrConflict) | |
| 142 | 142 | }) | |
| @@ -145,8 +145,8 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||
| 145 | 145 | t.Parallel() | |
| 146 | 146 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |
| 147 | 147 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil).Twice() | |
| 148 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(domain.ErrConflict).Once() | |
| 149 | - | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateClosed })).Return(nil).Once() | |
| 148 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once() | |
| 149 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateClosed }), mock.Anything).Return(nil).Once() | |
| 150 | 150 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { | |
| 151 | 151 | c, ok := ev.(domain.IssueClosed) | |
| 152 | 152 | return ok && c.Issue.State == domain.IssueStateClosed | |
| @@ -172,7 +172,7 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||
| 172 | 172 | closed := open | |
| 173 | 173 | closed.State = domain.IssueStateClosed | |
| 174 | 174 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(closed, nil) | |
| 175 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) | |
| 175 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(nil) | |
| 176 | 176 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { _, ok := ev.(domain.IssueReopened); return ok })).Return() | |
| 177 | 177 | got, err := u.issues.Reopen(ctx, p, u.repo.Name, 3) | |
| 178 | 178 | require.NoError(t, err) | |
| @@ -278,3 +278,79 @@ func TestIssueKind(t *testing.T) { | |||
| 278 | 278 | assert.ErrorIs(t, err, domain.ErrNotFound) | |
| 279 | 279 | }) | |
| 280 | 280 | } | |
| 281 | + | ||
| 282 | + | func TestIssueTransitionMatrix(t *testing.T) { | |
| 283 | + | for _, from := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |
| 284 | + | for _, to := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |
| 285 | + | t.Run(string(from)+"/"+string(to), func(t *testing.T) { | |
| 286 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |
| 287 | + | ctx, p := context.Background(), issuePrincipal(domain.ScopeIssueWrite) | |
| 288 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Repo: u.repo.Name, Number: 3, State: from, ClosedByMergeRequest: 9}, nil) | |
| 289 | + | allowed := (from == domain.IssueStateOpen && to != domain.IssueStateOpen) || (from != domain.IssueStateOpen && to == domain.IssueStateOpen) | |
| 290 | + | kind := map[domain.IssueState]domain.IssueEventKind{domain.IssueStateOpen: domain.IssueEventReopened, domain.IssueStateClosed: domain.IssueEventClosed, domain.IssueStateResolved: domain.IssueEventResolved}[to] | |
| 291 | + | if allowed { | |
| 292 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { | |
| 293 | + | return i.State == to && i.ClosedByMergeRequest == 0 && i.UpdatedAt.Equal(now) | |
| 294 | + | }), mock.MatchedBy(func(e []domain.IssueEvent) bool { | |
| 295 | + | return len(e) == 1 && e[0].Kind == kind && e[0].Actor == "agent-1" && e[0].CreatedAt.Equal(now) && e[0].MergeRequest == 0 | |
| 296 | + | })).Return(nil).Once() | |
| 297 | + | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool { return e.EventKind() == "issue."+string(kind) })).Return().Once() | |
| 298 | + | } | |
| 299 | + | fn := u.issues.Close | |
| 300 | + | if to == domain.IssueStateOpen { | |
| 301 | + | fn = u.issues.Reopen | |
| 302 | + | } | |
| 303 | + | if to == domain.IssueStateResolved { | |
| 304 | + | fn = u.issues.Resolve | |
| 305 | + | } | |
| 306 | + | got, err := fn(ctx, p, u.repo.Name, 3) | |
| 307 | + | if allowed { | |
| 308 | + | require.NoError(t, err) | |
| 309 | + | assert.Equal(t, to, got.State) | |
| 310 | + | } else { | |
| 311 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |
| 312 | + | } | |
| 313 | + | }) | |
| 314 | + | } | |
| 315 | + | } | |
| 316 | + | } | |
| 317 | + | ||
| 318 | + | func TestIssueHistoryTitles(t *testing.T) { | |
| 319 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |
| 320 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Number: 3}, nil) | |
| 321 | + | u.store.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(3)).Return(nil, nil) | |
| 322 | + | events := []domain.IssueEvent{{ID: 1, MergeRequest: 4}, {ID: 2, MergeRequest: 4}, {ID: 3, MergeRequest: 5}, {ID: 4}} | |
| 323 | + | u.store.EXPECT().ListEvents(mock.Anything, u.repo.Name, int64(3)).Return(events, nil) | |
| 324 | + | title, err := domain.ParseMRTitle("Fix crash") | |
| 325 | + | require.NoError(t, err) | |
| 326 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(4)).Return(domain.MergeRequest{Title: title}, nil).Once() | |
| 327 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(5)).Return(domain.MergeRequest{}, domain.ErrNotFound).Once() | |
| 328 | + | d, err := u.issues.Get(context.Background(), domain.Root(), u.repo.Name, 3) | |
| 329 | + | require.NoError(t, err) | |
| 330 | + | require.Len(t, d.Events, 4) | |
| 331 | + | for n, event := range events { | |
| 332 | + | assert.Equal(t, event, d.Events[n].Event) | |
| 333 | + | } | |
| 334 | + | assert.Equal(t, title, d.Events[0].MergeRequestTitle) | |
| 335 | + | assert.Equal(t, title, d.Events[1].MergeRequestTitle) | |
| 336 | + | assert.True(t, d.Events[2].MergeRequestTitle == (domain.MRTitle{})) | |
| 337 | + | } | |
| 338 | + | ||
| 339 | + | func TestResolveAccess(t *testing.T) { | |
| 340 | + | for _, tc := range []struct { | |
| 341 | + | name string | |
| 342 | + | mode domain.IssuesMode | |
| 343 | + | p domain.Principal | |
| 344 | + | want error | |
| 345 | + | }{ | |
| 346 | + | {"off", domain.IssuesOff, domain.Root(), domain.ErrNotFound}, | |
| 347 | + | {"anonymous", domain.IssuesOwner, domain.Anonymous(), domain.ErrUnauthenticated}, | |
| 348 | + | {"read only", domain.IssuesOwner, issuePrincipal(domain.ScopeRepoRead), domain.ErrForbidden}, | |
| 349 | + | } { | |
| 350 | + | t.Run(tc.name, func(t *testing.T) { | |
| 351 | + | u := newIssueUnit(t, domain.VisibilityPublic, tc.mode) | |
| 352 | + | _, err := u.issues.Resolve(context.Background(), tc.p, u.repo.Name, 1) | |
| 353 | + | require.ErrorIs(t, err, tc.want) | |
| 354 | + | }) | |
| 355 | + | } | |
| 356 | + | } | |
| @@ -136,7 +136,7 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||||
| 136 | t.Parallel() | 136 | t.Parallel() | ||
| 137 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | 137 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | ||
| 138 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil) | 138 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil) | ||
| 139 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(domain.ErrConflict) | 139 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict) |
| 140 | _, err := u.issues.Close(ctx, p, u.repo.Name, 3) | 140 | _, err := u.issues.Close(ctx, p, u.repo.Name, 3) | ||
| 141 | assert.ErrorIs(t, err, domain.ErrConflict) | 141 | assert.ErrorIs(t, err, domain.ErrConflict) | ||
| 142 | }) | 142 | }) | ||
| @@ -145,8 +145,8 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||||
| 145 | t.Parallel() | 145 | t.Parallel() | ||
| 146 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | 146 | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | ||
| 147 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil).Twice() | 147 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(open, nil).Twice() | ||
| 148 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(domain.ErrConflict).Once() | 148 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once() |
| 149 | - | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateClosed })).Return(nil).Once() | 149 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { return i.State == domain.IssueStateClosed }), mock.Anything).Return(nil).Once() |
| 150 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { | 150 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { | ||
| 151 | c, ok := ev.(domain.IssueClosed) | 151 | c, ok := ev.(domain.IssueClosed) | ||
| 152 | return ok && c.Issue.State == domain.IssueStateClosed | 152 | return ok && c.Issue.State == domain.IssueStateClosed | ||
| @@ -172,7 +172,7 @@ func TestIssueCreateAndTransitions(t *testing.T) { | |||||
| 172 | closed := open | 172 | closed := open | ||
| 173 | closed.State = domain.IssueStateClosed | 173 | closed.State = domain.IssueStateClosed | ||
| 174 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(closed, nil) | 174 | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(closed, nil) | ||
| 175 | - | u.store.EXPECT().Update(mock.Anything, mock.Anything).Return(nil) | 175 | + | u.store.EXPECT().Update(mock.Anything, mock.Anything, mock.Anything).Return(nil) |
| 176 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { _, ok := ev.(domain.IssueReopened); return ok })).Return() | 176 | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(ev domain.Event) bool { _, ok := ev.(domain.IssueReopened); return ok })).Return() | ||
| 177 | got, err := u.issues.Reopen(ctx, p, u.repo.Name, 3) | 177 | got, err := u.issues.Reopen(ctx, p, u.repo.Name, 3) | ||
| 178 | require.NoError(t, err) | 178 | require.NoError(t, err) | ||
| @@ -278,3 +278,79 @@ func TestIssueKind(t *testing.T) { | |||||
| 278 | assert.ErrorIs(t, err, domain.ErrNotFound) | 278 | assert.ErrorIs(t, err, domain.ErrNotFound) | ||
| 279 | }) | 279 | }) | ||
| 280 | } | 280 | } | ||
| 281 | + | ||||
| 282 | + | func TestIssueTransitionMatrix(t *testing.T) { | |||
| 283 | + | for _, from := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |||
| 284 | + | for _, to := range []domain.IssueState{domain.IssueStateOpen, domain.IssueStateClosed, domain.IssueStateResolved} { | |||
| 285 | + | t.Run(string(from)+"/"+string(to), func(t *testing.T) { | |||
| 286 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |||
| 287 | + | ctx, p := context.Background(), issuePrincipal(domain.ScopeIssueWrite) | |||
| 288 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Repo: u.repo.Name, Number: 3, State: from, ClosedByMergeRequest: 9}, nil) | |||
| 289 | + | allowed := (from == domain.IssueStateOpen && to != domain.IssueStateOpen) || (from != domain.IssueStateOpen && to == domain.IssueStateOpen) | |||
| 290 | + | kind := map[domain.IssueState]domain.IssueEventKind{domain.IssueStateOpen: domain.IssueEventReopened, domain.IssueStateClosed: domain.IssueEventClosed, domain.IssueStateResolved: domain.IssueEventResolved}[to] | |||
| 291 | + | if allowed { | |||
| 292 | + | u.store.EXPECT().Update(mock.Anything, mock.MatchedBy(func(i domain.Issue) bool { | |||
| 293 | + | return i.State == to && i.ClosedByMergeRequest == 0 && i.UpdatedAt.Equal(now) | |||
| 294 | + | }), mock.MatchedBy(func(e []domain.IssueEvent) bool { | |||
| 295 | + | return len(e) == 1 && e[0].Kind == kind && e[0].Actor == "agent-1" && e[0].CreatedAt.Equal(now) && e[0].MergeRequest == 0 | |||
| 296 | + | })).Return(nil).Once() | |||
| 297 | + | u.bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool { return e.EventKind() == "issue."+string(kind) })).Return().Once() | |||
| 298 | + | } | |||
| 299 | + | fn := u.issues.Close | |||
| 300 | + | if to == domain.IssueStateOpen { | |||
| 301 | + | fn = u.issues.Reopen | |||
| 302 | + | } | |||
| 303 | + | if to == domain.IssueStateResolved { | |||
| 304 | + | fn = u.issues.Resolve | |||
| 305 | + | } | |||
| 306 | + | got, err := fn(ctx, p, u.repo.Name, 3) | |||
| 307 | + | if allowed { | |||
| 308 | + | require.NoError(t, err) | |||
| 309 | + | assert.Equal(t, to, got.State) | |||
| 310 | + | } else { | |||
| 311 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |||
| 312 | + | } | |||
| 313 | + | }) | |||
| 314 | + | } | |||
| 315 | + | } | |||
| 316 | + | } | |||
| 317 | + | ||||
| 318 | + | func TestIssueHistoryTitles(t *testing.T) { | |||
| 319 | + | u := newIssueUnit(t, domain.VisibilityPublic, domain.IssuesOwner) | |||
| 320 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(3)).Return(domain.Issue{Number: 3}, nil) | |||
| 321 | + | u.store.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(3)).Return(nil, nil) | |||
| 322 | + | events := []domain.IssueEvent{{ID: 1, MergeRequest: 4}, {ID: 2, MergeRequest: 4}, {ID: 3, MergeRequest: 5}, {ID: 4}} | |||
| 323 | + | u.store.EXPECT().ListEvents(mock.Anything, u.repo.Name, int64(3)).Return(events, nil) | |||
| 324 | + | title, err := domain.ParseMRTitle("Fix crash") | |||
| 325 | + | require.NoError(t, err) | |||
| 326 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(4)).Return(domain.MergeRequest{Title: title}, nil).Once() | |||
| 327 | + | u.mrs.EXPECT().Get(mock.Anything, u.repo.Name, int64(5)).Return(domain.MergeRequest{}, domain.ErrNotFound).Once() | |||
| 328 | + | d, err := u.issues.Get(context.Background(), domain.Root(), u.repo.Name, 3) | |||
| 329 | + | require.NoError(t, err) | |||
| 330 | + | require.Len(t, d.Events, 4) | |||
| 331 | + | for n, event := range events { | |||
| 332 | + | assert.Equal(t, event, d.Events[n].Event) | |||
| 333 | + | } | |||
| 334 | + | assert.Equal(t, title, d.Events[0].MergeRequestTitle) | |||
| 335 | + | assert.Equal(t, title, d.Events[1].MergeRequestTitle) | |||
| 336 | + | assert.True(t, d.Events[2].MergeRequestTitle == (domain.MRTitle{})) | |||
| 337 | + | } | |||
| 338 | + | ||||
| 339 | + | func TestResolveAccess(t *testing.T) { | |||
| 340 | + | for _, tc := range []struct { | |||
| 341 | + | name string | |||
| 342 | + | mode domain.IssuesMode | |||
| 343 | + | p domain.Principal | |||
| 344 | + | want error | |||
| 345 | + | }{ | |||
| 346 | + | {"off", domain.IssuesOff, domain.Root(), domain.ErrNotFound}, | |||
| 347 | + | {"anonymous", domain.IssuesOwner, domain.Anonymous(), domain.ErrUnauthenticated}, | |||
| 348 | + | {"read only", domain.IssuesOwner, issuePrincipal(domain.ScopeRepoRead), domain.ErrForbidden}, | |||
| 349 | + | } { | |||
| 350 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 351 | + | u := newIssueUnit(t, domain.VisibilityPublic, tc.mode) | |||
| 352 | + | _, err := u.issues.Resolve(context.Background(), tc.p, u.repo.Name, 1) | |||
| 353 | + | require.ErrorIs(t, err, tc.want) | |||
| 354 | + | }) | |||
| 355 | + | } | |||
| 356 | + | } | |||
M internal/usecase/mergerequests.go +64 -23
| @@ -43,24 +43,26 @@ const mrTokenPrefix = "mp1:" | |||
| 43 | 43 | // merging mr:merge, on the repository (see scopeAccess), and that scope also | |
| 44 | 44 | // makes a private repository visible for the action (see writable). | |
| 45 | 45 | type MergeRequests struct { | |
| 46 | - | links ports.IssueLinkTrigger | |
| 47 | - | meta ports.RepoMetaStore | |
| 48 | - | store ports.MergeRequestStore | |
| 49 | - | repos ports.GitRepos | |
| 50 | - | git ports.MergeGit | |
| 51 | - | signer ports.CommitSigner | |
| 52 | - | committer domain.Identity | |
| 53 | - | bus ports.EventBus | |
| 54 | - | clock ports.Clock | |
| 55 | - | maint ports.MaintenanceScheduler | |
| 56 | - | ci ports.WorkflowTrigger // nil: no CI | |
| 57 | - | checks ports.CIChecksReader // nil: no CI, so no check blocks a merge | |
| 58 | - | pulls ports.PullRequestTrigger // nil: no pull_request workflows | |
| 59 | - | tips ports.CIRefTipStore // nil: who merges is not recorded | |
| 60 | - | verifier ports.CommitVerifier // nil: commits are not checked (ADR 0030) | |
| 61 | - | mergeab MergeabilityHook // nil: no mergeability | |
| 62 | - | autoMerge ports.AutoMergeTrigger | |
| 63 | - | mirror ports.MirrorTrigger // nil: no outbound mirror | |
| 46 | + | issues ports.IssueStore | |
| 47 | + | issueLinks ports.IssueLinkStore | |
| 48 | + | links ports.IssueLinkTrigger | |
| 49 | + | meta ports.RepoMetaStore | |
| 50 | + | store ports.MergeRequestStore | |
| 51 | + | repos ports.GitRepos | |
| 52 | + | git ports.MergeGit | |
| 53 | + | signer ports.CommitSigner | |
| 54 | + | committer domain.Identity | |
| 55 | + | bus ports.EventBus | |
| 56 | + | clock ports.Clock | |
| 57 | + | maint ports.MaintenanceScheduler | |
| 58 | + | ci ports.WorkflowTrigger // nil: no CI | |
| 59 | + | checks ports.CIChecksReader // nil: no CI, so no check blocks a merge | |
| 60 | + | pulls ports.PullRequestTrigger // nil: no pull_request workflows | |
| 61 | + | tips ports.CIRefTipStore // nil: who merges is not recorded | |
| 62 | + | verifier ports.CommitVerifier // nil: commits are not checked (ADR 0030) | |
| 63 | + | mergeab MergeabilityHook // nil: no mergeability | |
| 64 | + | autoMerge ports.AutoMergeTrigger | |
| 65 | + | mirror ports.MirrorTrigger // nil: no outbound mirror | |
| 64 | 66 | ||
| 65 | 67 | mu sync.Mutex | |
| 66 | 68 | locks map[domain.RepoName]chan struct{} | |
| @@ -79,9 +81,12 @@ var ( | |||
| 79 | 81 | // MergeRequestsConfig holds what the merge request use cases need. Every field | |
| 80 | 82 | // is required. | |
| 81 | 83 | type MergeRequestsConfig struct { | |
| 82 | - | Links ports.IssueLinkTrigger // Optional derived issue links. | |
| 83 | - | Meta ports.RepoMetaStore | |
| 84 | - | Store ports.MergeRequestStore | |
| 84 | + | // Issues and IssueLinks optionally enrich merge request details. | |
| 85 | + | Issues ports.IssueStore | |
| 86 | + | IssueLinks ports.IssueLinkStore | |
| 87 | + | Links ports.IssueLinkTrigger // Optional derived issue links. | |
| 88 | + | Meta ports.RepoMetaStore | |
| 89 | + | Store ports.MergeRequestStore | |
| 85 | 90 | // Repos gives the default branch of a repository. | |
| 86 | 91 | Repos ports.GitRepos | |
| 87 | 92 | Git ports.MergeGit | |
| @@ -114,7 +119,7 @@ type MergeRequestsConfig struct { | |||
| 114 | 119 | // NewMergeRequests builds the merge request use cases. | |
| 115 | 120 | func NewMergeRequests(cfg MergeRequestsConfig) *MergeRequests { | |
| 116 | 121 | return &MergeRequests{ | |
| 117 | - | links: cfg.Links, meta: cfg.Meta, store: cfg.Store, repos: cfg.Repos, git: cfg.Git, signer: cfg.Signer, | |
| 122 | + | issues: cfg.Issues, issueLinks: cfg.IssueLinks, links: cfg.Links, meta: cfg.Meta, store: cfg.Store, repos: cfg.Repos, git: cfg.Git, signer: cfg.Signer, | |
| 118 | 123 | committer: cfg.Committer, bus: cfg.Bus, clock: cfg.Clock, maint: cfg.Maint, ci: cfg.CI, checks: cfg.Checks, pulls: cfg.PullRequests, tips: cfg.RefTips, mergeab: cfg.Mergeability, | |
| 119 | 124 | locks: make(map[domain.RepoName]chan struct{}), | |
| 120 | 125 | } | |
| @@ -550,7 +555,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||
| 550 | 555 | if err != nil { | |
| 551 | 556 | return ports.MergeRequestDetail{}, err | |
| 552 | 557 | } | |
| 553 | - | _, mr, err := m.get(ctx, p, name, number) | |
| 558 | + | repo, mr, err := m.get(ctx, p, name, number) | |
| 554 | 559 | if err != nil { | |
| 555 | 560 | return ports.MergeRequestDetail{}, err | |
| 556 | 561 | } | |
| @@ -568,6 +573,42 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||
| 568 | 573 | return nil | |
| 569 | 574 | } | |
| 570 | 575 | d := ports.MergeRequestDetail{MergeRequest: mr} | |
| 576 | + | if m.issues != nil && m.issueLinks != nil && repo.Settings.IssuesEnabled() { | |
| 577 | + | links, err := m.issueLinks.LinksOfMergeRequest(ctx, name, number) | |
| 578 | + | if err != nil { | |
| 579 | + | return ports.MergeRequestDetail{}, err | |
| 580 | + | } | |
| 581 | + | links = slices.Clone(links) | |
| 582 | + | slices.SortFunc(links, func(a, b domain.IssueLink) int { | |
| 583 | + | if a.Issue < b.Issue { | |
| 584 | + | return -1 | |
| 585 | + | } | |
| 586 | + | if a.Issue > b.Issue { | |
| 587 | + | return 1 | |
| 588 | + | } | |
| 589 | + | return 0 | |
| 590 | + | }) | |
| 591 | + | if len(links) > MaxIssueLinksPerIssue { | |
| 592 | + | links = links[:MaxIssueLinksPerIssue] | |
| 593 | + | } | |
| 594 | + | for _, link := range links { | |
| 595 | + | issue, err := m.issues.Get(ctx, name, link.Issue) | |
| 596 | + | if errors.Is(err, domain.ErrNotFound) { | |
| 597 | + | continue | |
| 598 | + | } | |
| 599 | + | if err != nil { | |
| 600 | + | return ports.MergeRequestDetail{}, err | |
| 601 | + | } | |
| 602 | + | d.Issues = append(d.Issues, ports.MergeRequestIssue{Number: issue.Number, Title: issue.Title, State: issue.State, Closes: link.Closes}) | |
| 603 | + | } | |
| 604 | + | events, err := m.issueLinks.EventsOfMergeRequest(ctx, name, number) | |
| 605 | + | if err != nil { | |
| 606 | + | return ports.MergeRequestDetail{}, err | |
| 607 | + | } | |
| 608 | + | for _, event := range events { | |
| 609 | + | d.IssueEvents = append(d.IssueEvents, ports.IssueEventDetail{Event: event, MergeRequestTitle: mr.Title}) | |
| 610 | + | } | |
| 611 | + | } | |
| 571 | 612 | if m.mergeab != nil && mr.IsOpen() { | |
| 572 | 613 | tip, err := m.branchOrZero(ctx, name, mr.Target) | |
| 573 | 614 | if err != nil { | |
| @@ -43,24 +43,26 @@ const mrTokenPrefix = "mp1:" | |||||
| 43 | // merging mr:merge, on the repository (see scopeAccess), and that scope also | 43 | // merging mr:merge, on the repository (see scopeAccess), and that scope also | ||
| 44 | // makes a private repository visible for the action (see writable). | 44 | // makes a private repository visible for the action (see writable). | ||
| 45 | type MergeRequests struct { | 45 | type MergeRequests struct { | ||
| 46 | - | links ports.IssueLinkTrigger | 46 | + | issues ports.IssueStore |
| 47 | - | meta ports.RepoMetaStore | 47 | + | issueLinks ports.IssueLinkStore |
| 48 | - | store ports.MergeRequestStore | 48 | + | links ports.IssueLinkTrigger |
| 49 | - | repos ports.GitRepos | 49 | + | meta ports.RepoMetaStore |
| 50 | - | git ports.MergeGit | 50 | + | store ports.MergeRequestStore |
| 51 | - | signer ports.CommitSigner | 51 | + | repos ports.GitRepos |
| 52 | - | committer domain.Identity | 52 | + | git ports.MergeGit |
| 53 | - | bus ports.EventBus | 53 | + | signer ports.CommitSigner |
| 54 | - | clock ports.Clock | 54 | + | committer domain.Identity |
| 55 | - | maint ports.MaintenanceScheduler | 55 | + | bus ports.EventBus |
| 56 | - | ci ports.WorkflowTrigger // nil: no CI | 56 | + | clock ports.Clock |
| 57 | - | checks ports.CIChecksReader // nil: no CI, so no check blocks a merge | 57 | + | maint ports.MaintenanceScheduler |
| 58 | - | pulls ports.PullRequestTrigger // nil: no pull_request workflows | 58 | + | ci ports.WorkflowTrigger // nil: no CI |
| 59 | - | tips ports.CIRefTipStore // nil: who merges is not recorded | 59 | + | checks ports.CIChecksReader // nil: no CI, so no check blocks a merge |
| 60 | - | verifier ports.CommitVerifier // nil: commits are not checked (ADR 0030) | 60 | + | pulls ports.PullRequestTrigger // nil: no pull_request workflows |
| 61 | - | mergeab MergeabilityHook // nil: no mergeability | 61 | + | tips ports.CIRefTipStore // nil: who merges is not recorded |
| 62 | - | autoMerge ports.AutoMergeTrigger | 62 | + | verifier ports.CommitVerifier // nil: commits are not checked (ADR 0030) |
| 63 | - | mirror ports.MirrorTrigger // nil: no outbound mirror | 63 | + | mergeab MergeabilityHook // nil: no mergeability |
| 64 | + | autoMerge ports.AutoMergeTrigger | |||
| 65 | + | mirror ports.MirrorTrigger // nil: no outbound mirror | |||
| 64 | 66 | ||||
| 65 | mu sync.Mutex | 67 | mu sync.Mutex | ||
| 66 | locks map[domain.RepoName]chan struct{} | 68 | locks map[domain.RepoName]chan struct{} | ||
| @@ -79,9 +81,12 @@ var ( | |||||
| 79 | // MergeRequestsConfig holds what the merge request use cases need. Every field | 81 | // MergeRequestsConfig holds what the merge request use cases need. Every field | ||
| 80 | // is required. | 82 | // is required. | ||
| 81 | type MergeRequestsConfig struct { | 83 | type MergeRequestsConfig struct { | ||
| 82 | - | Links ports.IssueLinkTrigger // Optional derived issue links. | 84 | + | // Issues and IssueLinks optionally enrich merge request details. |
| 83 | - | Meta ports.RepoMetaStore | 85 | + | Issues ports.IssueStore |
| 84 | - | Store ports.MergeRequestStore | 86 | + | IssueLinks ports.IssueLinkStore |
| 87 | + | Links ports.IssueLinkTrigger // Optional derived issue links. | |||
| 88 | + | Meta ports.RepoMetaStore | |||
| 89 | + | Store ports.MergeRequestStore | |||
| 85 | // Repos gives the default branch of a repository. | 90 | // Repos gives the default branch of a repository. | ||
| 86 | Repos ports.GitRepos | 91 | Repos ports.GitRepos | ||
| 87 | Git ports.MergeGit | 92 | Git ports.MergeGit | ||
| @@ -114,7 +119,7 @@ type MergeRequestsConfig struct { | |||||
| 114 | // NewMergeRequests builds the merge request use cases. | 119 | // NewMergeRequests builds the merge request use cases. | ||
| 115 | func NewMergeRequests(cfg MergeRequestsConfig) *MergeRequests { | 120 | func NewMergeRequests(cfg MergeRequestsConfig) *MergeRequests { | ||
| 116 | return &MergeRequests{ | 121 | return &MergeRequests{ | ||
| 117 | - | links: cfg.Links, meta: cfg.Meta, store: cfg.Store, repos: cfg.Repos, git: cfg.Git, signer: cfg.Signer, | 122 | + | issues: cfg.Issues, issueLinks: cfg.IssueLinks, links: cfg.Links, meta: cfg.Meta, store: cfg.Store, repos: cfg.Repos, git: cfg.Git, signer: cfg.Signer, |
| 118 | committer: cfg.Committer, bus: cfg.Bus, clock: cfg.Clock, maint: cfg.Maint, ci: cfg.CI, checks: cfg.Checks, pulls: cfg.PullRequests, tips: cfg.RefTips, mergeab: cfg.Mergeability, | 123 | committer: cfg.Committer, bus: cfg.Bus, clock: cfg.Clock, maint: cfg.Maint, ci: cfg.CI, checks: cfg.Checks, pulls: cfg.PullRequests, tips: cfg.RefTips, mergeab: cfg.Mergeability, | ||
| 119 | locks: make(map[domain.RepoName]chan struct{}), | 124 | locks: make(map[domain.RepoName]chan struct{}), | ||
| 120 | } | 125 | } | ||
| @@ -550,7 +555,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||||
| 550 | if err != nil { | 555 | if err != nil { | ||
| 551 | return ports.MergeRequestDetail{}, err | 556 | return ports.MergeRequestDetail{}, err | ||
| 552 | } | 557 | } | ||
| 553 | - | _, mr, err := m.get(ctx, p, name, number) | 558 | + | repo, mr, err := m.get(ctx, p, name, number) |
| 554 | if err != nil { | 559 | if err != nil { | ||
| 555 | return ports.MergeRequestDetail{}, err | 560 | return ports.MergeRequestDetail{}, err | ||
| 556 | } | 561 | } | ||
| @@ -568,6 +573,42 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain | |||||
| 568 | return nil | 573 | return nil | ||
| 569 | } | 574 | } | ||
| 570 | d := ports.MergeRequestDetail{MergeRequest: mr} | 575 | d := ports.MergeRequestDetail{MergeRequest: mr} | ||
| 576 | + | if m.issues != nil && m.issueLinks != nil && repo.Settings.IssuesEnabled() { | |||
| 577 | + | links, err := m.issueLinks.LinksOfMergeRequest(ctx, name, number) | |||
| 578 | + | if err != nil { | |||
| 579 | + | return ports.MergeRequestDetail{}, err | |||
| 580 | + | } | |||
| 581 | + | links = slices.Clone(links) | |||
| 582 | + | slices.SortFunc(links, func(a, b domain.IssueLink) int { | |||
| 583 | + | if a.Issue < b.Issue { | |||
| 584 | + | return -1 | |||
| 585 | + | } | |||
| 586 | + | if a.Issue > b.Issue { | |||
| 587 | + | return 1 | |||
| 588 | + | } | |||
| 589 | + | return 0 | |||
| 590 | + | }) | |||
| 591 | + | if len(links) > MaxIssueLinksPerIssue { | |||
| 592 | + | links = links[:MaxIssueLinksPerIssue] | |||
| 593 | + | } | |||
| 594 | + | for _, link := range links { | |||
| 595 | + | issue, err := m.issues.Get(ctx, name, link.Issue) | |||
| 596 | + | if errors.Is(err, domain.ErrNotFound) { | |||
| 597 | + | continue | |||
| 598 | + | } | |||
| 599 | + | if err != nil { | |||
| 600 | + | return ports.MergeRequestDetail{}, err | |||
| 601 | + | } | |||
| 602 | + | d.Issues = append(d.Issues, ports.MergeRequestIssue{Number: issue.Number, Title: issue.Title, State: issue.State, Closes: link.Closes}) | |||
| 603 | + | } | |||
| 604 | + | events, err := m.issueLinks.EventsOfMergeRequest(ctx, name, number) | |||
| 605 | + | if err != nil { | |||
| 606 | + | return ports.MergeRequestDetail{}, err | |||
| 607 | + | } | |||
| 608 | + | for _, event := range events { | |||
| 609 | + | d.IssueEvents = append(d.IssueEvents, ports.IssueEventDetail{Event: event, MergeRequestTitle: mr.Title}) | |||
| 610 | + | } | |||
| 611 | + | } | |||
| 571 | if m.mergeab != nil && mr.IsOpen() { | 612 | if m.mergeab != nil && mr.IsOpen() { | ||
| 572 | tip, err := m.branchOrZero(ctx, name, mr.Target) | 613 | tip, err := m.branchOrZero(ctx, name, mr.Target) | ||
| 573 | if err != nil { | 614 | if err != nil { | ||
M internal/usecase/mergerequests_test.go +35 -0
| @@ -377,3 +377,38 @@ func TestHistoryAuthorizationAndEqualTips(t *testing.T) { | |||
| 377 | 377 | }) | |
| 378 | 378 | } | |
| 379 | 379 | } | |
| 380 | + | ||
| 381 | + | func TestMergeRequestIssueDetails(t *testing.T) { | |
| 382 | + | for _, mode := range []domain.IssuesMode{domain.IssuesOwner, domain.IssuesOff} { | |
| 383 | + | t.Run(string(mode), func(t *testing.T) { | |
| 384 | + | u := newMRUnit(t) | |
| 385 | + | u.repo.Settings.Issues = mode | |
| 386 | + | issues, links := mocks.NewIssueStore(t), mocks.NewIssueLinkStore(t) | |
| 387 | + | mrs := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Git: u.git, Issues: issues, IssueLinks: links}) | |
| 388 | + | title, err := domain.ParseMRTitle("Fix") | |
| 389 | + | require.NoError(t, err) | |
| 390 | + | mr := domain.MergeRequest{Repo: u.repo.Name, Number: 2, State: domain.MRStateClosed, Title: title} | |
| 391 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |
| 392 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(2)).Return(mr, nil) | |
| 393 | + | u.store.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(2)).Return(nil, nil) | |
| 394 | + | u.store.EXPECT().GetRead(mock.Anything, u.repo.Name, int64(2)).Return(domain.ObjectID{}, nil) | |
| 395 | + | event := domain.IssueEvent{Issue: 1, MergeRequest: 2, Kind: domain.IssueEventResolved} | |
| 396 | + | if mode == domain.IssuesOwner { | |
| 397 | + | links.EXPECT().LinksOfMergeRequest(mock.Anything, u.repo.Name, int64(2)).Return([]domain.IssueLink{{Issue: 1, Closes: true}}, nil) | |
| 398 | + | issues.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(domain.Issue{Number: 1, State: domain.IssueStateResolved}, nil) | |
| 399 | + | links.EXPECT().EventsOfMergeRequest(mock.Anything, u.repo.Name, int64(2)).Return([]domain.IssueEvent{event}, nil) | |
| 400 | + | } | |
| 401 | + | d, err := mrs.Get(context.Background(), domain.Root(), u.repo.Name, 2, ports.GetMergeRequestOptions{}) | |
| 402 | + | require.NoError(t, err) | |
| 403 | + | if mode == domain.IssuesOff { | |
| 404 | + | assert.Empty(t, d.Issues) | |
| 405 | + | assert.Empty(t, d.IssueEvents) | |
| 406 | + | } else { | |
| 407 | + | require.Len(t, d.Issues, 1) | |
| 408 | + | assert.Equal(t, domain.IssueStateResolved, d.Issues[0].State) | |
| 409 | + | assert.True(t, d.Issues[0].Closes) | |
| 410 | + | assert.Equal(t, []ports.IssueEventDetail{{Event: event, MergeRequestTitle: title}}, d.IssueEvents) | |
| 411 | + | } | |
| 412 | + | }) | |
| 413 | + | } | |
| 414 | + | } | |
| @@ -377,3 +377,38 @@ func TestHistoryAuthorizationAndEqualTips(t *testing.T) { | |||||
| 377 | }) | 377 | }) | ||
| 378 | } | 378 | } | ||
| 379 | } | 379 | } | ||
| 380 | + | ||||
| 381 | + | func TestMergeRequestIssueDetails(t *testing.T) { | |||
| 382 | + | for _, mode := range []domain.IssuesMode{domain.IssuesOwner, domain.IssuesOff} { | |||
| 383 | + | t.Run(string(mode), func(t *testing.T) { | |||
| 384 | + | u := newMRUnit(t) | |||
| 385 | + | u.repo.Settings.Issues = mode | |||
| 386 | + | issues, links := mocks.NewIssueStore(t), mocks.NewIssueLinkStore(t) | |||
| 387 | + | mrs := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Git: u.git, Issues: issues, IssueLinks: links}) | |||
| 388 | + | title, err := domain.ParseMRTitle("Fix") | |||
| 389 | + | require.NoError(t, err) | |||
| 390 | + | mr := domain.MergeRequest{Repo: u.repo.Name, Number: 2, State: domain.MRStateClosed, Title: title} | |||
| 391 | + | u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil) | |||
| 392 | + | u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(2)).Return(mr, nil) | |||
| 393 | + | u.store.EXPECT().ListComments(mock.Anything, u.repo.Name, int64(2)).Return(nil, nil) | |||
| 394 | + | u.store.EXPECT().GetRead(mock.Anything, u.repo.Name, int64(2)).Return(domain.ObjectID{}, nil) | |||
| 395 | + | event := domain.IssueEvent{Issue: 1, MergeRequest: 2, Kind: domain.IssueEventResolved} | |||
| 396 | + | if mode == domain.IssuesOwner { | |||
| 397 | + | links.EXPECT().LinksOfMergeRequest(mock.Anything, u.repo.Name, int64(2)).Return([]domain.IssueLink{{Issue: 1, Closes: true}}, nil) | |||
| 398 | + | issues.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(domain.Issue{Number: 1, State: domain.IssueStateResolved}, nil) | |||
| 399 | + | links.EXPECT().EventsOfMergeRequest(mock.Anything, u.repo.Name, int64(2)).Return([]domain.IssueEvent{event}, nil) | |||
| 400 | + | } | |||
| 401 | + | d, err := mrs.Get(context.Background(), domain.Root(), u.repo.Name, 2, ports.GetMergeRequestOptions{}) | |||
| 402 | + | require.NoError(t, err) | |||
| 403 | + | if mode == domain.IssuesOff { | |||
| 404 | + | assert.Empty(t, d.Issues) | |||
| 405 | + | assert.Empty(t, d.IssueEvents) | |||
| 406 | + | } else { | |||
| 407 | + | require.Len(t, d.Issues, 1) | |||
| 408 | + | assert.Equal(t, domain.IssueStateResolved, d.Issues[0].State) | |||
| 409 | + | assert.True(t, d.Issues[0].Closes) | |||
| 410 | + | assert.Equal(t, []ports.IssueEventDetail{{Event: event, MergeRequestTitle: title}}, d.IssueEvents) | |||
| 411 | + | } | |||
| 412 | + | }) | |||
| 413 | + | } | |||
| 414 | + | } | |||