detached b31a354

feat(mr): replies, line ranges, edits, deletes and verdicts in the merge request use case

commit
b31a35401436fb9f7ebb15f468633a0d2df68c5f
author
brice · · 2026-10-19 05:31 UTC
parent
3a784d4
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
7 files · +496 -7
Unified
UnifiedSplit
M internal/ports/mocks/MergeRequestService.go +174 -0
@@ -360,6 +360,180 @@ func (_c *MergeRequestService_Create_Call) RunAndReturn(run func(ctx context.Con
360360 return _c
361361 }
362362
363+// DeleteComment provides a mock function for the type MergeRequestService
364+func (_mock *MergeRequestService) DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error) {
365+ ret := _mock.Called(ctx, p, name, number, commentID)
366+
367+ if len(ret) == 0 {
368+ panic("no return value specified for DeleteComment")
369+ }
370+
371+ var r0 domain.Comment
372+ var r1 error
373+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) (domain.Comment, error)); ok {
374+ return returnFunc(ctx, p, name, number, commentID)
375+ }
376+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) domain.Comment); ok {
377+ r0 = returnFunc(ctx, p, name, number, commentID)
378+ } else {
379+ r0 = ret.Get(0).(domain.Comment)
380+ }
381+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) error); ok {
382+ r1 = returnFunc(ctx, p, name, number, commentID)
383+ } else {
384+ r1 = ret.Error(1)
385+ }
386+ return r0, r1
387+}
388+
389+// MergeRequestService_DeleteComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteComment'
390+type MergeRequestService_DeleteComment_Call struct {
391+ *mock.Call
392+}
393+
394+// DeleteComment is a helper method to define mock.On call
395+// - ctx context.Context
396+// - p domain.Principal
397+// - name domain.RepoName
398+// - number int64
399+// - commentID int64
400+func (_e *MergeRequestService_Expecter) DeleteComment(ctx any, p any, name any, number any, commentID any) *MergeRequestService_DeleteComment_Call {
401+ return &MergeRequestService_DeleteComment_Call{Call: _e.mock.On("DeleteComment", ctx, p, name, number, commentID)}
402+}
403+
404+func (_c *MergeRequestService_DeleteComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64)) *MergeRequestService_DeleteComment_Call {
405+ _c.Call.Run(func(args mock.Arguments) {
406+ var arg0 context.Context
407+ if args[0] != nil {
408+ arg0 = args[0].(context.Context)
409+ }
410+ var arg1 domain.Principal
411+ if args[1] != nil {
412+ arg1 = args[1].(domain.Principal)
413+ }
414+ var arg2 domain.RepoName
415+ if args[2] != nil {
416+ arg2 = args[2].(domain.RepoName)
417+ }
418+ var arg3 int64
419+ if args[3] != nil {
420+ arg3 = args[3].(int64)
421+ }
422+ var arg4 int64
423+ if args[4] != nil {
424+ arg4 = args[4].(int64)
425+ }
426+ run(
427+ arg0,
428+ arg1,
429+ arg2,
430+ arg3,
431+ arg4,
432+ )
433+ })
434+ return _c
435+}
436+
437+func (_c *MergeRequestService_DeleteComment_Call) Return(comment domain.Comment, err error) *MergeRequestService_DeleteComment_Call {
438+ _c.Call.Return(comment, err)
439+ return _c
440+}
441+
442+func (_c *MergeRequestService_DeleteComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error)) *MergeRequestService_DeleteComment_Call {
443+ _c.Call.Return(run)
444+ return _c
445+}
446+
447+// EditComment provides a mock function for the type MergeRequestService
448+func (_mock *MergeRequestService) EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error) {
449+ ret := _mock.Called(ctx, p, name, number, commentID, text)
450+
451+ if len(ret) == 0 {
452+ panic("no return value specified for EditComment")
453+ }
454+
455+ var r0 domain.Comment
456+ var r1 error
457+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) (domain.Comment, error)); ok {
458+ return returnFunc(ctx, p, name, number, commentID, text)
459+ }
460+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) domain.Comment); ok {
461+ r0 = returnFunc(ctx, p, name, number, commentID, text)
462+ } else {
463+ r0 = ret.Get(0).(domain.Comment)
464+ }
465+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) error); ok {
466+ r1 = returnFunc(ctx, p, name, number, commentID, text)
467+ } else {
468+ r1 = ret.Error(1)
469+ }
470+ return r0, r1
471+}
472+
473+// MergeRequestService_EditComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'EditComment'
474+type MergeRequestService_EditComment_Call struct {
475+ *mock.Call
476+}
477+
478+// EditComment is a helper method to define mock.On call
479+// - ctx context.Context
480+// - p domain.Principal
481+// - name domain.RepoName
482+// - number int64
483+// - commentID int64
484+// - text string
485+func (_e *MergeRequestService_Expecter) EditComment(ctx any, p any, name any, number any, commentID any, text any) *MergeRequestService_EditComment_Call {
486+ return &MergeRequestService_EditComment_Call{Call: _e.mock.On("EditComment", ctx, p, name, number, commentID, text)}
487+}
488+
489+func (_c *MergeRequestService_EditComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string)) *MergeRequestService_EditComment_Call {
490+ _c.Call.Run(func(args mock.Arguments) {
491+ var arg0 context.Context
492+ if args[0] != nil {
493+ arg0 = args[0].(context.Context)
494+ }
495+ var arg1 domain.Principal
496+ if args[1] != nil {
497+ arg1 = args[1].(domain.Principal)
498+ }
499+ var arg2 domain.RepoName
500+ if args[2] != nil {
501+ arg2 = args[2].(domain.RepoName)
502+ }
503+ var arg3 int64
504+ if args[3] != nil {
505+ arg3 = args[3].(int64)
506+ }
507+ var arg4 int64
508+ if args[4] != nil {
509+ arg4 = args[4].(int64)
510+ }
511+ var arg5 string
512+ if args[5] != nil {
513+ arg5 = args[5].(string)
514+ }
515+ run(
516+ arg0,
517+ arg1,
518+ arg2,
519+ arg3,
520+ arg4,
521+ arg5,
522+ )
523+ })
524+ return _c
525+}
526+
527+func (_c *MergeRequestService_EditComment_Call) Return(comment domain.Comment, err error) *MergeRequestService_EditComment_Call {
528+ _c.Call.Return(comment, err)
529+ return _c
530+}
531+
532+func (_c *MergeRequestService_EditComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error)) *MergeRequestService_EditComment_Call {
533+ _c.Call.Return(run)
534+ return _c
535+}
536+
363537 // Get provides a mock function for the type MergeRequestService
364538 func (_mock *MergeRequestService) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {
365539 ret := _mock.Called(ctx, p, name, number, opts)
@@ -360,6 +360,180 @@ func (_c *MergeRequestService_Create_Call) RunAndReturn(run func(ctx context.Con
360 return _c360 return _c
361 }361 }
362 362
363+// DeleteComment provides a mock function for the type MergeRequestService
364+func (_mock *MergeRequestService) DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error) {
365+ ret := _mock.Called(ctx, p, name, number, commentID)
366+
367+ if len(ret) == 0 {
368+ panic("no return value specified for DeleteComment")
369+ }
370+
371+ var r0 domain.Comment
372+ var r1 error
373+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) (domain.Comment, error)); ok {
374+ return returnFunc(ctx, p, name, number, commentID)
375+ }
376+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) domain.Comment); ok {
377+ r0 = returnFunc(ctx, p, name, number, commentID)
378+ } else {
379+ r0 = ret.Get(0).(domain.Comment)
380+ }
381+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64) error); ok {
382+ r1 = returnFunc(ctx, p, name, number, commentID)
383+ } else {
384+ r1 = ret.Error(1)
385+ }
386+ return r0, r1
387+}
388+
389+// MergeRequestService_DeleteComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteComment'
390+type MergeRequestService_DeleteComment_Call struct {
391+ *mock.Call
392+}
393+
394+// DeleteComment is a helper method to define mock.On call
395+// - ctx context.Context
396+// - p domain.Principal
397+// - name domain.RepoName
398+// - number int64
399+// - commentID int64
400+func (_e *MergeRequestService_Expecter) DeleteComment(ctx any, p any, name any, number any, commentID any) *MergeRequestService_DeleteComment_Call {
401+ return &MergeRequestService_DeleteComment_Call{Call: _e.mock.On("DeleteComment", ctx, p, name, number, commentID)}
402+}
403+
404+func (_c *MergeRequestService_DeleteComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64)) *MergeRequestService_DeleteComment_Call {
405+ _c.Call.Run(func(args mock.Arguments) {
406+ var arg0 context.Context
407+ if args[0] != nil {
408+ arg0 = args[0].(context.Context)
409+ }
410+ var arg1 domain.Principal
411+ if args[1] != nil {
412+ arg1 = args[1].(domain.Principal)
413+ }
414+ var arg2 domain.RepoName
415+ if args[2] != nil {
416+ arg2 = args[2].(domain.RepoName)
417+ }
418+ var arg3 int64
419+ if args[3] != nil {
420+ arg3 = args[3].(int64)
421+ }
422+ var arg4 int64
423+ if args[4] != nil {
424+ arg4 = args[4].(int64)
425+ }
426+ run(
427+ arg0,
428+ arg1,
429+ arg2,
430+ arg3,
431+ arg4,
432+ )
433+ })
434+ return _c
435+}
436+
437+func (_c *MergeRequestService_DeleteComment_Call) Return(comment domain.Comment, err error) *MergeRequestService_DeleteComment_Call {
438+ _c.Call.Return(comment, err)
439+ return _c
440+}
441+
442+func (_c *MergeRequestService_DeleteComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64) (domain.Comment, error)) *MergeRequestService_DeleteComment_Call {
443+ _c.Call.Return(run)
444+ return _c
445+}
446+
447+// EditComment provides a mock function for the type MergeRequestService
448+func (_mock *MergeRequestService) EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error) {
449+ ret := _mock.Called(ctx, p, name, number, commentID, text)
450+
451+ if len(ret) == 0 {
452+ panic("no return value specified for EditComment")
453+ }
454+
455+ var r0 domain.Comment
456+ var r1 error
457+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) (domain.Comment, error)); ok {
458+ return returnFunc(ctx, p, name, number, commentID, text)
459+ }
460+ if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) domain.Comment); ok {
461+ r0 = returnFunc(ctx, p, name, number, commentID, text)
462+ } else {
463+ r0 = ret.Get(0).(domain.Comment)
464+ }
465+ if returnFunc, ok := ret.Get(1).(func(context.Context, domain.Principal, domain.RepoName, int64, int64, string) error); ok {
466+ r1 = returnFunc(ctx, p, name, number, commentID, text)
467+ } else {
468+ r1 = ret.Error(1)
469+ }
470+ return r0, r1
471+}
472+
473+// MergeRequestService_EditComment_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'EditComment'
474+type MergeRequestService_EditComment_Call struct {
475+ *mock.Call
476+}
477+
478+// EditComment is a helper method to define mock.On call
479+// - ctx context.Context
480+// - p domain.Principal
481+// - name domain.RepoName
482+// - number int64
483+// - commentID int64
484+// - text string
485+func (_e *MergeRequestService_Expecter) EditComment(ctx any, p any, name any, number any, commentID any, text any) *MergeRequestService_EditComment_Call {
486+ return &MergeRequestService_EditComment_Call{Call: _e.mock.On("EditComment", ctx, p, name, number, commentID, text)}
487+}
488+
489+func (_c *MergeRequestService_EditComment_Call) Run(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string)) *MergeRequestService_EditComment_Call {
490+ _c.Call.Run(func(args mock.Arguments) {
491+ var arg0 context.Context
492+ if args[0] != nil {
493+ arg0 = args[0].(context.Context)
494+ }
495+ var arg1 domain.Principal
496+ if args[1] != nil {
497+ arg1 = args[1].(domain.Principal)
498+ }
499+ var arg2 domain.RepoName
500+ if args[2] != nil {
501+ arg2 = args[2].(domain.RepoName)
502+ }
503+ var arg3 int64
504+ if args[3] != nil {
505+ arg3 = args[3].(int64)
506+ }
507+ var arg4 int64
508+ if args[4] != nil {
509+ arg4 = args[4].(int64)
510+ }
511+ var arg5 string
512+ if args[5] != nil {
513+ arg5 = args[5].(string)
514+ }
515+ run(
516+ arg0,
517+ arg1,
518+ arg2,
519+ arg3,
520+ arg4,
521+ arg5,
522+ )
523+ })
524+ return _c
525+}
526+
527+func (_c *MergeRequestService_EditComment_Call) Return(comment domain.Comment, err error) *MergeRequestService_EditComment_Call {
528+ _c.Call.Return(comment, err)
529+ return _c
530+}
531+
532+func (_c *MergeRequestService_EditComment_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, commentID int64, text string) (domain.Comment, error)) *MergeRequestService_EditComment_Call {
533+ _c.Call.Return(run)
534+ return _c
535+}
536+
363 // Get provides a mock function for the type MergeRequestService537 // Get provides a mock function for the type MergeRequestService
364 func (_mock *MergeRequestService) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {538 func (_mock *MergeRequestService) Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts ports.GetMergeRequestOptions) (ports.MergeRequestDetail, error) {
365 ret := _mock.Called(ctx, p, name, number, opts)539 ret := _mock.Called(ctx, p, name, number, opts)
M internal/ports/services.go +2 -0
@@ -113,6 +113,8 @@ type MergeRequestService interface {
113113 List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error)
114114 Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts GetMergeRequestOptions) (MergeRequestDetail, error)
115115 AddComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params AddCommentParams) (domain.Comment, error)
116+ EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string) (domain.Comment, error)
117+ DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64) (domain.Comment, error)
116118 ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error)
117119 Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.MergeRequest, error)
118120 MarkRead(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, head domain.ObjectID) (domain.ObjectID, error)
@@ -113,6 +113,8 @@ type MergeRequestService interface {
113 List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error)113 List(ctx context.Context, p domain.Principal, name domain.RepoName, params ListMergeRequestsParams) (MergeRequestPage, error)
114 Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts GetMergeRequestOptions) (MergeRequestDetail, error)114 Get(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, opts GetMergeRequestOptions) (MergeRequestDetail, error)
115 AddComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params AddCommentParams) (domain.Comment, error)115 AddComment(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, params AddCommentParams) (domain.Comment, error)
116+ EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string) (domain.Comment, error)
117+ DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64) (domain.Comment, error)
116 ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error)118 ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error)
117 Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.MergeRequest, error)119 Close(ctx context.Context, p domain.Principal, name domain.RepoName, number int64) (domain.MergeRequest, error)
118 MarkRead(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, head domain.ObjectID) (domain.ObjectID, error)120 MarkRead(ctx context.Context, p domain.Principal, name domain.RepoName, number int64, head domain.ObjectID) (domain.ObjectID, error)
M internal/usecase/events.go +12 -0
@@ -178,6 +178,12 @@ func (e *Events) snapshot(ctx context.Context, ev domain.Event) bufferedEvent {
178178 return repoSnapshot(ev.Visibility, ev.CreatedAt)
179179 case domain.CommentAdded:
180180 return repoSnapshot(ev.Visibility, ev.CreatedAt)
181+ case domain.CommentEdited:
182+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
183+ case domain.CommentDeleted:
184+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
185+ case domain.CommentResolved:
186+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
181187 case domain.RunUpdated:
182188 return repoSnapshot(ev.Visibility, ev.CreatedAt)
183189 case domain.JobUpdated:
@@ -487,6 +493,12 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f
487493 return e.deliverRepo(ctx, w, ev.MergeRequest.Repo, false, be, out, send)
488494 case domain.CommentAdded:
489495 return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
496+ case domain.CommentEdited:
497+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
498+ case domain.CommentDeleted:
499+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
500+ case domain.CommentResolved:
501+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
490502 case domain.RunUpdated:
491503 // Home shows the CI state of each repository.
492504 return e.deliverRepo(ctx, w, ev.Run.Repo, false, be, out, send)
@@ -178,6 +178,12 @@ func (e *Events) snapshot(ctx context.Context, ev domain.Event) bufferedEvent {
178 return repoSnapshot(ev.Visibility, ev.CreatedAt)178 return repoSnapshot(ev.Visibility, ev.CreatedAt)
179 case domain.CommentAdded:179 case domain.CommentAdded:
180 return repoSnapshot(ev.Visibility, ev.CreatedAt)180 return repoSnapshot(ev.Visibility, ev.CreatedAt)
181+ case domain.CommentEdited:
182+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
183+ case domain.CommentDeleted:
184+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
185+ case domain.CommentResolved:
186+ return repoSnapshot(ev.Visibility, ev.CreatedAt)
181 case domain.RunUpdated:187 case domain.RunUpdated:
182 return repoSnapshot(ev.Visibility, ev.CreatedAt)188 return repoSnapshot(ev.Visibility, ev.CreatedAt)
183 case domain.JobUpdated:189 case domain.JobUpdated:
@@ -487,6 +493,12 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f
487 return e.deliverRepo(ctx, w, ev.MergeRequest.Repo, false, be, out, send)493 return e.deliverRepo(ctx, w, ev.MergeRequest.Repo, false, be, out, send)
488 case domain.CommentAdded:494 case domain.CommentAdded:
489 return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)495 return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
496+ case domain.CommentEdited:
497+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
498+ case domain.CommentDeleted:
499+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
500+ case domain.CommentResolved:
501+ return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send)
490 case domain.RunUpdated:502 case domain.RunUpdated:
491 // Home shows the CI state of each repository.503 // Home shows the CI state of each repository.
492 return e.deliverRepo(ctx, w, ev.Run.Repo, false, be, out, send)504 return e.deliverRepo(ctx, w, ev.Run.Repo, false, be, out, send)
M internal/usecase/events_test.go +32 -0
@@ -922,3 +922,35 @@ func TestWatchHomeGetsRunUpdates(t *testing.T) {
922922 root.quiet(t, r, pub)
923923 })
924924 }
925+
926+func TestNoteMutationEventVisibility(t *testing.T) {
927+ synctest.Test(t, func(t *testing.T) {
928+ r := newEventRig(t, 0)
929+ pub := r.repo("pub", domain.VisibilityPublic)
930+ priv := r.repo("priv", domain.VisibilityPrivate)
931+ ctx, cancel := context.WithCancel(context.Background())
932+ defer cancel()
933+ anon := r.watch(ctx, domain.Anonymous(), ports.WatchParams{})
934+ owner := r.watch(ctx, domain.Root(), ports.WatchParams{Topics: []string{"repo:priv"}})
935+ synctest.Wait()
936+ for _, kind := range []string{"edited", "deleted", "resolved"} {
937+ for _, n := range []domain.RepoName{priv, pub} {
938+ vis := domain.VisibilityPublic
939+ if n == priv {
940+ vis = domain.VisibilityPrivate
941+ }
942+ switch kind {
943+ case "edited":
944+ r.feed <- domain.CommentEdited{Repo: n, Visibility: vis, CreatedAt: now, At: now}
945+ case "deleted":
946+ r.feed <- domain.CommentDeleted{Repo: n, Visibility: vis, CreatedAt: now, At: now}
947+ case "resolved":
948+ r.feed <- domain.CommentResolved{Repo: n, Visibility: vis, CreatedAt: now, At: now}
949+ }
950+ }
951+ assert.Equal(t, "mr.comment_"+kind, anon.next(t).Event.EventKind())
952+ assert.Equal(t, "mr.comment_"+kind, owner.next(t).Event.EventKind())
953+ anon.quiet(t, r, pub)
954+ }
955+ })
956+}
@@ -922,3 +922,35 @@ func TestWatchHomeGetsRunUpdates(t *testing.T) {
922 root.quiet(t, r, pub)922 root.quiet(t, r, pub)
923 })923 })
924 }924 }
925+
926+func TestNoteMutationEventVisibility(t *testing.T) {
927+ synctest.Test(t, func(t *testing.T) {
928+ r := newEventRig(t, 0)
929+ pub := r.repo("pub", domain.VisibilityPublic)
930+ priv := r.repo("priv", domain.VisibilityPrivate)
931+ ctx, cancel := context.WithCancel(context.Background())
932+ defer cancel()
933+ anon := r.watch(ctx, domain.Anonymous(), ports.WatchParams{})
934+ owner := r.watch(ctx, domain.Root(), ports.WatchParams{Topics: []string{"repo:priv"}})
935+ synctest.Wait()
936+ for _, kind := range []string{"edited", "deleted", "resolved"} {
937+ for _, n := range []domain.RepoName{priv, pub} {
938+ vis := domain.VisibilityPublic
939+ if n == priv {
940+ vis = domain.VisibilityPrivate
941+ }
942+ switch kind {
943+ case "edited":
944+ r.feed <- domain.CommentEdited{Repo: n, Visibility: vis, CreatedAt: now, At: now}
945+ case "deleted":
946+ r.feed <- domain.CommentDeleted{Repo: n, Visibility: vis, CreatedAt: now, At: now}
947+ case "resolved":
948+ r.feed <- domain.CommentResolved{Repo: n, Visibility: vis, CreatedAt: now, At: now}
949+ }
950+ }
951+ assert.Equal(t, "mr.comment_"+kind, anon.next(t).Event.EventKind())
952+ assert.Equal(t, "mr.comment_"+kind, owner.next(t).Event.EventKind())
953+ anon.quiet(t, r, pub)
954+ }
955+ })
956+}
M internal/usecase/mergerequests.go +118 -6
@@ -468,6 +468,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain
468468 if d.Comments, err = m.store.ListComments(ctx, name, number); err != nil {
469469 return ports.MergeRequestDetail{}, fmt.Errorf("list comments of !%d in %q: %w", number, name, err)
470470 }
471+ d.Verdicts = domain.CurrentVerdicts(d.Comments, mr.HeadID)
471472 if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil {
472473 return ports.MergeRequestDetail{}, err
473474 }
@@ -547,7 +548,7 @@ func (m *MergeRequests) rangeOf(ctx context.Context, mr domain.MergeRequest) (ex
547548 }
548549
549550 // AddComment adds a comment and publishes CommentAdded. The principal needs
550-// mr:write. The text must follow the rules of ADR 0017 and not be blank; a
551+// mr:write. The text must follow the rules of ADR 0032 and not be blank unless carrying a verdict; a
551552 // comment on a line must name a line of the current diff of the request (see
552553 // domain.Diff.HasLine), else the call fails with domain.ErrInvalid. A request
553554 // that is not open gives domain.ErrPreconditionFailed, and one that already
@@ -566,21 +567,53 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
566567 switch {
567568 case err != nil:
568569 v.AddErr("text", err)
569- case text.IsBlank():
570+ case text.IsBlank() && params.Verdict == domain.VerdictNone:
570571 v.Add("text", "is required")
571572 }
572573 general := params.Path == ""
573574 switch {
574- case general && (params.Line != 0 || params.Side != domain.SideNone):
575+ case general && (params.Line != 0 || params.Side != domain.SideNone || params.StartLine != 0):
575576 v.Add("path", "is required with line and side")
576577 case !general && params.Line < 1:
577578 v.Add("line", "must be 1 or more with a path")
578579 case !general && params.Side != domain.SideOld && params.Side != domain.SideNew:
579580 v.Add("side", "is required with a path")
580581 }
582+ if params.StartLine != 0 && (params.StartLine < 1 || params.StartLine >= params.Line) {
583+ v.Add("start_line", "must be 1 or more and less than line")
584+ }
585+ if params.Verdict > domain.VerdictChangesRequested {
586+ v.Add("verdict", "is unknown")
587+ }
588+ if params.ReplyTo != 0 {
589+ if params.Path != "" || params.Line != 0 || params.StartLine != 0 || params.Side != domain.SideNone || params.Verdict != domain.VerdictNone {
590+ v.Add("reply_to", "a reply cannot specify an anchor or verdict")
591+ }
592+ }
581593 if err := v.Err(); err != nil {
582594 return domain.Comment{}, err
583595 }
596+ if params.ReplyTo != 0 {
597+ root, err := m.store.GetComment(ctx, name, number, params.ReplyTo)
598+ switch {
599+ case errors.Is(err, domain.ErrNotFound):
600+ v.Add("reply_to", fmt.Sprintf("no note ^%d", params.ReplyTo))
601+ case err != nil:
602+ return domain.Comment{}, err
603+ case root.IsReply() || root.Deleted:
604+ v.Add("reply_to", "must name a non-deleted root note")
605+ default:
606+ params.Path, params.Line, params.StartLine, params.Side = root.Path, root.Line, root.StartLine, root.Side
607+ }
608+ if err := v.Err(); err != nil {
609+ return domain.Comment{}, err
610+ }
611+ }
612+ if params.Verdict != domain.VerdictNone {
613+ if err := scopeAccess(p, repo, domain.ScopeMRReview); err != nil {
614+ return domain.Comment{}, err
615+ }
616+ }
584617 if !mr.IsOpen() {
585618 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
586619 }
@@ -588,7 +621,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
588621 if mr, err = m.withHead(ctx, mr); err != nil {
589622 return domain.Comment{}, err
590623 }
591- if !general {
624+ if !general && params.ReplyTo == 0 {
592625 if err := m.checkLine(ctx, mr, params); err != nil {
593626 return domain.Comment{}, err
594627 }
@@ -601,6 +634,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
601634 }
602635 c = domain.Comment{
603636 ID: mr.CommentSeq + 1, Author: domain.AuthorName(p), Path: params.Path, Line: params.Line, Side: params.Side,
637+ ReplyTo: params.ReplyTo, StartLine: params.StartLine, Verdict: params.Verdict, Resolved: params.Verdict == domain.VerdictApprove,
604638 CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),
605639 }
606640 mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt
@@ -624,7 +658,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
624658 }
625659 log := zerowrap.FromCtx(ctx)
626660 log.Info().Str("repo", name.String()).Int64("number", number).Int64("comment", c.ID).
627- Bool("on_line", !general).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment added")
661+ Int64("reply_to", c.ReplyTo).Str("verdict", c.Verdict.String()).Bool("on_line", !general).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment added")
628662 m.bus.Publish(ctx, domain.CommentAdded{
629663 Repo: name, Number: number, Comment: c, At: c.CreatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt,
630664 })
@@ -667,6 +701,9 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
667701 if idx < 0 || !diff.HasLine(params.Path, params.Side, params.Line) {
668702 v.Add("path", fmt.Sprintf("%s:%d on the %s side is not a line of the diff", params.Path, params.Line, params.Side))
669703 }
704+ if params.StartLine != 0 && !diff.HasLine(params.Path, params.Side, params.StartLine) {
705+ v.Add("start_line", "is not a line of the diff")
706+ }
670707 return v.Err()
671708 }
672709
@@ -683,7 +720,8 @@ func diffFileName(f domain.FileDiff) string {
683720 // not open domain.ErrPreconditionFailed. Setting the state the comment already
684721 // has succeeds and writes nothing.
685722 func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error) {
686- if _, err := m.writable(ctx, p, name, domain.ScopeMRWrite); err != nil {
723+ repo, err := m.writable(ctx, p, name, domain.ScopeMRWrite)
724+ if err != nil {
687725 return domain.Comment{}, err
688726 }
689727 for range maxWriteRetries {
@@ -698,6 +736,11 @@ func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal,
698736 if !mr.IsOpen() {
699737 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
700738 }
739+ if c.IsReply() {
740+ var v domain.Violations
741+ v.Add("comment_id", fmt.Sprintf("resolve the thread on its root ^%d", c.ReplyTo))
742+ return domain.Comment{}, v.Err()
743+ }
701744 if c.Resolved == resolved {
702745 return c, nil
703746 }
@@ -710,6 +753,7 @@ func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal,
710753 if err != nil {
711754 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, err)
712755 }
756+ m.bus.Publish(ctx, domain.CommentResolved{Repo: name, Number: number, Comment: c, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
713757 return c, nil
714758 }
715759 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, domain.ErrConflict)
@@ -1129,3 +1173,71 @@ func (m *MergeRequests) markMerged(ctx context.Context, name domain.RepoName, nu
11291173 Msg("the target branch moved but the merge request could not be marked merged")
11301174 return domain.MergeRequest{}, fmt.Errorf("record the merge of !%d in %q: %w", number, name, domain.ErrConflict)
11311175 }
1176+
1177+// EditComment changes only the author's text, preserving the immutable verdict.
1178+func (m *MergeRequests) EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string) (domain.Comment, error) {
1179+ return m.changeComment(ctx, p, name, number, commentID, text, false)
1180+}
1181+
1182+// DeleteComment removes text but retains the anchor, ID and verdict. Repeated deletion is a no-op.
1183+func (m *MergeRequests) DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64) (domain.Comment, error) {
1184+ return m.changeComment(ctx, p, name, number, commentID, "", true)
1185+}
1186+
1187+func (m *MergeRequests) changeComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string, remove bool) (domain.Comment, error) {
1188+ repo, err := m.writable(ctx, p, name, domain.ScopeMRWrite)
1189+ if err != nil {
1190+ return domain.Comment{}, err
1191+ }
1192+ for range maxWriteRetries {
1193+ mr, err := m.store.Get(ctx, name, number)
1194+ if err != nil {
1195+ return domain.Comment{}, err
1196+ }
1197+ c, err := m.store.GetComment(ctx, name, number, commentID)
1198+ if err != nil {
1199+ return domain.Comment{}, err
1200+ }
1201+ if !mr.IsOpen() {
1202+ return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
1203+ }
1204+ if domain.AuthorName(p) != c.Author {
1205+ return domain.Comment{}, fmt.Errorf("%w: only the author of a note can edit or delete it", domain.ErrForbidden)
1206+ }
1207+ if c.Deleted {
1208+ if remove {
1209+ return c, nil
1210+ }
1211+ return domain.Comment{}, fmt.Errorf("%w: the note was deleted", domain.ErrPreconditionFailed)
1212+ }
1213+ parsed, err := domain.ParsePlainText(text)
1214+ var v domain.Violations
1215+ if err != nil {
1216+ v.AddErr("text", err)
1217+ } else if !remove && parsed.IsBlank() && c.Verdict == domain.VerdictNone {
1218+ v.Add("text", "is required")
1219+ }
1220+ if err := v.Err(); err != nil {
1221+ return domain.Comment{}, err
1222+ }
1223+ if !remove && parsed == c.Text {
1224+ return c, nil
1225+ }
1226+ c.Text, c.Deleted, c.EditedAt = parsed, remove, m.clock.Now().UTC()
1227+ mr.UpdatedAt = c.EditedAt
1228+ if err := m.store.SaveComment(ctx, mr, c); errors.Is(err, domain.ErrConflict) {
1229+ continue
1230+ } else if err != nil {
1231+ return domain.Comment{}, fmt.Errorf("save note %d of !%d in %q: %w", commentID, number, name, err)
1232+ }
1233+ log := zerowrap.FromCtx(ctx)
1234+ log.Info().Str("repo", name.String()).Int64("number", number).Int64("comment", c.ID).Bool("deleted", remove).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment changed")
1235+ if remove {
1236+ m.bus.Publish(ctx, domain.CommentDeleted{Repo: name, Number: number, Comment: c, At: c.EditedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
1237+ } else {
1238+ m.bus.Publish(ctx, domain.CommentEdited{Repo: name, Number: number, Comment: c, At: c.EditedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
1239+ }
1240+ return c, nil
1241+ }
1242+ return domain.Comment{}, fmt.Errorf("save note %d of !%d in %q: %w", commentID, number, name, domain.ErrConflict)
1243+}
@@ -468,6 +468,7 @@ func (m *MergeRequests) Get(ctx context.Context, p domain.Principal, name domain
468 if d.Comments, err = m.store.ListComments(ctx, name, number); err != nil {468 if d.Comments, err = m.store.ListComments(ctx, name, number); err != nil {
469 return ports.MergeRequestDetail{}, fmt.Errorf("list comments of !%d in %q: %w", number, name, err)469 return ports.MergeRequestDetail{}, fmt.Errorf("list comments of !%d in %q: %w", number, name, err)
470 }470 }
471+ d.Verdicts = domain.CurrentVerdicts(d.Comments, mr.HeadID)
471 if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil {472 if d.LastReadID, err = m.store.GetRead(ctx, name, number); err != nil {
472 return ports.MergeRequestDetail{}, err473 return ports.MergeRequestDetail{}, err
473 }474 }
@@ -547,7 +548,7 @@ func (m *MergeRequests) rangeOf(ctx context.Context, mr domain.MergeRequest) (ex
547 }548 }
548 549
549 // AddComment adds a comment and publishes CommentAdded. The principal needs550 // AddComment adds a comment and publishes CommentAdded. The principal needs
550-// mr:write. The text must follow the rules of ADR 0017 and not be blank; a551+// mr:write. The text must follow the rules of ADR 0032 and not be blank unless carrying a verdict; a
551 // comment on a line must name a line of the current diff of the request (see552 // comment on a line must name a line of the current diff of the request (see
552 // domain.Diff.HasLine), else the call fails with domain.ErrInvalid. A request553 // domain.Diff.HasLine), else the call fails with domain.ErrInvalid. A request
553 // that is not open gives domain.ErrPreconditionFailed, and one that already554 // that is not open gives domain.ErrPreconditionFailed, and one that already
@@ -566,21 +567,53 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
566 switch {567 switch {
567 case err != nil:568 case err != nil:
568 v.AddErr("text", err)569 v.AddErr("text", err)
569- case text.IsBlank():570+ case text.IsBlank() && params.Verdict == domain.VerdictNone:
570 v.Add("text", "is required")571 v.Add("text", "is required")
571 }572 }
572 general := params.Path == ""573 general := params.Path == ""
573 switch {574 switch {
574- case general && (params.Line != 0 || params.Side != domain.SideNone):575+ case general && (params.Line != 0 || params.Side != domain.SideNone || params.StartLine != 0):
575 v.Add("path", "is required with line and side")576 v.Add("path", "is required with line and side")
576 case !general && params.Line < 1:577 case !general && params.Line < 1:
577 v.Add("line", "must be 1 or more with a path")578 v.Add("line", "must be 1 or more with a path")
578 case !general && params.Side != domain.SideOld && params.Side != domain.SideNew:579 case !general && params.Side != domain.SideOld && params.Side != domain.SideNew:
579 v.Add("side", "is required with a path")580 v.Add("side", "is required with a path")
580 }581 }
582+ if params.StartLine != 0 && (params.StartLine < 1 || params.StartLine >= params.Line) {
583+ v.Add("start_line", "must be 1 or more and less than line")
584+ }
585+ if params.Verdict > domain.VerdictChangesRequested {
586+ v.Add("verdict", "is unknown")
587+ }
588+ if params.ReplyTo != 0 {
589+ if params.Path != "" || params.Line != 0 || params.StartLine != 0 || params.Side != domain.SideNone || params.Verdict != domain.VerdictNone {
590+ v.Add("reply_to", "a reply cannot specify an anchor or verdict")
591+ }
592+ }
581 if err := v.Err(); err != nil {593 if err := v.Err(); err != nil {
582 return domain.Comment{}, err594 return domain.Comment{}, err
583 }595 }
596+ if params.ReplyTo != 0 {
597+ root, err := m.store.GetComment(ctx, name, number, params.ReplyTo)
598+ switch {
599+ case errors.Is(err, domain.ErrNotFound):
600+ v.Add("reply_to", fmt.Sprintf("no note ^%d", params.ReplyTo))
601+ case err != nil:
602+ return domain.Comment{}, err
603+ case root.IsReply() || root.Deleted:
604+ v.Add("reply_to", "must name a non-deleted root note")
605+ default:
606+ params.Path, params.Line, params.StartLine, params.Side = root.Path, root.Line, root.StartLine, root.Side
607+ }
608+ if err := v.Err(); err != nil {
609+ return domain.Comment{}, err
610+ }
611+ }
612+ if params.Verdict != domain.VerdictNone {
613+ if err := scopeAccess(p, repo, domain.ScopeMRReview); err != nil {
614+ return domain.Comment{}, err
615+ }
616+ }
584 if !mr.IsOpen() {617 if !mr.IsOpen() {
585 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)618 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
586 }619 }
@@ -588,7 +621,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
588 if mr, err = m.withHead(ctx, mr); err != nil {621 if mr, err = m.withHead(ctx, mr); err != nil {
589 return domain.Comment{}, err622 return domain.Comment{}, err
590 }623 }
591- if !general {624+ if !general && params.ReplyTo == 0 {
592 if err := m.checkLine(ctx, mr, params); err != nil {625 if err := m.checkLine(ctx, mr, params); err != nil {
593 return domain.Comment{}, err626 return domain.Comment{}, err
594 }627 }
@@ -601,6 +634,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
601 }634 }
602 c = domain.Comment{635 c = domain.Comment{
603 ID: mr.CommentSeq + 1, Author: domain.AuthorName(p), Path: params.Path, Line: params.Line, Side: params.Side,636 ID: mr.CommentSeq + 1, Author: domain.AuthorName(p), Path: params.Path, Line: params.Line, Side: params.Side,
637+ ReplyTo: params.ReplyTo, StartLine: params.StartLine, Verdict: params.Verdict, Resolved: params.Verdict == domain.VerdictApprove,
604 CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),638 CommitID: mr.HeadID, Text: text, CreatedAt: m.clock.Now().UTC(),
605 }639 }
606 mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt640 mr.CommentSeq, mr.UpdatedAt = c.ID, c.CreatedAt
@@ -624,7 +658,7 @@ func (m *MergeRequests) AddComment(ctx context.Context, p domain.Principal, name
624 }658 }
625 log := zerowrap.FromCtx(ctx)659 log := zerowrap.FromCtx(ctx)
626 log.Info().Str("repo", name.String()).Int64("number", number).Int64("comment", c.ID).660 log.Info().Str("repo", name.String()).Int64("number", number).Int64("comment", c.ID).
627- Bool("on_line", !general).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment added")661+ Int64("reply_to", c.ReplyTo).Str("verdict", c.Verdict.String()).Bool("on_line", !general).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment added")
628 m.bus.Publish(ctx, domain.CommentAdded{662 m.bus.Publish(ctx, domain.CommentAdded{
629 Repo: name, Number: number, Comment: c, At: c.CreatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt,663 Repo: name, Number: number, Comment: c, At: c.CreatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt,
630 })664 })
@@ -667,6 +701,9 @@ func (m *MergeRequests) checkLine(ctx context.Context, mr domain.MergeRequest, p
667 if idx < 0 || !diff.HasLine(params.Path, params.Side, params.Line) {701 if idx < 0 || !diff.HasLine(params.Path, params.Side, params.Line) {
668 v.Add("path", fmt.Sprintf("%s:%d on the %s side is not a line of the diff", params.Path, params.Line, params.Side))702 v.Add("path", fmt.Sprintf("%s:%d on the %s side is not a line of the diff", params.Path, params.Line, params.Side))
669 }703 }
704+ if params.StartLine != 0 && !diff.HasLine(params.Path, params.Side, params.StartLine) {
705+ v.Add("start_line", "is not a line of the diff")
706+ }
670 return v.Err()707 return v.Err()
671 }708 }
672 709
@@ -683,7 +720,8 @@ func diffFileName(f domain.FileDiff) string {
683 // not open domain.ErrPreconditionFailed. Setting the state the comment already720 // not open domain.ErrPreconditionFailed. Setting the state the comment already
684 // has succeeds and writes nothing.721 // has succeeds and writes nothing.
685 func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error) {722 func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, resolved bool) (domain.Comment, error) {
686- if _, err := m.writable(ctx, p, name, domain.ScopeMRWrite); err != nil {723+ repo, err := m.writable(ctx, p, name, domain.ScopeMRWrite)
724+ if err != nil {
687 return domain.Comment{}, err725 return domain.Comment{}, err
688 }726 }
689 for range maxWriteRetries {727 for range maxWriteRetries {
@@ -698,6 +736,11 @@ func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal,
698 if !mr.IsOpen() {736 if !mr.IsOpen() {
699 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)737 return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
700 }738 }
739+ if c.IsReply() {
740+ var v domain.Violations
741+ v.Add("comment_id", fmt.Sprintf("resolve the thread on its root ^%d", c.ReplyTo))
742+ return domain.Comment{}, v.Err()
743+ }
701 if c.Resolved == resolved {744 if c.Resolved == resolved {
702 return c, nil745 return c, nil
703 }746 }
@@ -710,6 +753,7 @@ func (m *MergeRequests) ResolveComment(ctx context.Context, p domain.Principal,
710 if err != nil {753 if err != nil {
711 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, err)754 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, err)
712 }755 }
756+ m.bus.Publish(ctx, domain.CommentResolved{Repo: name, Number: number, Comment: c, At: mr.UpdatedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
713 return c, nil757 return c, nil
714 }758 }
715 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, domain.ErrConflict)759 return domain.Comment{}, fmt.Errorf("resolve comment %d of !%d in %q: %w", commentID, number, name, domain.ErrConflict)
@@ -1129,3 +1173,71 @@ func (m *MergeRequests) markMerged(ctx context.Context, name domain.RepoName, nu
1129 Msg("the target branch moved but the merge request could not be marked merged")1173 Msg("the target branch moved but the merge request could not be marked merged")
1130 return domain.MergeRequest{}, fmt.Errorf("record the merge of !%d in %q: %w", number, name, domain.ErrConflict)1174 return domain.MergeRequest{}, fmt.Errorf("record the merge of !%d in %q: %w", number, name, domain.ErrConflict)
1131 }1175 }
1176+
1177+// EditComment changes only the author's text, preserving the immutable verdict.
1178+func (m *MergeRequests) EditComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string) (domain.Comment, error) {
1179+ return m.changeComment(ctx, p, name, number, commentID, text, false)
1180+}
1181+
1182+// DeleteComment removes text but retains the anchor, ID and verdict. Repeated deletion is a no-op.
1183+func (m *MergeRequests) DeleteComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64) (domain.Comment, error) {
1184+ return m.changeComment(ctx, p, name, number, commentID, "", true)
1185+}
1186+
1187+func (m *MergeRequests) changeComment(ctx context.Context, p domain.Principal, name domain.RepoName, number, commentID int64, text string, remove bool) (domain.Comment, error) {
1188+ repo, err := m.writable(ctx, p, name, domain.ScopeMRWrite)
1189+ if err != nil {
1190+ return domain.Comment{}, err
1191+ }
1192+ for range maxWriteRetries {
1193+ mr, err := m.store.Get(ctx, name, number)
1194+ if err != nil {
1195+ return domain.Comment{}, err
1196+ }
1197+ c, err := m.store.GetComment(ctx, name, number, commentID)
1198+ if err != nil {
1199+ return domain.Comment{}, err
1200+ }
1201+ if !mr.IsOpen() {
1202+ return domain.Comment{}, fmt.Errorf("%w: !%d is %s", domain.ErrPreconditionFailed, number, mr.State)
1203+ }
1204+ if domain.AuthorName(p) != c.Author {
1205+ return domain.Comment{}, fmt.Errorf("%w: only the author of a note can edit or delete it", domain.ErrForbidden)
1206+ }
1207+ if c.Deleted {
1208+ if remove {
1209+ return c, nil
1210+ }
1211+ return domain.Comment{}, fmt.Errorf("%w: the note was deleted", domain.ErrPreconditionFailed)
1212+ }
1213+ parsed, err := domain.ParsePlainText(text)
1214+ var v domain.Violations
1215+ if err != nil {
1216+ v.AddErr("text", err)
1217+ } else if !remove && parsed.IsBlank() && c.Verdict == domain.VerdictNone {
1218+ v.Add("text", "is required")
1219+ }
1220+ if err := v.Err(); err != nil {
1221+ return domain.Comment{}, err
1222+ }
1223+ if !remove && parsed == c.Text {
1224+ return c, nil
1225+ }
1226+ c.Text, c.Deleted, c.EditedAt = parsed, remove, m.clock.Now().UTC()
1227+ mr.UpdatedAt = c.EditedAt
1228+ if err := m.store.SaveComment(ctx, mr, c); errors.Is(err, domain.ErrConflict) {
1229+ continue
1230+ } else if err != nil {
1231+ return domain.Comment{}, fmt.Errorf("save note %d of !%d in %q: %w", commentID, number, name, err)
1232+ }
1233+ log := zerowrap.FromCtx(ctx)
1234+ log.Info().Str("repo", name.String()).Int64("number", number).Int64("comment", c.ID).Bool("deleted", remove).Str("principal", p.Label()).Str("token_id", p.TokenID()).Msg("comment changed")
1235+ if remove {
1236+ m.bus.Publish(ctx, domain.CommentDeleted{Repo: name, Number: number, Comment: c, At: c.EditedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
1237+ } else {
1238+ m.bus.Publish(ctx, domain.CommentEdited{Repo: name, Number: number, Comment: c, At: c.EditedAt, Visibility: repo.Visibility, CreatedAt: repo.CreatedAt})
1239+ }
1240+ return c, nil
1241+ }
1242+ return domain.Comment{}, fmt.Errorf("save note %d of !%d in %q: %w", commentID, number, name, domain.ErrConflict)
1243+}
M internal/usecase/mergerequests_integration_test.go +91 -1
@@ -689,7 +689,10 @@ func TestResolveComment(t *testing.T) {
689689 _, err = r.mrs.ResolveComment(r.ctx, mrW, r.repo, mr.Number, 0, true)
690690 assert.ErrorIs(t, err, domain.ErrNotFound)
691691
692- assert.Empty(t, r.drain(), "a resolved comment has no event")
692+ events := r.drain()
693+ require.Len(t, events, 2, "only state changes publish resolution events")
694+ assert.True(t, events[0].(domain.CommentResolved).Comment.Resolved)
695+ assert.False(t, events[1].(domain.CommentResolved).Comment.Resolved)
693696 }
694697
695698 func TestEventsOfAMergeRequest(t *testing.T) {
@@ -1342,3 +1345,90 @@ func TestCommentsPerMergeRequestAreBounded(t *testing.T) {
13421345 _, err = r.mrs.AddComment(r.ctx, mrW, r.repo, other.Number, ports.AddCommentParams{Text: "fine"})
13431346 require.NoError(t, err)
13441347 }
1348+
1349+func TestNoteThreadsRangesAndVerdicts(t *testing.T) {
1350+ r := newMRRig(t)
1351+ r.seed()
1352+ head := r.feature("feat")
1353+ mr := r.open("feat")
1354+ add := func(p domain.Principal, params ports.AddCommentParams) (domain.Comment, error) {
1355+ return r.mrs.AddComment(r.ctx, p, r.repo, mr.Number, params)
1356+ }
1357+ rootNote, err := add(root, ports.AddCommentParams{Path: "a.txt", StartLine: 1, Line: 3, Side: domain.SideNew, Text: "range"})
1358+ require.NoError(t, err)
1359+ reply, err := add(mrW, ports.AddCommentParams{ReplyTo: rootNote.ID, Text: "reply"})
1360+ require.NoError(t, err)
1361+ assert.Equal(t, rootNote.Path, reply.Path)
1362+ assert.Equal(t, rootNote.StartLine, reply.StartLine)
1363+ assert.Equal(t, rootNote.Line, reply.Line)
1364+ assert.Equal(t, rootNote.Side, reply.Side)
1365+ assert.Equal(t, head, reply.CommitID)
1366+ assert.False(t, reply.Resolved)
1367+ for _, params := range []ports.AddCommentParams{
1368+ {ReplyTo: reply.ID, Text: "x"}, {ReplyTo: 999, Text: "x"}, {ReplyTo: rootNote.ID, Path: "a.txt", Text: "x"}, {ReplyTo: rootNote.ID, Line: 1, Text: "x"}, {ReplyTo: rootNote.ID, Side: domain.SideNew, Text: "x"}, {ReplyTo: rootNote.ID, StartLine: 1, Text: "x"}, {ReplyTo: rootNote.ID, Verdict: domain.VerdictApprove},
1369+ {StartLine: 1, Text: "x"}, {Path: "a.txt", StartLine: 3, Line: 3, Side: domain.SideNew, Text: "x"}, {Path: "a.txt", StartLine: -1, Line: 3, Side: domain.SideNew, Text: "x"}, {Path: "a.txt", StartLine: 1, Line: 99, Side: domain.SideNew, Text: "x"},
1370+ } {
1371+ _, err := add(root, params)
1372+ assert.ErrorIs(t, err, domain.ErrInvalid, "%+v", params)
1373+ }
1374+ _, err = r.mrs.ResolveComment(r.ctx, root, r.repo, mr.Number, reply.ID, true)
1375+ assert.ErrorIs(t, err, domain.ErrInvalid)
1376+ assert.ErrorContains(t, err, fmt.Sprintf("root ^%d", rootNote.ID))
1377+ _, err = add(mrW, ports.AddCommentParams{Verdict: domain.VerdictApprove})
1378+ assert.ErrorIs(t, err, domain.ErrForbidden)
1379+ assert.ErrorContains(t, err, "this needs the mr:review scope")
1380+ reviewer := domain.TokenPrincipal(domain.Token{Label: "reviewer", Scopes: []domain.Scope{domain.ScopeMRWrite, domain.ScopeMRReview}})
1381+ approve, err := add(reviewer, ports.AddCommentParams{Verdict: domain.VerdictApprove})
1382+ require.NoError(t, err)
1383+ assert.True(t, approve.Resolved)
1384+ changes, err := add(root, ports.AddCommentParams{Verdict: domain.VerdictChangesRequested})
1385+ require.NoError(t, err)
1386+ assert.False(t, changes.Resolved)
1387+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, approve.ID, "no")
1388+ assert.ErrorIs(t, err, domain.ErrForbidden)
1389+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, approve.ID)
1390+ assert.ErrorIs(t, err, domain.ErrForbidden)
1391+ r.drain()
1392+ edited, err := r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "updated")
1393+ require.NoError(t, err)
1394+ assert.False(t, edited.EditedAt.IsZero())
1395+ assert.Equal(t, approve.Verdict, edited.Verdict)
1396+ _, err = r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "updated")
1397+ require.NoError(t, err)
1398+ events := r.drain()
1399+ require.Len(t, events, 1)
1400+ assert.IsType(t, domain.CommentEdited{}, events[0])
1401+ deleted, err := r.mrs.DeleteComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID)
1402+ require.NoError(t, err)
1403+ assert.True(t, deleted.Deleted)
1404+ assert.Equal(t, "", deleted.Text.String())
1405+ assert.Equal(t, approve.Verdict, deleted.Verdict)
1406+ again, err := r.mrs.DeleteComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID)
1407+ require.NoError(t, err)
1408+ deleted.Version = again.Version // SaveComment assigns the new version on the next read.
1409+ assert.Equal(t, deleted, again)
1410+ events = r.drain()
1411+ require.Len(t, events, 1)
1412+ assert.IsType(t, domain.CommentDeleted{}, events[0])
1413+ _, err = r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "x")
1414+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1415+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, rootNote.ID, "")
1416+ assert.ErrorIs(t, err, domain.ErrInvalid)
1417+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, rootNote.ID)
1418+ require.NoError(t, err)
1419+ _, err = add(root, ports.AddCommentParams{ReplyTo: rootNote.ID, Text: "x"})
1420+ assert.ErrorIs(t, err, domain.ErrInvalid)
1421+ r.commit(author, "a.txt", "changed\n2\nthree\n", "advance")
1422+ r.push("feat")
1423+ detail, err := r.mrs.Get(r.ctx, root, r.repo, mr.Number, ports.GetMergeRequestOptions{})
1424+ require.NoError(t, err)
1425+ require.Len(t, detail.Verdicts, 2)
1426+ assert.True(t, detail.Verdicts[0].Stale)
1427+ assert.False(t, detail.Verdicts[1].Stale)
1428+ _, err = r.mrs.Close(r.ctx, root, r.repo, mr.Number)
1429+ require.NoError(t, err)
1430+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, changes.ID, "x")
1431+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1432+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, changes.ID)
1433+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1434+}
@@ -689,7 +689,10 @@ func TestResolveComment(t *testing.T) {
689 _, err = r.mrs.ResolveComment(r.ctx, mrW, r.repo, mr.Number, 0, true)689 _, err = r.mrs.ResolveComment(r.ctx, mrW, r.repo, mr.Number, 0, true)
690 assert.ErrorIs(t, err, domain.ErrNotFound)690 assert.ErrorIs(t, err, domain.ErrNotFound)
691 691
692- assert.Empty(t, r.drain(), "a resolved comment has no event")692+ events := r.drain()
693+ require.Len(t, events, 2, "only state changes publish resolution events")
694+ assert.True(t, events[0].(domain.CommentResolved).Comment.Resolved)
695+ assert.False(t, events[1].(domain.CommentResolved).Comment.Resolved)
693 }696 }
694 697
695 func TestEventsOfAMergeRequest(t *testing.T) {698 func TestEventsOfAMergeRequest(t *testing.T) {
@@ -1342,3 +1345,90 @@ func TestCommentsPerMergeRequestAreBounded(t *testing.T) {
1342 _, err = r.mrs.AddComment(r.ctx, mrW, r.repo, other.Number, ports.AddCommentParams{Text: "fine"})1345 _, err = r.mrs.AddComment(r.ctx, mrW, r.repo, other.Number, ports.AddCommentParams{Text: "fine"})
1343 require.NoError(t, err)1346 require.NoError(t, err)
1344 }1347 }
1348+
1349+func TestNoteThreadsRangesAndVerdicts(t *testing.T) {
1350+ r := newMRRig(t)
1351+ r.seed()
1352+ head := r.feature("feat")
1353+ mr := r.open("feat")
1354+ add := func(p domain.Principal, params ports.AddCommentParams) (domain.Comment, error) {
1355+ return r.mrs.AddComment(r.ctx, p, r.repo, mr.Number, params)
1356+ }
1357+ rootNote, err := add(root, ports.AddCommentParams{Path: "a.txt", StartLine: 1, Line: 3, Side: domain.SideNew, Text: "range"})
1358+ require.NoError(t, err)
1359+ reply, err := add(mrW, ports.AddCommentParams{ReplyTo: rootNote.ID, Text: "reply"})
1360+ require.NoError(t, err)
1361+ assert.Equal(t, rootNote.Path, reply.Path)
1362+ assert.Equal(t, rootNote.StartLine, reply.StartLine)
1363+ assert.Equal(t, rootNote.Line, reply.Line)
1364+ assert.Equal(t, rootNote.Side, reply.Side)
1365+ assert.Equal(t, head, reply.CommitID)
1366+ assert.False(t, reply.Resolved)
1367+ for _, params := range []ports.AddCommentParams{
1368+ {ReplyTo: reply.ID, Text: "x"}, {ReplyTo: 999, Text: "x"}, {ReplyTo: rootNote.ID, Path: "a.txt", Text: "x"}, {ReplyTo: rootNote.ID, Line: 1, Text: "x"}, {ReplyTo: rootNote.ID, Side: domain.SideNew, Text: "x"}, {ReplyTo: rootNote.ID, StartLine: 1, Text: "x"}, {ReplyTo: rootNote.ID, Verdict: domain.VerdictApprove},
1369+ {StartLine: 1, Text: "x"}, {Path: "a.txt", StartLine: 3, Line: 3, Side: domain.SideNew, Text: "x"}, {Path: "a.txt", StartLine: -1, Line: 3, Side: domain.SideNew, Text: "x"}, {Path: "a.txt", StartLine: 1, Line: 99, Side: domain.SideNew, Text: "x"},
1370+ } {
1371+ _, err := add(root, params)
1372+ assert.ErrorIs(t, err, domain.ErrInvalid, "%+v", params)
1373+ }
1374+ _, err = r.mrs.ResolveComment(r.ctx, root, r.repo, mr.Number, reply.ID, true)
1375+ assert.ErrorIs(t, err, domain.ErrInvalid)
1376+ assert.ErrorContains(t, err, fmt.Sprintf("root ^%d", rootNote.ID))
1377+ _, err = add(mrW, ports.AddCommentParams{Verdict: domain.VerdictApprove})
1378+ assert.ErrorIs(t, err, domain.ErrForbidden)
1379+ assert.ErrorContains(t, err, "this needs the mr:review scope")
1380+ reviewer := domain.TokenPrincipal(domain.Token{Label: "reviewer", Scopes: []domain.Scope{domain.ScopeMRWrite, domain.ScopeMRReview}})
1381+ approve, err := add(reviewer, ports.AddCommentParams{Verdict: domain.VerdictApprove})
1382+ require.NoError(t, err)
1383+ assert.True(t, approve.Resolved)
1384+ changes, err := add(root, ports.AddCommentParams{Verdict: domain.VerdictChangesRequested})
1385+ require.NoError(t, err)
1386+ assert.False(t, changes.Resolved)
1387+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, approve.ID, "no")
1388+ assert.ErrorIs(t, err, domain.ErrForbidden)
1389+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, approve.ID)
1390+ assert.ErrorIs(t, err, domain.ErrForbidden)
1391+ r.drain()
1392+ edited, err := r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "updated")
1393+ require.NoError(t, err)
1394+ assert.False(t, edited.EditedAt.IsZero())
1395+ assert.Equal(t, approve.Verdict, edited.Verdict)
1396+ _, err = r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "updated")
1397+ require.NoError(t, err)
1398+ events := r.drain()
1399+ require.Len(t, events, 1)
1400+ assert.IsType(t, domain.CommentEdited{}, events[0])
1401+ deleted, err := r.mrs.DeleteComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID)
1402+ require.NoError(t, err)
1403+ assert.True(t, deleted.Deleted)
1404+ assert.Equal(t, "", deleted.Text.String())
1405+ assert.Equal(t, approve.Verdict, deleted.Verdict)
1406+ again, err := r.mrs.DeleteComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID)
1407+ require.NoError(t, err)
1408+ deleted.Version = again.Version // SaveComment assigns the new version on the next read.
1409+ assert.Equal(t, deleted, again)
1410+ events = r.drain()
1411+ require.Len(t, events, 1)
1412+ assert.IsType(t, domain.CommentDeleted{}, events[0])
1413+ _, err = r.mrs.EditComment(r.ctx, reviewer, r.repo, mr.Number, approve.ID, "x")
1414+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1415+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, rootNote.ID, "")
1416+ assert.ErrorIs(t, err, domain.ErrInvalid)
1417+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, rootNote.ID)
1418+ require.NoError(t, err)
1419+ _, err = add(root, ports.AddCommentParams{ReplyTo: rootNote.ID, Text: "x"})
1420+ assert.ErrorIs(t, err, domain.ErrInvalid)
1421+ r.commit(author, "a.txt", "changed\n2\nthree\n", "advance")
1422+ r.push("feat")
1423+ detail, err := r.mrs.Get(r.ctx, root, r.repo, mr.Number, ports.GetMergeRequestOptions{})
1424+ require.NoError(t, err)
1425+ require.Len(t, detail.Verdicts, 2)
1426+ assert.True(t, detail.Verdicts[0].Stale)
1427+ assert.False(t, detail.Verdicts[1].Stale)
1428+ _, err = r.mrs.Close(r.ctx, root, r.repo, mr.Number)
1429+ require.NoError(t, err)
1430+ _, err = r.mrs.EditComment(r.ctx, root, r.repo, mr.Number, changes.ID, "x")
1431+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1432+ _, err = r.mrs.DeleteComment(r.ctx, root, r.repo, mr.Number, changes.ID)
1433+ assert.ErrorIs(t, err, domain.ErrPreconditionFailed)
1434+}
M internal/usecase/mergerequests_test.go +67 -0
@@ -117,3 +117,70 @@ func TestMergeReturnsTheFailureToReadTheSettings(t *testing.T) {
117117 })
118118 }
119119 }
120+
121+func TestNoteMutationRetries(t *testing.T) {
122+ for _, action := range []string{"edit", "delete", "resolve"} {
123+ t.Run(action, func(t *testing.T) {
124+ u := newMRUnit(t)
125+ u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil)
126+ mr := domain.MergeRequest{Repo: u.repo.Name, Number: 1, State: domain.MRStateOpen}
127+ text, err := domain.ParsePlainText("before")
128+ require.NoError(t, err)
129+ c := domain.Comment{ID: 1, Author: "root", Text: text}
130+ u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Twice()
131+ u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Twice()
132+ u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once()
133+ u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
134+ // The bus expectation must be exactly once despite the conflict.
135+ bus := mocks.NewEventBus(t)
136+ bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool {
137+ return e.EventKind() == "mr.comment_"+map[string]string{"edit": "edited", "delete": "deleted", "resolve": "resolved"}[action]
138+ })).Return().Once()
139+ clock := mocks.NewClock(t)
140+ clock.EXPECT().Now().Return(now)
141+ svc := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Bus: bus, Clock: clock})
142+ switch action {
143+ case "edit":
144+ _, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")
145+ case "delete":
146+ _, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)
147+ case "resolve":
148+ _, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)
149+ }
150+ require.NoError(t, err)
151+ })
152+ }
153+}
154+
155+func TestAddNoteValidation(t *testing.T) {
156+ for _, tc := range []struct {
157+ name string
158+ params ports.AddCommentParams
159+ target domain.Comment
160+ missing bool
161+ }{
162+ {name: "range without path", params: ports.AddCommentParams{StartLine: 1, Text: "x"}},
163+ {name: "equal ends", params: ports.AddCommentParams{Path: "a", StartLine: 3, Line: 3, Side: domain.SideNew, Text: "x"}},
164+ {name: "reversed ends", params: ports.AddCommentParams{Path: "a", StartLine: 4, Line: 3, Side: domain.SideNew, Text: "x"}},
165+ {name: "reply anchor", params: ports.AddCommentParams{ReplyTo: 1, Path: "a", Line: 1, Side: domain.SideNew, Text: "x"}},
166+ {name: "reply verdict", params: ports.AddCommentParams{ReplyTo: 1, Verdict: domain.VerdictApprove}},
167+ {name: "reply to reply", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, target: domain.Comment{ReplyTo: 2}},
168+ {name: "deleted root", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, target: domain.Comment{Deleted: true}},
169+ {name: "missing root", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, missing: true},
170+ } {
171+ t.Run(tc.name, func(t *testing.T) {
172+ u := newMRUnit(t)
173+ u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil)
174+ u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(domain.MergeRequest{State: domain.MRStateOpen}, nil)
175+ if tc.name == "reply to reply" || tc.name == "deleted root" || tc.missing {
176+ var err error
177+ if tc.missing {
178+ err = domain.ErrNotFound
179+ }
180+ u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(tc.target, err)
181+ }
182+ _, err := u.mrs.AddComment(context.Background(), domain.Root(), u.repo.Name, 1, tc.params)
183+ require.ErrorIs(t, err, domain.ErrInvalid)
184+ })
185+ }
186+}
@@ -117,3 +117,70 @@ func TestMergeReturnsTheFailureToReadTheSettings(t *testing.T) {
117 })117 })
118 }118 }
119 }119 }
120+
121+func TestNoteMutationRetries(t *testing.T) {
122+ for _, action := range []string{"edit", "delete", "resolve"} {
123+ t.Run(action, func(t *testing.T) {
124+ u := newMRUnit(t)
125+ u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil)
126+ mr := domain.MergeRequest{Repo: u.repo.Name, Number: 1, State: domain.MRStateOpen}
127+ text, err := domain.ParsePlainText("before")
128+ require.NoError(t, err)
129+ c := domain.Comment{ID: 1, Author: "root", Text: text}
130+ u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(mr, nil).Twice()
131+ u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(c, nil).Twice()
132+ u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrConflict).Once()
133+ u.store.EXPECT().SaveComment(mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
134+ // The bus expectation must be exactly once despite the conflict.
135+ bus := mocks.NewEventBus(t)
136+ bus.EXPECT().Publish(mock.Anything, mock.MatchedBy(func(e domain.Event) bool {
137+ return e.EventKind() == "mr.comment_"+map[string]string{"edit": "edited", "delete": "deleted", "resolve": "resolved"}[action]
138+ })).Return().Once()
139+ clock := mocks.NewClock(t)
140+ clock.EXPECT().Now().Return(now)
141+ svc := usecase.NewMergeRequests(usecase.MergeRequestsConfig{Meta: u.meta, Store: u.store, Bus: bus, Clock: clock})
142+ switch action {
143+ case "edit":
144+ _, err = svc.EditComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, "after")
145+ case "delete":
146+ _, err = svc.DeleteComment(context.Background(), domain.Root(), u.repo.Name, 1, 1)
147+ case "resolve":
148+ _, err = svc.ResolveComment(context.Background(), domain.Root(), u.repo.Name, 1, 1, true)
149+ }
150+ require.NoError(t, err)
151+ })
152+ }
153+}
154+
155+func TestAddNoteValidation(t *testing.T) {
156+ for _, tc := range []struct {
157+ name string
158+ params ports.AddCommentParams
159+ target domain.Comment
160+ missing bool
161+ }{
162+ {name: "range without path", params: ports.AddCommentParams{StartLine: 1, Text: "x"}},
163+ {name: "equal ends", params: ports.AddCommentParams{Path: "a", StartLine: 3, Line: 3, Side: domain.SideNew, Text: "x"}},
164+ {name: "reversed ends", params: ports.AddCommentParams{Path: "a", StartLine: 4, Line: 3, Side: domain.SideNew, Text: "x"}},
165+ {name: "reply anchor", params: ports.AddCommentParams{ReplyTo: 1, Path: "a", Line: 1, Side: domain.SideNew, Text: "x"}},
166+ {name: "reply verdict", params: ports.AddCommentParams{ReplyTo: 1, Verdict: domain.VerdictApprove}},
167+ {name: "reply to reply", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, target: domain.Comment{ReplyTo: 2}},
168+ {name: "deleted root", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, target: domain.Comment{Deleted: true}},
169+ {name: "missing root", params: ports.AddCommentParams{ReplyTo: 1, Text: "x"}, missing: true},
170+ } {
171+ t.Run(tc.name, func(t *testing.T) {
172+ u := newMRUnit(t)
173+ u.meta.EXPECT().Get(mock.Anything, u.repo.Name).Return(u.repo, nil)
174+ u.store.EXPECT().Get(mock.Anything, u.repo.Name, int64(1)).Return(domain.MergeRequest{State: domain.MRStateOpen}, nil)
175+ if tc.name == "reply to reply" || tc.name == "deleted root" || tc.missing {
176+ var err error
177+ if tc.missing {
178+ err = domain.ErrNotFound
179+ }
180+ u.store.EXPECT().GetComment(mock.Anything, u.repo.Name, int64(1), int64(1)).Return(tc.target, err)
181+ }
182+ _, err := u.mrs.AddComment(context.Background(), domain.Root(), u.repo.Name, 1, tc.params)
183+ require.ErrorIs(t, err, domain.ErrInvalid)
184+ })
185+ }
186+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 3

Tags 43

↑↓ moveenter openesc close