fix(filters): initialize task positions for saved filters (#1477)

pkg/models/saved_filter_positions_test.go

@@ -0,0 +1,93 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package models
+
+import (
+	"testing"
+
+	"code.vikunja.io/api/pkg/db"
+	"code.vikunja.io/api/pkg/user"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSavedFilterUpdateInsertsNonZeroPosition(t *testing.T) {
+	db.LoadAndAssertFixtures(t)
+	s := db.NewSession()
+	defer s.Close()
+
+	sf := &SavedFilter{
+		Title:   "posfilter",
+		Filters: &TaskCollection{Filter: "id = 1"},
+	}
+
+	u := &user.User{ID: 1}
+	err := sf.Create(s, u)
+	require.NoError(t, err)
+
+	err = sf.Update(s, u)
+	require.NoError(t, err)
+
+	view := &ProjectView{}
+	exists, err := s.Where("project_id = ? AND view_kind = ?", getProjectIDFromSavedFilterID(sf.ID), ProjectViewKindKanban).Get(view)
+	require.NoError(t, err)
+	require.True(t, exists)
+
+	tp := &TaskPosition{}
+	exists, err = s.Where("project_view_id = ? AND task_id = ?", view.ID, 1).Get(tp)
+	require.NoError(t, err)
+	require.True(t, exists)
+	assert.NotZero(t, tp.Position)
+}
+
+func TestCronInsertsNonZeroPosition(t *testing.T) {
+	db.LoadAndAssertFixtures(t)
+	s := db.NewSession()
+	defer s.Close()
+
+	sf := &SavedFilter{
+		Title:   "cronfilter",
+		Filters: &TaskCollection{Filter: "due_date > '2018-01-01T00:00:00'"},
+	}
+
+	u := &user.User{ID: 1}
+	err := sf.Create(s, u)
+	require.NoError(t, err)
+
+	view := &ProjectView{}
+	exists, err := s.Where("project_id = ? AND view_kind = ?", getProjectIDFromSavedFilterID(sf.ID), ProjectViewKindKanban).Get(view)
+	require.NoError(t, err)
+	require.True(t, exists)
+
+	task := &Task{}
+	exists, err = s.Where("id = ?", 5).Get(task)
+	require.NoError(t, err)
+	require.True(t, exists)
+
+	tp := &TaskPosition{TaskID: task.ID, ProjectViewID: view.ID, Position: 0}
+	_, err = s.Insert(tp)
+	require.NoError(t, err)
+
+	_, err = calculateNewPositionForTask(s, u, task, view)
+	require.NoError(t, err)
+
+	exists, err = s.Where("project_view_id = ? AND task_id = ?", view.ID, task.ID).Get(tp)
+	require.NoError(t, err)
+	require.True(t, exists)
+	assert.NotZero(t, tp.Position)
+}

pkg/models/saved_filters.go

@@ -240,9 +240,6 @@ func (sf *SavedFilter) Update(s *xorm.Session, _ web.Auth) error {
 		return err
 	}
 
-	taskBuckets := []*TaskBucket{}
-	taskPositions := []*TaskPosition{}
-
 	for _, view := range kanbanFilterViews {
 		// Fetch all tasks in the filter but not in task_bucket
 		// select * from tasks where id not in (select task_id from task_buckets where project_view_id = ?) and FILTER_COND
@@ -265,6 +262,8 @@ func (sf *SavedFilter) Update(s *xorm.Session, _ web.Auth) error {
 			return err
 		}
 
+		taskBuckets := make([]*TaskBucket, 0, len(tasksToAdd))
+		taskPositions := make([]*TaskPosition, 0, len(tasksToAdd))
 		for _, task := range tasksToAdd {
 			taskBuckets = append(taskBuckets, &TaskBucket{
 				TaskID:        task.ID,
@@ -278,16 +277,21 @@ func (sf *SavedFilter) Update(s *xorm.Session, _ web.Auth) error {
 				Position:      0,
 			})
 		}
-	}
 
-	if len(taskBuckets) > 0 && len(taskPositions) > 0 {
-		_, err = s.Insert(taskBuckets)
-		if err != nil {
-			return err
+		if len(taskBuckets) > 0 {
+			if _, err = s.Insert(taskBuckets); err != nil {
+				return err
+			}
 		}
-		_, err = s.Insert(taskPositions)
-		if err != nil {
-			return err
+
+		if len(taskPositions) > 0 {
+			if _, err = s.Insert(taskPositions); err != nil {
+				return err
+			}
+
+			if err = RecalculateTaskPositions(s, view, &user.User{ID: sf.OwnerID}); err != nil {
+				return err
+			}
 		}
 	}
 
@@ -374,10 +378,9 @@ func addTaskToFilter(s *xorm.Session, filter *SavedFilter, view *ProjectView, fa
 		return nil, nil, err
 	}
 	if !existingTaskPosition {
-		taskPosition = &TaskPosition{
-			TaskID:        task.ID,
-			ProjectViewID: view.ID,
-			Position:      calculateDefaultPosition(task.Index, task.Position),
+		taskPosition, err = calculateNewPositionForTask(s, &user.User{ID: filter.OwnerID}, task, view)
+		if err != nil {
+			return nil, nil, err
 		}
 	}
 
@@ -436,6 +439,10 @@ func RegisterAddTaskToFilterViewCron() {
 		newTaskPositions := []*TaskPosition{}
 		deleteCond := []builder.Cond{}
 		taskIDsToRemove := []int64{}
+		viewsToRecalc := map[int64]struct {
+			view    *ProjectView
+			ownerID int64
+		}{}
 		for _, view := range kanbanFilterViews {
 			filterID := GetSavedFilterIDFromProjectID(view.ProjectID)
 			filter, exists := filters[filterID]
@@ -500,9 +507,16 @@ func RegisterAddTaskToFilterViewCron() {
 					tp := &TaskPosition{
 						TaskID:        task.ID,
 						ProjectViewID: view.ID,
-						Position:      task.Position,
+						Position:      0,
 					}
 					newTaskPositions = append(newTaskPositions, tp)
+
+					if _, ok := viewsToRecalc[view.ID]; !ok {
+						viewsToRecalc[view.ID] = struct {
+							view    *ProjectView
+							ownerID int64
+						}{view: view, ownerID: filter.OwnerID}
+					}
 				}
 			}
 
@@ -526,6 +540,12 @@ func RegisterAddTaskToFilterViewCron() {
 		}
 
 		upsertRelatedTaskProperties(s, logPrefix, newTaskBuckets, newTaskPositions, deleteCond, taskIDsToRemove)
+
+		for _, data := range viewsToRecalc {
+			if err := RecalculateTaskPositions(s, data.view, &user.User{ID: data.ownerID}); err != nil {
+				log.Errorf("%sError recalculating task positions for view %d: %s", logPrefix, data.view.ID, err)
+			}
+		}
 	})
 	if err != nil {
 		log.Fatalf("Could register add task to filter view cron: %s", err)
@@ -546,7 +566,6 @@ func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBucke
 			log.Errorf("%sError inserting task positions: %s", logPrefix, err)
 		}
 	}
-
 	if len(deleteCond) > 0 {
 		_, err = s.Where(builder.Or(deleteCond...)).Delete(&TaskBucket{})
 		if err != nil {