Add default board to new projects, remove uncategorized pseudo-board (#29874)

On creation of an empty project (no template) a default board will be
created instead of falling back to the uneditable pseudo-board.

Every project now has to have exactly one default boards. As a
consequence, you cannot unset a board as default, instead you have to
set another board as default. Existing projects will be modified using a
cron job, additionally this check will run every midnight by default.

Deleting the default board is not allowed, you have to set another board
as default to do it.

Fixes #29873
Fixes #14679 along the way
Fixes #29853

Co-authored-by: delvh <dev.lh@web.de>
(cherry picked from commit e5160185ed65fd1c2bcb2fc7dc7e0b5514ddb299)

Conflicts:
	options/locale/locale_en-US.ini
	trivial conflict because Forgejo strings do not have
	surrounding double quotes

models/fixtures/project.yml

@@ -45,3 +45,27 @@
   type: 2
   created_unix: 1688973000
   updated_unix: 1688973000
+
+-
+  id: 5
+  title: project without default column
+  owner_id: 2
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 2
+  created_unix: 1688973000
+  updated_unix: 1688973000
+
+-
+  id: 6
+  title: project with multiple default columns
+  owner_id: 2
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 2
+  created_unix: 1688973000
+  updated_unix: 1688973000

models/fixtures/project_board.yml

@@ -3,6 +3,7 @@
   project_id: 1
   title: To Do
   creator_id: 2
+  default: true
   created_unix: 1588117528
   updated_unix: 1588117528
 
@@ -29,3 +30,48 @@
   creator_id: 2
   created_unix: 1588117528
   updated_unix: 1588117528
+
+-
+  id: 5
+  project_id: 2
+  title: Backlog
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 6
+  project_id: 4
+  title: Backlog
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 7
+  project_id: 5
+  title: Done
+  creator_id: 2
+  default: false
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 8
+  project_id: 6
+  title: Backlog
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 9
+  project_id: 6
+  title: Uncategorized
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528

models/issues/issue_project.go

@@ -49,18 +49,13 @@ func (issue *Issue) ProjectBoardID(ctx context.Context) int64 {
 
 // LoadIssuesFromBoard load issues assigned to this board
 func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList, error) {
-	issueList := make(IssueList, 0, 10)
-
-	if b.ID > 0 {
-		issues, err := Issues(ctx, &IssuesOptions{
-			ProjectBoardID: b.ID,
-			ProjectID:      b.ProjectID,
-			SortType:       "project-column-sorting",
-		})
-		if err != nil {
-			return nil, err
-		}
-		issueList = issues
+	issueList, err := Issues(ctx, &IssuesOptions{
+		ProjectBoardID: b.ID,
+		ProjectID:      b.ProjectID,
+		SortType:       "project-column-sorting",
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	if b.Default {

models/migrations/fixtures/Test_CheckProjectColumnsConsistency/project.yml

@@ -0,0 +1,23 @@
+-
+  id: 1
+  title: project without default column
+  owner_id: 2
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 2
+  created_unix: 1688973000
+  updated_unix: 1688973000
+
+-
+  id: 2
+  title: project with multiple default columns
+  owner_id: 2
+  repo_id: 0
+  is_closed: false
+  creator_id: 2
+  board_type: 1
+  type: 2
+  created_unix: 1688973000
+  updated_unix: 1688973000

models/migrations/fixtures/Test_CheckProjectColumnsConsistency/project_board.yml

@@ -0,0 +1,26 @@
+-
+  id: 1
+  project_id: 1
+  title: Done
+  creator_id: 2
+  default: false
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 2
+  project_id: 2
+  title: Backlog
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528
+
+-
+  id: 3
+  project_id: 2
+  title: Uncategorized
+  creator_id: 2
+  default: true
+  created_unix: 1588117528
+  updated_unix: 1588117528

models/migrations/migrations.go

@@ -570,6 +570,8 @@ var migrations = []Migration{
 	NewMigration("Add PayloadVersion to HookTask", v1_22.AddPayloadVersionToHookTaskTable),
 	// v291 -> v292
 	NewMigration("Add Index to attachment.comment_id", v1_22.AddCommentIDIndexofAttachment),
+	// v292 -> v293
+	NewMigration("Ensure every project has exactly one default column", v1_22.CheckProjectColumnsConsistency),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v1_22/v292.go

@@ -0,0 +1,85 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_22 //nolint
+
+import (
+	"code.gitea.io/gitea/models/project"
+	"code.gitea.io/gitea/modules/setting"
+
+	"xorm.io/builder"
+	"xorm.io/xorm"
+)
+
+// CheckProjectColumnsConsistency ensures there is exactly one default board per project present
+func CheckProjectColumnsConsistency(x *xorm.Engine) error {
+	sess := x.NewSession()
+	defer sess.Close()
+
+	if err := sess.Begin(); err != nil {
+		return err
+	}
+
+	limit := setting.Database.IterateBufferSize
+	if limit <= 0 {
+		limit = 50
+	}
+
+	start := 0
+
+	for {
+		var projects []project.Project
+		if err := sess.SQL("SELECT DISTINCT `p`.`id`, `p`.`creator_id` FROM `project` `p` WHERE (SELECT COUNT(*) FROM `project_board` `pb` WHERE `pb`.`project_id` = `p`.`id` AND `pb`.`default` = ?) != 1", true).
+			Limit(limit, start).
+			Find(&projects); err != nil {
+			return err
+		}
+
+		if len(projects) == 0 {
+			break
+		}
+		start += len(projects)
+
+		for _, p := range projects {
+			var boards []project.Board
+			if err := sess.Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil {
+				return err
+			}
+
+			if len(boards) == 0 {
+				if _, err := sess.Insert(project.Board{
+					ProjectID: p.ID,
+					Default:   true,
+					Title:     "Uncategorized",
+					CreatorID: p.CreatorID,
+				}); err != nil {
+					return err
+				}
+				continue
+			}
+
+			var boardsToUpdate []int64
+			for id, b := range boards {
+				if id > 0 {
+					boardsToUpdate = append(boardsToUpdate, b.ID)
+				}
+			}
+
+			if _, err := sess.Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))).
+				Cols("`default`").Update(&project.Board{Default: false}); err != nil {
+				return err
+			}
+		}
+
+		if start%1000 == 0 {
+			if err := sess.Commit(); err != nil {
+				return err
+			}
+			if err := sess.Begin(); err != nil {
+				return err
+			}
+		}
+	}
+
+	return sess.Commit()
+}

models/migrations/v1_22/v292_test.go

@@ -0,0 +1,44 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_22 //nolint
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/migrations/base"
+	"code.gitea.io/gitea/models/project"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_CheckProjectColumnsConsistency(t *testing.T) {
+	// Prepare and load the testing database
+	x, deferable := base.PrepareTestEnv(t, 0, new(project.Project), new(project.Board))
+	defer deferable()
+	if x == nil || t.Failed() {
+		return
+	}
+
+	assert.NoError(t, CheckProjectColumnsConsistency(x))
+
+	// check if default board was added
+	var defaultBoard project.Board
+	has, err := x.Where("project_id=? AND `default` = ?", 1, true).Get(&defaultBoard)
+	assert.NoError(t, err)
+	assert.True(t, has)
+	assert.Equal(t, int64(1), defaultBoard.ProjectID)
+	assert.True(t, defaultBoard.Default)
+
+	// check if multiple defaults were removed
+	expectDefaultBoard, err := project.GetBoard(db.DefaultContext, 2)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(2), expectDefaultBoard.ProjectID)
+	assert.True(t, expectDefaultBoard.Default)
+
+	expectNonDefaultBoard, err := project.GetBoard(db.DefaultContext, 3)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(2), expectNonDefaultBoard.ProjectID)
+	assert.False(t, expectNonDefaultBoard.Default)
+}

models/project/board.go

@@ -123,6 +123,17 @@ func createBoardsForProjectsType(ctx context.Context, project *Project) error {
 		return nil
 	}
 
+	board := Board{
+		CreatedUnix: timeutil.TimeStampNow(),
+		CreatorID:   project.CreatorID,
+		Title:       "Backlog",
+		ProjectID:   project.ID,
+		Default:     true,
+	}
+	if err := db.Insert(ctx, board); err != nil {
+		return err
+	}
+
 	if len(items) == 0 {
 		return nil
 	}
@@ -176,6 +187,10 @@ func deleteBoardByID(ctx context.Context, boardID int64) error {
 		return err
 	}
 
+	if board.Default {
+		return fmt.Errorf("deleteBoardByID: cannot delete default board")
+	}
+
 	if err = board.removeIssues(ctx); err != nil {
 		return err
 	}
@@ -228,7 +243,6 @@ func UpdateBoard(ctx context.Context, board *Board) error {
 }
 
 // GetBoards fetches all boards related to a project
-// if no default board set, first board is a temporary "Uncategorized" board
 func (p *Project) GetBoards(ctx context.Context) (BoardList, error) {
 	boards := make([]*Board, 0, 5)
 
@@ -244,41 +258,61 @@ func (p *Project) GetBoards(ctx context.Context) (BoardList, error) {
 	return append([]*Board{defaultB}, boards...), nil
 }
 
-// getDefaultBoard return default board and create a dummy if none exist
+// getDefaultBoard return default board and ensure only one exists
 func (p *Project) getDefaultBoard(ctx context.Context) (*Board, error) {
-	var board Board
-	exist, err := db.GetEngine(ctx).Where("project_id=? AND `default`=?", p.ID, true).Get(&board)
-	if err != nil {
+	var boards []Board
+	if err := db.GetEngine(ctx).Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil {
 		return nil, err
 	}
-	if exist {
+
+	// create a default board if none is found
+	if len(boards) == 0 {
+		board := Board{
+			ProjectID: p.ID,
+			Default:   true,
+			Title:     "Uncategorized",
+			CreatorID: p.CreatorID,
+		}
+		if _, err := db.GetEngine(ctx).Insert(); err != nil {
+			return nil, err
+		}
 		return &board, nil
 	}
 
-	// represents a board for issues not assigned to one
-	return &Board{
-		ProjectID: p.ID,
-		Title:     "Uncategorized",
-		Default:   true,
-	}, nil
+	// unset default boards where too many default boards exist
+	if len(boards) > 1 {
+		var boardsToUpdate []int64
+		for id, b := range boards {
+			if id > 0 {
+				boardsToUpdate = append(boardsToUpdate, b.ID)
+			}
+		}
+
+		if _, err := db.GetEngine(ctx).Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))).
+			Cols("`default`").Update(&Board{Default: false}); err != nil {
+			return nil, err
+		}
+	}
+
+	return &boards[0], nil
 }
 
 // SetDefaultBoard represents a board for issues not assigned to one
-// if boardID is 0 unset default
 func SetDefaultBoard(ctx context.Context, projectID, boardID int64) error {
-	_, err := db.GetEngine(ctx).Where(builder.Eq{
-		"project_id": projectID,
-		"`default`":  true,
-	}).Cols("`default`").Update(&Board{Default: false})
-	if err != nil {
+	if _, err := GetBoard(ctx, boardID); err != nil {
 		return err
 	}
 
-	if boardID > 0 {
-		_, err = db.GetEngine(ctx).ID(boardID).Where(builder.Eq{"project_id": projectID}).
-			Cols("`default`").Update(&Board{Default: true})
+	if _, err := db.GetEngine(ctx).Where(builder.Eq{
+		"project_id": projectID,
+		"`default`":  true,
+	}).Cols("`default`").Update(&Board{Default: false}); err != nil {
+		return err
 	}
 
+	_, err := db.GetEngine(ctx).ID(boardID).Where(builder.Eq{"project_id": projectID}).
+		Cols("`default`").Update(&Board{Default: true})
+
 	return err
 }
 

models/project/board_test.go

@@ -0,0 +1,40 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package project
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/unittest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetDefaultBoard(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	projectWithoutDefault, err := GetProjectByID(db.DefaultContext, 5)
+	assert.NoError(t, err)
+
+	// check if default board was added
+	board, err := projectWithoutDefault.getDefaultBoard(db.DefaultContext)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(5), board.ProjectID)
+	assert.Equal(t, "Uncategorized", board.Title)
+
+	projectWithMultipleDefaults, err := GetProjectByID(db.DefaultContext, 6)
+	assert.NoError(t, err)
+
+	// check if multiple defaults were removed
+	board, err = projectWithMultipleDefaults.getDefaultBoard(db.DefaultContext)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(6), board.ProjectID)
+	assert.Equal(t, int64(8), board.ID)
+
+	board, err = GetBoard(db.DefaultContext, 9)
+	assert.NoError(t, err)
+	assert.Equal(t, int64(6), board.ProjectID)
+	assert.False(t, board.Default)
+}

models/project/project_test.go

@@ -92,19 +92,19 @@ func TestProjectsSort(t *testing.T) {
 	}{
 		{
 			sortType: "default",
-			wants:    []int64{1, 3, 2, 4},
+			wants:    []int64{1, 3, 2, 6, 5, 4},
 		},
 		{
 			sortType: "oldest",
-			wants:    []int64{4, 2, 3, 1},
+			wants:    []int64{4, 5, 6, 2, 3, 1},
 		},
 		{
 			sortType: "recentupdate",
-			wants:    []int64{1, 3, 2, 4},
+			wants:    []int64{1, 3, 2, 6, 5, 4},
 		},
 		{
 			sortType: "leastupdate",
-			wants:    []int64{4, 2, 3, 1},
+			wants:    []int64{4, 5, 6, 2, 3, 1},
 		},
 	}
 
@@ -113,8 +113,8 @@ func TestProjectsSort(t *testing.T) {
 			OrderBy: GetSearchOrderByBySortType(tt.sortType),
 		})
 		assert.NoError(t, err)
-		assert.EqualValues(t, int64(4), count)
-		if assert.Len(t, projects, 4) {
+		assert.EqualValues(t, int64(6), count)
+		if assert.Len(t, projects, 6) {
 			for i := range projects {
 				assert.EqualValues(t, tt.wants[i], projects[i].ID)
 			}