Make sure there can be only one done bucket per list

docs/content/doc/usage/errors.md

@@ -131,10 +131,11 @@ This document describes the different errors Vikunja can return.
 
 | ErrorCode | HTTP Status Code | Description |
 |-----------|------------------|-------------|
-| 10001 | 404 | The bucket does not exist. | 
-| 10002 | 400 | The bucket does not belong to that list. | 
-| 10003 | 412 | You cannot remove the last bucket on a list. | 
-| 10004 | 412 | You cannot add the task to this bucket as it already exceeded the limit of tasks it can hold. | 
+| 10001 | 404 | The bucket does not exist. |
+| 10002 | 400 | The bucket does not belong to that list. |
+| 10003 | 412 | You cannot remove the last bucket on a list. |
+| 10004 | 412 | You cannot add the task to this bucket as it already exceeded the limit of tasks it can hold. |
+| 10005 | 412 | There can be only one done bucket per list. |
 
 ## Saved Filters
 

pkg/models/error.go

@@ -1365,6 +1365,35 @@ func (err ErrBucketLimitExceeded) HTTPError() web.HTTPError {
 	}
 }
 
+// ErrOnlyOneDoneBucketPerList represents an error where a bucket is set to the done bucket but one already exists for its list.
+type ErrOnlyOneDoneBucketPerList struct {
+	BucketID     int64
+	ListID       int64
+	DoneBucketID int64
+}
+
+// IsErrOnlyOneDoneBucketPerList checks if an error is ErrBucketLimitExceeded.
+func IsErrOnlyOneDoneBucketPerList(err error) bool {
+	_, ok := err.(*ErrOnlyOneDoneBucketPerList)
+	return ok
+}
+
+func (err *ErrOnlyOneDoneBucketPerList) Error() string {
+	return fmt.Sprintf("There can be only one done bucket per list [BucketID: %d, ListID: %d]", err.BucketID, err.ListID)
+}
+
+// ErrCodeOnlyOneDoneBucketPerList holds the unique world-error code of this error
+const ErrCodeOnlyOneDoneBucketPerList = 10005
+
+// HTTPError holds the http error description
+func (err *ErrOnlyOneDoneBucketPerList) HTTPError() web.HTTPError {
+	return web.HTTPError{
+		HTTPCode: http.StatusPreconditionFailed,
+		Code:     ErrCodeOnlyOneDoneBucketPerList,
+		Message:  "There can be only one done bucket per list.",
+	}
+}
+
 // =============
 // Saved Filters
 // =============

pkg/models/kanban.go

@@ -83,6 +83,21 @@ func getDefaultBucket(s *xorm.Session, listID int64) (bucket *Bucket, err error)
 	return
 }
 
+func getDoneBucketForList(s *xorm.Session, listID int64) (bucket *Bucket, err error) {
+	bucket = &Bucket{}
+	exists, err := s.
+		Where("list_id = ? and is_done_bucket = ?", listID, true).
+		Get(bucket)
+	if err != nil {
+		return nil, err
+	}
+	if !exists {
+		bucket = nil
+	}
+
+	return
+}
+
 // ReadAll returns all buckets with their tasks for a certain list
 // @Summary Get all kanban buckets of a list
 // @Description Returns all kanban buckets with belong to a list including their tasks.
@@ -241,9 +256,26 @@ func (b *Bucket) Create(s *xorm.Session, a web.Auth) (err error) {
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /lists/{listID}/buckets/{bucketID} [post]
 func (b *Bucket) Update(s *xorm.Session, a web.Auth) (err error) {
+	doneBucket, err := getDoneBucketForList(s, b.ListID)
+	if err != nil {
+		return err
+	}
+
+	if doneBucket != nil && doneBucket.IsDoneBucket && b.IsDoneBucket {
+		return &ErrOnlyOneDoneBucketPerList{
+			BucketID:     b.ID,
+			ListID:       b.ListID,
+			DoneBucketID: doneBucket.ID,
+		}
+	}
+
 	_, err = s.
 		Where("id = ?", b.ID).
-		Cols("title", "limit").
+		Cols(
+			"title",
+			"limit",
+			"is_done_bucket",
+		).
 		Update(b)
 	return
 }

pkg/models/kanban_test.go

@@ -182,4 +182,19 @@ func TestBucket_Update(t *testing.T) {
 
 		testAndAssertBucketUpdate(t, b, s)
 	})
+	t.Run("only one done bucket per list", func(t *testing.T) {
+		db.LoadAndAssertFixtures(t)
+		s := db.NewSession()
+		defer s.Close()
+
+		b := &Bucket{
+			ID:           1,
+			ListID:       1,
+			IsDoneBucket: true,
+		}
+
+		err := b.Update(s, &user.User{ID: 1})
+		assert.Error(t, err)
+		assert.True(t, IsErrOnlyOneDoneBucketPerList(err))
+	})
 }

pkg/models/tasks.go

@@ -739,16 +739,10 @@ func setTaskBucket(s *xorm.Session, task *Task, originalTask *Task, doCheckBucke
 	// Make sure we have a bucket
 	var bucket *Bucket
 	if task.Done {
-		bucket = &Bucket{}
-		exists, err := s.
-			Where("list_id = ? and is_done_bucket = ?", task.ListID, true).
-			Get(bucket)
+		bucket, err := getDoneBucketForList(s, task.ListID)
 		if err != nil {
 			return err
 		}
-		if !exists {
-			bucket = nil
-		}
 		if bucket != nil {
 			task.BucketID = bucket.ID
 		}