fix: prevent duplicate label creation from Android sync engine

apiserver/internal/migrations/008_unique_label_names.go

@@ -0,0 +1,108 @@
+package migrations
+
+import (
+	"context"
+	"fmt"
+
+	"gorm.io/gorm"
+)
+
+func init() {
+	Register(&UniqueLabelNamesMigration{})
+}
+
+type UniqueLabelNamesMigration struct{}
+
+func (m *UniqueLabelNamesMigration) Version() int {
+	return 8
+}
+
+func (m *UniqueLabelNamesMigration) Name() string {
+	return "unique_label_names"
+}
+
+func (m *UniqueLabelNamesMigration) Up(ctx context.Context, db *gorm.DB) error {
+	dbCtx := db.WithContext(ctx)
+
+	dedup := []string{
+		// Reassign task_labels from duplicate labels to the kept (lowest ID) label,
+		// skipping rows that would collide with an existing (task_id, keep_id) pair
+		`INSERT OR IGNORE INTO task_labels (task_id, label_id)
+		SELECT tl.task_id, keep.id
+		FROM task_labels tl
+		JOIN labels dup ON tl.label_id = dup.id
+		JOIN (
+			SELECT MIN(id) AS id, name, created_by
+			FROM labels
+			GROUP BY created_by, name
+		) keep ON dup.created_by = keep.created_by AND dup.name = keep.name
+		WHERE dup.id != keep.id`,
+
+		// Remove task_labels pointing to duplicate labels
+		`DELETE FROM task_labels WHERE label_id IN (
+			SELECT l.id FROM labels l
+			JOIN (
+				SELECT MIN(id) AS keep_id, name, created_by
+				FROM labels
+				GROUP BY created_by, name
+			) k ON l.created_by = k.created_by AND l.name = k.name
+			WHERE l.id != k.keep_id
+		)`,
+
+		// Delete the duplicate labels themselves
+		`DELETE FROM labels WHERE id NOT IN (
+			SELECT MIN(id) FROM labels GROUP BY created_by, name
+		)`,
+	}
+
+	dialect := db.Dialector.Name()
+	if dialect == "mysql" {
+		dedup = []string{
+			`INSERT IGNORE INTO task_labels (task_id, label_id)
+			SELECT tl.task_id, keep_tbl.keep_id
+			FROM task_labels tl
+			JOIN labels dup ON tl.label_id = dup.id
+			JOIN (
+				SELECT MIN(id) AS keep_id, name, created_by
+				FROM labels
+				GROUP BY created_by, name
+			) keep_tbl ON dup.created_by = keep_tbl.created_by AND dup.name = keep_tbl.name
+			WHERE dup.id != keep_tbl.keep_id`,
+
+			`DELETE tl FROM task_labels tl
+			JOIN labels l ON tl.label_id = l.id
+			JOIN (
+				SELECT MIN(id) AS keep_id, name, created_by
+				FROM labels
+				GROUP BY created_by, name
+			) k ON l.created_by = k.created_by AND l.name = k.name
+			WHERE l.id != k.keep_id`,
+
+			`DELETE l FROM labels l
+			JOIN (
+				SELECT MIN(id) AS keep_id, name, created_by
+				FROM labels
+				GROUP BY created_by, name
+			) k ON l.created_by = k.created_by AND l.name = k.name
+			WHERE l.id != k.keep_id`,
+		}
+	}
+
+	for _, stmt := range dedup {
+		if err := dbCtx.Exec(stmt).Error; err != nil {
+			return fmt.Errorf("failed to deduplicate labels: %w", err)
+		}
+	}
+
+	return dbCtx.Exec("CREATE UNIQUE INDEX idx_labels_created_by_name ON labels(created_by, name)").Error
+}
+
+func (m *UniqueLabelNamesMigration) Down(ctx context.Context, db *gorm.DB) error {
+	dbCtx := db.WithContext(ctx)
+
+	if db.Dialector.Name() == "mysql" {
+		return dbCtx.Exec("DROP INDEX idx_labels_created_by_name ON labels").Error
+	}
+
+	return dbCtx.Exec("DROP INDEX IF EXISTS idx_labels_created_by_name").Error
+}

apiserver/internal/models/label.go

