From c4950964443a9bffc4cdd8fc25004ad951520f20 Mon Sep 17 00:00:00 2001
From: kolaente <k@knt.li>
Date: Wed, 2 Nov 2022 17:40:43 +0100
Subject: [PATCH] fix: make sure task indexes are calculated correctly when
 moving tasks between lists

Resolves https://github.com/go-vikunja/api/issues/52
---
 pkg/models/tasks.go      | 26 +++++++++++++++++---------
 pkg/models/tasks_test.go | 19 +++++++++++++++++--
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go
index a8e41dddd56..16b14d73d8d 100644
--- a/pkg/models/tasks.go
+++ b/pkg/models/tasks.go
@@ -869,6 +869,19 @@ func calculateDefaultPosition(entityID int64, position float64) float64 {
 	return position
 }
 
+func getNextTaskIndex(s *xorm.Session, listID int64) (nextIndex int64, err error) {
+	latestTask := &Task{}
+	_, err = s.
+		Where("list_id = ?", listID).
+		OrderBy("`index` desc").
+		Get(latestTask)
+	if err != nil {
+		return 0, err
+	}
+
+	return latestTask.Index + 1, nil
+}
+
 // Create is the implementation to create a list task
 // @Summary Create a task
 // @Description Inserts a task into a list.
@@ -920,16 +933,14 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err
 	}
 
 	// Get the index for this task
-	latestTask := &Task{}
-	_, err = s.Where("list_id = ?", t.ListID).OrderBy("id desc").Get(latestTask)
+	t.Index, err = getNextTaskIndex(s, t.ListID)
 	if err != nil {
 		return err
 	}
 
-	t.Index = latestTask.Index + 1
 	// If no position was supplied, set a default one
-	t.Position = calculateDefaultPosition(latestTask.ID+1, t.Position)
-	t.KanbanPosition = calculateDefaultPosition(latestTask.ID+1, t.KanbanPosition)
+	t.Position = calculateDefaultPosition(t.Index, t.Position)
+	t.KanbanPosition = calculateDefaultPosition(t.Index, t.KanbanPosition)
 	if _, err = s.Insert(t); err != nil {
 		return err
 	}
@@ -1047,13 +1058,10 @@ func (t *Task) Update(s *xorm.Session, a web.Auth) (err error) {
 
 	// If the task is being moved between lists, make sure to move the bucket + index as well
 	if t.ListID != 0 && ot.ListID != t.ListID {
-		latestTask := &Task{}
-		_, err = s.Where("list_id = ?", t.ListID).OrderBy("id desc").Get(latestTask)
+		t.Index, err = getNextTaskIndex(s, t.ListID)
 		if err != nil {
 			return err
 		}
-
-		t.Index = latestTask.Index + 1
 		colsToUpdate = append(colsToUpdate, "index")
 	}
 
diff --git a/pkg/models/tasks_test.go b/pkg/models/tasks_test.go
index 49cb590c799..49207c4f94c 100644
--- a/pkg/models/tasks_test.go
+++ b/pkg/models/tasks_test.go
@@ -20,10 +20,10 @@ import (
 	"testing"
 	"time"
 
-	"code.vikunja.io/api/pkg/events"
-
 	"code.vikunja.io/api/pkg/db"
+	"code.vikunja.io/api/pkg/events"
 	"code.vikunja.io/api/pkg/user"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -324,6 +324,21 @@ func TestTask_Update(t *testing.T) {
 			"bucket_id": 1,
 		}, false)
 	})
+	t.Run("moving a task between lists should give it a correct index", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		task := &Task{
+			ID:     12,
+			ListID: 2, // From list 1
+		}
+		err := task.Update(s, u)
+		assert.NoError(t, err)
+		err = s.Commit()
+		assert.NoError(t, err)
+		assert.Equal(t, int64(3), task.Index)
+	})
 }
 
 func TestTask_Delete(t *testing.T) {