From ad1095a5ecd73e8b6585f2a35c96e0c7d2a6b196 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Apr 2024 11:44:04 +0800 Subject: [PATCH 01/49] Fix moving column start multiple POST requests and default column will not be moved. --- models/project/board.go | 19 +++++---- models/project/issue.go | 27 ++++++++++++ routers/web/org/projects.go | 63 ++++++++++++++++++++++++++++ routers/web/web.go | 1 + templates/projects/view.tmpl | 2 +- web_src/js/features/repo-projects.js | 25 ++++++----- 6 files changed, 117 insertions(+), 20 deletions(-) diff --git a/models/project/board.go b/models/project/board.go index 7faabc52c58b..aa995caa804c 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -242,17 +242,11 @@ func UpdateBoard(ctx context.Context, board *Board) error { // GetBoards fetches all boards related to a project func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { boards := make([]*Board, 0, 5) - - if err := db.GetEngine(ctx).Where("project_id=? AND `default`=?", p.ID, false).OrderBy("sorting").Find(&boards); err != nil { - return nil, err - } - - defaultB, err := p.getDefaultBoard(ctx) - if err != nil { + if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting").Find(&boards); err != nil { return nil, err } - return append([]*Board{defaultB}, boards...), nil + return boards, nil } // getDefaultBoard return default board and ensure only one exists @@ -316,3 +310,12 @@ func UpdateBoardSorting(ctx context.Context, bs BoardList) error { return nil }) } + +func GetColumnsByIDs(ctx context.Context, columnsIDs []int64) (BoardList, error) { + columns := make([]*Board, 0, 5) + if err := db.GetEngine(ctx).In("id", columnsIDs).OrderBy("sorting").Find(&columns); err != nil { + return nil, err + } + + return columns, nil +} diff --git a/models/project/issue.go b/models/project/issue.go index ebc9719de55d..95d0853aa542 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -106,3 +106,30 @@ func (b *Board) removeIssues(ctx context.Context) error { _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = 0 WHERE project_board_id = ? ", b.ID) return err } + +// MoveColumnsOnProject moves or keeps issues in a column and sorts them inside that column +func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) + + columnIDs := make([]int64, 0, len(sortedColumnIDs)) + for _, columnID := range sortedColumnIDs { + columnIDs = append(columnIDs, columnID) + } + count, err := sess.Table(new(Board)).Where("project_id=?", project.ID).In("id", columnIDs).Count() + if err != nil { + return err + } + if int(count) != len(sortedColumnIDs) { + return fmt.Errorf("all issues have to be added to a project first") + } + + for sorting, columnID := range sortedColumnIDs { + _, err = sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID) + if err != nil { + return err + } + } + return nil + }) +} diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 7f78d1c830b7..1b0ea613afcf 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -678,3 +678,66 @@ func MoveIssues(ctx *context.Context) { ctx.JSONOK() } + +// MoveColumns moves or keeps columns in a project and sorts them inside that project +func MoveColumns(ctx *context.Context) { + if ctx.Doer == nil { + ctx.JSON(http.StatusForbidden, map[string]string{ + "message": "Only signed in users are allowed to perform this action.", + }) + return + } + + project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) + if err != nil { + ctx.NotFoundOrServerError("GetProjectByID", project_model.IsErrProjectNotExist, err) + return + } + if project.OwnerID != ctx.ContextUser.ID { + ctx.NotFound("InvalidRepoID", nil) + return + } + + type movedColumnsForm struct { + Columns []struct { + ColumnID int64 `json:"columnID"` + Sorting int64 `json:"sorting"` + } `json:"columns"` + } + + form := &movedColumnsForm{} + if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { + ctx.ServerError("DecodeMovedColumnsForm", err) + } + + columnIDs := make([]int64, 0, len(form.Columns)) + sortedColumnIDs := make(map[int64]int64) + for _, column := range form.Columns { + columnIDs = append(columnIDs, column.ColumnID) + sortedColumnIDs[column.Sorting] = column.ColumnID + } + movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) + if err != nil { + ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) + return + } + + if len(movedColumns) != len(form.Columns) { + ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) + return + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) + return + } + } + + if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { + ctx.ServerError("MoveColumnsOnProject", err) + return + } + + ctx.JSONOK() +} diff --git a/routers/web/web.go b/routers/web/web.go index c6132f0d61b9..3aa1b8aed084 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -999,6 +999,7 @@ func registerRoutes(m *web.Route) { m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), org.AddBoardToProjectPost) + m.Post("/move", org.MoveColumns) m.Post("/delete", org.DeleteProject) m.Get("/edit", org.RenderEditProject) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 3e000660e285..47f214a44e0d 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -64,7 +64,7 @@
-
+
{{range .Columns}}
diff --git a/web_src/js/features/repo-projects.js b/web_src/js/features/repo-projects.js index a869c24c823a..0ea90bcebdd4 100644 --- a/web_src/js/features/repo-projects.js +++ b/web_src/js/features/repo-projects.js @@ -63,17 +63,20 @@ async function initRepoProjectSortable() { delay: 500, onSort: async () => { boardColumns = mainBoard.getElementsByClassName('project-column'); - for (let i = 0; i < boardColumns.length; i++) { - const column = boardColumns[i]; - if (parseInt(column.getAttribute('data-sorting')) !== i) { - try { - const bgColor = column.style.backgroundColor; // will be rgb() string - const color = bgColor ? tinycolor(bgColor).toHexString() : ''; - await PUT(column.getAttribute('data-url'), {data: {sorting: i, color}}); - } catch (error) { - console.error(error); - } - } + + const columnSorting = { + columns: Array.from(boardColumns, (column, i) => ({ + columnID: parseInt(column.getAttribute('data-id')), + sorting: i, + })), + }; + + try { + await POST(`${mainBoard.getAttribute('data-url')}`, { + data: columnSorting, + }); + } catch (error) { + console.error(error); } }, }); From 50a573f75387d5d3bc59efd0809ee1c532c90bae Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Apr 2024 11:53:46 +0800 Subject: [PATCH 02/49] Fix lint --- web_src/js/features/repo-projects.js | 1 - 1 file changed, 1 deletion(-) diff --git a/web_src/js/features/repo-projects.js b/web_src/js/features/repo-projects.js index 0ea90bcebdd4..a857e94015e0 100644 --- a/web_src/js/features/repo-projects.js +++ b/web_src/js/features/repo-projects.js @@ -2,7 +2,6 @@ import $ from 'jquery'; import {contrastColor} from '../utils/color.js'; import {createSortable} from '../modules/sortable.js'; import {POST, DELETE, PUT} from '../modules/fetch.js'; -import tinycolor from 'tinycolor2'; function updateIssueCount(cards) { const parent = cards.parentElement; From 753608a6cf1edfb3b9c4798af241976545283d68 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Apr 2024 12:01:07 +0800 Subject: [PATCH 03/49] Use star as defult column's icon but not a pin and add a star icon on title of column --- templates/projects/view.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 47f214a44e0d..8e4f0e794bed 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -72,7 +72,7 @@
{{.NumIssues ctx}}
- {{.Title}} + {{.Title}}{{if .Default}}{{svg "octicon-star"}}{{end}}
{{if $canWriteProject}}
@@ -90,7 +90,7 @@ data-modal-default-project-column-header="{{ctx.Locale.Tr "repo.projects.column.set_default"}}" data-modal-default-project-column-content="{{ctx.Locale.Tr "repo.projects.column.set_default_desc"}}" data-url="{{$.Link}}/{{.ID}}/default"> - {{svg "octicon-pin"}} + {{svg "octicon-star"}} {{ctx.Locale.Tr "repo.projects.column.set_default"}} Date: Thu, 25 Apr 2024 14:11:06 +0800 Subject: [PATCH 04/49] Fix repository project sorting --- models/db/engine.go | 1 + models/project/board.go | 9 +++++ routers/web/repo/projects.go | 67 ++++++++++++++++++++++++++++++++++++ routers/web/web.go | 2 ++ 4 files changed, 79 insertions(+) diff --git a/models/db/engine.go b/models/db/engine.go index 25f4066ea138..847ba58c2675 100755 --- a/models/db/engine.go +++ b/models/db/engine.go @@ -57,6 +57,7 @@ type Engine interface { SumInt(bean any, columnName string) (res int64, err error) Sync(...any) error Select(string) *xorm.Session + SetExpr(string, any) *xorm.Session NotIn(string, ...any) *xorm.Session OrderBy(any, ...any) *xorm.Session Exist(...any) (bool, error) diff --git a/models/project/board.go b/models/project/board.go index aa995caa804c..dc4d0561e05a 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -156,6 +156,15 @@ func NewBoard(ctx context.Context, board *Board) error { return fmt.Errorf("bad color code: %s", board.Color) } + var maxSorting int8 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { + return err + } + if maxSorting > 0 { + board.Sorting = maxSorting + } + _, err := db.GetEngine(ctx).Insert(board) return err } diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 9b765e89e877..63473e23b08d 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -666,3 +666,70 @@ func MoveIssues(ctx *context.Context) { ctx.JSONOK() } + +// MoveColumns moves or keeps columns in a project and sorts them inside that project +func MoveColumns(ctx *context.Context) { + if ctx.Doer == nil { + ctx.JSON(http.StatusForbidden, map[string]string{ + "message": "Only signed in users are allowed to perform this action.", + }) + return + } + + project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) + if err != nil { + if project_model.IsErrProjectNotExist(err) { + ctx.NotFound("ProjectNotExist", nil) + } else { + ctx.ServerError("GetProjectByID", err) + } + return + } + if project.RepoID != ctx.Repo.Repository.ID { + ctx.NotFound("InvalidRepoID", nil) + return + } + + type movedColumnsForm struct { + Columns []struct { + ColumnID int64 `json:"columnID"` + Sorting int64 `json:"sorting"` + } `json:"columns"` + } + + form := &movedColumnsForm{} + if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { + ctx.ServerError("DecodeMovedColumnsForm", err) + } + + columnIDs := make([]int64, 0, len(form.Columns)) + sortedColumnIDs := make(map[int64]int64) + for _, column := range form.Columns { + columnIDs = append(columnIDs, column.ColumnID) + sortedColumnIDs[column.Sorting] = column.ColumnID + } + movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) + if err != nil { + ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) + return + } + + if len(movedColumns) != len(form.Columns) { + ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) + return + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) + return + } + } + + if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { + ctx.ServerError("MoveColumnsOnProject", err) + return + } + + ctx.JSONOK() +} diff --git a/routers/web/web.go b/routers/web/web.go index 3aa1b8aed084..10670ea68c78 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1351,10 +1351,12 @@ func registerRoutes(m *web.Route) { m.Get("", repo.Projects) m.Get("/{id}", repo.ViewProject) m.Group("", func() { //nolint:dupl + m.Get("/new", repo.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), repo.AddBoardToProjectPost) + m.Post("/move", repo.MoveColumns) m.Post("/delete", repo.DeleteProject) m.Get("/edit", repo.RenderEditProject) From 42bfa04a0471001bf52f9ca0ec89429a27c34670 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Apr 2024 14:14:52 +0800 Subject: [PATCH 05/49] Fix lint --- models/project/board.go | 2 +- routers/web/web.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/models/project/board.go b/models/project/board.go index dc4d0561e05a..47a330ebb9c9 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -158,7 +158,7 @@ func NewBoard(ctx context.Context, board *Board) error { var maxSorting int8 if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { + Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { return err } if maxSorting > 0 { diff --git a/routers/web/web.go b/routers/web/web.go index 10670ea68c78..4abacbe4d7cf 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1351,7 +1351,6 @@ func registerRoutes(m *web.Route) { m.Get("", repo.Projects) m.Get("/{id}", repo.ViewProject) m.Group("", func() { //nolint:dupl - m.Get("/new", repo.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { From 715ec4cebb3ba88b68534734692cc50f721cca90 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 25 Apr 2024 21:48:47 +0800 Subject: [PATCH 06/49] Update web_src/js/features/repo-projects.js Co-authored-by: silverwind --- web_src/js/features/repo-projects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web_src/js/features/repo-projects.js b/web_src/js/features/repo-projects.js index a857e94015e0..a1cc4b346bb8 100644 --- a/web_src/js/features/repo-projects.js +++ b/web_src/js/features/repo-projects.js @@ -71,7 +71,7 @@ async function initRepoProjectSortable() { }; try { - await POST(`${mainBoard.getAttribute('data-url')}`, { + await POST(mainBoard.getAttribute('data-url'), { data: columnSorting, }); } catch (error) { From 0610385dbd5d9e8a6262369f3dbd45e7ebdc43b2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Apr 2024 09:33:25 +0800 Subject: [PATCH 07/49] Update templates/projects/view.tmpl Co-authored-by: silverwind --- templates/projects/view.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 8e4f0e794bed..200c1819eafd 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -72,7 +72,7 @@
{{.NumIssues ctx}}
- {{.Title}}{{if .Default}}{{svg "octicon-star"}}{{end}} + {{.Title}}{{if .Default}}{{svg "octicon-star" 16 "tw-ml-2"}}{{end}}
{{if $canWriteProject}}
From 1c67fe6f5fb1c07a52ff4a87c4fe341093218826 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Apr 2024 16:34:55 +0800 Subject: [PATCH 08/49] Remove default column icon --- templates/projects/view.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 8e4f0e794bed..91ddf3812702 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -72,7 +72,7 @@
{{.NumIssues ctx}}
- {{.Title}}{{if .Default}}{{svg "octicon-star"}}{{end}} + {{.Title}}
{{if $canWriteProject}}
From 0613d7f8d5d3c1f6ec784d8716fac22f4e7da1dc Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Apr 2024 17:14:09 +0800 Subject: [PATCH 09/49] Add the issue to default column when adding to a project --- models/issues/issue_project.go | 42 +++++++++++++++++++++------------- models/project/board.go | 16 ++++++++++--- models/project/issue.go | 6 ++--- routers/web/org/projects.go | 17 +++++++++++++- routers/web/repo/projects.go | 17 +++++++++++++- routers/web/repo/pull.go | 15 ++++++++++-- services/issue/issue.go | 10 +++++++- 7 files changed, 96 insertions(+), 27 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 907a5a17b9f2..35b6810786b6 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -90,22 +90,16 @@ func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (m return issuesMap, nil } -// ChangeProjectAssign changes the project associated with an issue -func ChangeProjectAssign(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - if err := addUpdateIssueProject(ctx, issue, doer, newProjectID); err != nil { - return err - } - - return committer.Commit() +// ChangeProjectAssign changes the project associated with an issue, if newProjectID is 0, the issue is removed from the project +func ChangeProjectAssign(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + return addUpdateIssueProject(ctx, issue, doer, newProjectID, newColumnID) + }) } -func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID int64) error { +// addUpdateIssueProject adds or updates the project the default column associated with an issue +// If newProjectID is 0, the issue is removed from the project +func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { oldProjectID := issue.projectID(ctx) if err := issue.LoadRepo(ctx); err != nil { @@ -139,9 +133,25 @@ func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.U return err } } + if newProjectID == 0 || newColumnID == 0 { + return nil + } + + var maxSorting int64 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + Where("project_id=?", newProjectID). + And("project_board_id=?", newColumnID). + Get(&maxSorting); err != nil { + return err + } + if maxSorting > 0 { + maxSorting++ + } return db.Insert(ctx, &project_model.ProjectIssue{ - IssueID: issue.ID, - ProjectID: newProjectID, + IssueID: issue.ID, + ProjectID: newProjectID, + ProjectBoardID: newColumnID, + Sorting: maxSorting, }) } diff --git a/models/project/board.go b/models/project/board.go index 47a330ebb9c9..cb24aafb5108 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -198,7 +198,17 @@ func deleteBoardByID(ctx context.Context, boardID int64) error { return fmt.Errorf("deleteBoardByID: cannot delete default board") } - if err = board.removeIssues(ctx); err != nil { + // move all issues to the default column + project, err := GetProjectByID(ctx, board.ProjectID) + if err != nil { + return err + } + defaultBoard, err := project.GetDefaultBoard(ctx) + if err != nil { + return err + } + + if err = board.moveIssuesToDefault(ctx, defaultBoard.ID); err != nil { return err } @@ -258,8 +268,8 @@ func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { return boards, nil } -// getDefaultBoard return default board and ensure only one exists -func (p *Project) getDefaultBoard(ctx context.Context) (*Board, error) { +// GetDefaultBoard return default board and ensure only one exists +func (p *Project) GetDefaultBoard(ctx context.Context) (*Board, error) { var board Board has, err := db.GetEngine(ctx). Where("project_id=? AND `default` = ?", p.ID, true). diff --git a/models/project/issue.go b/models/project/issue.go index 95d0853aa542..faa3819e2d6d 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -17,7 +17,7 @@ type ProjectIssue struct { //revive:disable-line:exported IssueID int64 `xorm:"INDEX"` ProjectID int64 `xorm:"INDEX"` - // If 0, then it has not been added to a specific board in the project + // If this should not be zero from 1.22. If it's zero, it will not be displayed on UI and maybe result in errors. ProjectBoardID int64 `xorm:"INDEX"` // the sorting order on the board @@ -102,8 +102,8 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs }) } -func (b *Board) removeIssues(ctx context.Context) error { - _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = 0 WHERE project_board_id = ? ", b.ID) +func (b *Board) moveIssuesToDefault(ctx context.Context, defaultBoardID int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", defaultBoardID, b.ID) return err } diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 1b0ea613afcf..be045a2212ed 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -442,6 +442,21 @@ func UpdateIssueProject(ctx *context.Context) { } projectID := ctx.FormInt64("id") + var dstColumnID int64 + if projectID > 0 { + dstProject, err := project_model.GetProjectByID(ctx, projectID) + if err != nil { + ctx.ServerError("GetProjectByID", err) + return + } + dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) + if err != nil { + ctx.ServerError("GetDefaultBoard", err) + return + } + dstColumnID = dstDefaultColumn.ID + } + for _, issue := range issues { if issue.Project != nil { if issue.Project.ID == projectID { @@ -449,7 +464,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { + if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 63473e23b08d..13df083deed5 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -385,6 +385,21 @@ func UpdateIssueProject(ctx *context.Context) { } projectID := ctx.FormInt64("id") + var dstColumnID int64 + if projectID > 0 { + dstProject, err := project_model.GetProjectByID(ctx, projectID) + if err != nil { + ctx.ServerError("GetProjectByID", err) + return + } + dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) + if err != nil { + ctx.ServerError("GetDefaultBoard", err) + return + } + dstColumnID = dstDefaultColumn.ID + } + for _, issue := range issues { if issue.Project != nil { if issue.Project.ID == projectID { @@ -392,7 +407,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID); err != nil { + if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index acdba4bcdc02..9937bbdceb95 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -20,6 +20,7 @@ import ( git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" access_model "code.gitea.io/gitea/models/perm/access" + project_model "code.gitea.io/gitea/models/project" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -1332,10 +1333,20 @@ func CompareAndPullRequestPost(ctx *context.Context) { if projectID > 0 { if !ctx.Repo.CanWrite(unit.TypeProjects) { - ctx.Error(http.StatusBadRequest, "user hasn't the permission to write to projects") + log.Error("user hasn't the permission to write to projects") return } - if err := issues_model.ChangeProjectAssign(ctx, pullIssue, ctx.Doer, projectID); err != nil { + dstProject, err := project_model.GetProjectByID(ctx, projectID) + if err != nil { + ctx.ServerError("GetProjectByID", err) + return + } + dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) + if err != nil { + ctx.ServerError("GetDefaultBoard", err) + return + } + if err := issues_model.ChangeProjectAssign(ctx, pullIssue, ctx.Doer, projectID, dstDefaultColumn.ID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/services/issue/issue.go b/services/issue/issue.go index c7fa9f3300a5..d43a1d599a7c 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -42,7 +42,15 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo } } if projectID > 0 { - if err := issues_model.ChangeProjectAssign(ctx, issue, issue.Poster, projectID); err != nil { + project, err := project_model.GetProjectByID(ctx, projectID) + if err != nil { + return err + } + defaultBoard, err := project.GetDefaultBoard(ctx) + if err != nil { + return err + } + if err := issues_model.ChangeProjectAssign(ctx, issue, issue.Poster, projectID, defaultBoard.ID); err != nil { return err } } From 53aea60bbc7288d61cda4b35bd8b1333b8a6e5c0 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 26 Apr 2024 22:07:23 +0800 Subject: [PATCH 10/49] Fix lint --- models/project/board_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/project/board_test.go b/models/project/board_test.go index 71ba29a5896d..8d954d89a3ee 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -19,7 +19,7 @@ func TestGetDefaultBoard(t *testing.T) { assert.NoError(t, err) // check if default board was added - board, err := projectWithoutDefault.getDefaultBoard(db.DefaultContext) + board, err := projectWithoutDefault.GetDefaultBoard(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(5), board.ProjectID) assert.Equal(t, "Uncategorized", board.Title) @@ -28,7 +28,7 @@ func TestGetDefaultBoard(t *testing.T) { assert.NoError(t, err) // check if multiple defaults were removed - board, err = projectWithMultipleDefaults.getDefaultBoard(db.DefaultContext) + board, err = projectWithMultipleDefaults.GetDefaultBoard(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), board.ProjectID) assert.Equal(t, int64(9), board.ID) From d300bfdc9c5e0b23bb6667590e7bd9d1c57f4b92 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 27 Apr 2024 21:00:21 +0800 Subject: [PATCH 11/49] Follow maintainers' suggestions --- routers/web/org/projects.go | 63 ----------------------- routers/web/repo/projects.go | 67 ------------------------- routers/web/repo/pull.go | 3 +- routers/web/shared/project/column.go | 74 ++++++++++++++++++++++++++++ routers/web/web.go | 5 +- templates/projects/view.tmpl | 2 +- 6 files changed, 79 insertions(+), 135 deletions(-) create mode 100644 routers/web/shared/project/column.go diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index be045a2212ed..bc313f659360 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -693,66 +693,3 @@ func MoveIssues(ctx *context.Context) { ctx.JSONOK() } - -// MoveColumns moves or keeps columns in a project and sorts them inside that project -func MoveColumns(ctx *context.Context) { - if ctx.Doer == nil { - ctx.JSON(http.StatusForbidden, map[string]string{ - "message": "Only signed in users are allowed to perform this action.", - }) - return - } - - project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) - if err != nil { - ctx.NotFoundOrServerError("GetProjectByID", project_model.IsErrProjectNotExist, err) - return - } - if project.OwnerID != ctx.ContextUser.ID { - ctx.NotFound("InvalidRepoID", nil) - return - } - - type movedColumnsForm struct { - Columns []struct { - ColumnID int64 `json:"columnID"` - Sorting int64 `json:"sorting"` - } `json:"columns"` - } - - form := &movedColumnsForm{} - if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { - ctx.ServerError("DecodeMovedColumnsForm", err) - } - - columnIDs := make([]int64, 0, len(form.Columns)) - sortedColumnIDs := make(map[int64]int64) - for _, column := range form.Columns { - columnIDs = append(columnIDs, column.ColumnID) - sortedColumnIDs[column.Sorting] = column.ColumnID - } - movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) - if err != nil { - ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) - return - } - - if len(movedColumns) != len(form.Columns) { - ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) - return - } - - for _, column := range movedColumns { - if column.ProjectID != project.ID { - ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) - return - } - } - - if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { - ctx.ServerError("MoveColumnsOnProject", err) - return - } - - ctx.JSONOK() -} diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 13df083deed5..4c03f436f62c 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -681,70 +681,3 @@ func MoveIssues(ctx *context.Context) { ctx.JSONOK() } - -// MoveColumns moves or keeps columns in a project and sorts them inside that project -func MoveColumns(ctx *context.Context) { - if ctx.Doer == nil { - ctx.JSON(http.StatusForbidden, map[string]string{ - "message": "Only signed in users are allowed to perform this action.", - }) - return - } - - project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) - if err != nil { - if project_model.IsErrProjectNotExist(err) { - ctx.NotFound("ProjectNotExist", nil) - } else { - ctx.ServerError("GetProjectByID", err) - } - return - } - if project.RepoID != ctx.Repo.Repository.ID { - ctx.NotFound("InvalidRepoID", nil) - return - } - - type movedColumnsForm struct { - Columns []struct { - ColumnID int64 `json:"columnID"` - Sorting int64 `json:"sorting"` - } `json:"columns"` - } - - form := &movedColumnsForm{} - if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { - ctx.ServerError("DecodeMovedColumnsForm", err) - } - - columnIDs := make([]int64, 0, len(form.Columns)) - sortedColumnIDs := make(map[int64]int64) - for _, column := range form.Columns { - columnIDs = append(columnIDs, column.ColumnID) - sortedColumnIDs[column.Sorting] = column.ColumnID - } - movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) - if err != nil { - ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) - return - } - - if len(movedColumns) != len(form.Columns) { - ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) - return - } - - for _, column := range movedColumns { - if column.ProjectID != project.ID { - ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) - return - } - } - - if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { - ctx.ServerError("MoveColumnsOnProject", err) - return - } - - ctx.JSONOK() -} diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 06bfa010f89f..05c2a6458c46 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1327,12 +1327,11 @@ func CompareAndPullRequestPost(ctx *context.Context) { } ctx.JSONError(flashError) } - return } if projectID > 0 { if !ctx.Repo.CanWrite(unit.TypeProjects) { - log.Error("user hasn't the permission to write to projects") + ctx.Error(http.StatusBadRequest, "user hasn't the permission to write to projects") return } dstProject, err := project_model.GetProjectByID(ctx, projectID) diff --git a/routers/web/shared/project/column.go b/routers/web/shared/project/column.go new file mode 100644 index 000000000000..5fde05f0f76c --- /dev/null +++ b/routers/web/shared/project/column.go @@ -0,0 +1,74 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package project + +import ( + "errors" + + issues_model "code.gitea.io/gitea/models/issues" + project_model "code.gitea.io/gitea/models/project" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/services/context" +) + +// MoveColumns moves or keeps columns in a project and sorts them inside that project +func MoveColumns(ctx *context.Context) { + project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) + if err != nil { + ctx.NotFoundOrServerError("GetProjectByID", project_model.IsErrProjectNotExist, err) + return + } + if project.OwnerID > 0 && project.OwnerID != ctx.ContextUser.ID { + ctx.NotFound("InvalidOwnerID", nil) + return + } + if project.RepoID > 0 && project.RepoID != ctx.Repo.Repository.ID { + ctx.NotFound("InvalidRepoID", nil) + return + } + + type movedColumnsForm struct { + Columns []struct { + ColumnID int64 `json:"columnID"` + Sorting int64 `json:"sorting"` + } `json:"columns"` + } + + form := &movedColumnsForm{} + if err = json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil { + ctx.ServerError("DecodeMovedColumnsForm", err) + return + } + + columnIDs := make([]int64, 0, len(form.Columns)) + sortedColumnIDs := make(map[int64]int64) + for _, column := range form.Columns { + columnIDs = append(columnIDs, column.ColumnID) + sortedColumnIDs[column.Sorting] = column.ColumnID + } + movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) + if err != nil { + ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) + return + } + + if len(movedColumns) != len(form.Columns) { + ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) + return + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) + return + } + } + + if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { + ctx.ServerError("MoveColumnsOnProject", err) + return + } + + ctx.JSONOK() +} diff --git a/routers/web/web.go b/routers/web/web.go index 619bca711240..f8d793510d0d 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -37,6 +37,7 @@ import ( "code.gitea.io/gitea/routers/web/repo" "code.gitea.io/gitea/routers/web/repo/actions" repo_setting "code.gitea.io/gitea/routers/web/repo/setting" + "code.gitea.io/gitea/routers/web/shared/project" "code.gitea.io/gitea/routers/web/user" user_setting "code.gitea.io/gitea/routers/web/user/setting" "code.gitea.io/gitea/routers/web/user/setting/security" @@ -999,7 +1000,7 @@ func registerRoutes(m *web.Route) { m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), org.AddBoardToProjectPost) - m.Post("/move", org.MoveColumns) + m.Post("/move", project.MoveColumns) m.Post("/delete", org.DeleteProject) m.Get("/edit", org.RenderEditProject) @@ -1355,7 +1356,7 @@ func registerRoutes(m *web.Route) { m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) m.Group("/{id}", func() { m.Post("", web.Bind(forms.EditProjectBoardForm{}), repo.AddBoardToProjectPost) - m.Post("/move", repo.MoveColumns) + m.Post("/move", project.MoveColumns) m.Post("/delete", repo.DeleteProject) m.Get("/edit", repo.RenderEditProject) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index 91ddf3812702..47f214a44e0d 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -90,7 +90,7 @@ data-modal-default-project-column-header="{{ctx.Locale.Tr "repo.projects.column.set_default"}}" data-modal-default-project-column-content="{{ctx.Locale.Tr "repo.projects.column.set_default_desc"}}" data-url="{{$.Link}}/{{.ID}}/default"> - {{svg "octicon-star"}} + {{svg "octicon-pin"}} {{ctx.Locale.Tr "repo.projects.column.set_default"}} Date: Sat, 27 Apr 2024 21:02:16 +0800 Subject: [PATCH 12/49] Update models/project/issue.go Co-authored-by: wxiaoguang --- models/project/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/issue.go b/models/project/issue.go index faa3819e2d6d..10b6bc425112 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -17,7 +17,7 @@ type ProjectIssue struct { //revive:disable-line:exported IssueID int64 `xorm:"INDEX"` ProjectID int64 `xorm:"INDEX"` - // If this should not be zero from 1.22. If it's zero, it will not be displayed on UI and maybe result in errors. + // ProjectBoardID should not be zero since 1.22. If it's zero, the issue will not be displayed on UI and it might result in errors. ProjectBoardID int64 `xorm:"INDEX"` // the sorting order on the board From c2158063c11e43c208db481a3f424362e8d291d8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 28 Apr 2024 15:17:03 +0800 Subject: [PATCH 13/49] Fix bug --- models/project/board.go | 2 +- routers/web/repo/projects.go | 10 ++++++++++ routers/web/repo/pull.go | 4 ++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/models/project/board.go b/models/project/board.go index cb24aafb5108..86c95af9143d 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -162,7 +162,7 @@ func NewBoard(ctx context.Context, board *Board) error { return err } if maxSorting > 0 { - board.Sorting = maxSorting + board.Sorting = maxSorting + 1 } _, err := db.GetEngine(ctx).Insert(board) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 4c03f436f62c..6cd932d76a72 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -383,6 +383,10 @@ func UpdateIssueProject(ctx *context.Context) { ctx.ServerError("LoadProjects", err) return } + if _, err := issues.LoadRepositories(ctx); err != nil { + ctx.ServerError("LoadProjects", err) + return + } projectID := ctx.FormInt64("id") var dstColumnID int64 @@ -392,6 +396,12 @@ func UpdateIssueProject(ctx *context.Context) { ctx.ServerError("GetProjectByID", err) return } + for _, issue := range issues { + if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != issue.Repo.OwnerID { + ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") + return + } + } dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) if err != nil { ctx.ServerError("GetDefaultBoard", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 05c2a6458c46..bdc33acb5001 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1339,6 +1339,10 @@ func CompareAndPullRequestPost(ctx *context.Context) { ctx.ServerError("GetProjectByID", err) return } + if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != repo.OwnerID { + ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") + return + } dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) if err != nil { ctx.ServerError("GetDefaultBoard", err) From 6844f6f43f7ff4ff96198e6b3e0745c83c139918 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 28 Apr 2024 15:22:29 +0800 Subject: [PATCH 14/49] Fix function name --- models/project/board.go | 4 ++-- models/project/issue.go | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/models/project/board.go b/models/project/board.go index 86c95af9143d..7898d3dd6a90 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -203,12 +203,12 @@ func deleteBoardByID(ctx context.Context, boardID int64) error { if err != nil { return err } - defaultBoard, err := project.GetDefaultBoard(ctx) + defaultColumn, err := project.GetDefaultBoard(ctx) if err != nil { return err } - if err = board.moveIssuesToDefault(ctx, defaultBoard.ID); err != nil { + if err = board.moveIssuesToAnotherColumn(ctx, defaultColumn.ID); err != nil { return err } diff --git a/models/project/issue.go b/models/project/issue.go index 10b6bc425112..ad36f1e1b175 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -102,8 +102,11 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs }) } -func (b *Board) moveIssuesToDefault(ctx context.Context, defaultBoardID int64) error { - _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", defaultBoardID, b.ID) +func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, columnID int64) error { + if b.ID == columnID { + return nil + } + _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", columnID, b.ID) return err } From 235c9d4fd0155a4ae4d943534ac1a4daad682854 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 28 Apr 2024 15:28:06 +0800 Subject: [PATCH 15/49] Fix bug --- routers/web/org/projects.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index bc313f659360..7e26ffbc0080 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -449,6 +449,13 @@ func UpdateIssueProject(ctx *context.Context) { ctx.ServerError("GetProjectByID", err) return } + if dstProject.OwnerID != ctx.ContextUser.ID { + ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ + "message": fmt.Sprintf("Project[%d] is not in Owner[%d] as expected", dstProject.ID, ctx.ContextUser.ID), + }) + return + } + dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) if err != nil { ctx.ServerError("GetDefaultBoard", err) From cca2e9523ae9fa094124ab1b975f7d8cc4404afe Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 29 Apr 2024 13:51:45 +0800 Subject: [PATCH 16/49] Follow delvh's suggestion --- models/project/board.go | 10 ++++---- models/project/issue.go | 34 ++++++++++++++++------------ modules/container/map.go | 14 ++++++++++++ routers/web/shared/project/column.go | 23 +------------------ 4 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 modules/container/map.go diff --git a/models/project/board.go b/models/project/board.go index 7898d3dd6a90..e45a0ba676bb 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -208,7 +208,7 @@ func deleteBoardByID(ctx context.Context, boardID int64) error { return err } - if err = board.moveIssuesToAnotherColumn(ctx, defaultColumn.ID); err != nil { + if err = board.moveIssuesToAnotherColumn(ctx, defaultColumn); err != nil { return err } @@ -330,11 +330,13 @@ func UpdateBoardSorting(ctx context.Context, bs BoardList) error { }) } -func GetColumnsByIDs(ctx context.Context, columnsIDs []int64) (BoardList, error) { +func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (BoardList, error) { columns := make([]*Board, 0, 5) - if err := db.GetEngine(ctx).In("id", columnsIDs).OrderBy("sorting").Find(&columns); err != nil { + if err := db.GetEngine(ctx). + Where("project_id =?", projectID). + In("id", columnsIDs). + OrderBy("sorting").Find(&columns); err != nil { return nil, err } - return columns, nil } diff --git a/models/project/issue.go b/models/project/issue.go index ad36f1e1b175..920fe65ed61a 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -5,9 +5,11 @@ package project import ( "context" + "errors" "fmt" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" ) @@ -102,34 +104,38 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs }) } -func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, columnID int64) error { - if b.ID == columnID { +func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) error { + if b.ID == newColumn.ID { return nil } - _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", columnID, b.ID) + if b.ProjectID != newColumn.ProjectID { + return fmt.Errorf("columns have to be in the same project") + } + _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", newColumn.ID, b.ID) return err } -// MoveColumnsOnProject moves or keeps issues in a column and sorts them inside that column +// MoveColumnsOnProject moves or keeps column in a project and sorts them inside that column func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { return db.WithTx(ctx, func(ctx context.Context) error { sess := db.GetEngine(ctx) - - columnIDs := make([]int64, 0, len(sortedColumnIDs)) - for _, columnID := range sortedColumnIDs { - columnIDs = append(columnIDs, columnID) - } - count, err := sess.Table(new(Board)).Where("project_id=?", project.ID).In("id", columnIDs).Count() + columnIDs := container.MapValues(sortedColumnIDs) + movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) if err != nil { return err } - if int(count) != len(sortedColumnIDs) { - return fmt.Errorf("all issues have to be added to a project first") + if len(movedColumns) != len(sortedColumnIDs) { + return errors.New("some columns do not exist") + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + return errors.New("Some column's projectID is not equal to project's ID") + } } for sorting, columnID := range sortedColumnIDs { - _, err = sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID) - if err != nil { + if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { return err } } diff --git a/modules/container/map.go b/modules/container/map.go new file mode 100644 index 000000000000..8fad8f4fbf11 --- /dev/null +++ b/modules/container/map.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package container + +// MapValues returns a slice of all values in a map. +// TODO: remove this after Golang std lib has similar function but not golang.org/x/exp +func MapValues[K comparable, V any](m map[K]V) []V { + values := make([]V, 0, len(m)) + for _, v := range m { + values = append(values, v) + } + return values +} diff --git a/routers/web/shared/project/column.go b/routers/web/shared/project/column.go index 5fde05f0f76c..9564ab9c4936 100644 --- a/routers/web/shared/project/column.go +++ b/routers/web/shared/project/column.go @@ -4,8 +4,6 @@ package project import ( - "errors" - issues_model "code.gitea.io/gitea/models/issues" project_model "code.gitea.io/gitea/models/project" "code.gitea.io/gitea/modules/json" @@ -41,32 +39,13 @@ func MoveColumns(ctx *context.Context) { return } - columnIDs := make([]int64, 0, len(form.Columns)) sortedColumnIDs := make(map[int64]int64) for _, column := range form.Columns { - columnIDs = append(columnIDs, column.ColumnID) sortedColumnIDs[column.Sorting] = column.ColumnID } - movedColumns, err := project_model.GetColumnsByIDs(ctx, columnIDs) - if err != nil { - ctx.NotFoundOrServerError("GetColumnsByIDs", issues_model.IsErrIssueNotExist, err) - return - } - - if len(movedColumns) != len(form.Columns) { - ctx.ServerError("some columns do not exist", errors.New("some columns do not exist")) - return - } - - for _, column := range movedColumns { - if column.ProjectID != project.ID { - ctx.ServerError("Some column's projectID is not equal to project's ID", errors.New("Some column's projectID is not equal to project's ID")) - return - } - } if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { - ctx.ServerError("MoveColumnsOnProject", err) + ctx.NotFoundOrServerError("MoveColumnsOnProject", issues_model.IsErrIssueNotExist, err) return } From b6fccc058103439981a6ffe483b168f053ca5392 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Apr 2024 09:58:25 +0800 Subject: [PATCH 17/49] Follow delvh's suggestion --- models/project/issue.go | 6 +++--- modules/container/map.go | 14 -------------- 2 files changed, 3 insertions(+), 17 deletions(-) delete mode 100644 modules/container/map.go diff --git a/models/project/issue.go b/models/project/issue.go index 920fe65ed61a..90143be2b9d6 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -9,8 +9,8 @@ import ( "fmt" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // ProjectIssue saves relation from issue to a project @@ -115,11 +115,11 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return err } -// MoveColumnsOnProject moves or keeps column in a project and sorts them inside that column +// MoveColumnsOnProject sorts columns in a project func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { return db.WithTx(ctx, func(ctx context.Context) error { sess := db.GetEngine(ctx) - columnIDs := container.MapValues(sortedColumnIDs) + columnIDs := util.ValuesOfMap(sortedColumnIDs) movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) if err != nil { return err diff --git a/modules/container/map.go b/modules/container/map.go deleted file mode 100644 index 8fad8f4fbf11..000000000000 --- a/modules/container/map.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -// MapValues returns a slice of all values in a map. -// TODO: remove this after Golang std lib has similar function but not golang.org/x/exp -func MapValues[K comparable, V any](m map[K]V) []V { - values := make([]V, 0, len(m)) - for _, v := range m { - values = append(values, v) - } - return values -} From 81d455fa6cd22994ee99eb819f13e2b10fc2e874 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Apr 2024 15:10:35 +0800 Subject: [PATCH 18/49] Use json return but not 500 directly --- routers/web/repo/projects.go | 32 ++++++++++++++++---------------- services/context/context.go | 12 ++++++++++++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 6cd932d76a72..f7de4c9b21a7 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -380,11 +380,11 @@ func UpdateIssueProject(ctx *context.Context) { } if err := issues.LoadProjects(ctx); err != nil { - ctx.ServerError("LoadProjects", err) + ctx.JSONServerError("Load projects failed", err) return } if _, err := issues.LoadRepositories(ctx); err != nil { - ctx.ServerError("LoadProjects", err) + ctx.JSONServerError("Load repositories failed", err) return } @@ -393,18 +393,18 @@ func UpdateIssueProject(ctx *context.Context) { if projectID > 0 { dstProject, err := project_model.GetProjectByID(ctx, projectID) if err != nil { - ctx.ServerError("GetProjectByID", err) + ctx.JSONServerError("Get project failed", err) return } for _, issue := range issues { if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != issue.Repo.OwnerID { - ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") + ctx.JSON(http.StatusBadRequest, "project doesn't belong to the repository") return } } dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) if err != nil { - ctx.ServerError("GetDefaultBoard", err) + ctx.JSONServerError("Get default board failed", err) return } dstColumnID = dstDefaultColumn.ID @@ -418,7 +418,7 @@ func UpdateIssueProject(ctx *context.Context) { } if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) + ctx.JSONServerError("Change project assign failed", err) return } } @@ -445,16 +445,16 @@ func DeleteProjectBoard(ctx *context.Context) { project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.NotFound("", nil) + ctx.JSONServerError("Project doesn't exist", err) } else { - ctx.ServerError("GetProjectByID", err) + ctx.JSONServerError("Get project failed", err) } return } pb, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.JSONServerError("Get project board failed", err) return } if pb.ProjectID != ctx.ParamsInt64(":id") { @@ -472,7 +472,7 @@ func DeleteProjectBoard(ctx *context.Context) { } if err := project_model.DeleteBoardByID(ctx, ctx.ParamsInt64(":boardID")); err != nil { - ctx.ServerError("DeleteProjectBoardByID", err) + ctx.JSONServerError("Delete project board failed", err) return } @@ -492,9 +492,9 @@ func AddBoardToProjectPost(ctx *context.Context) { project, err := project_model.GetProjectForRepoByID(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.NotFound("", nil) + ctx.JSONServerError("Project doesn't exist", err) } else { - ctx.ServerError("GetProjectByID", err) + ctx.JSONServerError("Get project failed", err) } return } @@ -505,7 +505,7 @@ func AddBoardToProjectPost(ctx *context.Context) { Color: form.Color, CreatorID: ctx.Doer.ID, }); err != nil { - ctx.ServerError("NewProjectBoard", err) + ctx.JSONServerError("create project board failed", err) return } @@ -530,16 +530,16 @@ func checkProjectBoardChangePermissions(ctx *context.Context) (*project_model.Pr project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.NotFound("", nil) + ctx.JSONServerError("Project doesn't exist", err) } else { - ctx.ServerError("GetProjectByID", err) + ctx.JSONServerError("Get project failed", err) } return nil, nil } board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) if err != nil { - ctx.ServerError("GetProjectBoard", err) + ctx.JSONServerError("Get project board failed", err) return nil, nil } if board.ProjectID != ctx.ParamsInt64(":id") { diff --git a/services/context/context.go b/services/context/context.go index aab0485f1a49..723d58d165a5 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" @@ -267,3 +268,14 @@ func (ctx *Context) JSONError(msg any) { panic(fmt.Sprintf("unsupported type: %T", msg)) } } + +func (ctx *Context) JSONServerError(msg string, err error) { + errorMessage := msg + if err != nil { + log.Log(1, log.ERROR, "%s: %v", msg, err) + if ctx.Doer != nil && ctx.Doer.IsAdmin { + errorMessage = fmt.Sprintf("%s: %v", errorMessage, err) + } + } + ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": errorMessage, "renderFormat": "text"}) +} From 03562c7ff70c9d51e5140b1f38eb6c62ad40663b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Apr 2024 15:21:38 +0800 Subject: [PATCH 19/49] Add limitation for a project's column to 20 --- models/project/board.go | 21 +++++++++++++++------ models/project/issue.go | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/models/project/board.go b/models/project/board.go index e45a0ba676bb..c577c0a3375d 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -150,22 +150,31 @@ func createBoardsForProjectsType(ctx context.Context, project *Project) error { return db.Insert(ctx, boards) } +// maxProjectColumns max columns allowed in a project, this should not bigger than 127 +// because sorting is int8 in database +const maxProjectColumns = 20 + // NewBoard adds a new project board to a given project func NewBoard(ctx context.Context, board *Board) error { if len(board.Color) != 0 && !BoardColorPattern.MatchString(board.Color) { return fmt.Errorf("bad color code: %s", board.Color) } - var maxSorting int8 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { + totalColumns, err := db.GetEngine(ctx).Table("project_board"). + Where("project_id=?", board.ProjectID).Count() + if err != nil { return err } - if maxSorting > 0 { - board.Sorting = maxSorting + 1 + + if totalColumns >= maxProjectColumns { + return fmt.Errorf("NewBoard: maximum number of columns reached") + } + + if totalColumns > 0 { + board.Sorting = int8(totalColumns) } - _, err := db.GetEngine(ctx).Insert(board) + _, err = db.GetEngine(ctx).Insert(board) return err } diff --git a/models/project/issue.go b/models/project/issue.go index 90143be2b9d6..11d83d88bc8d 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -130,7 +130,7 @@ func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs for _, column := range movedColumns { if column.ProjectID != project.ID { - return errors.New("Some column's projectID is not equal to project's ID") + return errors.New("some column's projectID is not equal to project's ID") } } From 24c2c13c418e501fdcd82e6b74b5d2c5a33b68d4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Apr 2024 15:24:56 +0800 Subject: [PATCH 20/49] Reuse method JSONError --- services/context/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/context/context.go b/services/context/context.go index 723d58d165a5..493fa14fd9e6 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -277,5 +277,5 @@ func (ctx *Context) JSONServerError(msg string, err error) { errorMessage = fmt.Sprintf("%s: %v", errorMessage, err) } } - ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": errorMessage, "renderFormat": "text"}) + ctx.JSONError(errorMessage) } From e3d553157a640cf2418fa7adb6fa50eddd4c066b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 30 Apr 2024 15:44:49 +0800 Subject: [PATCH 21/49] Fix possible sorting overflow --- models/project/board.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/models/project/board.go b/models/project/board.go index c577c0a3375d..781bce0b42c8 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -171,7 +171,14 @@ func NewBoard(ctx context.Context, board *Board) error { } if totalColumns > 0 { - board.Sorting = int8(totalColumns) + var maxSorting int8 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { + return err + } + if maxSorting > 0 { + board.Sorting = maxSorting + 1 + } } _, err = db.GetEngine(ctx).Insert(board) From db9362d73c7c2d2681dce4011258fa73ff8f92c2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 May 2024 14:23:46 +0800 Subject: [PATCH 22/49] rename PROJECT_BOARD_CHANGED -> PROJECT_COLUMN_CHANGED --- templates/repo/issue/view_content/comments.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index acc04e4c6151..05a34d772485 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -10,7 +10,7 @@ 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED, 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, - 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_BOARD_CHANGED + 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_COLUMN_CHANGED 32 = DISMISSED_REVIEW, 33 = COMMENT_TYPE_CHANGE_ISSUE_REF, 34 = PR_SCHEDULE_TO_AUTO_MERGE, 35 = CANCEL_SCHEDULED_AUTO_MERGE_PR, 36 = PIN_ISSUE, 37 = UNPIN_ISSUE --> {{if eq .Type 0}} From 2bb832ece1e526b0d8155b632962dd8d870badaf Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 May 2024 14:24:37 +0800 Subject: [PATCH 23/49] Revert "rename PROJECT_BOARD_CHANGED -> PROJECT_COLUMN_CHANGED" This reverts commit db9362d73c7c2d2681dce4011258fa73ff8f92c2. --- templates/repo/issue/view_content/comments.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 05a34d772485..acc04e4c6151 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -10,7 +10,7 @@ 18 = REMOVED_DEADLINE, 19 = ADD_DEPENDENCY, 20 = REMOVE_DEPENDENCY, 21 = CODE, 22 = REVIEW, 23 = ISSUE_LOCKED, 24 = ISSUE_UNLOCKED, 25 = TARGET_BRANCH_CHANGED, 26 = DELETE_TIME_MANUAL, 27 = REVIEW_REQUEST, 28 = MERGE_PULL_REQUEST, - 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_COLUMN_CHANGED + 29 = PULL_PUSH_EVENT, 30 = PROJECT_CHANGED, 31 = PROJECT_BOARD_CHANGED 32 = DISMISSED_REVIEW, 33 = COMMENT_TYPE_CHANGE_ISSUE_REF, 34 = PR_SCHEDULE_TO_AUTO_MERGE, 35 = CANCEL_SCHEDULED_AUTO_MERGE_PR, 36 = PIN_ISSUE, 37 = UNPIN_ISSUE --> {{if eq .Type 0}} From e9f58eb5f9ed664e3156692467918c6b1ca6093d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 May 2024 14:25:42 +0800 Subject: [PATCH 24/49] Revert "Reuse method JSONError" This reverts commit 24c2c13c418e501fdcd82e6b74b5d2c5a33b68d4. --- services/context/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/context/context.go b/services/context/context.go index 493fa14fd9e6..723d58d165a5 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -277,5 +277,5 @@ func (ctx *Context) JSONServerError(msg string, err error) { errorMessage = fmt.Sprintf("%s: %v", errorMessage, err) } } - ctx.JSONError(errorMessage) + ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": errorMessage, "renderFormat": "text"}) } From 85d9fe6f793ab8deec7cee1333d92965039b091e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 May 2024 14:27:45 +0800 Subject: [PATCH 25/49] Revert "Use json return but not 500 directly" This reverts commit 81d455fa6cd22994ee99eb819f13e2b10fc2e874. --- routers/web/repo/projects.go | 32 ++++++++++++++++---------------- services/context/context.go | 12 ------------ 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index f7de4c9b21a7..6cd932d76a72 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -380,11 +380,11 @@ func UpdateIssueProject(ctx *context.Context) { } if err := issues.LoadProjects(ctx); err != nil { - ctx.JSONServerError("Load projects failed", err) + ctx.ServerError("LoadProjects", err) return } if _, err := issues.LoadRepositories(ctx); err != nil { - ctx.JSONServerError("Load repositories failed", err) + ctx.ServerError("LoadProjects", err) return } @@ -393,18 +393,18 @@ func UpdateIssueProject(ctx *context.Context) { if projectID > 0 { dstProject, err := project_model.GetProjectByID(ctx, projectID) if err != nil { - ctx.JSONServerError("Get project failed", err) + ctx.ServerError("GetProjectByID", err) return } for _, issue := range issues { if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != issue.Repo.OwnerID { - ctx.JSON(http.StatusBadRequest, "project doesn't belong to the repository") + ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") return } } dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) if err != nil { - ctx.JSONServerError("Get default board failed", err) + ctx.ServerError("GetDefaultBoard", err) return } dstColumnID = dstDefaultColumn.ID @@ -418,7 +418,7 @@ func UpdateIssueProject(ctx *context.Context) { } if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { - ctx.JSONServerError("Change project assign failed", err) + ctx.ServerError("ChangeProjectAssign", err) return } } @@ -445,16 +445,16 @@ func DeleteProjectBoard(ctx *context.Context) { project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.JSONServerError("Project doesn't exist", err) + ctx.NotFound("", nil) } else { - ctx.JSONServerError("Get project failed", err) + ctx.ServerError("GetProjectByID", err) } return } pb, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) if err != nil { - ctx.JSONServerError("Get project board failed", err) + ctx.ServerError("GetProjectBoard", err) return } if pb.ProjectID != ctx.ParamsInt64(":id") { @@ -472,7 +472,7 @@ func DeleteProjectBoard(ctx *context.Context) { } if err := project_model.DeleteBoardByID(ctx, ctx.ParamsInt64(":boardID")); err != nil { - ctx.JSONServerError("Delete project board failed", err) + ctx.ServerError("DeleteProjectBoardByID", err) return } @@ -492,9 +492,9 @@ func AddBoardToProjectPost(ctx *context.Context) { project, err := project_model.GetProjectForRepoByID(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.JSONServerError("Project doesn't exist", err) + ctx.NotFound("", nil) } else { - ctx.JSONServerError("Get project failed", err) + ctx.ServerError("GetProjectByID", err) } return } @@ -505,7 +505,7 @@ func AddBoardToProjectPost(ctx *context.Context) { Color: form.Color, CreatorID: ctx.Doer.ID, }); err != nil { - ctx.JSONServerError("create project board failed", err) + ctx.ServerError("NewProjectBoard", err) return } @@ -530,16 +530,16 @@ func checkProjectBoardChangePermissions(ctx *context.Context) (*project_model.Pr project, err := project_model.GetProjectByID(ctx, ctx.ParamsInt64(":id")) if err != nil { if project_model.IsErrProjectNotExist(err) { - ctx.JSONServerError("Project doesn't exist", err) + ctx.NotFound("", nil) } else { - ctx.JSONServerError("Get project failed", err) + ctx.ServerError("GetProjectByID", err) } return nil, nil } board, err := project_model.GetBoard(ctx, ctx.ParamsInt64(":boardID")) if err != nil { - ctx.JSONServerError("Get project board failed", err) + ctx.ServerError("GetProjectBoard", err) return nil, nil } if board.ProjectID != ctx.ParamsInt64(":id") { diff --git a/services/context/context.go b/services/context/context.go index 723d58d165a5..aab0485f1a49 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" @@ -268,14 +267,3 @@ func (ctx *Context) JSONError(msg any) { panic(fmt.Sprintf("unsupported type: %T", msg)) } } - -func (ctx *Context) JSONServerError(msg string, err error) { - errorMessage := msg - if err != nil { - log.Log(1, log.ERROR, "%s: %v", msg, err) - if ctx.Doer != nil && ctx.Doer.IsAdmin { - errorMessage = fmt.Sprintf("%s: %v", errorMessage, err) - } - } - ctx.JSON(http.StatusBadRequest, map[string]any{"errorMessage": errorMessage, "renderFormat": "text"}) -} From 699c9e618764134fa318606be9449edfc2ec9724 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 2 May 2024 21:44:08 +0800 Subject: [PATCH 26/49] Follow yp05327's suggestion --- models/project/issue.go | 2 +- routers/web/shared/project/column.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/models/project/issue.go b/models/project/issue.go index 11d83d88bc8d..8175187458eb 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -130,7 +130,7 @@ func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs for _, column := range movedColumns { if column.ProjectID != project.ID { - return errors.New("some column's projectID is not equal to project's ID") + return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) } } diff --git a/routers/web/shared/project/column.go b/routers/web/shared/project/column.go index 9564ab9c4936..f20f8b3b19a6 100644 --- a/routers/web/shared/project/column.go +++ b/routers/web/shared/project/column.go @@ -4,7 +4,6 @@ package project import ( - issues_model "code.gitea.io/gitea/models/issues" project_model "code.gitea.io/gitea/models/project" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/services/context" @@ -45,7 +44,7 @@ func MoveColumns(ctx *context.Context) { } if err = project_model.MoveColumnsOnProject(ctx, project, sortedColumnIDs); err != nil { - ctx.NotFoundOrServerError("MoveColumnsOnProject", issues_model.IsErrIssueNotExist, err) + ctx.ServerError("MoveColumnsOnProject", err) return } From f4dd17289d15bfa1bb38c28c5891216323db4876 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 02:26:56 +0800 Subject: [PATCH 27/49] Add tests --- models/fixtures/project_board.yml | 3 ++ models/project/board.go | 40 ++++++++++++++++++++++ models/project/board_test.go | 56 +++++++++++++++++++++++++++++++ models/project/issue.go | 30 ----------------- models/repo/repo.go | 2 +- tests/integration/project_test.go | 38 +++++++++++++++++++++ 6 files changed, 138 insertions(+), 31 deletions(-) diff --git a/models/fixtures/project_board.yml b/models/fixtures/project_board.yml index 3293dea6edf4..1939c1d317c6 100644 --- a/models/fixtures/project_board.yml +++ b/models/fixtures/project_board.yml @@ -4,6 +4,7 @@ title: To Do creator_id: 2 default: true + sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -12,6 +13,7 @@ project_id: 1 title: In Progress creator_id: 2 + sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 @@ -20,6 +22,7 @@ project_id: 1 title: Done creator_id: 2 + sorting: 2 created_unix: 1588117528 updated_unix: 1588117528 diff --git a/models/project/board.go b/models/project/board.go index 781bce0b42c8..65a52d0a6a28 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -5,12 +5,14 @@ package project import ( "context" + "errors" "fmt" "regexp" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" ) @@ -82,6 +84,16 @@ func (b *Board) NumIssues(ctx context.Context) int { return int(c) } +func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { + issues := make([]*ProjectIssue, 0, 5) + if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID). + And("project_board_id=?", b.ID). + Find(&issues); err != nil { + return nil, err + } + return issues, nil +} + func init() { db.RegisterModel(new(Board)) } @@ -356,3 +368,31 @@ func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) ( } return columns, nil } + +// MoveColumnsOnProject sorts columns in a project +func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + sess := db.GetEngine(ctx) + columnIDs := util.ValuesOfMap(sortedColumnIDs) + movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) + if err != nil { + return err + } + if len(movedColumns) != len(sortedColumnIDs) { + return errors.New("some columns do not exist") + } + + for _, column := range movedColumns { + if column.ProjectID != project.ID { + return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) + } + } + + for sorting, columnID := range sortedColumnIDs { + if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { + return err + } + } + return nil + }) +} diff --git a/models/project/board_test.go b/models/project/board_test.go index 8d954d89a3ee..2841edb861b8 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -42,3 +42,59 @@ func TestGetDefaultBoard(t *testing.T) { assert.Equal(t, int64(6), board.ProjectID) assert.False(t, board.Default) } + +func Test_moveIssuesToAnotherColumn(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + column1 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 1, ProjectID: 1}) + + issues, err := column1.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 1) + assert.EqualValues(t, 1, issues[0].ID) + + column2 := unittest.AssertExistsAndLoadBean(t, &Board{ID: 2, ProjectID: 1}) + issues, err = column2.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 1) + assert.EqualValues(t, 3, issues[0].ID) + + err = column1.moveIssuesToAnotherColumn(db.DefaultContext, column2) + assert.NoError(t, err) + + issues, err = column1.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 0) + + issues, err = column2.GetIssues(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issues, 2) + assert.EqualValues(t, 1, issues[0].ID) + assert.EqualValues(t, 3, issues[1].ID) +} + +func Test_MoveColumnsOnProject(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, 0, columns[0].Sorting) + assert.EqualValues(t, 1, columns[1].Sorting) + assert.EqualValues(t, 2, columns[2].Sorting) + + err = MoveColumnsOnProject(db.DefaultContext, project1, map[int64]int64{ + 0: columns[1].ID, + 1: columns[2].ID, + 2: columns[0].ID, + }) + assert.NoError(t, err) + + columnsAfter, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) + assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) + assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) +} diff --git a/models/project/issue.go b/models/project/issue.go index 8175187458eb..fcc365e6af9c 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -5,12 +5,10 @@ package project import ( "context" - "errors" "fmt" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/util" ) // ProjectIssue saves relation from issue to a project @@ -114,31 +112,3 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", newColumn.ID, b.ID) return err } - -// MoveColumnsOnProject sorts columns in a project -func MoveColumnsOnProject(ctx context.Context, project *Project, sortedColumnIDs map[int64]int64) error { - return db.WithTx(ctx, func(ctx context.Context) error { - sess := db.GetEngine(ctx) - columnIDs := util.ValuesOfMap(sortedColumnIDs) - movedColumns, err := GetColumnsByIDs(ctx, project.ID, columnIDs) - if err != nil { - return err - } - if len(movedColumns) != len(sortedColumnIDs) { - return errors.New("some columns do not exist") - } - - for _, column := range movedColumns { - if column.ProjectID != project.ID { - return fmt.Errorf("column[%d]'s projectID is not equal to project's ID [%d]", column.ProjectID, project.ID) - } - } - - for sorting, columnID := range sortedColumnIDs { - if _, err := sess.Exec("UPDATE `project_board` SET sorting=? WHERE id=?", sorting, columnID); err != nil { - return err - } - } - return nil - }) -} diff --git a/models/repo/repo.go b/models/repo/repo.go index 5d5707d1acbe..24d39c70079c 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -724,7 +724,7 @@ func (err ErrRepoNotExist) Unwrap() error { } // GetRepositoryByOwnerAndName returns the repository by given owner name and repo name -func GetRepositoryByOwnerAndName(ctx context.Context, ownerName, repoName string) (*Repository, error) { +func GetRepositoryByOwnerAndName(ctx context.Context, ownerName, repoName string) (*Repository, error) { var repo Repository has, err := db.GetEngine(ctx).Table("repository").Select("repository.*"). Join("INNER", "`user`", "`user`.id = repository.owner_id"). diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 45061c5b24f1..4351fcbbec9f 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -7,7 +7,12 @@ import ( "net/http" "testing" + "code.gitea.io/gitea/models/db" + project_model "code.gitea.io/gitea/models/project" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" ) func TestPrivateRepoProject(t *testing.T) { @@ -21,3 +26,36 @@ func TestPrivateRepoProject(t *testing.T) { req = NewRequest(t, "GET", "/user31/-/projects") sess.MakeRequest(t, req, http.StatusOK) } + +func TestMoveRepoProjectColumns(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + project1 := unittest.AssertExistsAndLoadBean(t, &project_model.Project{ID: 1}) + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, 0, columns[0].Sorting) + assert.EqualValues(t, 1, columns[1].Sorting) + assert.EqualValues(t, 2, columns[2].Sorting) + + sess := loginUser(t, "user1") + req := NewRequest(t, "GET", "/user2/repo1/projects/1") + resp := sess.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + req = NewRequestWithJSON(t, "POST", "/user2/repo1/projects/1/move?_csrf="+htmlDoc.GetCSRF(), map[string]any{ + "columns": []map[string]any{ + {"columnID": columns[1].ID, "sorting": 0}, + {"columnID": columns[2].ID, "sorting": 1}, + {"columnID": columns[0].ID, "sorting": 2}, + }, + }) + sess.MakeRequest(t, req, http.StatusOK) + + columnsAfter, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) + assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) + assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) +} From da28c1c3a4d810579d0f251c4bfe0864707154ab Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 02:29:01 +0800 Subject: [PATCH 28/49] Revert unnecessary change --- models/repo/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 24d39c70079c..5d5707d1acbe 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -724,7 +724,7 @@ func (err ErrRepoNotExist) Unwrap() error { } // GetRepositoryByOwnerAndName returns the repository by given owner name and repo name -func GetRepositoryByOwnerAndName(ctx context.Context, ownerName, repoName string) (*Repository, error) { +func GetRepositoryByOwnerAndName(ctx context.Context, ownerName, repoName string) (*Repository, error) { var repo Repository has, err := db.GetEngine(ctx).Table("repository").Select("repository.*"). Join("INNER", "`user`", "`user`.id = repository.owner_id"). From ed56a0f5fcdee24e6804a0070770d80840183a81 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 02:36:09 +0800 Subject: [PATCH 29/49] Add test for NewBoard --- models/project/board_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/models/project/board_test.go b/models/project/board_test.go index 2841edb861b8..fa40330b7466 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -4,6 +4,8 @@ package project import ( + "fmt" + "strings" "testing" "code.gitea.io/gitea/models/db" @@ -98,3 +100,26 @@ func Test_MoveColumnsOnProject(t *testing.T) { assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) } + +func Test_NewBoard(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + project1 := unittest.AssertExistsAndLoadBean(t, &Project{ID: 1}) + columns, err := project1.GetBoards(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, columns, 3) + + for i := 0; i < maxProjectColumns-3; i++ { + err := NewBoard(db.DefaultContext, &Board{ + Title: fmt.Sprintf("board-%d", i+4), + ProjectID: project1.ID, + }) + assert.NoError(t, err) + } + err = NewBoard(db.DefaultContext, &Board{ + Title: "board-21", + ProjectID: project1.ID, + }) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "maximum number of columns reached")) +} From b27dde914d39baa089df150b144d4e81dece1ee4 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 11:38:48 +0800 Subject: [PATCH 30/49] recover the changes on test --- tests/integration/project_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 4351fcbbec9f..d4676b413b86 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -58,4 +58,14 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) + + // update the sorting back + req = NewRequestWithJSON(t, "POST", "/user2/repo1/projects/1/move?_csrf="+htmlDoc.GetCSRF(), map[string]any{ + "columns": []map[string]any{ + {"columnID": columns[0].ID, "sorting": 0}, + {"columnID": columns[1].ID, "sorting": 1}, + {"columnID": columns[2].ID, "sorting": 2}, + }, + }) + sess.MakeRequest(t, req, http.StatusOK) } From cf9ef105101fbc13f1e971b45036a6a46edf5b19 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 12:05:44 +0800 Subject: [PATCH 31/49] Fix move issues sorting problem and possible tests failure --- models/issues/issue_project.go | 10 ++++----- models/project/issue.go | 40 +++++++++++++++++++++++++++------- routers/web/org/projects.go | 2 +- routers/web/repo/projects.go | 2 +- routers/web/repo/pull.go | 2 +- services/issue/issue.go | 2 +- 6 files changed, 41 insertions(+), 17 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 35b6810786b6..8c731a227830 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -90,16 +90,16 @@ func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (m return issuesMap, nil } -// ChangeProjectAssign changes the project associated with an issue, if newProjectID is 0, the issue is removed from the project -func ChangeProjectAssign(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { +// IssueAssignOrRemoveProject changes the project associated with an issue, if newProjectID is 0, the issue is removed from the project +func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { return db.WithTx(ctx, func(ctx context.Context) error { - return addUpdateIssueProject(ctx, issue, doer, newProjectID, newColumnID) + return issueAssignOrRemoveProject(ctx, issue, doer, newProjectID, newColumnID) }) } -// addUpdateIssueProject adds or updates the project the default column associated with an issue +// issueAssignOrRemoveProject adds or updates the project the default column associated with an issue // If newProjectID is 0, the issue is removed from the project -func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { +func issueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { oldProjectID := issue.projectID(ctx) if err := issue.LoadRepo(ctx); err != nil { diff --git a/models/project/issue.go b/models/project/issue.go index fcc365e6af9c..78689efd8def 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // ProjectIssue saves relation from issue to a project @@ -79,11 +80,8 @@ func (p *Project) NumOpenIssues(ctx context.Context) int { func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs map[int64]int64) error { return db.WithTx(ctx, func(ctx context.Context) error { sess := db.GetEngine(ctx) + issueIDs := util.ValuesOfMap(sortedIssueIDs) - issueIDs := make([]int64, 0, len(sortedIssueIDs)) - for _, issueID := range sortedIssueIDs { - issueIDs = append(issueIDs, issueID) - } count, err := sess.Table(new(ProjectIssue)).Where("project_id=?", board.ProjectID).In("issue_id", issueIDs).Count() if err != nil { return err @@ -103,12 +101,38 @@ func MoveIssuesOnProjectBoard(ctx context.Context, board *Board, sortedIssueIDs } func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) error { + if b.ProjectID != newColumn.ProjectID { + return fmt.Errorf("columns have to be in the same project") + } + if b.ID == newColumn.ID { return nil } - if b.ProjectID != newColumn.ProjectID { - return fmt.Errorf("columns have to be in the same project") + + var maxSorting int8 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + Where("project_id=?", newColumn.ProjectID). + And("project_board_id=?", newColumn.ID). + Get(&maxSorting); err != nil { + return err } - _, err := db.GetEngine(ctx).Exec("UPDATE `project_issue` SET project_board_id = ? WHERE project_board_id = ? ", newColumn.ID, b.ID) - return err + if maxSorting > 0 { + maxSorting++ + } + + issues, err := b.GetIssues(ctx) + if err != nil { + return err + } + + return db.WithTx(ctx, func(ctx context.Context) error { + for i, issue := range issues { + issue.ProjectBoardID = newColumn.ID + issue.Sorting = int64(maxSorting) + int64(i) + if _, err := db.GetEngine(ctx).Cols("project_board_id", "sorting").Update(issue); err != nil { + return err + } + } + return nil + }) } diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 7e26ffbc0080..dbface9061ff 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -471,7 +471,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 6cd932d76a72..efbf8db0dc1a 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -417,7 +417,7 @@ func UpdateIssueProject(ctx *context.Context) { } } - if err := issues_model.ChangeProjectAssign(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index bdc33acb5001..3a06d0f5eac4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1348,7 +1348,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { ctx.ServerError("GetDefaultBoard", err) return } - if err := issues_model.ChangeProjectAssign(ctx, pullIssue, ctx.Doer, projectID, dstDefaultColumn.ID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, pullIssue, ctx.Doer, projectID, dstDefaultColumn.ID); err != nil { ctx.ServerError("ChangeProjectAssign", err) return } diff --git a/services/issue/issue.go b/services/issue/issue.go index 6ef4b53c8832..7e4048c110ca 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -50,7 +50,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo if err != nil { return err } - if err := issues_model.ChangeProjectAssign(ctx, issue, issue.Poster, projectID, defaultBoard.ID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, issue.Poster, projectID, defaultBoard.ID); err != nil { return err } } From f7f36ddbaaf0619ce0d47bbb2d75763d56ca4a0c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 13:33:34 +0800 Subject: [PATCH 32/49] remove unnecessary wrap --- models/issues/issue_project.go | 97 ++++++++++++++++------------------ 1 file changed, 46 insertions(+), 51 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 8c731a227830..c07bee4d49f0 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -90,68 +90,63 @@ func LoadIssuesFromBoardList(ctx context.Context, bs project_model.BoardList) (m return issuesMap, nil } -// IssueAssignOrRemoveProject changes the project associated with an issue, if newProjectID is 0, the issue is removed from the project +// IssueAssignOrRemoveProject changes the project associated with an issue +// If newProjectID is 0, the issue is removed from the project func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { return db.WithTx(ctx, func(ctx context.Context) error { - return issueAssignOrRemoveProject(ctx, issue, doer, newProjectID, newColumnID) - }) -} + oldProjectID := issue.projectID(ctx) -// issueAssignOrRemoveProject adds or updates the project the default column associated with an issue -// If newProjectID is 0, the issue is removed from the project -func issueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_model.User, newProjectID, newColumnID int64) error { - oldProjectID := issue.projectID(ctx) + if err := issue.LoadRepo(ctx); err != nil { + return err + } - if err := issue.LoadRepo(ctx); err != nil { - return err - } + // Only check if we add a new project and not remove it. + if newProjectID > 0 { + newProject, err := project_model.GetProjectByID(ctx, newProjectID) + if err != nil { + return err + } + if newProject.RepoID != issue.RepoID && newProject.OwnerID != issue.Repo.OwnerID { + return fmt.Errorf("issue's repository is not the same as project's repository") + } + } - // Only check if we add a new project and not remove it. - if newProjectID > 0 { - newProject, err := project_model.GetProjectByID(ctx, newProjectID) - if err != nil { + if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { return err } - if newProject.RepoID != issue.RepoID && newProject.OwnerID != issue.Repo.OwnerID { - return fmt.Errorf("issue's repository is not the same as project's repository") - } - } - if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { - return err - } + if oldProjectID > 0 || newProjectID > 0 { + if _, err := CreateComment(ctx, &CreateCommentOptions{ + Type: CommentTypeProject, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + OldProjectID: oldProjectID, + ProjectID: newProjectID, + }); err != nil { + return err + } + } + if newProjectID == 0 || newColumnID == 0 { + return nil + } - if oldProjectID > 0 || newProjectID > 0 { - if _, err := CreateComment(ctx, &CreateCommentOptions{ - Type: CommentTypeProject, - Doer: doer, - Repo: issue.Repo, - Issue: issue, - OldProjectID: oldProjectID, - ProjectID: newProjectID, - }); err != nil { + var maxSorting int64 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + Where("project_id=?", newProjectID). + And("project_board_id=?", newColumnID). + Get(&maxSorting); err != nil { return err } - } - if newProjectID == 0 || newColumnID == 0 { - return nil - } - - var maxSorting int64 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). - Where("project_id=?", newProjectID). - And("project_board_id=?", newColumnID). - Get(&maxSorting); err != nil { - return err - } - if maxSorting > 0 { - maxSorting++ - } + if maxSorting > 0 { + maxSorting++ + } - return db.Insert(ctx, &project_model.ProjectIssue{ - IssueID: issue.ID, - ProjectID: newProjectID, - ProjectBoardID: newColumnID, - Sorting: maxSorting, + return db.Insert(ctx, &project_model.ProjectIssue{ + IssueID: issue.ID, + ProjectID: newProjectID, + ProjectBoardID: newColumnID, + Sorting: maxSorting, + }) }) } From cd2350eca38d4676dbd69f1f09cbfbed6564da64 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 15:03:46 +0800 Subject: [PATCH 33/49] Fix bug --- models/project/issue.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/project/issue.go b/models/project/issue.go index 78689efd8def..65b218b73edb 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -129,7 +129,7 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) for i, issue := range issues { issue.ProjectBoardID = newColumn.ID issue.Sorting = int64(maxSorting) + int64(i) - if _, err := db.GetEngine(ctx).Cols("project_board_id", "sorting").Update(issue); err != nil { + if _, err := db.GetEngine(ctx).ID(issue.ID).Cols("project_board_id", "sorting").Update(issue); err != nil { return err } } From 0aa451b9ab630b802503a29d4ed22ab3139dbe25 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 17:39:14 +0800 Subject: [PATCH 34/49] Fix bug --- models/fixtures/project_board.yml | 4 ++++ models/issues/issue_project.go | 7 ++++--- models/project/board.go | 8 +++++--- models/project/board_test.go | 6 ++++-- models/project/issue.go | 10 +++++++--- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/models/fixtures/project_board.yml b/models/fixtures/project_board.yml index 1939c1d317c6..1bbb81baa560 100644 --- a/models/fixtures/project_board.yml +++ b/models/fixtures/project_board.yml @@ -31,6 +31,7 @@ project_id: 4 title: Done creator_id: 2 + sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -49,6 +50,7 @@ title: Backlog creator_id: 2 default: true + sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 @@ -67,6 +69,7 @@ title: Backlog creator_id: 2 default: true + sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -76,5 +79,6 @@ title: Uncategorized creator_id: 2 default: true + sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index c07bee4d49f0..0353dd14dae4 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -132,13 +132,14 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo } var maxSorting int64 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). Where("project_id=?", newProjectID). And("project_board_id=?", newColumnID). - Get(&maxSorting); err != nil { + Get(&maxSorting) + if err != nil { return err } - if maxSorting > 0 { + if has { maxSorting++ } diff --git a/models/project/board.go b/models/project/board.go index 65a52d0a6a28..0b119275e72e 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -88,6 +88,7 @@ func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { issues := make([]*ProjectIssue, 0, 5) if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID). And("project_board_id=?", b.ID). + OrderBy("sorting"). Find(&issues); err != nil { return nil, err } @@ -184,11 +185,12 @@ func NewBoard(ctx context.Context, board *Board) error { if totalColumns > 0 { var maxSorting int8 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { + has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&maxSorting) + if err != nil { return err } - if maxSorting > 0 { + if has { board.Sorting = maxSorting + 1 } } diff --git a/models/project/board_test.go b/models/project/board_test.go index fa40330b7466..758611a4c34a 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -71,8 +71,10 @@ func Test_moveIssuesToAnotherColumn(t *testing.T) { issues, err = column2.GetIssues(db.DefaultContext) assert.NoError(t, err) assert.Len(t, issues, 2) - assert.EqualValues(t, 1, issues[0].ID) - assert.EqualValues(t, 3, issues[1].ID) + assert.EqualValues(t, 3, issues[0].ID) + assert.EqualValues(t, 0, issues[0].Sorting) + assert.EqualValues(t, 1, issues[1].ID) + assert.EqualValues(t, 1, issues[1].Sorting) } func Test_MoveColumnsOnProject(t *testing.T) { diff --git a/models/project/issue.go b/models/project/issue.go index 65b218b73edb..7e4730d9870e 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -110,13 +110,14 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) } var maxSorting int8 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). Where("project_id=?", newColumn.ProjectID). And("project_board_id=?", newColumn.ID). - Get(&maxSorting); err != nil { + Get(&maxSorting) + if err != nil { return err } - if maxSorting > 0 { + if has { maxSorting++ } @@ -124,6 +125,9 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) if err != nil { return err } + if len(issues) == 0 { + return nil + } return db.WithTx(ctx, func(ctx context.Context) error { for i, issue := range issues { From be61c914920285943b0cfbd7d78c1f1a3cff9961 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 17:51:41 +0800 Subject: [PATCH 35/49] Fix bug --- models/issues/issue_project.go | 14 +++++++------- models/project/board.go | 12 ++++++------ models/project/issue.go | 14 +++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 0353dd14dae4..a9b9286a47f4 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -5,6 +5,7 @@ package issues import ( "context" + "database/sql" "fmt" "code.gitea.io/gitea/models/db" @@ -131,23 +132,22 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo return nil } - var maxSorting int64 - has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + var maxSorting sql.NullInt64 + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). Where("project_id=?", newProjectID). And("project_board_id=?", newColumnID). - Get(&maxSorting) - if err != nil { + Get(&maxSorting); err != nil { return err } - if has { - maxSorting++ + if maxSorting.Valid { + maxSorting.Int64++ } return db.Insert(ctx, &project_model.ProjectIssue{ IssueID: issue.ID, ProjectID: newProjectID, ProjectBoardID: newColumnID, - Sorting: maxSorting, + Sorting: maxSorting.Int64, }) }) } diff --git a/models/project/board.go b/models/project/board.go index 0b119275e72e..c04424e30444 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -5,6 +5,7 @@ package project import ( "context" + "database/sql" "errors" "fmt" "regexp" @@ -184,14 +185,13 @@ func NewBoard(ctx context.Context, board *Board) error { } if totalColumns > 0 { - var maxSorting int8 - has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&maxSorting) - if err != nil { + var maxSorting sql.NullByte + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { return err } - if has { - board.Sorting = maxSorting + 1 + if maxSorting.Valid { + board.Sorting = int8(maxSorting.Byte) + 1 } } diff --git a/models/project/issue.go b/models/project/issue.go index 7e4730d9870e..eefa71c7cc44 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -5,6 +5,7 @@ package project import ( "context" + "database/sql" "fmt" "code.gitea.io/gitea/models/db" @@ -109,16 +110,15 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return nil } - var maxSorting int8 - has, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + var maxSorting sql.NullByte + if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). Where("project_id=?", newColumn.ProjectID). And("project_board_id=?", newColumn.ID). - Get(&maxSorting) - if err != nil { + Get(&maxSorting); err != nil { return err } - if has { - maxSorting++ + if maxSorting.Valid { + maxSorting.Byte++ } issues, err := b.GetIssues(ctx) @@ -132,7 +132,7 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return db.WithTx(ctx, func(ctx context.Context) error { for i, issue := range issues { issue.ProjectBoardID = newColumn.ID - issue.Sorting = int64(maxSorting) + int64(i) + issue.Sorting = int64(maxSorting.Byte) + int64(i) if _, err := db.GetEngine(ctx).ID(issue.ID).Cols("project_board_id", "sorting").Update(issue); err != nil { return err } From e0a7867e7e1f393fdbbfd08d56aa96b379ad6a8f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 17:58:50 +0800 Subject: [PATCH 36/49] revert unnecessary change --- routers/web/repo/pull.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 3a06d0f5eac4..86cec004c14d 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1327,6 +1327,7 @@ func CompareAndPullRequestPost(ctx *context.Context) { } ctx.JSONError(flashError) } + return } if projectID > 0 { From 4ce383f73102a854a41e8e68f03c29bd83145bc7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 18:41:40 +0800 Subject: [PATCH 37/49] Use another user --- tests/integration/project_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index d4676b413b86..848f4f2b03ba 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -38,7 +38,7 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, 1, columns[1].Sorting) assert.EqualValues(t, 2, columns[2].Sorting) - sess := loginUser(t, "user1") + sess := loginUser(t, "user2") req := NewRequest(t, "GET", "/user2/repo1/projects/1") resp := sess.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) From e56f89fdba24c3f0f55dc2728241a9fd58f38f98 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 4 May 2024 21:29:02 +0800 Subject: [PATCH 38/49] Create a project dynamically to avoid override --- tests/integration/project_test.go | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 848f4f2b03ba..d36c83b41186 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -4,12 +4,12 @@ package integration import ( + "fmt" "net/http" "testing" "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" - "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -30,7 +30,23 @@ func TestPrivateRepoProject(t *testing.T) { func TestMoveRepoProjectColumns(t *testing.T) { defer tests.PrepareTestEnv(t)() - project1 := unittest.AssertExistsAndLoadBean(t, &project_model.Project{ID: 1}) + project1 := project_model.Project{ + Title: "new created project", + RepoID: 1, + Type: project_model.TypeRepository, + BoardType: project_model.BoardTypeNone, + } + err := project_model.NewProject(db.DefaultContext, &project1) + assert.NoError(t, err) + + for i := 0; i < 3; i++ { + err = project_model.NewBoard(db.DefaultContext, &project_model.Board{ + Title: fmt.Sprintf("column %d", i+1), + ProjectID: project1.ID, + }) + assert.NoError(t, err) + } + columns, err := project1.GetBoards(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) @@ -39,11 +55,11 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, 2, columns[2].Sorting) sess := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user2/repo1/projects/1") + req := NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/projects/%d", project1.ID)) resp := sess.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) - req = NewRequestWithJSON(t, "POST", "/user2/repo1/projects/1/move?_csrf="+htmlDoc.GetCSRF(), map[string]any{ + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/user2/repo1/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), project1.ID), map[string]any{ "columns": []map[string]any{ {"columnID": columns[1].ID, "sorting": 0}, {"columnID": columns[2].ID, "sorting": 1}, @@ -60,7 +76,7 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) // update the sorting back - req = NewRequestWithJSON(t, "POST", "/user2/repo1/projects/1/move?_csrf="+htmlDoc.GetCSRF(), map[string]any{ + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/user2/repo1/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), project1.ID), map[string]any{ "columns": []map[string]any{ {"columnID": columns[0].ID, "sorting": 0}, {"columnID": columns[1].ID, "sorting": 1}, @@ -68,4 +84,6 @@ func TestMoveRepoProjectColumns(t *testing.T) { }, }) sess.MakeRequest(t, req, http.StatusOK) + + assert.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) } From 76465a4490261b6eddff9d40330a39c3739ef135 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 May 2024 10:05:57 +0800 Subject: [PATCH 39/49] Fix tests --- models/fixtures/repo_unit.yml | 6 ++++++ tests/integration/project_test.go | 22 ++++++++-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index 8a22db0445c6..e019c0f58599 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -712,3 +712,9 @@ type: 3 config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}" created_unix: 946684810 + +- + id: 108 + repo_id: 4 + type: 8 + created_unix: 946684810 diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index d36c83b41186..9ac441d6bec1 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -10,6 +10,8 @@ import ( "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -30,9 +32,11 @@ func TestPrivateRepoProject(t *testing.T) { func TestMoveRepoProjectColumns(t *testing.T) { defer tests.PrepareTestEnv(t)() + repo4 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + project1 := project_model.Project{ Title: "new created project", - RepoID: 1, + RepoID: repo4.ID, Type: project_model.TypeRepository, BoardType: project_model.BoardTypeNone, } @@ -54,12 +58,12 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, 1, columns[1].Sorting) assert.EqualValues(t, 2, columns[2].Sorting) - sess := loginUser(t, "user2") - req := NewRequest(t, "GET", fmt.Sprintf("/user2/repo1/projects/%d", project1.ID)) + sess := loginUser(t, "user1") + req := NewRequest(t, "GET", fmt.Sprintf("/%s/projects/%d", repo4.FullName(), project1.ID)) resp := sess.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/user2/repo1/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), project1.ID), map[string]any{ + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), repo4.FullName(), project1.ID), map[string]any{ "columns": []map[string]any{ {"columnID": columns[1].ID, "sorting": 0}, {"columnID": columns[2].ID, "sorting": 1}, @@ -75,15 +79,5 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) - // update the sorting back - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/user2/repo1/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), project1.ID), map[string]any{ - "columns": []map[string]any{ - {"columnID": columns[0].ID, "sorting": 0}, - {"columnID": columns[1].ID, "sorting": 1}, - {"columnID": columns[2].ID, "sorting": 2}, - }, - }) - sess.MakeRequest(t, req, http.StatusOK) - assert.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) } From 5514594bc8fcea6ab76215719e68e5877669d503 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 May 2024 10:44:27 +0800 Subject: [PATCH 40/49] Add repo unit for repo4 dynamically in test --- models/fixtures/repo_unit.yml | 6 ------ tests/integration/project_test.go | 11 ++++++++++- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/models/fixtures/repo_unit.yml b/models/fixtures/repo_unit.yml index e019c0f58599..8a22db0445c6 100644 --- a/models/fixtures/repo_unit.yml +++ b/models/fixtures/repo_unit.yml @@ -712,9 +712,3 @@ type: 3 config: "{\"IgnoreWhitespaceConflicts\":false,\"AllowMerge\":true,\"AllowRebase\":true,\"AllowRebaseMerge\":true,\"AllowSquash\":true}" created_unix: 946684810 - -- - id: 108 - repo_id: 4 - type: 8 - created_unix: 946684810 diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 9ac441d6bec1..8c157ad26c18 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" @@ -33,6 +34,12 @@ func TestMoveRepoProjectColumns(t *testing.T) { defer tests.PrepareTestEnv(t)() repo4 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + repoUnit := repo_model.RepoUnit{ + RepoID: repo4.ID, + Type: unit.TypeProjects, + } + err := db.Insert(db.DefaultContext, &repoUnit) + assert.NoError(t, err) project1 := project_model.Project{ Title: "new created project", @@ -40,7 +47,7 @@ func TestMoveRepoProjectColumns(t *testing.T) { Type: project_model.TypeRepository, BoardType: project_model.BoardTypeNone, } - err := project_model.NewProject(db.DefaultContext, &project1) + err = project_model.NewProject(db.DefaultContext, &project1) assert.NoError(t, err) for i := 0; i < 3; i++ { @@ -80,4 +87,6 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) assert.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) + _, err = db.DeleteByID[repo_model.RepoUnit](db.DefaultContext, repoUnit.ID) + assert.NoError(t, err) } From 21f019bd1967e26618b3899958029421a0e90fa2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 May 2024 11:54:27 +0800 Subject: [PATCH 41/49] Fix tests --- tests/integration/project_test.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 8c157ad26c18..5ddea4314a81 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -11,7 +11,6 @@ import ( "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" @@ -33,21 +32,15 @@ func TestPrivateRepoProject(t *testing.T) { func TestMoveRepoProjectColumns(t *testing.T) { defer tests.PrepareTestEnv(t)() - repo4 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - repoUnit := repo_model.RepoUnit{ - RepoID: repo4.ID, - Type: unit.TypeProjects, - } - err := db.Insert(db.DefaultContext, &repoUnit) - assert.NoError(t, err) + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) project1 := project_model.Project{ Title: "new created project", - RepoID: repo4.ID, + RepoID: repo2.ID, Type: project_model.TypeRepository, BoardType: project_model.BoardTypeNone, } - err = project_model.NewProject(db.DefaultContext, &project1) + err := project_model.NewProject(db.DefaultContext, &project1) assert.NoError(t, err) for i := 0; i < 3; i++ { @@ -66,11 +59,11 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, 2, columns[2].Sorting) sess := loginUser(t, "user1") - req := NewRequest(t, "GET", fmt.Sprintf("/%s/projects/%d", repo4.FullName(), project1.ID)) + req := NewRequest(t, "GET", fmt.Sprintf("/%s/projects/%d", repo2.FullName(), project1.ID)) resp := sess.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), repo4.FullName(), project1.ID), map[string]any{ + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s/projects/%d/move?_csrf="+htmlDoc.GetCSRF(), repo2.FullName(), project1.ID), map[string]any{ "columns": []map[string]any{ {"columnID": columns[1].ID, "sorting": 0}, {"columnID": columns[2].ID, "sorting": 1}, @@ -87,6 +80,4 @@ func TestMoveRepoProjectColumns(t *testing.T) { assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) assert.NoError(t, project_model.DeleteProjectByID(db.DefaultContext, project1.ID)) - _, err = db.DeleteByID[repo_model.RepoUnit](db.DefaultContext, repoUnit.ID) - assert.NoError(t, err) } From 94ea1bed2a8c7f488c408f8f3d41aba5cbc54b22 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 May 2024 12:26:26 +0800 Subject: [PATCH 42/49] Add some permissions check --- tests/integration/project_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/project_test.go b/tests/integration/project_test.go index 5ddea4314a81..1d9c3aae5361 100644 --- a/tests/integration/project_test.go +++ b/tests/integration/project_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/tests" @@ -34,6 +35,9 @@ func TestMoveRepoProjectColumns(t *testing.T) { repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + projectsUnit := repo2.MustGetUnit(db.DefaultContext, unit.TypeProjects) + assert.True(t, projectsUnit.ProjectsConfig().IsProjectsAllowed(repo_model.ProjectsModeRepo)) + project1 := project_model.Project{ Title: "new created project", RepoID: repo2.ID, From ca73f6c78e091491b436b942f7926809341425a6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 5 May 2024 15:29:54 +0800 Subject: [PATCH 43/49] Fix project test to recover the changes of global variable --- tests/integration/org_project_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/org_project_test.go b/tests/integration/org_project_test.go index a14004f6b028..4b67637a2730 100644 --- a/tests/integration/org_project_test.go +++ b/tests/integration/org_project_test.go @@ -14,8 +14,12 @@ import ( func TestOrgProjectAccess(t *testing.T) { defer tests.PrepareTestEnv(t)() + oldDisabledRepoUnits := unit_model.DisabledRepoUnits // disable repo project unit unit_model.DisabledRepoUnits = []unit_model.Type{unit_model.TypeProjects} + defer func() { + unit_model.DisabledRepoUnits = oldDisabledRepoUnits + }() // repo project, 404 req := NewRequest(t, "GET", "/user2/repo1/projects") From f5865770081fa21d80aee2a7b287f4df344eefa0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 May 2024 12:09:02 +0800 Subject: [PATCH 44/49] fix sorting --- models/fixtures/project_board.yml | 7 ------- models/issues/issue_project.go | 18 +++++++++--------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/models/fixtures/project_board.yml b/models/fixtures/project_board.yml index 1bbb81baa560..3293dea6edf4 100644 --- a/models/fixtures/project_board.yml +++ b/models/fixtures/project_board.yml @@ -4,7 +4,6 @@ title: To Do creator_id: 2 default: true - sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -13,7 +12,6 @@ project_id: 1 title: In Progress creator_id: 2 - sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 @@ -22,7 +20,6 @@ project_id: 1 title: Done creator_id: 2 - sorting: 2 created_unix: 1588117528 updated_unix: 1588117528 @@ -31,7 +28,6 @@ project_id: 4 title: Done creator_id: 2 - sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -50,7 +46,6 @@ title: Backlog creator_id: 2 default: true - sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 @@ -69,7 +64,6 @@ title: Backlog creator_id: 2 default: true - sorting: 0 created_unix: 1588117528 updated_unix: 1588117528 @@ -79,6 +73,5 @@ title: Uncategorized creator_id: 2 default: true - sorting: 1 created_unix: 1588117528 updated_unix: 1588117528 diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index a9b9286a47f4..3ae3358209db 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -5,12 +5,12 @@ package issues import ( "context" - "database/sql" "fmt" "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/util" ) // LoadProject load the project the issue was assigned to @@ -132,22 +132,22 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo return nil } - var maxSorting sql.NullInt64 - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + res := struct { + MaxSorting int64 + IssueCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as MaxSorting, count(*) as IssueCount").Table("project_issue"). Where("project_id=?", newProjectID). And("project_board_id=?", newColumnID). - Get(&maxSorting); err != nil { + Get(&res); err != nil { return err } - if maxSorting.Valid { - maxSorting.Int64++ - } - + newSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) return db.Insert(ctx, &project_model.ProjectIssue{ IssueID: issue.ID, ProjectID: newProjectID, ProjectBoardID: newColumnID, - Sorting: maxSorting.Int64, + Sorting: newSorting, }) }) } From 94cb84d16eba0898ad42b4652ca5dceb92fb626a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 May 2024 12:50:26 +0800 Subject: [PATCH 45/49] fix --- models/issues/issue_project.go | 17 +++++-- models/project/project.go | 7 +++ routers/web/org/projects.go | 91 ---------------------------------- routers/web/repo/projects.go | 34 +++---------- routers/web/repo/pull.go | 29 +++-------- services/issue/issue.go | 10 +--- 6 files changed, 34 insertions(+), 154 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 3ae3358209db..616bcc753e64 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -5,7 +5,6 @@ package issues import ( "context" - "fmt" "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" @@ -107,8 +106,15 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo if err != nil { return err } - if newProject.RepoID != issue.RepoID && newProject.OwnerID != issue.Repo.OwnerID { - return fmt.Errorf("issue's repository is not the same as project's repository") + if newColumnID == 0 { + newDefaultColumn, err := newProject.GetDefaultBoard(ctx) + if err != nil { + return err + } + newColumnID = newDefaultColumn.ID + } + if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo.ID) { + return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) } } @@ -128,9 +134,12 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo return err } } - if newProjectID == 0 || newColumnID == 0 { + if newProjectID == 0 { return nil } + if newColumnID == 0 { + panic("newColumnID must not be zero") // shouldn't happen + } res := struct { MaxSorting int64 diff --git a/models/project/project.go b/models/project/project.go index 8f9ee2a99e9c..a11465c05c23 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -161,6 +161,13 @@ func (p *Project) IsRepositoryProject() bool { return p.Type == TypeRepository } +func (p *Project) CanBeAccessedByOwnerRepo(ownerID, repoID int64) bool { + if p.Type == TypeOrganization { + return p.OwnerID == ownerID && p.RepoID == 0 + } + return p.OwnerID == ownerID && p.RepoID == repoID +} + func init() { db.RegisterModel(new(Project)) } diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index dbface9061ff..50effbe9633f 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "strings" "code.gitea.io/gitea/models/db" @@ -390,96 +389,6 @@ func ViewProject(ctx *context.Context) { ctx.HTML(http.StatusOK, tplProjectsView) } -func getActionIssues(ctx *context.Context) issues_model.IssueList { - commaSeparatedIssueIDs := ctx.FormString("issue_ids") - if len(commaSeparatedIssueIDs) == 0 { - return nil - } - issueIDs := make([]int64, 0, 10) - for _, stringIssueID := range strings.Split(commaSeparatedIssueIDs, ",") { - issueID, err := strconv.ParseInt(stringIssueID, 10, 64) - if err != nil { - ctx.ServerError("ParseInt", err) - return nil - } - issueIDs = append(issueIDs, issueID) - } - issues, err := issues_model.GetIssuesByIDs(ctx, issueIDs) - if err != nil { - ctx.ServerError("GetIssuesByIDs", err) - return nil - } - // Check access rights for all issues - issueUnitEnabled := ctx.Repo.CanRead(unit.TypeIssues) - prUnitEnabled := ctx.Repo.CanRead(unit.TypePullRequests) - for _, issue := range issues { - if issue.RepoID != ctx.Repo.Repository.ID { - ctx.NotFound("some issue's RepoID is incorrect", errors.New("some issue's RepoID is incorrect")) - return nil - } - if issue.IsPull && !prUnitEnabled || !issue.IsPull && !issueUnitEnabled { - ctx.NotFound("IssueOrPullRequestUnitNotAllowed", nil) - return nil - } - if err = issue.LoadAttributes(ctx); err != nil { - ctx.ServerError("LoadAttributes", err) - return nil - } - } - return issues -} - -// UpdateIssueProject change an issue's project -func UpdateIssueProject(ctx *context.Context) { - issues := getActionIssues(ctx) - if ctx.Written() { - return - } - - if err := issues.LoadProjects(ctx); err != nil { - ctx.ServerError("LoadProjects", err) - return - } - - projectID := ctx.FormInt64("id") - var dstColumnID int64 - if projectID > 0 { - dstProject, err := project_model.GetProjectByID(ctx, projectID) - if err != nil { - ctx.ServerError("GetProjectByID", err) - return - } - if dstProject.OwnerID != ctx.ContextUser.ID { - ctx.JSON(http.StatusUnprocessableEntity, map[string]string{ - "message": fmt.Sprintf("Project[%d] is not in Owner[%d] as expected", dstProject.ID, ctx.ContextUser.ID), - }) - return - } - - dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) - if err != nil { - ctx.ServerError("GetDefaultBoard", err) - return - } - dstColumnID = dstDefaultColumn.ID - } - - for _, issue := range issues { - if issue.Project != nil { - if issue.Project.ID == projectID { - continue - } - } - - if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) - return - } - } - - ctx.JSONOK() -} - // DeleteProjectBoard allows for the deletion of a project board func DeleteProjectBoard(ctx *context.Context) { if ctx.Doer == nil { diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index efbf8db0dc1a..6186ee150cbf 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" @@ -389,36 +390,15 @@ func UpdateIssueProject(ctx *context.Context) { } projectID := ctx.FormInt64("id") - var dstColumnID int64 - if projectID > 0 { - dstProject, err := project_model.GetProjectByID(ctx, projectID) - if err != nil { - ctx.ServerError("GetProjectByID", err) - return - } - for _, issue := range issues { - if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != issue.Repo.OwnerID { - ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") - return - } - } - dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) - if err != nil { - ctx.ServerError("GetDefaultBoard", err) - return - } - dstColumnID = dstDefaultColumn.ID - } - for _, issue := range issues { - if issue.Project != nil { - if issue.Project.ID == projectID { + if issue.Project != nil && issue.Project.ID == projectID { + continue + } + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, 0); err != nil { + if errors.Is(err, util.ErrPermissionDenied) { continue } - } - - if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, dstColumnID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) + ctx.ServerError("IssueAssignOrRemoveProject", err) return } } diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 86cec004c14d..7041175d9ae1 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -20,7 +20,6 @@ import ( git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" access_model "code.gitea.io/gitea/models/perm/access" - project_model "code.gitea.io/gitea/models/project" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" @@ -1330,28 +1329,12 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } - if projectID > 0 { - if !ctx.Repo.CanWrite(unit.TypeProjects) { - ctx.Error(http.StatusBadRequest, "user hasn't the permission to write to projects") - return - } - dstProject, err := project_model.GetProjectByID(ctx, projectID) - if err != nil { - ctx.ServerError("GetProjectByID", err) - return - } - if dstProject.RepoID != ctx.Repo.Repository.ID && dstProject.OwnerID != repo.OwnerID { - ctx.Error(http.StatusBadRequest, "project doesn't belong to the repository") - return - } - dstDefaultColumn, err := dstProject.GetDefaultBoard(ctx) - if err != nil { - ctx.ServerError("GetDefaultBoard", err) - return - } - if err := issues_model.IssueAssignOrRemoveProject(ctx, pullIssue, ctx.Doer, projectID, dstDefaultColumn.ID); err != nil { - ctx.ServerError("ChangeProjectAssign", err) - return + if projectID > 0 && ctx.Repo.CanWrite(unit.TypeProjects) { + if err := issues_model.IssueAssignOrRemoveProject(ctx, pullIssue, ctx.Doer, projectID, 0); err != nil { + if !errors.Is(err, util.ErrPermissionDenied) { + ctx.ServerError("IssueAssignOrRemoveProject", err) + return + } } } diff --git a/services/issue/issue.go b/services/issue/issue.go index 7e4048c110ca..72ea66c8d98c 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -42,15 +42,7 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo } } if projectID > 0 { - project, err := project_model.GetProjectByID(ctx, projectID) - if err != nil { - return err - } - defaultBoard, err := project.GetDefaultBoard(ctx) - if err != nil { - return err - } - if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, issue.Poster, projectID, defaultBoard.ID); err != nil { + if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, issue.Poster, projectID, 0); err != nil { return err } } From 6ecdbb6aca0bb9be35e0a8778dacb4aea1b249b8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 May 2024 13:16:08 +0800 Subject: [PATCH 46/49] fix --- models/project/board.go | 33 +++++++++------------------ tests/integration/org_project_test.go | 10 +++----- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/models/project/board.go b/models/project/board.go index c04424e30444..7cddd823d281 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -5,7 +5,6 @@ package project import ( "context" - "database/sql" "errors" "fmt" "regexp" @@ -89,7 +88,7 @@ func (b *Board) GetIssues(ctx context.Context) ([]*ProjectIssue, error) { issues := make([]*ProjectIssue, 0, 5) if err := db.GetEngine(ctx).Where("project_id=?", b.ProjectID). And("project_board_id=?", b.ID). - OrderBy("sorting"). + OrderBy("sorting, id"). Find(&issues); err != nil { return nil, err } @@ -173,29 +172,19 @@ func NewBoard(ctx context.Context, board *Board) error { if len(board.Color) != 0 && !BoardColorPattern.MatchString(board.Color) { return fmt.Errorf("bad color code: %s", board.Color) } - - totalColumns, err := db.GetEngine(ctx).Table("project_board"). - Where("project_id=?", board.ProjectID).Count() - if err != nil { + res := struct { + MaxSorting int64 + ColumnCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as MaxSorting, count(*) as ColumnCount").Table("project_board"). + Where("project_id=?", board.ProjectID).Get(&res); err != nil { return err } - - if totalColumns >= maxProjectColumns { + if res.ColumnCount >= maxProjectColumns { return fmt.Errorf("NewBoard: maximum number of columns reached") } - - if totalColumns > 0 { - var maxSorting sql.NullByte - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_board"). - Where("project_id=?", board.ProjectID).Get(&maxSorting); err != nil { - return err - } - if maxSorting.Valid { - board.Sorting = int8(maxSorting.Byte) + 1 - } - } - - _, err = db.GetEngine(ctx).Insert(board) + board.Sorting = int8(util.Iif(res.MaxSorting > 0, res.MaxSorting+1, 0)) + _, err := db.GetEngine(ctx).Insert(board) return err } @@ -291,7 +280,7 @@ func UpdateBoard(ctx context.Context, board *Board) error { // GetBoards fetches all boards related to a project func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { boards := make([]*Board, 0, 5) - if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting").Find(&boards); err != nil { + if err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Find(&boards); err != nil { return nil, err } diff --git a/tests/integration/org_project_test.go b/tests/integration/org_project_test.go index 4b67637a2730..ca39cf513020 100644 --- a/tests/integration/org_project_test.go +++ b/tests/integration/org_project_test.go @@ -5,21 +5,17 @@ package integration import ( "net/http" + "slices" "testing" unit_model "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" ) func TestOrgProjectAccess(t *testing.T) { defer tests.PrepareTestEnv(t)() - - oldDisabledRepoUnits := unit_model.DisabledRepoUnits - // disable repo project unit - unit_model.DisabledRepoUnits = []unit_model.Type{unit_model.TypeProjects} - defer func() { - unit_model.DisabledRepoUnits = oldDisabledRepoUnits - }() + defer test.MockVariableValue(&unit_model.DisabledRepoUnits, append(slices.Clone(unit_model.DisabledRepoUnits), unit_model.TypeProjects))() // repo project, 404 req := NewRequest(t, "GET", "/user2/repo1/projects") From 68c3fdc32e2770f85af32b61bacb2e217bd23d00 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 May 2024 13:27:51 +0800 Subject: [PATCH 47/49] fix --- models/project/project.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/models/project/project.go b/models/project/project.go index a11465c05c23..a1cdf56eff5c 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -162,10 +162,10 @@ func (p *Project) IsRepositoryProject() bool { } func (p *Project) CanBeAccessedByOwnerRepo(ownerID, repoID int64) bool { - if p.Type == TypeOrganization { - return p.OwnerID == ownerID && p.RepoID == 0 + if p.Type == TypeRepository { + return p.RepoID == repoID // if a project belongs to a repository, then its OwnerID is 0 and can be ignored } - return p.OwnerID == ownerID && p.RepoID == repoID + return p.OwnerID == ownerID && p.RepoID == 0 } func init() { From 104c29bcdfdd721c5e6e7170168901d850b645c7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 6 May 2024 14:26:10 +0800 Subject: [PATCH 48/49] fix --- models/issues/issue_project.go | 4 ++-- models/project/board.go | 4 ++-- models/project/board_test.go | 8 ++++---- models/project/project.go | 4 ++-- routers/web/shared/project/column.go | 8 ++------ 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 616bcc753e64..7db08bdcfe73 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -113,7 +113,7 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo } newColumnID = newDefaultColumn.ID } - if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo.ID) { + if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) { return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) } } @@ -145,7 +145,7 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo MaxSorting int64 IssueCount int64 }{} - if _, err := db.GetEngine(ctx).Select("max(sorting) as MaxSorting, count(*) as IssueCount").Table("project_issue"). + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as issue_count").Table("project_issue"). Where("project_id=?", newProjectID). And("project_board_id=?", newColumnID). Get(&res); err != nil { diff --git a/models/project/board.go b/models/project/board.go index 7cddd823d281..a52baa0c185f 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -176,14 +176,14 @@ func NewBoard(ctx context.Context, board *Board) error { MaxSorting int64 ColumnCount int64 }{} - if _, err := db.GetEngine(ctx).Select("max(sorting) as MaxSorting, count(*) as ColumnCount").Table("project_board"). + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as column_count").Table("project_board"). Where("project_id=?", board.ProjectID).Get(&res); err != nil { return err } if res.ColumnCount >= maxProjectColumns { return fmt.Errorf("NewBoard: maximum number of columns reached") } - board.Sorting = int8(util.Iif(res.MaxSorting > 0, res.MaxSorting+1, 0)) + board.Sorting = int8(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0)) _, err := db.GetEngine(ctx).Insert(board) return err } diff --git a/models/project/board_test.go b/models/project/board_test.go index 758611a4c34a..da922ff7adaa 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -84,9 +84,9 @@ func Test_MoveColumnsOnProject(t *testing.T) { columns, err := project1.GetBoards(db.DefaultContext) assert.NoError(t, err) assert.Len(t, columns, 3) - assert.EqualValues(t, 0, columns[0].Sorting) - assert.EqualValues(t, 1, columns[1].Sorting) - assert.EqualValues(t, 2, columns[2].Sorting) + assert.EqualValues(t, 0, columns[0].Sorting) // even if there is no default sorting, the code should also work + assert.EqualValues(t, 0, columns[1].Sorting) + assert.EqualValues(t, 0, columns[2].Sorting) err = MoveColumnsOnProject(db.DefaultContext, project1, map[int64]int64{ 0: columns[1].ID, @@ -97,7 +97,7 @@ func Test_MoveColumnsOnProject(t *testing.T) { columnsAfter, err := project1.GetBoards(db.DefaultContext) assert.NoError(t, err) - assert.Len(t, columns, 3) + assert.Len(t, columnsAfter, 3) assert.EqualValues(t, columns[1].ID, columnsAfter[0].ID) assert.EqualValues(t, columns[2].ID, columnsAfter[1].ID) assert.EqualValues(t, columns[0].ID, columnsAfter[2].ID) diff --git a/models/project/project.go b/models/project/project.go index a1cdf56eff5c..8be38694c522 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -161,9 +161,9 @@ func (p *Project) IsRepositoryProject() bool { return p.Type == TypeRepository } -func (p *Project) CanBeAccessedByOwnerRepo(ownerID, repoID int64) bool { +func (p *Project) CanBeAccessedByOwnerRepo(ownerID int64, repo *repo_model.Repository) bool { if p.Type == TypeRepository { - return p.RepoID == repoID // if a project belongs to a repository, then its OwnerID is 0 and can be ignored + return repo != nil && p.RepoID == repo.ID // if a project belongs to a repository, then its OwnerID is 0 and can be ignored } return p.OwnerID == ownerID && p.RepoID == 0 } diff --git a/routers/web/shared/project/column.go b/routers/web/shared/project/column.go index f20f8b3b19a6..599842ea9e9b 100644 --- a/routers/web/shared/project/column.go +++ b/routers/web/shared/project/column.go @@ -16,12 +16,8 @@ func MoveColumns(ctx *context.Context) { ctx.NotFoundOrServerError("GetProjectByID", project_model.IsErrProjectNotExist, err) return } - if project.OwnerID > 0 && project.OwnerID != ctx.ContextUser.ID { - ctx.NotFound("InvalidOwnerID", nil) - return - } - if project.RepoID > 0 && project.RepoID != ctx.Repo.Repository.ID { - ctx.NotFound("InvalidRepoID", nil) + if !project.CanBeAccessedByOwnerRepo(ctx.ContextUser.ID, ctx.Repo.Repository) { + ctx.NotFound("CanBeAccessedByOwnerRepo", nil) return } From 59b473902b2a1a56150ffeb8d7aec74db2bb88b1 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 8 May 2024 10:19:20 +0800 Subject: [PATCH 49/49] move perm check, fix incorrect column type --- models/issues/issue_project.go | 6 +++--- models/project/issue.go | 17 +++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 7db08bdcfe73..e31d2ef15162 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -106,6 +106,9 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo if err != nil { return err } + if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) { + return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) + } if newColumnID == 0 { newDefaultColumn, err := newProject.GetDefaultBoard(ctx) if err != nil { @@ -113,9 +116,6 @@ func IssueAssignOrRemoveProject(ctx context.Context, issue *Issue, doer *user_mo } newColumnID = newDefaultColumn.ID } - if !newProject.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) { - return util.NewPermissionDeniedErrorf("issue %d can't be accessed by project %d", issue.ID, newProject.ID) - } } if _, err := db.GetEngine(ctx).Where("project_issue.issue_id=?", issue.ID).Delete(&project_model.ProjectIssue{}); err != nil { diff --git a/models/project/issue.go b/models/project/issue.go index eefa71c7cc44..32e72e909d5c 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -5,7 +5,6 @@ package project import ( "context" - "database/sql" "fmt" "code.gitea.io/gitea/models/db" @@ -110,16 +109,17 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return nil } - var maxSorting sql.NullByte - if _, err := db.GetEngine(ctx).Select("Max(sorting)").Table("project_issue"). + res := struct { + MaxSorting int64 + IssueCount int64 + }{} + if _, err := db.GetEngine(ctx).Select("max(sorting) as max_sorting, count(*) as issue_count"). + Table("project_issue"). Where("project_id=?", newColumn.ProjectID). And("project_board_id=?", newColumn.ID). - Get(&maxSorting); err != nil { + Get(&res); err != nil { return err } - if maxSorting.Valid { - maxSorting.Byte++ - } issues, err := b.GetIssues(ctx) if err != nil { @@ -129,10 +129,11 @@ func (b *Board) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Board) return nil } + nextSorting := util.Iif(res.IssueCount > 0, res.MaxSorting+1, 0) return db.WithTx(ctx, func(ctx context.Context) error { for i, issue := range issues { issue.ProjectBoardID = newColumn.ID - issue.Sorting = int64(maxSorting.Byte) + int64(i) + issue.Sorting = nextSorting + int64(i) if _, err := db.GetEngine(ctx).ID(issue.ID).Cols("project_board_id", "sorting").Update(issue); err != nil { return err }