fix(views): make setting task position in saved filters work

pkg/models/project_rights.go

@@ -118,6 +118,16 @@ func (p *Project) CanUpdate(s *xorm.Session, a web.Auth) (canUpdate bool, err er
 		return false, nil
 	}
 
+	fid := getSavedFilterIDFromProjectID(p.ID)
+	if fid > 0 {
+		sf, err := getSavedFilterSimpleByID(s, fid)
+		if err != nil {
+			return false, err
+		}
+
+		return sf.CanUpdate(s, a)
+	}
+
 	// Get the project
 	ol, err := GetProjectSimpleByID(s, p.ID)
 	if err != nil {
@@ -137,16 +147,6 @@ func (p *Project) CanUpdate(s *xorm.Session, a web.Auth) (canUpdate bool, err er
 		}
 	}
 
-	fid := getSavedFilterIDFromProjectID(p.ID)
-	if fid > 0 {
-		sf, err := getSavedFilterSimpleByID(s, fid)
-		if err != nil {
-			return false, err
-		}
-
-		return sf.CanUpdate(s, a)
-	}
-
 	canUpdate, err = p.CanWrite(s, a)
 	// If the project is archived and the user tries to un-archive it, let the request through
 	archivedErr := ErrProjectIsArchived{}

pkg/models/project_view.go

@@ -303,7 +303,7 @@ func createProjectView(s *xorm.Session, p *ProjectView, a web.Auth, createBacklo
 		}
 	}
 
-	return RecalculateTaskPositions(s, p)
+	return RecalculateTaskPositions(s, p, a)
 }
 
 // Update is the handler to update a project view

pkg/models/project_view_rights.go

@@ -54,5 +54,5 @@ func (p *ProjectView) CanCreate(s *xorm.Session, a web.Auth) (bool, error) {
 }
 
 func (p *ProjectView) getProject(s *xorm.Session) (pp *Project, err error) {
-	return GetProjectSimpleByID(s, p.ProjectID)
+	return &Project{ID: p.ProjectID}, nil
 }

pkg/models/task_collection.go

@@ -130,6 +130,34 @@ func getTaskOrTasksInBuckets(s *xorm.Session, a web.Auth, projects []*Project, v
 	return getTasksForProjects(s, projects, a, opts, view)
 }
 
+func getRelevantProjectsFromCollection(s *xorm.Session, a web.Auth, tf *TaskCollection) (projects []*Project, err error) {
+	if tf.ProjectID == 0 || tf.isSavedFilter {
+		projects, _, _, err = getRawProjectsForUser(
+			s,
+			&projectOptions{
+				user: &user.User{ID: a.GetID()},
+				page: -1,
+			},
+		)
+		return projects, err
+	}
+
+	// Check the project exists and the user has access on it
+	project := &Project{ID: tf.ProjectID}
+	canRead, _, err := project.CanRead(s, a)
+	if err != nil {
+		return nil, err
+	}
+	if !canRead {
+		return nil, ErrUserDoesNotHaveAccessToProject{
+			ProjectID: tf.ProjectID,
+			UserID:    a.GetID(),
+		}
+	}
+
+	return []*Project{{ID: tf.ProjectID}}, nil
+}
+
 // ReadAll gets all tasks for a collection
 // @Summary Get tasks in a project
 // @Description Returns all tasks for the current project.
@@ -234,35 +262,10 @@ func (tf *TaskCollection) ReadAll(s *xorm.Session, a web.Auth, search string, pa
 		return getTaskOrTasksInBuckets(s, a, []*Project{project}, view, opts)
 	}
 