@@ -6,9 +6,9 @@ import (
 
 type Label struct {
 	ID        int        `json:"id" gorm:"primary_key"`
-	Name      string     `json:"name" gorm:"column:name;not null"`
+	Name      string     `json:"name" gorm:"column:name;not null;uniqueIndex:idx_labels_created_by_name"`
 	Color     string     `json:"color" gorm:"type:varchar(7);column:color;not null"`
-	CreatedBy int        `json:"created_by" gorm:"column:created_by;not null;index:idx_labels_created_by"`
+	CreatedBy int        `json:"created_by" gorm:"column:created_by;not null;index:idx_labels_created_by;uniqueIndex:idx_labels_created_by_name"`
 	CreatedAt time.Time  `json:"-" gorm:"column:created_at;default:CURRENT_TIMESTAMP"`
 	UpdatedAt *time.Time `json:"-" gorm:"column:updated_at;default:NULL;autoUpdateTime"`
 

apiserver/internal/repos/label/label.go

@@ -30,6 +30,18 @@ func (r *LabelRepository) CreateLabels(ctx context.Context, labels []*models.Lab
 	return r.db.WithContext(ctx).Create(&labels).Error
 }
 
+func (r *LabelRepository) LabelExistsByName(ctx context.Context, userID int, name string, excludeLabelID int) (bool, error) {
+	var count int64
+	q := r.db.WithContext(ctx).Model(&models.Label{}).Where("created_by = ? AND name = ?", userID, name)
+	if excludeLabelID > 0 {
+		q = q.Where("id != ?", excludeLabelID)
+	}
+	if err := q.Count(&count).Error; err != nil {
+		return false, err
+	}
+	return count > 0, nil
+}
+
 func (r *LabelRepository) AreLabelsAssignableByUser(ctx context.Context, userID int, labels []int) bool {
 	var count int64
 

apiserver/internal/repos/label/label_test.go

@@ -70,6 +70,34 @@ func (s *LabelTestSuite) TestCreateLabels() {
 	s.Equal(int64(2), count)
 }
 
+func (s *LabelTestSuite) TestLabelExistsByName() {
+	ctx := context.Background()
+
+	label := &models.Label{Name: "Work", Color: "#FF0000", CreatedBy: s.testUser.ID}
+	err := s.DB.Create(label).Error
+	s.Require().NoError(err)
+
+	exists, err := s.repo.LabelExistsByName(ctx, s.testUser.ID, "Work", 0)
+	s.Require().NoError(err)
+	s.True(exists)
+
+	exists, err = s.repo.LabelExistsByName(ctx, s.testUser.ID, "Nonexistent", 0)
+	s.Require().NoError(err)
+	s.False(exists)
+
+	exists, err = s.repo.LabelExistsByName(ctx, s.testUser.ID, "Work", label.ID)
+	s.Require().NoError(err)
+	s.False(exists)
+
+	anotherUser := &models.User{}
+	err = s.DB.Create(anotherUser).Error
+	s.Require().NoError(err)
+
+	exists, err = s.repo.LabelExistsByName(ctx, anotherUser.ID, "Work", 0)
+	s.Require().NoError(err)
+	s.False(exists)
+}
+
 func (s *LabelTestSuite) TestAreLabelsAssignableByUser() {
 	ctx := context.Background()
 

apiserver/internal/services/labels/label.go

@@ -3,6 +3,7 @@ package labels
 import (
 	"context"
 	"net/http"
+	"strings"
 
 	"dkhalife.com/tasks/core/internal/models"
 	repos "dkhalife.com/tasks/core/internal/repos/label"
@@ -49,13 +50,34 @@ func (s *LabelService) GetUserLabels(ctx context.Context, userID int) (int, inte
 func (s *LabelService) CreateLabel(ctx context.Context, userId int, req models.CreateLabelReq) (int, interface{}) {
 	log := logging.FromContext(ctx)
 
+	exists, err := s.r.LabelExistsByName(ctx, userId, req.Name, 0)
+	if err != nil {
+		log.Errorf("Failed to check label existence: %s", err.Error())
+		telemetry.TrackError(ctx, "label_check_failed", "label-service", err, nil)
+		return http.StatusInternalServerError, gin.H{
+			"error": "Failed to create label",
+		}
+	}
+
+	if exists {
+		return http.StatusConflict, gin.H{
+			"error": "A label with this name already exists",
+		}
+	}
+
 	label := &models.Label{
 		Name:      req.Name,
 		Color:     req.Color,
 		CreatedBy: userId,
 	}
 
 	if err := s.r.CreateLabels(ctx, []*models.Label{label}); err != nil {
+		if isDuplicateKeyError(err) {
+			return http.StatusConflict, gin.H{
+				"error": "A label with this name already exists",
+			}
+		}
+
 		log.Errorf("Failed to create label: %s", err.Error())
 		telemetry.TrackError(ctx, "label_create_failed", "label-service", err, nil)
 		return http.StatusInternalServerError, gin.H{
@@ -76,7 +98,9 @@ func (s *LabelService) CreateLabel(ctx context.Context, userId int, req models.C
 		Data:   newLabel,
 	})
 
-	return http.StatusCreated, newLabel
+	return http.StatusCreated, gin.H{
+		"label": label.ID,
+	}
 }
 
 func (s *LabelService) UpdateLabel(ctx context.Context, userId int, req models.UpdateLabelReq) (int, interface{}) {
@@ -95,7 +119,28 @@ func (s *LabelService) UpdateLabel(ctx context.Context, userId int, req models.U
 		}
 	}
 
+	exists, err := s.r.LabelExistsByName(ctx, userId, req.Name, req.ID)
+	if err != nil {
+		log.Errorf("Failed to check label existence: %s", err.Error())
+		telemetry.TrackError(ctx, "label_check_failed", "label-service", err, nil)
+		return http.StatusInternalServerError, gin.H{
+			"error": "Error updating label",
+		}
+	}
+
+	if exists {
+		return http.StatusConflict, gin.H{
+			"error": "A label with this name already exists",
+		}
+	}
+
 	if err := s.r.UpdateLabel(ctx, userId, label); err != nil {
+		if isDuplicateKeyError(err) {
+			return http.StatusConflict, gin.H{
+				"error": "A label with this name already exists",
+			}
+		}
+
 		log.Errorf("Failed to update label: %s", err.Error())
 		telemetry.TrackError(ctx, "label_update_failed", "label-service", err, nil)
 		return http.StatusInternalServerError, gin.H{
@@ -137,3 +182,12 @@ func (s *LabelService) DeleteLabel(ctx context.Context, userID int, labelID int)
 	})
 	return http.StatusNoContent, nil
 }
+
+func isDuplicateKeyError(err error) bool {
+	msg := err.Error()
+	// SQLite: "UNIQUE constraint failed: ..."
+	// MySQL: "Error 1062 (23000): Duplicate entry ..."
+	return strings.Contains(msg, "UNIQUE constraint failed") ||
+		strings.Contains(msg, "Duplicate entry") ||
+		strings.Contains(msg, "Error 1062")
+}

frontend/src/api/labels.ts

@@ -6,13 +6,17 @@ type LabelsResponse = {
   labels: Label[]
 }
 
+type LabelIdResponse = {
+  label: number
+}
+
 type SingleLabelResponse = {
   label: Label
 }
 
 export const CreateLabel = async (label: Omit<Label, 'id'>) =>
   await transport({
-    http: () => Request<SingleLabelResponse>(`/labels`, 'POST', label),
+    http: () => Request<LabelIdResponse>(`/labels`, 'POST', label),
     ws: (ws) => ws.request('create_label', label),
   })
 

frontend/src/store/labelsSlice.ts

@@ -87,8 +87,15 @@ const labelsSlice = createSlice({
       })
       .addCase(createLabel.fulfilled, (state, action) => {
         state.status = 'succeeded'
+
+        const labelId = action.payload.label
+        const label: Label = {
+          ...action.meta.arg,
+          id: labelId,
+        }
+
         labelsSlice.caseReducers.labelUpserted(state, {
-          payload: action.payload.label,
+          payload: label,
           type: 'labels/labelUpserted',
         })
         state.error = null
@@ -103,9 +110,8 @@ const labelsSlice = createSlice({
       })
       .addCase(updateLabel.fulfilled, (state, action) => {
         state.status = 'succeeded'
-        const label = action.payload.label
         labelsSlice.caseReducers.labelUpserted(state, {
-          payload: label,
+          payload: action.payload.label,
           type: 'labels/labelUpserted',
         })
         state.error = null