fix(comments): return correct status codes for comment reactions

apps/api/internal/handler/comment.go

@@ -1,6 +1,7 @@
 package handler
 
 import (
+	"errors"
 	"net/http"
 
 	"github.com/Devlaner/devlane/api/internal/middleware"
@@ -9,6 +10,14 @@ import (
 	"github.com/google/uuid"
 )
 
+// commentAccessNotFound reports whether err is one of the access/lookup errors
+// that should surface as a 404 to the client.
+func commentAccessNotFound(err error) bool {
+	return errors.Is(err, service.ErrCommentNotFound) ||
+		errors.Is(err, service.ErrProjectForbidden) ||
+		errors.Is(err, service.ErrProjectNotFound)
+}
+
 // CommentHandler serves issue comments.
 type CommentHandler struct {
 	Comment *service.CommentService
@@ -209,7 +218,14 @@ func (h *CommentHandler) AddReaction(c *gin.Context) {
 	}
 	r, err := h.Comment.AddReaction(c.Request.Context(), slug, projectID, commentID, user.ID, body.Reaction)
 	if err != nil {
-		c.JSON(http.StatusConflict, gin.H{"error": err.Error()})
+		switch {
+		case errors.Is(err, service.ErrReactionExists):
+			c.JSON(http.StatusConflict, gin.H{"error": "Already reacted"})
+		case commentAccessNotFound(err):
+			c.JSON(http.StatusNotFound, gin.H{"error": "Not found"})
+		default:
+			c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to add reaction"})
+		}
 		return
 	}
 	c.JSON(http.StatusCreated, r)
@@ -240,6 +256,10 @@ func (h *CommentHandler) RemoveReaction(c *gin.Context) {
 		return
 	}
 	if err := h.Comment.RemoveReaction(c.Request.Context(), slug, projectID, commentID, user.ID, reaction); err != nil {
+		if commentAccessNotFound(err) {
+			c.JSON(http.StatusNotFound, gin.H{"error": "Not found"})
+			return
+		}
 		c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to remove reaction"})
 		return
 	}

apps/api/internal/handler/comment_reaction_test.go

@@ -0,0 +1,55 @@
+package handler_test
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/Devlaner/devlane/api/internal/testutil"
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/require"
+)
+
+func TestComment_AddReaction_StatusCodes(t *testing.T) {
+	ts := testutil.NewTestServer(t)
+	w := testutil.SeedWorld(t, ts.DB)
+	issue := testutil.CreateIssue(t, ts.DB, w.Project.ID, w.Workspace.ID, w.User.ID)
+	comment := testutil.CreateComment(t, ts.DB, issue.ID, w.Project.ID, w.Workspace.ID, w.User.ID)
+
+	base := "/api/workspaces/" + w.Workspace.Slug + "/projects/" + w.Project.ID.String() +
+		"/issues/" + issue.ID.String() + "/comments/" + comment.ID.String() + "/reactions/"
+
+	// First reaction succeeds.
+	rr := ts.POST(base, map[string]any{"reaction": "👍"}, w.Session)
+	require.Equal(t, http.StatusCreated, rr.Code, "body=%s", rr.Body.String())
+
+	// Reacting again with the same emoji is a conflict, not a leaked 409-for-all.
+	rr2 := ts.POST(base, map[string]any{"reaction": "👍"}, w.Session)
+	require.Equal(t, http.StatusConflict, rr2.Code, "body=%s", rr2.Body.String())
+
+	// A missing comment is a 404, not a 409.
+	missing := "/api/workspaces/" + w.Workspace.Slug + "/projects/" + w.Project.ID.String() +
+		"/issues/" + issue.ID.String() + "/comments/" + uuid.NewString() + "/reactions/"
+	rr3 := ts.POST(missing, map[string]any{"reaction": "👍"}, w.Session)
+	require.Equal(t, http.StatusNotFound, rr3.Code, "body=%s", rr3.Body.String())
+}
+
+func TestComment_RemoveReaction_StatusCodes(t *testing.T) {
+	ts := testutil.NewTestServer(t)
+	w := testutil.SeedWorld(t, ts.DB)
+	issue := testutil.CreateIssue(t, ts.DB, w.Project.ID, w.Workspace.ID, w.User.ID)
+	comment := testutil.CreateComment(t, ts.DB, issue.ID, w.Project.ID, w.Workspace.ID, w.User.ID)
+
+	base := "/api/workspaces/" + w.Workspace.Slug + "/projects/" + w.Project.ID.String() +
+		"/issues/" + issue.ID.String() + "/comments/" + comment.ID.String() + "/reactions/"
+
+	// Add then remove succeeds.
+	require.Equal(t, http.StatusCreated, ts.POST(base, map[string]any{"reaction": "🎉"}, w.Session).Code)
+	rr := ts.DELETE(base+"%F0%9F%8E%89/", w.Session)
+	require.Equal(t, http.StatusNoContent, rr.Code, "body=%s", rr.Body.String())
+
+	// Removing from a missing comment is a 404, not a generic 500.
+	missing := "/api/workspaces/" + w.Workspace.Slug + "/projects/" + w.Project.ID.String() +
+		"/issues/" + issue.ID.String() + "/comments/" + uuid.NewString() + "/reactions/x/"
+	rr2 := ts.DELETE(missing, w.Session)
+	require.Equal(t, http.StatusNotFound, rr2.Code, "body=%s", rr2.Body.String())
+}

apps/api/internal/service/comment.go

@@ -8,6 +8,7 @@ import (
 	"github.com/Devlaner/devlane/api/internal/store"
 	"github.com/Devlaner/devlane/api/internal/text"
 	"github.com/google/uuid"
+	"gorm.io/gorm"
 )
 
 var ErrCommentNotFound = errors.New("comment not found")
@@ -188,8 +189,10 @@ func (s *CommentService) AddReaction(ctx context.Context, workspaceSlug string,
 		WorkspaceID: c.WorkspaceID,
 	}
 	if err := s.reactions.Add(ctx, r); err != nil {
-		// Unique-constraint violation = already reacted, return existing row.
-		// We don't bother fetching it; caller can refetch the list.
+		// Unique-constraint violation = the user already reacted with this emoji.
+		if errors.Is(err, gorm.ErrDuplicatedKey) {
+			return nil, ErrReactionExists
+		}
 		return nil, err
 	}
 	return r, nil