From 4ff27c2cbd45173be17ceb8139e81357a2d596ca Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:34:20 -0400 Subject: [PATCH 01/11] fix: Fixes issues w/ shared pointers to structs (#378) --- cmd/app/assignee_test.go | 6 ++-- cmd/app/attachment_test.go | 6 ++-- cmd/app/comment_test.go | 36 ++++++++++---------- cmd/app/create_mr_test.go | 10 +++--- cmd/app/draft_note_publisher_test.go | 8 ++--- cmd/app/draft_notes_test.go | 36 ++++++++++---------- cmd/app/job_test.go | 6 ++-- cmd/app/list_discussions_test.go | 10 +++--- cmd/app/merge_mr_test.go | 6 ++-- cmd/app/merge_requests_by_username_test.go | 12 +++---- cmd/app/merge_requests_test.go | 8 ++--- cmd/app/middleware.go | 17 +++++++--- cmd/app/middleware_test.go | 4 +-- cmd/app/reply_test.go | 6 ++-- cmd/app/resolve_discussion_test.go | 8 ++--- cmd/app/server.go | 38 +++++++++++----------- lua/gitlab/actions/comment.lua | 4 +++ 17 files changed, 117 insertions(+), 104 deletions(-) diff --git a/cmd/app/assignee_test.go b/cmd/app/assignee_test.go index 8cdde646..dca4f83f 100644 --- a/cmd/app/assignee_test.go +++ b/cmd/app/assignee_test.go @@ -27,7 +27,7 @@ func TestAssigneeHandler(t *testing.T) { svc := middleware( assigneesService{testProjectData, fakeAssigneeClient{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &AssigneeUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[AssigneeUpdateRequest]}), withMethodCheck(http.MethodPut), ) data := getSuccessData(t, svc, request) @@ -40,7 +40,7 @@ func TestAssigneeHandler(t *testing.T) { svc := middleware( assigneesService{testProjectData, client}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &AssigneeUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[AssigneeUpdateRequest]}), withMethodCheck(http.MethodPut), ) data, _ := getFailData(t, svc, request) @@ -53,7 +53,7 @@ func TestAssigneeHandler(t *testing.T) { svc := middleware( assigneesService{testProjectData, client}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &AssigneeUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[AssigneeUpdateRequest]}), withMethodCheck(http.MethodPut), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/attachment_test.go b/cmd/app/attachment_test.go index 38a1a0d5..bb2093ed 100644 --- a/cmd/app/attachment_test.go +++ b/cmd/app/attachment_test.go @@ -38,7 +38,7 @@ func TestAttachmentHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/attachment", attachmentTestRequestData) svc := middleware( attachmentService{testProjectData, fakeFileReader{}, fakeFileUploaderClient{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &AttachmentRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AttachmentRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -49,7 +49,7 @@ func TestAttachmentHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/attachment", attachmentTestRequestData) svc := middleware( attachmentService{testProjectData, fakeFileReader{}, fakeFileUploaderClient{testBase{errFromGitlab: true}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &AttachmentRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AttachmentRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -60,7 +60,7 @@ func TestAttachmentHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/attachment", attachmentTestRequestData) svc := middleware( attachmentService{testProjectData, fakeFileReader{}, fakeFileUploaderClient{testBase{status: http.StatusSeeOther}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &AttachmentRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AttachmentRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/comment_test.go b/cmd/app/comment_test.go index f86902f4..2e933502 100644 --- a/cmd/app/comment_test.go +++ b/cmd/app/comment_test.go @@ -44,9 +44,9 @@ func TestPostComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) @@ -66,9 +66,9 @@ func TestPostComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) @@ -82,9 +82,9 @@ func TestPostComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) @@ -98,9 +98,9 @@ func TestPostComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{testBase{status: http.StatusSeeOther}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) @@ -117,9 +117,9 @@ func TestDeleteComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) @@ -136,9 +136,9 @@ func TestEditComment(t *testing.T) { commentService{testProjectData, fakeCommentClient{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), ) diff --git a/cmd/app/create_mr_test.go b/cmd/app/create_mr_test.go index e10bd7a7..f3c39155 100644 --- a/cmd/app/create_mr_test.go +++ b/cmd/app/create_mr_test.go @@ -31,7 +31,7 @@ func TestCreateMr(t *testing.T) { request := makeRequest(t, http.MethodPost, "/create_mr", testCreateMrRequestData) svc := middleware( mergeRequestCreatorService{testProjectData, fakeMergeCreatorClient{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -42,7 +42,7 @@ func TestCreateMr(t *testing.T) { request := makeRequest(t, http.MethodPost, "/create_mr", testCreateMrRequestData) svc := middleware( mergeRequestCreatorService{testProjectData, fakeMergeCreatorClient{testBase{errFromGitlab: true}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -53,7 +53,7 @@ func TestCreateMr(t *testing.T) { request := makeRequest(t, http.MethodPost, "/create_mr", testCreateMrRequestData) svc := middleware( mergeRequestCreatorService{testProjectData, fakeMergeCreatorClient{testBase{status: http.StatusSeeOther}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -66,7 +66,7 @@ func TestCreateMr(t *testing.T) { request := makeRequest(t, http.MethodPost, "/create_mr", reqData) svc := middleware( mergeRequestCreatorService{testProjectData, fakeMergeCreatorClient{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -80,7 +80,7 @@ func TestCreateMr(t *testing.T) { request := makeRequest(t, http.MethodPost, "/create_mr", reqData) svc := middleware( mergeRequestCreatorService{testProjectData, fakeMergeCreatorClient{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/draft_note_publisher_test.go b/cmd/app/draft_note_publisher_test.go index 36e3e4f0..0ebfa554 100644 --- a/cmd/app/draft_note_publisher_test.go +++ b/cmd/app/draft_note_publisher_test.go @@ -25,7 +25,7 @@ func TestPublishDraftNote(t *testing.T) { svc := middleware( draftNotePublisherService{testProjectData, fakeDraftNotePublisher{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DraftNotePublishRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DraftNotePublishRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -36,7 +36,7 @@ func TestPublishDraftNote(t *testing.T) { svc := middleware( draftNotePublisherService{testProjectData, fakeDraftNotePublisher{testBase: testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DraftNotePublishRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DraftNotePublishRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -51,7 +51,7 @@ func TestPublishAllDraftNotes(t *testing.T) { svc := middleware( draftNotePublisherService{testProjectData, fakeDraftNotePublisher{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DraftNotePublishRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DraftNotePublishRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -62,7 +62,7 @@ func TestPublishAllDraftNotes(t *testing.T) { svc := middleware( draftNotePublisherService{testProjectData, fakeDraftNotePublisher{testBase: testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DraftNotePublishRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DraftNotePublishRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/draft_notes_test.go b/cmd/app/draft_notes_test.go index 3702a69e..8fe38da1 100644 --- a/cmd/app/draft_notes_test.go +++ b/cmd/app/draft_notes_test.go @@ -46,8 +46,8 @@ func TestListDraftNotes(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -61,8 +61,8 @@ func TestListDraftNotes(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{testBase: testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -75,8 +75,8 @@ func TestListDraftNotes(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{testBase: testBase{status: http.StatusSeeOther}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -96,8 +96,8 @@ func TestPostDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -113,8 +113,8 @@ func TestDeleteDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -127,8 +127,8 @@ func TestDeleteDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -146,8 +146,8 @@ func TestEditDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -160,8 +160,8 @@ func TestEditDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) @@ -177,8 +177,8 @@ func TestEditDraftNote(t *testing.T) { draftNoteService{testProjectData, fakeDraftNoteManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), ) diff --git a/cmd/app/job_test.go b/cmd/app/job_test.go index f9465484..34ca0db8 100644 --- a/cmd/app/job_test.go +++ b/cmd/app/job_test.go @@ -40,7 +40,7 @@ func TestJobHandler(t *testing.T) { request := makeRequest(t, http.MethodGet, "/job", JobTraceRequest{JobId: 3}) svc := middleware( traceFileService{testProjectData, fakeTraceFileGetter{}}, - withPayloadValidation(methodToPayload{http.MethodGet: &JobTraceRequest{}}), + withPayloadValidation(methodToPayload{http.MethodGet: newPayload[JobTraceRequest]}), withMethodCheck(http.MethodGet), ) data := getTraceFileData(t, svc, request) @@ -51,7 +51,7 @@ func TestJobHandler(t *testing.T) { request := makeRequest(t, http.MethodGet, "/job", JobTraceRequest{JobId: 2}) svc := middleware( traceFileService{testProjectData, fakeTraceFileGetter{testBase{errFromGitlab: true}}}, - withPayloadValidation(methodToPayload{http.MethodGet: &JobTraceRequest{}}), + withPayloadValidation(methodToPayload{http.MethodGet: newPayload[JobTraceRequest]}), withMethodCheck(http.MethodGet), ) data, _ := getFailData(t, svc, request) @@ -62,7 +62,7 @@ func TestJobHandler(t *testing.T) { request := makeRequest(t, http.MethodGet, "/job", JobTraceRequest{JobId: 1}) svc := middleware( traceFileService{testProjectData, fakeTraceFileGetter{testBase{status: http.StatusSeeOther}}}, - withPayloadValidation(methodToPayload{http.MethodGet: &JobTraceRequest{}}), + withPayloadValidation(methodToPayload{http.MethodGet: newPayload[JobTraceRequest]}), withMethodCheck(http.MethodGet), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/list_discussions_test.go b/cmd/app/list_discussions_test.go index 25d284c2..c0366f81 100644 --- a/cmd/app/list_discussions_test.go +++ b/cmd/app/list_discussions_test.go @@ -71,7 +71,7 @@ func TestListDiscussions(t *testing.T) { svc := middleware( discussionsListerService{testProjectData, fakeDiscussionsLister{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), ) data := getDiscussionsList(t, svc, request) @@ -85,7 +85,7 @@ func TestListDiscussions(t *testing.T) { svc := middleware( discussionsListerService{testProjectData, fakeDiscussionsLister{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), ) data := getDiscussionsList(t, svc, request) @@ -98,7 +98,7 @@ func TestListDiscussions(t *testing.T) { svc := middleware( discussionsListerService{testProjectData, fakeDiscussionsLister{testBase: testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -109,7 +109,7 @@ func TestListDiscussions(t *testing.T) { svc := middleware( discussionsListerService{testProjectData, fakeDiscussionsLister{testBase: testBase{status: http.StatusSeeOther}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -120,7 +120,7 @@ func TestListDiscussions(t *testing.T) { svc := middleware( discussionsListerService{testProjectData, fakeDiscussionsLister{badEmojiResponse: true, testBase: testBase{}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/merge_mr_test.go b/cmd/app/merge_mr_test.go index 9c48cf41..332f94db 100644 --- a/cmd/app/merge_mr_test.go +++ b/cmd/app/merge_mr_test.go @@ -28,7 +28,7 @@ func TestAcceptAndMergeHandler(t *testing.T) { mergeRequestAccepterService{testProjectData, fakeMergeRequestAccepter{}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &AcceptMergeRequestRequest{}, + http.MethodPost: newPayload[AcceptMergeRequestRequest], }), withMethodCheck(http.MethodPost), ) @@ -41,7 +41,7 @@ func TestAcceptAndMergeHandler(t *testing.T) { mergeRequestAccepterService{testProjectData, fakeMergeRequestAccepter{testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &AcceptMergeRequestRequest{}, + http.MethodPost: newPayload[AcceptMergeRequestRequest], }), withMethodCheck(http.MethodPost), ) @@ -54,7 +54,7 @@ func TestAcceptAndMergeHandler(t *testing.T) { mergeRequestAccepterService{testProjectData, fakeMergeRequestAccepter{testBase{status: http.StatusSeeOther}}}, withMr(testProjectData, fakeMergeRequestLister{}), withPayloadValidation(methodToPayload{ - http.MethodPost: &AcceptMergeRequestRequest{}, + http.MethodPost: newPayload[AcceptMergeRequestRequest], }), withMethodCheck(http.MethodPost), ) diff --git a/cmd/app/merge_requests_by_username_test.go b/cmd/app/merge_requests_by_username_test.go index a6c2d010..8518e3c3 100644 --- a/cmd/app/merge_requests_by_username_test.go +++ b/cmd/app/merge_requests_by_username_test.go @@ -32,7 +32,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", testListMrsByUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -43,7 +43,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", testListMrsByUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{emptyResponse: true}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -58,7 +58,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", missingUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -73,7 +73,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", missingUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -86,7 +86,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", testListMrsByUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{testBase: testBase{errFromGitlab: true}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -99,7 +99,7 @@ func TestListMergeRequestByUsername(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests_by_username", testListMrsByUsernamePayload) svc := middleware( mergeRequestListerByUsernameService{testProjectData, fakeMergeRequestListerByUsername{testBase: testBase{status: http.StatusSeeOther}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) diff --git a/cmd/app/merge_requests_test.go b/cmd/app/merge_requests_test.go index 5f8cd1c3..29762cf4 100644 --- a/cmd/app/merge_requests_test.go +++ b/cmd/app/merge_requests_test.go @@ -36,7 +36,7 @@ func TestMergeRequestHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest) svc := middleware( mergeRequestListerService{testProjectData, fakeMergeRequestLister{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &gitlab.ListProjectMergeRequestsOptions{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -46,7 +46,7 @@ func TestMergeRequestHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest) svc := middleware( mergeRequestListerService{testProjectData, fakeMergeRequestLister{testBase: testBase{errFromGitlab: true}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &gitlab.ListProjectMergeRequestsOptions{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -57,7 +57,7 @@ func TestMergeRequestHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest) svc := middleware( mergeRequestListerService{testProjectData, fakeMergeRequestLister{testBase: testBase{status: http.StatusSeeOther}}}, - withPayloadValidation(methodToPayload{http.MethodPost: &gitlab.ListProjectMergeRequestsOptions{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) @@ -68,7 +68,7 @@ func TestMergeRequestHandler(t *testing.T) { request := makeRequest(t, http.MethodPost, "/merge_requests", testListMergeRequestsRequest) svc := middleware( mergeRequestListerService{testProjectData, fakeMergeRequestLister{emptyResponse: true}}, - withPayloadValidation(methodToPayload{http.MethodPost: &gitlab.ListProjectMergeRequestsOptions{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}), withMethodCheck(http.MethodPost), ) data, status := getFailData(t, svc, request) diff --git a/cmd/app/middleware.go b/cmd/app/middleware.go index ae991911..9af69679 100644 --- a/cmd/app/middleware.go +++ b/cmd/app/middleware.go @@ -28,7 +28,13 @@ func middleware(h http.Handler, middlewares ...mw) http.HandlerFunc { var validate = validator.New() -type methodToPayload map[string]any +type methodToPayload map[string]func() any + +// Generic factory function to create new payload instances per request +func newPayload[T any]() any { + var p T + return &p +} type validatorMiddleware struct { validate *validator.Validate @@ -40,7 +46,8 @@ type validatorMiddleware struct { func (p validatorMiddleware) handle(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if p.methodToPayload[r.Method] == nil { // If no payload to validate for this method type... + constructor, exists := p.methodToPayload[r.Method] + if !exists { // If no payload to validate for this method type... next.ServeHTTP(w, r) return } @@ -51,7 +58,9 @@ func (p validatorMiddleware) handle(next http.Handler) http.Handler { return } - pl := p.methodToPayload[r.Method] + // Create a new instance for this request + pl := constructor() + err = json.Unmarshal(body, &pl) if err != nil { @@ -72,7 +81,7 @@ func (p validatorMiddleware) handle(next http.Handler) http.Handler { } // Pass the parsed data so we don't have to re-parse it in the handler - ctx := context.WithValue(r.Context(), payload(payload("payload")), pl) + ctx := context.WithValue(r.Context(), payload("payload"), pl) r = r.WithContext(ctx) next.ServeHTTP(w, r) diff --git a/cmd/app/middleware_test.go b/cmd/app/middleware_test.go index 6e9afdfb..0402f9d4 100644 --- a/cmd/app/middleware_test.go +++ b/cmd/app/middleware_test.go @@ -97,7 +97,7 @@ func TestValidatorMiddleware(t *testing.T) { request := makeRequest(t, http.MethodPost, "/foo", FakePayload{}) // No Foo field data, status := getFailData(t, middleware( fakeHandler{}, - withPayloadValidation(methodToPayload{http.MethodPost: &FakePayload{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[FakePayload]}), ), request) assert(t, data.Message, "Invalid payload") assert(t, data.Details, "Foo is required") @@ -107,7 +107,7 @@ func TestValidatorMiddleware(t *testing.T) { request := makeRequest(t, http.MethodPost, "/foo", FakePayload{Foo: "Some payload"}) data := getSuccessData(t, middleware( fakeHandler{}, - withPayloadValidation(methodToPayload{http.MethodPost: &FakePayload{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[FakePayload]}), ), request) assert(t, data.Message, "Some message") }) diff --git a/cmd/app/reply_test.go b/cmd/app/reply_test.go index 27d4416e..4234711c 100644 --- a/cmd/app/reply_test.go +++ b/cmd/app/reply_test.go @@ -27,7 +27,7 @@ func TestReplyHandler(t *testing.T) { svc := middleware( replyService{testProjectData, fakeReplyManager{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &ReplyRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[ReplyRequest]}), withMethodCheck(http.MethodPost), ) data := getSuccessData(t, svc, request) @@ -38,7 +38,7 @@ func TestReplyHandler(t *testing.T) { svc := middleware( replyService{testProjectData, fakeReplyManager{testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &ReplyRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[ReplyRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) @@ -50,7 +50,7 @@ func TestReplyHandler(t *testing.T) { svc := middleware( replyService{testProjectData, fakeReplyManager{testBase{status: http.StatusSeeOther}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPost: &ReplyRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[ReplyRequest]}), withMethodCheck(http.MethodPost), ) data, _ := getFailData(t, svc, request) diff --git a/cmd/app/resolve_discussion_test.go b/cmd/app/resolve_discussion_test.go index 18918e1a..26332dae 100644 --- a/cmd/app/resolve_discussion_test.go +++ b/cmd/app/resolve_discussion_test.go @@ -30,7 +30,7 @@ func TestResolveDiscussion(t *testing.T) { svc := middleware( discussionsResolutionService{testProjectData, fakeDiscussionResolver{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &DiscussionResolveRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[DiscussionResolveRequest]}), withMethodCheck(http.MethodPut), ) request := makeRequest(t, http.MethodPut, "/mr/discussions/resolve", testResolveMergeRequestPayload) @@ -44,7 +44,7 @@ func TestResolveDiscussion(t *testing.T) { svc := middleware( discussionsResolutionService{testProjectData, fakeDiscussionResolver{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &DiscussionResolveRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[DiscussionResolveRequest]}), withMethodCheck(http.MethodPut), ) request := makeRequest(t, http.MethodPut, "/mr/discussions/resolve", payload) @@ -58,7 +58,7 @@ func TestResolveDiscussion(t *testing.T) { svc := middleware( discussionsResolutionService{testProjectData, fakeDiscussionResolver{}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &DiscussionResolveRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[DiscussionResolveRequest]}), withMethodCheck(http.MethodPut), ) request := makeRequest(t, http.MethodPut, "/mr/discussions/resolve", payload) @@ -72,7 +72,7 @@ func TestResolveDiscussion(t *testing.T) { svc := middleware( discussionsResolutionService{testProjectData, fakeDiscussionResolver{testBase: testBase{errFromGitlab: true}}}, withMr(testProjectData, fakeMergeRequestLister{}), - withPayloadValidation(methodToPayload{http.MethodPut: &DiscussionResolveRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[DiscussionResolveRequest]}), withMethodCheck(http.MethodPut), ) request := makeRequest(t, http.MethodPut, "/mr/discussions/resolve", testResolveMergeRequestPayload) diff --git a/cmd/app/server.go b/cmd/app/server.go index 342eb947..ca7ac8aa 100644 --- a/cmd/app/server.go +++ b/cmd/app/server.go @@ -99,28 +99,28 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer commentService{d, gitlabClient}, withMr(d, gitlabClient), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostCommentRequest{}, - http.MethodDelete: &DeleteCommentRequest{}, - http.MethodPatch: &EditCommentRequest{}, + http.MethodPost: newPayload[PostCommentRequest], + http.MethodDelete: newPayload[DeleteCommentRequest], + http.MethodPatch: newPayload[EditCommentRequest], }), withMethodCheck(http.MethodPost, http.MethodDelete, http.MethodPatch), )) m.HandleFunc("/mr/merge", middleware( mergeRequestAccepterService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPost: &AcceptMergeRequestRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AcceptMergeRequestRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/mr/discussions/list", middleware( discussionsListerService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPost: &DiscussionsRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DiscussionsRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/mr/discussions/resolve", middleware( discussionsResolutionService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPut: &DiscussionResolveRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[DiscussionResolveRequest]}), withMethodCheck(http.MethodPut), )) m.HandleFunc("/mr/info", middleware( @@ -131,19 +131,19 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer m.HandleFunc("/mr/assignee", middleware( assigneesService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPut: &AssigneeUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[AssigneeUpdateRequest]}), withMethodCheck(http.MethodPut), )) m.HandleFunc("/mr/summary", middleware( summaryService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPut: &SummaryUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[SummaryUpdateRequest]}), withMethodCheck(http.MethodPut), )) m.HandleFunc("/mr/reviewer", middleware( reviewerService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPut: &ReviewerUpdateRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPut: newPayload[ReviewerUpdateRequest]}), withMethodCheck(http.MethodPut), )) m.HandleFunc("/mr/revisions", middleware( @@ -154,7 +154,7 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer m.HandleFunc("/mr/reply", middleware( replyService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPost: &ReplyRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[ReplyRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/mr/label", middleware( @@ -175,15 +175,15 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer draftNoteService{d, gitlabClient}, withMr(d, gitlabClient), withPayloadValidation(methodToPayload{ - http.MethodPost: &PostDraftNoteRequest{}, - http.MethodPatch: &UpdateDraftNoteRequest{}, + http.MethodPost: newPayload[PostDraftNoteRequest], + http.MethodPatch: newPayload[UpdateDraftNoteRequest], }), withMethodCheck(http.MethodGet, http.MethodPost, http.MethodPatch, http.MethodDelete), )) m.HandleFunc("/mr/draft_notes/publish", middleware( draftNotePublisherService{d, gitlabClient}, withMr(d, gitlabClient), - withPayloadValidation(methodToPayload{http.MethodPost: &DraftNotePublishRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[DraftNotePublishRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/pipeline", middleware( @@ -200,17 +200,17 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer )) m.HandleFunc("/attachment", middleware( attachmentService{data: d, client: gitlabClient, fileReader: attachmentReader{}}, - withPayloadValidation(methodToPayload{http.MethodPost: &AttachmentRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[AttachmentRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/create_mr", middleware( mergeRequestCreatorService{d, gitlabClient}, - withPayloadValidation(methodToPayload{http.MethodPost: &CreateMrRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[CreateMrRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/job", middleware( traceFileService{d, gitlabClient}, - withPayloadValidation(methodToPayload{http.MethodGet: &JobTraceRequest{}}), + withPayloadValidation(methodToPayload{http.MethodGet: newPayload[JobTraceRequest]}), withMethodCheck(http.MethodGet), )) m.HandleFunc("/project/members", middleware( @@ -219,17 +219,17 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer )) m.HandleFunc("/merge_requests", middleware( mergeRequestListerService{d, gitlabClient}, - withPayloadValidation(methodToPayload{http.MethodPost: &gitlab.ListProjectMergeRequestsOptions{}}), // TODO: How to validate external object + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[gitlab.ListProjectMergeRequestsOptions]}), // TODO: How to validate external object withMethodCheck(http.MethodPost), )) m.HandleFunc("/merge_requests_by_username", middleware( mergeRequestListerByUsernameService{d, gitlabClient}, - withPayloadValidation(methodToPayload{http.MethodPost: &MergeRequestByUsernameRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[MergeRequestByUsernameRequest]}), withMethodCheck(http.MethodPost), )) m.HandleFunc("/shutdown", middleware( *s, - withPayloadValidation(methodToPayload{http.MethodPost: &ShutdownRequest{}}), + withPayloadValidation(methodToPayload{http.MethodPost: newPayload[ShutdownRequest]}), withMethodCheck(http.MethodPost), )) diff --git a/lua/gitlab/actions/comment.lua b/lua/gitlab/actions/comment.lua index 95abe305..a2f3d1fd 100644 --- a/lua/gitlab/actions/comment.lua +++ b/lua/gitlab/actions/comment.lua @@ -69,6 +69,8 @@ local confirm_create_comment = function(text, visual_range, unlinked, discussion return end + vim.print("Here: ", unlinked, discussion_id) + local reviewer_data = reviewer.get_reviewer_data() if reviewer_data == nil then u.notify("Error getting reviewer data", vim.log.levels.ERROR) @@ -255,6 +257,7 @@ end --- This function will open a comment popup in order to create a comment on the changed/updated --- line in the current MR M.create_comment = function() + vim.print("Creating comment...") local has_clean_tree, err = git.has_clean_tree() if err ~= nil then return @@ -298,6 +301,7 @@ end --- This function will open a a popup to create a "note" (e.g. unlinked comment) --- on the changed/updated line in the current MR M.create_note = function() + vim.print("Creating note...") local layout = M.create_comment_layout({ ranged = false, unlinked = true }) if layout ~= nil then layout:mount() From 87e224a1f43ff8fdbb86a70a4b059d6f651d3dfe Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:34:48 -0400 Subject: [PATCH 02/11] feat: adds even better debugging and linting support (#376) --- .github/CONTRIBUTING.md | 5 +++-- .github/workflows/go.yaml | 8 ++++++-- .golangci.yml | 3 +++ cmd/app/assignee.go | 2 +- cmd/app/client.go | 22 +++++++++++----------- cmd/app/draft_notes.go | 2 +- cmd/app/emoji.go | 11 ++++++++--- cmd/app/git/git.go | 16 ++++++++-------- cmd/app/list_discussions.go | 2 +- cmd/app/logging.go | 6 +++--- cmd/app/merge_requests.go | 4 ++-- cmd/app/merge_requests_by_username_test.go | 2 +- cmd/app/middleware.go | 12 ++++++------ cmd/app/middleware_test.go | 4 ++-- cmd/app/pipeline.go | 2 +- cmd/app/resolve_discussion_test.go | 2 +- cmd/app/server.go | 12 +++++++++--- cmd/app/test_helpers.go | 2 +- cmd/main.go | 8 ++++++-- 19 files changed, 74 insertions(+), 51 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 0a07071c..98d3c074 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -31,11 +31,12 @@ If you are making changes to the Go codebase, don't forget to run `make compile` 3. Apply formatters and linters to your changes -For changes to the Go codebase: We use gofmt to check formatting and golangci-lint to check linting. Run these commands in the root of the repository: +For changes to the Go codebase: We use gofmt to check formatting and golangci-lint to check linting, and staticcheck. Run these commands in the root of the repository: ```bash $ go fmt ./... -$ golangci-lint run +$ golangci-lint run ./... +$ staticcheck ./... ``` If you are writing tests and have added something to the Go client, you can test with: diff --git a/.github/workflows/go.yaml b/.github/workflows/go.yaml index 01e1dbee..2786b74b 100644 --- a/.github/workflows/go.yaml +++ b/.github/workflows/go.yaml @@ -12,14 +12,18 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-go@v4 with: - go-version: '1.19' + go-version: '1.23.1' cache: false - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.54 + version: v1.61.0 only-new-issues: true skip-cache: true + - name: Install staticcheck + run: go install honnef.co/go/tools/cmd/staticcheck@2024.1.1 + - name: Run staticcheck + run: staticcheck ./... go_test: name: Test Go 🧪 needs: [go_lint] diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..2ba75e1f --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,3 @@ +run: + tests: true + timeout: 30s diff --git a/cmd/app/assignee.go b/cmd/app/assignee.go index b7be1670..17fb1b66 100644 --- a/cmd/app/assignee.go +++ b/cmd/app/assignee.go @@ -28,7 +28,7 @@ func (a assigneesService) ServeHTTP(w http.ResponseWriter, r *http.Request) { assigneeUpdateRequest, ok := r.Context().Value(payload("payload")).(*AssigneeUpdateRequest) if !ok { - handleError(w, errors.New("Could not get payload from context"), "Bad payload", http.StatusInternalServerError) + handleError(w, errors.New("could not get payload from context"), "Bad payload", http.StatusInternalServerError) return } diff --git a/cmd/app/client.go b/cmd/app/client.go index 756371f7..4d3da2ed 100644 --- a/cmd/app/client.go +++ b/cmd/app/client.go @@ -33,13 +33,13 @@ type Client struct { } /* NewClient parses and validates the project settings and initializes the Gitlab client. */ -func NewClient() (error, *Client) { +func NewClient() (*Client, error) { if pluginOptions.GitlabUrl == "" { - return errors.New("GitLab instance URL cannot be empty"), nil + return nil, errors.New("GitLab instance URL cannot be empty") } - var apiCustUrl = fmt.Sprintf(pluginOptions.GitlabUrl + "/api/v4") + var apiCustUrl = fmt.Sprintf("%s/api/v4", pluginOptions.GitlabUrl) gitlabOptions := []gitlab.ClientOptionFunc{ gitlab.WithBaseURL(apiCustUrl), @@ -73,10 +73,10 @@ func NewClient() (error, *Client) { client, err := gitlab.NewClient(pluginOptions.AuthToken, gitlabOptions...) if err != nil { - return fmt.Errorf("Failed to create client: %v", err), nil + return nil, fmt.Errorf("failed to create client: %v", err) } - return nil, &Client{ + return &Client{ MergeRequestsService: client.MergeRequests, MergeRequestApprovalsService: client.MergeRequestApprovals, DiscussionsService: client.Discussions, @@ -88,28 +88,28 @@ func NewClient() (error, *Client) { AwardEmojiService: client.AwardEmoji, UsersService: client.Users, DraftNotesService: client.DraftNotes, - } + }, nil } /* InitProjectSettings fetch the project ID using the client */ -func InitProjectSettings(c *Client, gitInfo git.GitData) (error, *ProjectInfo) { +func InitProjectSettings(c *Client, gitInfo git.GitData) (*ProjectInfo, error) { opt := gitlab.GetProjectOptions{} project, _, err := c.GetProject(gitInfo.ProjectPath(), &opt) if err != nil { - return fmt.Errorf(fmt.Sprintf("Error getting project at %s", gitInfo.RemoteUrl), err), nil + return nil, fmt.Errorf(fmt.Sprintf("Error getting project at %s", gitInfo.RemoteUrl), err) } if project == nil { - return fmt.Errorf(fmt.Sprintf("Could not find project at %s", gitInfo.RemoteUrl), err), nil + return nil, fmt.Errorf(fmt.Sprintf("Could not find project at %s", gitInfo.RemoteUrl), err) } projectId := fmt.Sprint(project.ID) - return nil, &ProjectInfo{ + return &ProjectInfo{ ProjectId: projectId, - } + }, nil } /* handleError is a utililty handler that returns errors to the client along with their statuses and messages */ diff --git a/cmd/app/draft_notes.go b/cmd/app/draft_notes.go index 6c248115..42daaa39 100644 --- a/cmd/app/draft_notes.go +++ b/cmd/app/draft_notes.go @@ -183,7 +183,7 @@ func (a draftNoteService) updateDraftNote(w http.ResponseWriter, r *http.Request payload := r.Context().Value(payload("payload")).(*UpdateDraftNoteRequest) if payload.Note == "" { - handleError(w, errors.New("Draft note text missing"), "Must provide draft note text", http.StatusBadRequest) + handleError(w, errors.New("draft note text missing"), "Must provide draft note text", http.StatusBadRequest) return } diff --git a/cmd/app/emoji.go b/cmd/app/emoji.go index 4a535038..2dbfe6fb 100644 --- a/cmd/app/emoji.go +++ b/cmd/app/emoji.go @@ -64,6 +64,11 @@ func (a emojiService) deleteEmojiFromNote(w http.ResponseWriter, r *http.Request suffix := strings.TrimPrefix(r.URL.Path, "/mr/awardable/note/") ids := strings.Split(suffix, "/") + if len(ids) < 2 { + handleError(w, errors.New("missing IDs"), "Must provide note ID and awardable ID", http.StatusBadRequest) + return + } + noteId, err := strconv.Atoi(ids[0]) if err != nil { handleError(w, err, "Could not convert note ID to integer", http.StatusBadRequest) @@ -158,18 +163,18 @@ func attachEmojis(a *data, fr FileReader) error { reader, err := fr.ReadFile(filePath) if err != nil { - return fmt.Errorf("Could not find emojis at %s", filePath) + return fmt.Errorf("could not find emojis at %s", filePath) } bytes, err := io.ReadAll(reader) if err != nil { - return errors.New("Could not read emoji file") + return errors.New("could not read emoji file") } var emojiMap EmojiMap err = json.Unmarshal(bytes, &emojiMap) if err != nil { - return errors.New("Could not unmarshal emojis") + return errors.New("could not unmarshal emojis") } a.emojiMap = emojiMap diff --git a/cmd/app/git/git.go b/cmd/app/git/git.go index 7307430b..b4e36f87 100644 --- a/cmd/app/git/git.go +++ b/cmd/app/git/git.go @@ -39,12 +39,12 @@ Gitlab project and the branch must be a feature branch func NewGitData(remote string, g GitManager) (GitData, error) { err := g.RefreshProjectInfo(remote) if err != nil { - return GitData{}, fmt.Errorf("Could not get latest information from remote: %v", err) + return GitData{}, fmt.Errorf("could not get latest information from remote: %v", err) } url, err := g.GetProjectUrlFromNativeGitCmd(remote) if err != nil { - return GitData{}, fmt.Errorf("Could not get project Url: %v", err) + return GitData{}, fmt.Errorf("could not get project Url: %v", err) } /* @@ -62,7 +62,7 @@ func NewGitData(remote string, g GitManager) (GitData, error) { re := regexp.MustCompile(`^(?:git@[^\/:]*|https?:\/\/[^\/]+|ssh:\/\/[^\/:]+)(?::\d+)?[\/:](.*)\/([^\/]+?)(?:\.git)?\/?$`) matches := re.FindStringSubmatch(url) if len(matches) != 3 { - return GitData{}, fmt.Errorf("Invalid Git URL format: %s", url) + return GitData{}, fmt.Errorf("invalid git URL format: %s", url) } namespace := matches[1] @@ -70,7 +70,7 @@ func NewGitData(remote string, g GitManager) (GitData, error) { branchName, err := g.GetCurrentBranchNameFromNativeGitCmd() if err != nil { - return GitData{}, fmt.Errorf("Failed to get current branch: %v", err) + return GitData{}, fmt.Errorf("failed to get current branch: %v", err) } return GitData{ @@ -88,7 +88,7 @@ func (g Git) GetCurrentBranchNameFromNativeGitCmd() (res string, e error) { output, err := gitCmd.Output() if err != nil { - return "", fmt.Errorf("Error running git rev-parse: %w", err) + return "", fmt.Errorf("error running git rev-parse: %w", err) } branchName := strings.TrimSpace(string(output)) @@ -101,7 +101,7 @@ func (g Git) GetProjectUrlFromNativeGitCmd(remote string) (string, error) { cmd := exec.Command("git", "remote", "get-url", remote) url, err := cmd.Output() if err != nil { - return "", fmt.Errorf("Could not get remote") + return "", fmt.Errorf("could not get remote") } return strings.TrimSpace(string(url)), nil @@ -112,7 +112,7 @@ func (g Git) RefreshProjectInfo(remote string) error { cmd := exec.Command("git", "fetch", remote) _, err := cmd.Output() if err != nil { - return fmt.Errorf("Failed to run `git fetch %s`: %v", remote, err) + return fmt.Errorf("failed to run `git fetch %s`: %v", remote, err) } return nil @@ -123,7 +123,7 @@ func (g Git) GetLatestCommitOnRemote(remote string, branchName string) (string, out, err := cmd.Output() if err != nil { - return "", fmt.Errorf("Failed to run `git log -1 --format=%%H " + fmt.Sprintf("%s/%s", remote, branchName)) + return "", fmt.Errorf("failed to run `git log -1 --format=%%H %s/%s`", remote, branchName) } commit := strings.TrimSpace(string(out)) diff --git a/cmd/app/list_discussions.go b/cmd/app/list_discussions.go index a75134ce..555a151e 100644 --- a/cmd/app/list_discussions.go +++ b/cmd/app/list_discussions.go @@ -87,7 +87,7 @@ func (a discussionsListerService) ServeHTTP(w http.ResponseWriter, r *http.Reque var linkedDiscussions []*gitlab.Discussion for _, discussion := range discussions { - if discussion.Notes == nil || len(discussion.Notes) == 0 || Contains(request.Blacklist, discussion.Notes[0].Author.Username) { + if len(discussion.Notes) == 0 || Contains(request.Blacklist, discussion.Notes[0].Author.Username) { continue } for _, note := range discussion.Notes { diff --git a/cmd/app/logging.go b/cmd/app/logging.go index e7fda5d9..d85820db 100644 --- a/cmd/app/logging.go +++ b/cmd/app/logging.go @@ -47,7 +47,7 @@ func (l LoggingServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { Request: r, } if pluginOptions.Debug.Response { - logResponse("RESPONSE FROM GO SERVER", resp) + logResponse("RESPONSE FROM GO SERVER", resp) //nolint:errcheck } } @@ -62,7 +62,7 @@ func logRequest(prefix string, r *http.Request) { os.Exit(1) } r.Header.Set("Private-Token", token) - _, err = file.Write([]byte(fmt.Sprintf("\n-- %s --\n%s\n", prefix, res))) //nolint:all + fmt.Fprintf(file, "\n-- %s --\n%s\n", prefix, res) //nolint:errcheck } func logResponse(prefix string, r *http.Response) { @@ -75,7 +75,7 @@ func logResponse(prefix string, r *http.Response) { os.Exit(1) } - _, err = file.Write([]byte(fmt.Sprintf("\n-- %s --\n%s\n", prefix, res))) //nolint:all + fmt.Fprintf(file, "\n-- %s --\n%s\n", prefix, res) //nolint:errcheck } func openLogFile() *os.File { diff --git a/cmd/app/merge_requests.go b/cmd/app/merge_requests.go index 4fd58b6d..7f1510e7 100644 --- a/cmd/app/merge_requests.go +++ b/cmd/app/merge_requests.go @@ -48,7 +48,7 @@ func (a mergeRequestListerService) ServeHTTP(w http.ResponseWriter, r *http.Requ } if len(mergeRequests) == 0 { - handleError(w, errors.New("No merge requests found"), "No merge requests found", http.StatusNotFound) + handleError(w, errors.New("no merge requests found"), "No merge requests found", http.StatusNotFound) return } @@ -60,6 +60,6 @@ func (a mergeRequestListerService) ServeHTTP(w http.ResponseWriter, r *http.Requ err = json.NewEncoder(w).Encode(response) if err != nil { - handleError(w, err, "Could not encode response", http.StatusInternalServerError) + handleError(w, err, "could not encode response", http.StatusInternalServerError) } } diff --git a/cmd/app/merge_requests_by_username_test.go b/cmd/app/merge_requests_by_username_test.go index 8518e3c3..fb3aa847 100644 --- a/cmd/app/merge_requests_by_username_test.go +++ b/cmd/app/merge_requests_by_username_test.go @@ -91,7 +91,7 @@ func TestListMergeRequestByUsername(t *testing.T) { ) data, status := getFailData(t, svc, request) assert(t, data.Message, "An error occurred") - assert(t, data.Details, strings.Repeat("Some error from Gitlab; ", 3)) + assert(t, data.Details, strings.Repeat("some error from Gitlab; ", 3)) assert(t, status, http.StatusInternalServerError) }) diff --git a/cmd/app/middleware.go b/cmd/app/middleware.go index 9af69679..5408626b 100644 --- a/cmd/app/middleware.go +++ b/cmd/app/middleware.go @@ -110,18 +110,18 @@ func (m withMrMiddleware) handle(next http.Handler) http.Handler { mergeRequests, _, err := m.client.ListProjectMergeRequests(m.data.projectInfo.ProjectId, &options) if err != nil { - handleError(w, fmt.Errorf("Failed to list merge requests: %w", err), "Failed to list merge requests", http.StatusInternalServerError) + handleError(w, fmt.Errorf("failed to list merge requests: %w", err), "Failed to list merge requests", http.StatusInternalServerError) return } if len(mergeRequests) == 0 { - err := fmt.Errorf("Branch '%s' does not have any merge requests", m.data.gitInfo.BranchName) + err := fmt.Errorf("branch '%s' does not have any merge requests", m.data.gitInfo.BranchName) handleError(w, err, "No MRs Found", http.StatusNotFound) return } if len(mergeRequests) > 1 { - err := errors.New("Please call gitlab.choose_merge_request()") + err := errors.New("please call gitlab.choose_merge_request()") handleError(w, err, "Multiple MRs found", http.StatusBadRequest) return } @@ -164,9 +164,9 @@ func withMethodCheck(methods ...string) mw { } // Helper function to format validation errors into more readable strings -func formatValidationErrors(errors validator.ValidationErrors) error { +func formatValidationErrors(errs validator.ValidationErrors) error { var s strings.Builder - for i, e := range errors { + for i, e := range errs { if i > 0 { s.WriteString("; ") } @@ -178,5 +178,5 @@ func formatValidationErrors(errors validator.ValidationErrors) error { } } - return fmt.Errorf(s.String()) + return errors.New(s.String()) } diff --git a/cmd/app/middleware_test.go b/cmd/app/middleware_test.go index 0402f9d4..598c3d59 100644 --- a/cmd/app/middleware_test.go +++ b/cmd/app/middleware_test.go @@ -75,7 +75,7 @@ func TestWithMrMiddleware(t *testing.T) { data, status := getFailData(t, handler, request) assert(t, status, http.StatusNotFound) assert(t, data.Message, "No MRs Found") - assert(t, data.Details, "Branch 'foo' does not have any merge requests") + assert(t, data.Details, "branch 'foo' does not have any merge requests") }) t.Run("Handles when there are too many MRs", func(t *testing.T) { request := makeRequest(t, http.MethodGet, "/foo", nil) @@ -88,7 +88,7 @@ func TestWithMrMiddleware(t *testing.T) { data, status := getFailData(t, handler, request) assert(t, status, http.StatusBadRequest) assert(t, data.Message, "Multiple MRs found") - assert(t, data.Details, "Please call gitlab.choose_merge_request()") + assert(t, data.Details, "please call gitlab.choose_merge_request()") }) } diff --git a/cmd/app/pipeline.go b/cmd/app/pipeline.go index 8640613a..174b4acc 100644 --- a/cmd/app/pipeline.go +++ b/cmd/app/pipeline.go @@ -67,7 +67,7 @@ func (a pipelineService) GetLastPipeline(commit string) (*gitlab.PipelineInfo, e } if res.StatusCode >= 300 { - return nil, errors.New("Could not get pipelines") + return nil, errors.New("could not get pipelines") } if len(pipes) == 0 { diff --git a/cmd/app/resolve_discussion_test.go b/cmd/app/resolve_discussion_test.go index 26332dae..24b04ddd 100644 --- a/cmd/app/resolve_discussion_test.go +++ b/cmd/app/resolve_discussion_test.go @@ -78,7 +78,7 @@ func TestResolveDiscussion(t *testing.T) { request := makeRequest(t, http.MethodPut, "/mr/discussions/resolve", testResolveMergeRequestPayload) data, status := getFailData(t, svc, request) assert(t, data.Message, "Could not resolve discussion") - assert(t, data.Details, "Some error from Gitlab") + assert(t, data.Details, "some error from Gitlab") assert(t, status, http.StatusInternalServerError) }) } diff --git a/cmd/app/server.go b/cmd/app/server.go index ca7ac8aa..c0b1af63 100644 --- a/cmd/app/server.go +++ b/cmd/app/server.go @@ -86,7 +86,13 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer for _, optFunc := range optFuncs { err := optFunc(&d) if err != nil { - panic(err) + if os.Getenv("DEBUG") != "" { + // TODO: We have some JSON files (emojis.json) we import relative to the binary in production and + // expect to break during debugging, do not throw when that occurs. + fmt.Fprintf(os.Stdout, "Issue occured setting up router: %s\n", err) + } else { + panic(err) + } } } @@ -245,13 +251,13 @@ func CreateRouter(gitlabClient *Client, projectInfo *ProjectInfo, s *shutdownSer func checkServer(port int) error { for i := 0; i < 10; i++ { resp, err := http.Get("http://localhost:" + fmt.Sprintf("%d", port) + "/ping") - if resp.StatusCode == 200 && err == nil { + if resp != nil && resp.StatusCode == 200 && err == nil { return nil } time.Sleep(100 * time.Microsecond) } - return errors.New("Could not start server!") + return errors.New("could not start server") } /* Creates a TCP listener on the port specified by the user or a random port */ diff --git a/cmd/app/test_helpers.go b/cmd/app/test_helpers.go index 98ec8d2b..2c3e1bf8 100644 --- a/cmd/app/test_helpers.go +++ b/cmd/app/test_helpers.go @@ -14,7 +14,7 @@ import ( "github.com/xanzy/go-gitlab" ) -var errorFromGitlab = errors.New("Some error from Gitlab") +var errorFromGitlab = errors.New("some error from Gitlab") /* The assert function is a helper function used to check two comparables */ func assert[T comparable](t *testing.T, got T, want T) { diff --git a/cmd/main.go b/cmd/main.go index 175aa30a..b46a880f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -14,6 +14,10 @@ var pluginOptions app.PluginOptions func main() { log.SetFlags(0) + if len(os.Args) < 2 { + log.Fatal("Must provide server configuration") + } + err := json.Unmarshal([]byte(os.Args[1]), &pluginOptions) app.SetPluginOptions(pluginOptions) @@ -28,12 +32,12 @@ func main() { log.Fatalf("Failure initializing plugin: %v", err) } - err, client := app.NewClient() + client, err := app.NewClient() if err != nil { log.Fatalf("Failed to initialize Gitlab client: %v", err) } - err, projectInfo := app.InitProjectSettings(client, gitData) + projectInfo, err := app.InitProjectSettings(client, gitData) if err != nil { log.Fatalf("Failed to initialize project settings: %v", err) } From e17d713b265a91d7388228da29a72e4f59756e22 Mon Sep 17 00:00:00 2001 From: George Kontridze Date: Sat, 21 Sep 2024 14:34:48 -0700 Subject: [PATCH 03/11] fix: error messages and run all tests (#381) Fixes scripts for running go tests and git tests --- cmd/app/git/git_test.go | 4 ++-- makefile | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/app/git/git_test.go b/cmd/app/git/git_test.go index e8864fb2..c2f88045 100644 --- a/cmd/app/git/git_test.go +++ b/cmd/app/git/git_test.go @@ -198,7 +198,7 @@ func TestExtractGitInfo_FailToGetProjectRemoteUrl(t *testing.T) { tC := FailTestCase{ desc: "Error returned by function to get the project remote url", errMsg: "Some error", - expectedErr: "Could not get project Url: Some error", + expectedErr: "could not get project Url: Some error", } t.Run(tC.desc, func(t *testing.T) { g := failingUrlManager{ @@ -227,7 +227,7 @@ func TestExtractGitInfo_FailToGetCurrentBranchName(t *testing.T) { tC := FailTestCase{ desc: "Error returned by function to get the project remote url", errMsg: "Some error", - expectedErr: "Failed to get current branch: Some error", + expectedErr: "failed to get current branch: Some error", } t.Run(tC.desc, func(t *testing.T) { g := failingBranchManager{ diff --git a/makefile b/makefile index 83e9c65f..151ce822 100644 --- a/makefile +++ b/makefile @@ -4,10 +4,10 @@ PROJECTNAME=$(shell basename "$(PWD)") ## compile: build golang project compile: - @cd cmd && go build -o bin && mv bin ../bin + @go build -o bin ./cmd ## test: run golang project tests test: - @cd cmd/app && go test + @go test -v ./... .PHONY: help all: help From 523bdd4726b13c715798cde77ef6af19c1bd5cd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Mon, 23 Sep 2024 21:08:27 +0200 Subject: [PATCH 04/11] feat: Automatically open fold under cursor (#380) feat: Automatically open fold under cursor, redraw screen with cursor at center --- lua/gitlab/reviewer/init.lua | 2 ++ lua/gitlab/utils/init.lua | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 9ef37ea2..185c01bc 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -145,6 +145,8 @@ M.jump = function(file_name, line_number, new_buffer) line_number = number_of_lines end vim.api.nvim_win_set_cursor(0, { line_number, 0 }) + u.open_fold_under_cursor() + vim.cmd("normal! zz") end ---Get the data from diffview, such as line information and file name. May be used by diff --git a/lua/gitlab/utils/init.lua b/lua/gitlab/utils/init.lua index 3018fd44..2729da1a 100644 --- a/lua/gitlab/utils/init.lua +++ b/lua/gitlab/utils/init.lua @@ -752,4 +752,10 @@ M.get_nested_field = function(table, field) end end +M.open_fold_under_cursor = function() + if vim.fn.foldclosed(vim.fn.line(".")) > -1 then + vim.cmd("normal! zo") + end +end + return M From 879ee20c1e80d82eb5a47e35bccc73aeba34c2ce Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:08:25 -0400 Subject: [PATCH 05/11] fix: discussion ID is not required (#383) --- cmd/app/draft_notes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/app/draft_notes.go b/cmd/app/draft_notes.go index 42daaa39..4f550537 100644 --- a/cmd/app/draft_notes.go +++ b/cmd/app/draft_notes.go @@ -90,7 +90,7 @@ func (a draftNoteService) listDraftNotes(w http.ResponseWriter, r *http.Request) type PostDraftNoteRequest struct { Comment string `json:"comment" validate:"required"` - DiscussionId string `json:"discussion_id,omitempty" validate:"required"` + DiscussionId string `json:"discussion_id,omitempty"` PositionData // TODO: How to add validations to data from external package??? } From 125cfbb54460861117f930e31cc4a0ec7013ecb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Fri, 4 Oct 2024 22:03:34 +0200 Subject: [PATCH 06/11] Chore: Add more emojis (#384) --- config/emojis.json | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/config/emojis.json b/config/emojis.json index 5df086aa..e349f871 100644 --- a/config/emojis.json +++ b/config/emojis.json @@ -6351,6 +6351,38 @@ ], "moji": "😮" }, + "package": { + "unicode": "1F4E6", + "unicode_alternates": [], + "name": "package", + "shortname": ":package:", + "category": "people", + "aliases": [], + "aliases_ascii": [], + "keywords": [ + "release" + ], + "moji": "📦" + }, + "party": { + "unicode": "1F389", + "unicode_alternates": [], + "name": "party popper as a 'tada' celebration", + "shortname": ":tada:", + "category": "people", + "aliases": [ + ":party_popper:" + ], + "aliases_ascii": [], + "keywords": [ + "celebrate", + "celebration", + "hooray", + "hurrah", + "hurray" + ], + "moji": "🎉" + }, "pensive": { "unicode": "1F614", "unicode_alternates": [], @@ -8445,6 +8477,19 @@ ], "moji": "🤖" }, + "rocket": { + "unicode": "1F680", + "unicode_alternates": [], + "name": "rocket", + "shortname": ":rocket:", + "category": "people", + "aliases": [], + "aliases_ascii": [], + "keywords": [ + "space" + ], + "moji": "🚀" + }, "rofl": { "unicode": "1F923", "unicode_alternates": [], From 7171f4c7d75561bf83b397fed888a470a94e75ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Thu, 10 Oct 2024 21:57:25 +0200 Subject: [PATCH 07/11] Fix: Publish all drafts (#391) * fix: use body with /mr/draft_notes/publish endpoint * docs: notify about successfully building server --- lua/gitlab/actions/draft_notes/init.lua | 2 +- lua/gitlab/server.lua | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lua/gitlab/actions/draft_notes/init.lua b/lua/gitlab/actions/draft_notes/init.lua index a8716750..ea7b5b42 100755 --- a/lua/gitlab/actions/draft_notes/init.lua +++ b/lua/gitlab/actions/draft_notes/init.lua @@ -84,7 +84,7 @@ end ---Publishes all draft notes and comments. Re-renders all discussion views. M.confirm_publish_all_drafts = function() - local body = {} + local body = { publish_all = true } job.run_job("/mr/draft_notes/publish", "POST", body, function(data) u.notify(data.message, vim.log.levels.INFO) state.DRAFT_NOTES = {} diff --git a/lua/gitlab/server.lua b/lua/gitlab/server.lua index 8efb338a..d770b071 100644 --- a/lua/gitlab/server.lua +++ b/lua/gitlab/server.lua @@ -100,6 +100,7 @@ M.build = function(override) u.notify("Could not install gitlab.nvim!", vim.log.levels.ERROR) return false end + u.notify("Gitlab.nvim installed successfully!", vim.log.levels.INFO) return true end From 0e19857f49a6e844ec5b7489c8a226abf1eff387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Fri, 11 Oct 2024 21:18:12 +0200 Subject: [PATCH 08/11] fix: make discussion tree buffers nomodifiable (#394) --- lua/gitlab/actions/common.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/lua/gitlab/actions/common.lua b/lua/gitlab/actions/common.lua index e409454c..ace7bbe4 100644 --- a/lua/gitlab/actions/common.lua +++ b/lua/gitlab/actions/common.lua @@ -82,6 +82,7 @@ M.add_empty_titles = function() { end_row = linnr - 1, end_col = string.len(v.title), hl_group = "TitleHighlight" } ) end + M.switch_can_edit_bufs(false, v.bufnr) end end end From 96efdc268f1c9e7c0e5f1c781e9d7f1730b47748 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sat, 12 Oct 2024 10:19:35 -0400 Subject: [PATCH 09/11] fix: Incorrect warning about commits (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed some bad warnings when the branch was out of date or ahead of the remote tracking branch. --------- Co-authored-by: Jakub F. Bortlík --- lua/gitlab/actions/create_mr.lua | 2 +- lua/gitlab/actions/summary.lua | 3 +- lua/gitlab/git.lua | 110 +++++++++++++++++++++++-------- lua/gitlab/reviewer/init.lua | 15 +---- 4 files changed, 89 insertions(+), 41 deletions(-) diff --git a/lua/gitlab/actions/create_mr.lua b/lua/gitlab/actions/create_mr.lua index 401162e2..2e366946 100644 --- a/lua/gitlab/actions/create_mr.lua +++ b/lua/gitlab/actions/create_mr.lua @@ -47,7 +47,7 @@ end --- continue working on it. ---@param args? Mr M.start = function(args) - if not git.current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then + if not git.check_current_branch_up_to_date_on_remote(vim.log.levels.ERROR) then return end diff --git a/lua/gitlab/actions/summary.lua b/lua/gitlab/actions/summary.lua index 613b4df7..dfde1d73 100644 --- a/lua/gitlab/actions/summary.lua +++ b/lua/gitlab/actions/summary.lua @@ -82,7 +82,8 @@ M.summary = function() vim.api.nvim_set_current_buf(description_popup.bufnr) end) - git.current_branch_up_to_date_on_remote(vim.log.levels.WARN) + git.check_current_branch_up_to_date_on_remote(vim.log.levels.WARN) + git.check_mr_in_good_condition() end -- Builds a lua list of strings that contain metadata about the current MR. Only builds the diff --git a/lua/gitlab/git.lua b/lua/gitlab/git.lua index cced16ad..aa74abc0 100644 --- a/lua/gitlab/git.lua +++ b/lua/gitlab/git.lua @@ -43,6 +43,58 @@ M.switch_branch = function(branch) return run_system({ "git", "checkout", "-q", branch }) end +---Fetches the name of the remote tracking branch for the current branch +---@return string|nil, string|nil +M.get_remote_branch = function() + return run_system({ "git", "rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}" }) +end + +---Determines whether the tracking branch is ahead of or behind the current branch, and warns the user if so +---@param current_branch string +---@param remote_branch string +---@param log_level number +---@return boolean +M.get_ahead_behind = function(current_branch, remote_branch, log_level) + local u = require("gitlab.utils") + local result, err = + run_system({ "git", "rev-list", "--left-right", "--count", current_branch .. "..." .. remote_branch }) + if err ~= nil or result == nil then + u.notify("Could not determine if branch is up-to-date: " .. err, vim.log.levels.ERROR) + return false + end + + local ahead, behind = result:match("(%d+)%s+(%d+)") + if ahead == nil or behind == nil then + u.notify("Error parsing ahead/behind information.", vim.log.levels.ERROR) + return false + end + + ahead = tonumber(ahead) + behind = tonumber(behind) + + if ahead > 0 and behind == 0 then + u.notify(string.format("There are local changes that haven't been pushed to %s yet", remote_branch), log_level) + return false + end + if behind > 0 and ahead == 0 then + u.notify(string.format("There are remote changes on %s that haven't been pulled yet", remote_branch), log_level) + return false + end + + if ahead > 0 and behind > 0 then + u.notify( + string.format( + "Your branch and the remote %s have diverged. You need to pull, possibly rebase, and then push.", + remote_branch + ), + log_level + ) + return false + end + + return true -- Checks passed, branch is up-to-date +end + ---Return the name of the current branch ---@return string|nil, string|nil M.get_current_branch = function() @@ -93,39 +145,45 @@ M.contains_branch = function(current_branch) return run_system({ "git", "branch", "-r", "--contains", current_branch }) end ----Returns true if `branch` is up-to-date on remote, false otherwise. +---Returns true if `branch` is up-to-date on remote, otherwise false and warns user ---@param log_level integer ----@return boolean|nil -M.current_branch_up_to_date_on_remote = function(log_level) - local state = require("gitlab.state") - local current_branch = M.get_current_branch() - local handle = io.popen("git branch -r --contains " .. current_branch .. " 2>&1") - if not handle then - require("gitlab.utils").notify("Error running 'git branch' command.", vim.log.levels.ERROR) - return nil +---@return boolean +M.check_current_branch_up_to_date_on_remote = function(log_level) + local u = require("gitlab.utils") + + -- Get current branch + local current_branch, err_current_branch = M.get_current_branch() + if err_current_branch or not current_branch then + u.notify("Could not get current branch: " .. err_current_branch, vim.log.levels.ERROR) + return false end - local remote_branches_with_current_head = {} - for line in handle:lines() do - table.insert(remote_branches_with_current_head, line) + -- Get remote tracking branch + local remote_branch, err_remote_branch = M.get_remote_branch() + if err_remote_branch or not remote_branch then + u.notify("Could not get remote branch: " .. err_remote_branch, vim.log.levels.ERROR) + return false end - handle:close() - local current_head_on_remote = List.new(remote_branches_with_current_head):filter(function(line) - return line == string.format(" %s/", state.settings.connection_settings.remote) .. current_branch - end) - local remote_up_to_date = #current_head_on_remote == 1 + return M.get_ahead_behind(current_branch, remote_branch, log_level) +end - if not remote_up_to_date then - require("gitlab.utils").notify( - string.format( - "You have local commits that are not on %s. Have you forgotten to push?", - state.settings.connection_settings.remote - ), - log_level - ) +---Warns user if the current MR is in a bad state (closed, has conflicts, merged) +M.check_mr_in_good_condition = function() + local state = require("gitlab.state") + local u = require("gitlab.utils") + + if state.INFO.has_conflicts then + u.notify("This merge request has conflicts!", vim.log.levels.WARN) + end + + if state.INFO.state == "closed" then + u.notify(string.format("This MR was closed %s", u.time_since(state.INFO.closed_at)), vim.log.levels.WARN) + end + + if state.INFO.state == "merged" then + u.notify(string.format("This MR was merged %s", u.time_since(state.INFO.merged_at)), vim.log.levels.WARN) end - return remote_up_to_date end return M diff --git a/lua/gitlab/reviewer/init.lua b/lua/gitlab/reviewer/init.lua index 185c01bc..89654c97 100644 --- a/lua/gitlab/reviewer/init.lua +++ b/lua/gitlab/reviewer/init.lua @@ -62,18 +62,6 @@ M.open = function() ) end - if state.INFO.has_conflicts then - u.notify("This merge request has conflicts!", vim.log.levels.WARN) - end - - if state.INFO.state == "closed" then - u.notify(string.format("This MR was closed %s", u.time_since(state.INFO.closed_at)), vim.log.levels.WARN) - end - - if state.INFO.state == "merged" then - u.notify(string.format("This MR was merged %s", u.time_since(state.INFO.merged_at)), vim.log.levels.WARN) - end - if state.settings.discussion_diagnostic ~= nil or state.settings.discussion_sign ~= nil then u.notify( "Diagnostics are now configured as settings.discussion_signs, see :h gitlab.nvim.signs-and-diagnostics", @@ -98,7 +86,8 @@ M.open = function() require("gitlab").toggle_discussions() -- Fetches data and opens discussions end - git.current_branch_up_to_date_on_remote(vim.log.levels.WARN) + git.check_current_branch_up_to_date_on_remote(vim.log.levels.WARN) + git.check_mr_in_good_condition() end -- Closes the reviewer and cleans up From b359b474a4892fdd88d6b9c1f68ac2924d1f8b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20F=2E=20Bortl=C3=ADk?= Date: Sun, 13 Oct 2024 18:59:09 +0200 Subject: [PATCH 10/11] Fix: Show draft replies in the correct tree (#396) Fix: Show draft replies in the correct tree --- lua/gitlab/actions/discussions/winbar.lua | 13 ++++++++++--- lua/gitlab/utils/list.lua | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lua/gitlab/actions/discussions/winbar.lua b/lua/gitlab/actions/discussions/winbar.lua index 09fb9934..d97c17d5 100644 --- a/lua/gitlab/actions/discussions/winbar.lua +++ b/lua/gitlab/actions/discussions/winbar.lua @@ -49,9 +49,16 @@ local function content() local resolvable_notes, resolved_notes = get_data(state.DISCUSSION_DATA.unlinked_discussions) local draft_notes = require("gitlab.actions.draft_notes") - local inline_draft_notes = List.new(state.DRAFT_NOTES):filter(draft_notes.has_position) - local unlinked_draft_notes = List.new(state.DRAFT_NOTES):filter(function(note) - return not draft_notes.has_position(note) + local inline_draft_notes, unlinked_draft_notes = List.new(state.DRAFT_NOTES):partition(function(note) + if note.discussion_id == "" then + return draft_notes.has_position(note) + end + for _, discussion in ipairs(state.DISCUSSION_DATA.unlinked_discussions) do + if discussion.id == note.discussion_id then + return false + end + end + return true end) local t = { diff --git a/lua/gitlab/utils/list.lua b/lua/gitlab/utils/list.lua index ea54b90c..775522ff 100644 --- a/lua/gitlab/utils/list.lua +++ b/lua/gitlab/utils/list.lua @@ -33,6 +33,23 @@ function List:filter(func) return result end +---Partitions a given list into two lists +---@generic T +---@param func fun(v: T, i: integer):boolean +---@return List, List @Returns two lists: the 1st with elements for which func returns true, the 2nd with elements for which it returns false +function List:partition(func) + local result_true = List.new() + local result_false = List.new() + for i, v in ipairs(self) do + if func(v, i) == true then + table.insert(result_true, v) + else + table.insert(result_false, v) + end + end + return result_true, result_false +end + function List:reduce(func, agg) for i, v in ipairs(self) do agg = func(agg, v, i) From a63823c196d6a6bdf7eee463e262ce7f5002c663 Mon Sep 17 00:00:00 2001 From: "Harrison (Harry) Cramer" <32515581+harrisoncramer@users.noreply.github.com> Date: Sun, 13 Oct 2024 15:12:14 -0400 Subject: [PATCH 11/11] fix: Cannot choose merge requests (#398) --- cmd/app/config.go | 2 +- cmd/app/middleware.go | 5 ++++- lua/gitlab/actions/merge_requests.lua | 2 +- lua/gitlab/server.lua | 4 ++-- lua/gitlab/state.lua | 5 ++--- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/cmd/app/config.go b/cmd/app/config.go index 5861b4b3..dc40dfe2 100644 --- a/cmd/app/config.go +++ b/cmd/app/config.go @@ -11,7 +11,7 @@ type PluginOptions struct { GitlabRequest bool `json:"gitlab_request"` GitlabResponse bool `json:"gitlab_response"` } `json:"debug"` - ChosenTargetBranch *string `json:"chosen_target_branch,omitempty"` + ChosenMrIID int `json:"chosen_mr_iid"` ConnectionSettings struct { Insecure bool `json:"insecure"` Remote string `json:"remote"` diff --git a/cmd/app/middleware.go b/cmd/app/middleware.go index 5408626b..ec5c534a 100644 --- a/cmd/app/middleware.go +++ b/cmd/app/middleware.go @@ -105,7 +105,10 @@ func (m withMrMiddleware) handle(next http.Handler) http.Handler { options := gitlab.ListProjectMergeRequestsOptions{ Scope: gitlab.Ptr("all"), SourceBranch: &m.data.gitInfo.BranchName, - TargetBranch: pluginOptions.ChosenTargetBranch, + } + + if pluginOptions.ChosenMrIID != 0 { + options.IIDs = gitlab.Ptr([]int{pluginOptions.ChosenMrIID}) } mergeRequests, _, err := m.client.ListProjectMergeRequests(m.data.projectInfo.ProjectId, &options) diff --git a/lua/gitlab/actions/merge_requests.lua b/lua/gitlab/actions/merge_requests.lua index b42b94b6..0af9db0c 100644 --- a/lua/gitlab/actions/merge_requests.lua +++ b/lua/gitlab/actions/merge_requests.lua @@ -45,7 +45,7 @@ M.choose_merge_request = function(opts) end vim.schedule(function() - state.chosen_target_branch = choice.target_branch + state.chosen_mr_iid = choice.iid require("gitlab.server").restart(function() if opts.open_reviewer then require("gitlab").review() diff --git a/lua/gitlab/server.lua b/lua/gitlab/server.lua index d770b071..aa739b45 100644 --- a/lua/gitlab/server.lua +++ b/lua/gitlab/server.lua @@ -20,10 +20,10 @@ M.start = function(callback) debug = state.settings.debug, log_path = state.settings.log_path, connection_settings = state.settings.connection_settings, - chosen_target_branch = state.chosen_target_branch, + chosen_mr_iid = state.chosen_mr_iid, } - state.chosen_target_branch = nil -- Do not let this interfere with subsequent reviewer.open() calls + state.chosen_mr_iid = 0 -- Do not let this interfere with subsequent reviewer.open() calls local settings = vim.json.encode(go_server_settings) local command = string.format("%s '%s'", state.settings.bin, settings) diff --git a/lua/gitlab/state.lua b/lua/gitlab/state.lua index a6349b50..12686151 100644 --- a/lua/gitlab/state.lua +++ b/lua/gitlab/state.lua @@ -251,9 +251,8 @@ M.unlinked_discussion_tree = { unresolved_expanded = false, } --- Used to set a specific target when choosing a merge request, due to the fact --- that it's technically possible to have multiple target branches -M.chosen_target_branch = nil +-- Used to set a specific MR when choosing a merge request +M.chosen_mr_iid = 0 -- These keymaps are set globally when the plugin is initialized M.set_global_keymaps = function()