-	// If the project ID is not set, we get all tasks for the user.
+	projects, err := getRelevantProjectsFromCollection(s, a, tf)
-	// This allows to use this function in Task.ReadAll with a possibility to deprecate the latter at some point.
-	var projects []*Project
-	if tf.ProjectID == 0 || tf.isSavedFilter {
-		projects, _, _, err = getRawProjectsForUser(
-			s,
-			&projectOptions{
-				user: &user.User{ID: a.GetID()},
-				page: -1,
-			},
-		)
 	if err != nil {
 		return nil, 0, 0, err
 	}
-	} else {
-		// Check the project exists and the user has access on it
-		project := &Project{ID: tf.ProjectID}
-		canRead, _, err := project.CanRead(s, a)
-		if err != nil {
-			return nil, 0, 0, err
-		}
-		if !canRead {
-			return nil, 0, 0, ErrUserDoesNotHaveAccessToProject{
-				ProjectID: tf.ProjectID,
-				UserID:    a.GetID(),
-			}
-		}
-		projects = []*Project{{ID: tf.ProjectID}}
-	}
 
 	return getTaskOrTasksInBuckets(s, a, projects, view, opts)
 }

pkg/models/task_position.go

@@ -66,50 +66,81 @@ func (tp *TaskPosition) CanUpdate(s *xorm.Session, a web.Auth) (bool, error) {
 // @Failure 400 {object} web.HTTPError "Invalid task position object provided."
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /tasks/{id}/position [post]
-func (tp *TaskPosition) Update(s *xorm.Session, _ web.Auth) (err error) {
+func (tp *TaskPosition) Update(s *xorm.Session, a web.Auth) (err error) {
 
 	// Update all positions if the newly saved position is < 0.1
+	var shouldRecalculate bool
+	var view *ProjectView
 	if tp.Position < 0.1 {
-		view, err := GetProjectViewByID(s, tp.ProjectViewID)
+		shouldRecalculate = true
+		view, err = GetProjectViewByID(s, tp.ProjectViewID)
 		if err != nil {
 			return err
 		}
-
-		return RecalculateTaskPositions(s, view)
 	}
 
 	exists, err := s.
 		Where("task_id = ? AND project_view_id = ?", tp.TaskID, tp.ProjectViewID).
-		Get(&TaskPosition{})
+		Exist(&TaskPosition{})
 	if err != nil {
 		return err
 	}
 
 	if !exists {
 		_, err = s.Insert(tp)
+		if err != nil {
 			return
 		}
+		if shouldRecalculate {
+			return RecalculateTaskPositions(s, view, a)
+		}
+		return nil
+	}
 
 	_, err = s.
 		Where("task_id = ?", tp.TaskID).
 		Cols("project_view_id", "position").
 		Update(tp)
-	return
-}
-
-func RecalculateTaskPositions(s *xorm.Session, view *ProjectView) (err error) {
-
-	allTasks := []*Task{}
-	err = s.
-		Select("tasks.*, task_positions.position AS position").
-		Join("LEFT", "task_positions", "task_positions.task_id = tasks.id AND task_positions.project_view_id = ?", view.ID).
-		Where("project_id = ?", view.ProjectID).
-		OrderBy("position asc").
-		Find(&allTasks)
 	if err != nil {
 		return
 	}
 
+	if shouldRecalculate {
+		return RecalculateTaskPositions(s, view, a)
+	}
+
+	return
+}
+
+func RecalculateTaskPositions(s *xorm.Session, view *ProjectView, a web.Auth) (err error) {
+
+	// Using the collection so that we get all tasks, even in cases where we're dealing with a saved filter underneath
+	tc := &TaskCollection{
+		ProjectID: view.ProjectID,
+	}
+	if view.ProjectID < -1 {
+		tc.ProjectID = 0
+	}
+
+	projects, err := getRelevantProjectsFromCollection(s, a, tc)
+	if err != nil {
+		return err
+	}
+
+	opts := &taskSearchOptions{
+		sortby: []*sortParam{
+			{
+				projectViewID: view.ID,
+				sortBy:        taskPropertyPosition,
+				orderBy:       orderAscending,
+			},
+		},
+	}
+
+	allTasks, _, _, err := getRawTasksForProjects(s, projects, a, opts)
+	if err != nil {
+		return err
+	}
 	if len(allTasks) == 0 {
 		return
 	}