Release

cmd/app/client.go

@@ -131,7 +131,7 @@ func handleError(w http.ResponseWriter, err error, message string, status int) {
 	w.WriteHeader(status)
 	response := ErrorResponse{
 		Message: message,
-		Details: err.Error(),
+		Error:   err.Error(),
 	}
 
 	err = json.NewEncoder(w).Encode(response)

cmd/app/comment_helpers.go

@@ -7,17 +7,27 @@ import (
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 )
 
-/* LinePosition represents a position in a line range. Unlike the Gitlab struct, this does not contain LineCode with a sha1 of the filename */
-type LinePosition struct {
+/* PositionInfo represents one endpoint (start or end) of a line range, as sent by the Lua
+* plugin. Unlike the Gitlab struct, it has no LineCode - Lua can't compute a sha1, so
+* buildCommentPosition computes one below from OldLine and NewLine.
+*
+* OldLine and NewLine are always real, non-nil integers, even when Type is "old" or "new"
+* and only one side actually has a line. On the side that doesn't, the value is a position
+* marker, not a claim that a line exists there: it's wherever that side's cursor was
+* sitting when the other side's line was found. LineCode is always built from this
+* unzeroed pair; buildCommentPosition separately zeroes the inapplicable side before
+* setting it on the request's LineRange.{Start,End}.{OldLine,NewLine} - see
+* zeroInapplicableLine. */
+type PositionInfo struct {
 	Type    string `json:"type"`
 	OldLine int64  `json:"old_line"`
 	NewLine int64  `json:"new_line"`
 }
 
 /* LineRange represents the range of a note. */
 type LineRange struct {
-	StartRange *LinePosition `json:"start"`
-	EndRange   *LinePosition `json:"end"`
+	Start *PositionInfo `json:"start" validate:"required"`
+	End   *PositionInfo `json:"end" validate:"required"`
 }
 
 /* PositionData represents the position of a comment or note (relative to a file diff) */
@@ -30,7 +40,7 @@ type PositionData struct {
 	BaseCommitSHA  string     `json:"base_commit_sha"`
 	StartCommitSHA string     `json:"start_commit_sha"`
 	Type           string     `json:"type"`
-	LineRange      *LineRange `json:"line_range,omitempty"`
+	LineRange      *LineRange `json:"line_range" validate:"required_with=FileName"`
 }
 
 /* RequestWithPosition is an interface that abstracts the handling of position data for a comment or a draft comment */
@@ -42,48 +52,65 @@ type RequestWithPosition interface {
 func buildCommentPosition(commentWithPositionData RequestWithPosition) *gitlab.PositionOptions {
 	positionData := commentWithPositionData.GetPositionData()
 
-	// If the file has been renamed, then this is a relevant part of the payload
-	oldFileName := positionData.OldFileName
-	if oldFileName == "" {
-		oldFileName = positionData.FileName
-	}
-
 	opt := &gitlab.PositionOptions{
 		PositionType: &positionData.Type,
 		StartSHA:     &positionData.StartCommitSHA,
 		HeadSHA:      &positionData.HeadCommitSHA,
 		BaseSHA:      &positionData.BaseCommitSHA,
 		NewPath:      &positionData.FileName,
-		OldPath:      &oldFileName,
+		OldPath:      &positionData.OldFileName,
 		NewLine:      positionData.NewLine,
 		OldLine:      positionData.OldLine,
 	}
 
-	if positionData.LineRange != nil {
-		shaFormat := "%x_%d_%d"
-		startFilenameSha := fmt.Sprintf(
-			shaFormat,
-			sha1.Sum([]byte(positionData.FileName)),
-			positionData.LineRange.StartRange.OldLine,
-			positionData.LineRange.StartRange.NewLine,
-		)
-		endFilenameSha := fmt.Sprintf(
-			shaFormat,
-			sha1.Sum([]byte(positionData.FileName)),
-			positionData.LineRange.EndRange.OldLine,
-			positionData.LineRange.EndRange.NewLine,
-		)
-		opt.LineRange = &gitlab.LineRangeOptions{
-			Start: &gitlab.LinePositionOptions{
-				Type:     &positionData.LineRange.StartRange.Type,
-				LineCode: &startFilenameSha,
-			},
-			End: &gitlab.LinePositionOptions{
-				Type:     &positionData.LineRange.EndRange.Type,
-				LineCode: &endFilenameSha,
-			},
-		}
+	shaFormat := "%x_%d_%d"
+	startFilenameSha := fmt.Sprintf(
+		shaFormat,
+		sha1.Sum([]byte(positionData.FileName)),
+		positionData.LineRange.Start.OldLine,
+		positionData.LineRange.Start.NewLine,
+	)
+	endFilenameSha := fmt.Sprintf(
+		shaFormat,
+		sha1.Sum([]byte(positionData.FileName)),
+		positionData.LineRange.End.OldLine,
+		positionData.LineRange.End.NewLine,
+	)
+
+	startOldLine, startNewLine := zeroInapplicableLine(positionData.LineRange.Start)
+	endOldLine, endNewLine := zeroInapplicableLine(positionData.LineRange.End)
+
+	opt.LineRange = &gitlab.LineRangeOptions{
+		Start: &gitlab.LinePositionOptions{
+			Type:     &positionData.LineRange.Start.Type,
+			LineCode: &startFilenameSha,
+			OldLine:  &startOldLine,
+			NewLine:  &startNewLine,
+		},
+		End: &gitlab.LinePositionOptions{
+			Type:     &positionData.LineRange.End.Type,
+			LineCode: &endFilenameSha,
+			OldLine:  &endOldLine,
+			NewLine:  &endNewLine,
+		},
 	}
 
 	return opt
 }
+
+/* zeroInapplicableLine returns a line_range endpoint's OldLine/NewLine with the side
+* that its Type doesn't apply to zeroed out: NewLine for a deleted ("old") line, OldLine
+* for an added ("new") line. Both stay real for an unmodified ("") or "expanded" line.
+* The unzeroed pair is still what the LineCode hash above is computed from - Gitlab
+* expects LineCode to encode the real old/new correspondence even when the displayed
+* OldLine or NewLine is zeroed. */
+func zeroInapplicableLine(position *PositionInfo) (oldLine int64, newLine int64) {
+	oldLine, newLine = position.OldLine, position.NewLine
+	switch position.Type {
+	case "old":
+		newLine = 0
+	case "new":
+		oldLine = 0
+	}
+	return oldLine, newLine
+}

cmd/app/comment_helpers_test.go

@@ -0,0 +1,65 @@
+package app
+
+import (
+	"testing"
+)
+
+func TestBuildCommentPosition(t *testing.T) {
+	makePositionData := func(startType string, startOld, startNew int64, endType string, endOld, endNew int64) PositionData {
+		return PositionData{
+			FileName:       "file.txt",
+			HeadCommitSHA:  "head-sha",
+			BaseCommitSHA:  "base-sha",
+			StartCommitSHA: "start-sha",
+			Type:           "text",
+			LineRange: &LineRange{
+				Start: &PositionInfo{Type: startType, OldLine: startOld, NewLine: startNew},
+				End:   &PositionInfo{Type: endType, OldLine: endOld, NewLine: endNew},
+			},
+		}
+	}
+
+	t.Run("zeroes NewLine for a deleted (\"old\") line, keeping LineCode's real pair", func(t *testing.T) {
+		positionData := makePositionData("", 4, 4, "old", 5, 5)
+		opt := buildCommentPosition(CommentWithPosition{PositionData: positionData})
+
+		assert(t, *opt.LineRange.End.OldLine, int64(5))
+		assert(t, *opt.LineRange.End.NewLine, int64(0))
+		assert(t, *opt.LineRange.End.LineCode, "5436437fa01a7d3e41d46741da54b451446774ca_5_5")
+	})
+
+	t.Run("zeroes OldLine for an added (\"new\") line, keeping LineCode's real pair", func(t *testing.T) {
+		positionData := makePositionData("", 4, 4, "new", 5, 5)
+		opt := buildCommentPosition(CommentWithPosition{PositionData: positionData})
+
+		assert(t, *opt.LineRange.End.OldLine, int64(0))
+		assert(t, *opt.LineRange.End.NewLine, int64(5))
+		assert(t, *opt.LineRange.End.LineCode, "5436437fa01a7d3e41d46741da54b451446774ca_5_5")
+	})
+
+	t.Run("keeps both lines real for an unmodified (\"\") line", func(t *testing.T) {
+		positionData := makePositionData("", 4, 4, "", 5, 6)
+		opt := buildCommentPosition(CommentWithPosition{PositionData: positionData})
+
+		assert(t, *opt.LineRange.End.OldLine, int64(5))
+		assert(t, *opt.LineRange.End.NewLine, int64(6))
+	})
+
+	t.Run("keeps both lines real for an expanded line", func(t *testing.T) {
+		positionData := makePositionData("", 4, 4, "expanded", 59, 61)
+		opt := buildCommentPosition(CommentWithPosition{PositionData: positionData})
+
+		assert(t, *opt.LineRange.End.OldLine, int64(59))
+		assert(t, *opt.LineRange.End.NewLine, int64(61))
+	})
+
+	t.Run("zeroes the start and end independently", func(t *testing.T) {
+		positionData := makePositionData("new", 0, 50, "", 60, 62)
+		opt := buildCommentPosition(CommentWithPosition{PositionData: positionData})
+
+		assert(t, *opt.LineRange.Start.OldLine, int64(0))
+		assert(t, *opt.LineRange.Start.NewLine, int64(50))
+		assert(t, *opt.LineRange.End.OldLine, int64(60))
+		assert(t, *opt.LineRange.End.NewLine, int64(62))
+	})
+}

cmd/app/comment_test.go

@@ -59,6 +59,10 @@ func TestPostComment(t *testing.T) {
 			Comment: "Some comment",
 			PositionData: PositionData{
 				FileName: "file.txt",
+				LineRange: &LineRange{
+					Start: &PositionInfo{Type: "", OldLine: 4, NewLine: 4},
+					End:   &PositionInfo{Type: "", OldLine: 4, NewLine: 4},
+				},
 			},
 		}
 		request := makeRequest(t, http.MethodPost, "/mr/comment", testCommentCreationData)

cmd/app/create_mr_test.go

@@ -71,7 +71,7 @@ func TestCreateMr(t *testing.T) {
 		)
 		data, _ := getFailData(t, svc, request)
 		assert(t, data.Message, "Invalid payload")
-		assert(t, data.Details, "Title is required")
+		assert(t, data.Error, "Title is required")
 	})
 
 	t.Run("Handles missing target branch", func(t *testing.T) {
@@ -85,6 +85,6 @@ func TestCreateMr(t *testing.T) {
 		)
 		data, _ := getFailData(t, svc, request)
 		assert(t, data.Message, "Invalid payload")
-		assert(t, data.Details, "TargetBranch is required")
+		assert(t, data.Error, "TargetBranch is required")
 	})
 }

cmd/app/draft_notes_test.go

@@ -184,7 +184,7 @@ func TestEditDraftNote(t *testing.T) {
 		)
 		data, status := getFailData(t, svc, request)
 		assert(t, data.Message, "Invalid payload")
-		assert(t, data.Details, "Note is required")
+		assert(t, data.Error, "Note is required")
 		assert(t, status, http.StatusBadRequest)
 	})
 }

cmd/app/emoji.go

@@ -3,31 +3,14 @@ package app
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
-	"os"
-	"path/filepath"
 	"strconv"
 	"strings"
 
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 )
 
-type Emoji struct {
-	Unicode           string   `json:"unicode"`
-	UnicodeAlternates []string `json:"unicode_alternates"`
-	Name              string   `json:"name"`
-	Shortname         string   `json:"shortname"`
-	Category          string   `json:"category"`
-	Aliases           []string `json:"aliases"`
-	AliasesASCII      []string `json:"aliases_ascii"`
-	Keywords          []string `json:"keywords"`
-	Moji              string   `json:"moji"`
-}
-
-type EmojiMap map[string]Emoji
-
 type CreateNoteEmojiPost struct {
 	Emoji  string `json:"emoji"`
 	NoteId int64  `json:"note_id"`
@@ -145,38 +128,3 @@ func (a emojiService) postEmojiOnNote(w http.ResponseWriter, r *http.Request) {
 		handleError(w, err, "Could not encode response", http.StatusInternalServerError)
 	}
 }
-
-/*
-attachEmojis reads the emojis from our external JSON file
-and attaches them to the data so that they can be looked up later
-*/
-func attachEmojis(a *data, fr FileReader) error {
-
-	e, err := os.Executable()
-	if err != nil {
-		return err
-	}
-
-	binPath := filepath.Dir(e)
-	filePath := filepath.Join(binPath, "config", "emojis.json")
-
-	reader, err := fr.ReadFile(filePath)
-
-	if err != nil {
-		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")
-	}
-
-	var emojiMap EmojiMap
-	err = json.Unmarshal(bytes, &emojiMap)
-	if err != nil {
-		return errors.New("could not unmarshal emojis")
-	}
-
-	a.emojiMap = emojiMap
-	return nil
-}

cmd/app/list_discussions_test.go

@@ -137,6 +137,6 @@ func TestListDiscussions(t *testing.T) {
 		)
 		data, _ := getFailData(t, svc, request)
 		assert(t, data.Message, "Could not fetch emojis")
-		assert(t, data.Details, "Some error from emoji service")
+		assert(t, data.Error, "Some error from emoji service")
 	})
 }

cmd/app/members.go

@@ -3,6 +3,7 @@ package app
 import (
 	"encoding/json"
 	"net/http"
+	"slices"
 
 	gitlab "gitlab.com/gitlab-org/api/client-go"
 )
@@ -30,26 +31,24 @@ func (a projectMemberService) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		},
 	}
 
-	projectMembers, res, err := a.client.ListAllProjectMembers(a.projectInfo.ProjectId, &projectMemberOptions)
+	it, hasErr := gitlab.Scan(func(p gitlab.PaginationOptionFunc) ([]*gitlab.ProjectMember, *gitlab.Response, error) {
+		return a.client.ListAllProjectMembers(a.projectInfo.ProjectId, &projectMemberOptions, p)
+	})
+	projectMembers := slices.Collect(it)
 
-	if err != nil {
+	if err := hasErr(); err != nil {
 		handleError(w, err, "Could not retrieve project members", http.StatusInternalServerError)
 		return
 	}
 
-	if res.StatusCode >= 300 {
-		handleError(w, GenericError{r.URL.Path}, "Could not retrieve project members", res.StatusCode)
-		return
-	}
-
 	w.WriteHeader(http.StatusOK)
 
 	response := ProjectMembersResponse{
 		SuccessResponse: SuccessResponse{Message: "Project members retrieved"},
 		ProjectMembers:  projectMembers,
 	}
 
-	err = json.NewEncoder(w).Encode(response)
+	err := json.NewEncoder(w).Encode(response)
 	if err != nil {
 		handleError(w, err, "Could not encode response", http.StatusInternalServerError)
 	}

cmd/app/members_test.go

@@ -38,13 +38,4 @@ func TestMembersHandler(t *testing.T) {
 		data, _ := getFailData(t, svc, request)
 		checkErrorFromGitlab(t, data, "Could not retrieve project members")
 	})
-	t.Run("Handles non-200s from Gitlab client", func(t *testing.T) {
-		request := makeRequest(t, http.MethodGet, "/project/members", nil)
-		svc := middleware(
-			projectMemberService{testProjectData, fakeMemberLister{testBase{status: http.StatusSeeOther}}},
-			withMethodCheck(http.MethodGet),
-		)
-		data, _ := getFailData(t, svc, request)
-		checkNon200(t, data, "Could not retrieve project members", "/project/members")
-	})
 }