0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-06-12 05:04:04 +02:00

Address remaining review feedback

- Use ParseIssueFilterStateIsClosed for ListProjects state parsing
- Add SortTypeProjectColumnSorting const, replace magic string
- Use GetIssueByRepoID and dedupe Add/Remove issue handlers
- Migrate Column.Sorting from int8 to int (drops 127-column limit, allows
  the API to expose a normal int without truncation)
- Introduce project_service.UpdateProject with optional.Option fields,
  use it from the API EditProject handler

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
silverwind 2026-04-27 12:08:29 +02:00 committed by beardev-in
parent 795c6bc944
commit 45832f4d68
No known key found for this signature in database
GPG Key ID: 625E0CB70AFF32B9
12 changed files with 229 additions and 105 deletions

View File

@ -22,7 +22,11 @@ import (
"xorm.io/xorm"
)
const ScopeSortPrefix = "scope-"
const (
ScopeSortPrefix = "scope-"
// SortTypeProjectColumnSorting orders issues within a project column by their project_issue.sorting value.
SortTypeProjectColumnSorting = "project-column-sorting"
)
// IssuesOptions represents options of an issue.
type IssuesOptions struct { //nolint:revive // export stutter
@ -122,7 +126,7 @@ func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) {
"ELSE 2 END ASC", priorityRepoID).
Desc("issue.created_unix").
Desc("issue.id")
case "project-column-sorting":
case SortTypeProjectColumnSorting:
sess.Asc("project_issue.sorting").Desc("issue.created_unix").Desc("issue.id")
default:
sess.Desc("issue.created_unix").Desc("issue.id")

View File

@ -409,6 +409,7 @@ func prepareMigrationTasks() []*migration {
// Gitea 1.26.0 ends at migration ID number 330 (database version 331)
newMigration(331, "Add ActionRunAttempt model and related action fields", v1_27.AddActionRunAttemptModel),
newMigration(332, "Widen project_board.sorting from int8 to int", v1_27.WidenProjectBoardSorting),
}
return preparedMigrations
}

View File

@ -0,0 +1,26 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v1_27
import (
"code.gitea.io/gitea/models/migrations/base"
"xorm.io/xorm"
"xorm.io/xorm/schemas"
)
// WidenProjectBoardSorting changes project_board.sorting from int8 (TINYINT) to int (INTEGER)
// so the public API can expose a regular int and lift the 127 column upper bound.
func WidenProjectBoardSorting(x *xorm.Engine) error {
if x.Dialect().URI().DBType == schemas.SQLITE {
return nil
}
return base.ModifyColumn(x, "project_board", &schemas.Column{
Name: "sorting",
SQLType: schemas.SQLType{Name: "INT"},
Nullable: false,
Default: "0",
DefaultIsEmpty: false,
})
}

View File

@ -42,7 +42,7 @@ type Column struct {
ID int64 `xorm:"pk autoincr"`
Title string
Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific column will be assigned to this column
Sorting int8 `xorm:"NOT NULL DEFAULT 0"`
Sorting int `xorm:"NOT NULL DEFAULT 0"`
Color string `xorm:"VARCHAR(7)"`
ProjectID int64 `xorm:"INDEX NOT NULL"`
@ -128,8 +128,7 @@ func createDefaultColumnsForProject(ctx context.Context, project *Project) error
})
}
// maxProjectColumns max columns allowed in a project, this should not bigger than 127
// because sorting is int8 in database
// maxProjectColumns is the maximum number of columns allowed in a project.
const maxProjectColumns = 20
// NewColumn adds a new project column to a given project
@ -149,7 +148,7 @@ func NewColumn(ctx context.Context, column *Column) error {
if res.ColumnCount >= maxProjectColumns {
return errors.New("NewBoard: maximum number of columns reached")
}
column.Sorting = int8(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0))
column.Sorting = int(util.Iif(res.ColumnCount > 0, res.MaxSorting+1, 0))
_, err := db.GetEngine(ctx).Insert(column)
return err
}

View File

@ -83,9 +83,9 @@ func Test_MoveColumnsOnProject(t *testing.T) {
columns, err := GetProjectColumns(t.Context(), project1.ID, db.ListOptionsAll)
assert.NoError(t, err)
assert.Len(t, columns, 3)
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)
assert.Equal(t, 0, columns[0].Sorting) // even if there is no default sorting, the code should also work
assert.Equal(t, 0, columns[1].Sorting)
assert.Equal(t, 0, columns[2].Sorting)
err = MoveColumnsOnProject(t.Context(), project1, map[int64]int64{
0: columns[1].ID,

View File

@ -97,7 +97,7 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp
searchOpt.SortBy = SortByDeadlineAsc
case "farduedate":
searchOpt.SortBy = SortByDeadlineDesc
case "priority", "priorityrepo", "project-column-sorting":
case "priority", "priorityrepo", issues_model.SortTypeProjectColumnSorting:
// Unsupported sort type for search
fallthrough
default:

View File

@ -13,6 +13,7 @@ import (
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/routers/api/v1/utils"
"code.gitea.io/gitea/routers/common"
"code.gitea.io/gitea/services/context"
"code.gitea.io/gitea/services/convert"
project_service "code.gitea.io/gitea/services/projects"
@ -97,18 +98,7 @@ func ListProjects(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"
state := ctx.FormTrim("state")
var isClosed optional.Option[bool]
switch state {
case "closed":
isClosed = optional.Some(true)
case "open":
isClosed = optional.Some(false)
case "all":
isClosed = optional.None[bool]()
default:
isClosed = optional.Some(false)
}
isClosed := common.ParseIssueFilterStateIsClosed(ctx.FormTrim("state"))
listOptions := utils.GetListOptions(ctx)
@ -275,30 +265,21 @@ func EditProject(ctx *context.APIContext) {
form := web.GetForm(ctx).(*api.EditProjectOption)
if form.Title != nil {
project.Title = *form.Title
}
if form.Description != nil {
project.Description = *form.Description
opts := project_service.UpdateProjectOptions{
Title: optional.FromPtr(form.Title),
Description: optional.FromPtr(form.Description),
}
if form.CardType != nil {
project.CardType = project_model.CardType(*form.CardType)
opts.CardType = optional.Some(project_model.CardType(*form.CardType))
}
if err := project_model.UpdateProject(ctx, project); err != nil {
if form.State != nil {
opts.IsClosed = optional.Some(*form.State == string(api.StateClosed))
}
if err := project_service.UpdateProject(ctx, project, opts); err != nil {
ctx.APIErrorInternal(err)
return
}
if form.State != nil {
isClosed := *form.State == string(api.StateClosed)
if isClosed != project.IsClosed {
if err := project_model.ChangeProjectStatus(ctx, project, isClosed); err != nil {
ctx.APIErrorInternal(err)
return
}
}
}
if err := project_service.LoadIssueNumbersForProject(ctx, project, ctx.Doer); err != nil {
ctx.APIErrorInternal(err)
return
@ -527,12 +508,7 @@ func EditProjectColumn(ctx *context.APIContext) {
column.Color = *form.Color
}
if form.Sorting != nil {
sorting := int8(*form.Sorting)
if int(sorting) != *form.Sorting {
ctx.APIError(http.StatusUnprocessableEntity, "sorting out of range")
return
}
column.Sorting = sorting
column.Sorting = *form.Sorting
}
if err := project_model.UpdateColumn(ctx, column); err != nil {
@ -645,7 +621,7 @@ func ListProjectColumnIssues(ctx *context.APIContext) {
RepoIDs: []int64{ctx.Repo.Repository.ID},
ProjectID: column.ProjectID,
ProjectColumnID: column.ID,
SortType: "project-column-sorting",
SortType: issues_model.SortTypeProjectColumnSorting,
}
count, err := issues_model.CountIssues(ctx, issuesOpts)
@ -713,32 +689,7 @@ func AddIssueToProjectColumn(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"
column := getRepoProjectColumn(ctx)
if ctx.Written() {
return
}
issue, err := issues_model.GetIssueByID(ctx, ctx.PathParamInt64("issue_id"))
if err != nil {
if issues_model.IsErrIssueNotExist(err) {
ctx.APIError(http.StatusUnprocessableEntity, "issue not found")
} else {
ctx.APIErrorInternal(err)
}
return
}
if issue.RepoID != ctx.Repo.Repository.ID {
ctx.APIError(http.StatusUnprocessableEntity, "issue does not belong to this repository")
return
}
if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, column.ProjectID, column.ID); err != nil {
ctx.APIErrorInternal(err)
return
}
ctx.Status(http.StatusCreated)
assignIssueToProjectColumn(ctx, true)
}
// RemoveIssueFromProjectColumn remove an issue from a project column
@ -789,31 +740,39 @@ func RemoveIssueFromProjectColumn(ctx *context.APIContext) {
// "422":
// "$ref": "#/responses/validationError"
assignIssueToProjectColumn(ctx, false)
}
// assignIssueToProjectColumn assigns an issue to a project column when add is true,
// or removes the issue from any project assignment when add is false.
func assignIssueToProjectColumn(ctx *context.APIContext, add bool) {
column := getRepoProjectColumn(ctx)
if ctx.Written() {
return
}
issue, err := issues_model.GetIssueByID(ctx, ctx.PathParamInt64("issue_id"))
issue, err := issues_model.GetIssueByRepoID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("issue_id"))
if err != nil {
if issues_model.IsErrIssueNotExist(err) {
ctx.APIError(http.StatusUnprocessableEntity, "issue not found")
ctx.APIErrorNotFound()
} else {
ctx.APIErrorInternal(err)
}
return
}
if issue.RepoID != ctx.Repo.Repository.ID {
ctx.APIError(http.StatusUnprocessableEntity, "issue does not belong to this repository")
return
projectID := int64(0)
if add {
projectID = column.ProjectID
}
// 0 means remove
if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, 0, column.ID); err != nil {
if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, column.ID); err != nil {
ctx.APIErrorInternal(err)
return
}
ctx.Status(http.StatusNoContent)
if add {
ctx.Status(http.StatusCreated)
} else {
ctx.Status(http.StatusNoContent)
}
}

View File

@ -55,7 +55,7 @@ func ToProjectColumn(ctx context.Context, column *project_model.Column) *api.Pro
ID: column.ID,
Title: column.Title,
Default: column.Default,
Sorting: int(column.Sorting),
Sorting: column.Sorting,
Color: column.Color,
ProjectID: column.ProjectID,
CreatorID: column.CreatorID,

View File

@ -469,7 +469,7 @@ type CreateProjectForm struct {
// EditProjectColumnForm is a form for editing a project column
type EditProjectColumnForm struct {
Title string `binding:"Required;MaxSize(100)"`
Sorting int8
Sorting int
Color string `binding:"MaxSize(7)"`
}

View File

@ -59,13 +59,11 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum
continue
}
projectColumnMap, err := curIssue.ProjectColumnMap(ctx)
projectColumnID, err := curIssue.ProjectColumnID(ctx)
if err != nil {
return err
}
projectColumnID := projectColumnMap[column.ProjectID]
if projectColumnID != column.ID {
// add timeline to issue
if _, err := issues_model.CreateComment(ctx, &issues_model.CreateCommentOptions{
@ -82,16 +80,7 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum
}
}
// Update the column and sorting for this specific issue in this specific project.
// IMPORTANT: The WHERE clause must include both issue_id AND project_id to ensure
// that moving an issue's column in one project doesn't affect its column in other
// projects when the issue is assigned to multiple projects.
_, err = db.GetEngine(ctx).Table("project_issue").
Where("issue_id = ? AND project_id = ?", issueID, column.ProjectID).
Update(map[string]any{
"project_board_id": column.ID,
"sorting": sorting,
})
_, err = db.Exec(ctx, "UPDATE `project_issue` SET project_board_id=?, sorting=? WHERE issue_id=?", column.ID, sorting, issueID)
if err != nil {
return err
}
@ -128,8 +117,8 @@ func LoadIssuesAssigneesForProject(ctx context.Context, issuesMap map[int64]issu
// LoadIssuesFromProject load issues assigned to each project column inside the given project
func LoadIssuesFromProject(ctx context.Context, project *project_model.Project, opts *issues_model.IssuesOptions) (results map[int64]issues_model.IssueList, _ error) {
issueList, err := issues_model.Issues(ctx, opts.Copy(func(o *issues_model.IssuesOptions) {
o.ProjectIDs = []int64{project.ID}
o.SortType = "project-column-sorting"
o.ProjectID = project.ID
o.SortType = issues_model.SortTypeProjectColumnSorting
}))
if err != nil {
return nil, err
@ -222,10 +211,10 @@ func LoadIssueNumbersForProject(ctx context.Context, project *project_model.Proj
// for user or org projects, we need to check access permissions
opts := issues_model.IssuesOptions{
ProjectIDs: []int64{project.ID},
Doer: doer,
AllPublic: doer == nil,
Owner: project.Owner,
ProjectID: project.ID,
Doer: doer,
AllPublic: doer == nil,
Owner: project.Owner,
}
var err error

View File

@ -0,0 +1,41 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package project
import (
"context"
project_model "code.gitea.io/gitea/models/project"
"code.gitea.io/gitea/modules/optional"
)
// UpdateProjectOptions represents updatable project fields. Fields with no value are left unchanged.
type UpdateProjectOptions struct {
Title optional.Option[string]
Description optional.Option[string]
CardType optional.Option[project_model.CardType]
IsClosed optional.Option[bool]
}
// UpdateProject applies the provided options to the project.
func UpdateProject(ctx context.Context, project *project_model.Project, opts UpdateProjectOptions) error {
if opts.Title.Has() {
project.Title = opts.Title.Value()
}
if opts.Description.Has() {
project.Description = opts.Description.Value()
}
if opts.CardType.Has() {
project.CardType = opts.CardType.Value()
}
if err := project_model.UpdateProject(ctx, project); err != nil {
return err
}
if opts.IsClosed.Has() && opts.IsClosed.Value() != project.IsClosed {
if err := project_model.ChangeProjectStatus(ctx, project, opts.IsClosed.Value()); err != nil {
return err
}
}
return nil
}

View File

@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"strconv"
"strings"
"testing"
"code.gitea.io/gitea/models/db"
@ -63,9 +64,9 @@ func TestMoveRepoProjectColumns(t *testing.T) {
columns, err := project_model.GetProjectColumns(t.Context(), project1.ID, db.ListOptionsAll)
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.Equal(t, 0, columns[0].Sorting)
assert.Equal(t, 1, columns[1].Sorting)
assert.Equal(t, 2, columns[2].Sorting)
sess := loginUser(t, "user1")
req := NewRequest(t, "GET", fmt.Sprintf("/%s/projects/%d", repo2.FullName(), project1.ID))
@ -90,6 +91,110 @@ func TestMoveRepoProjectColumns(t *testing.T) {
assert.NoError(t, project_model.DeleteProjectByID(t.Context(), project1.ID))
}
func TestUpdateIssueProjectColumn(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// fixture: issue 3 is in project 1 of repo user2/repo1, column "In Progress" (id=2)
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 3})
assert.EqualValues(t, 1, issue.RepoID)
sess := loginUser(t, "user2")
t.Run("MoveColumn", func(t *testing.T) {
req := NewRequestWithValues(t, "POST", "/user2/repo1/issues/projects/column", map[string]string{
"issue_id": "3",
"id": "3",
})
sess.MakeRequest(t, req, http.StatusOK)
pi := unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{IssueID: 3})
assert.EqualValues(t, 3, pi.ProjectColumnID)
})
t.Run("InvalidIssueID", func(t *testing.T) {
req := NewRequestWithValues(t, "POST", "/user2/repo1/issues/projects/column", map[string]string{
"issue_id": "0",
"id": "3",
})
sess.MakeRequest(t, req, http.StatusNotFound)
})
t.Run("WrongRepo", func(t *testing.T) {
req := NewRequestWithValues(t, "POST", "/user2/repo1/issues/projects/column", map[string]string{
"issue_id": "6",
"id": "3",
})
sess.MakeRequest(t, req, http.StatusNotFound)
})
t.Run("WrongProject", func(t *testing.T) {
project2 := project_model.Project{
Title: "second project on repo1",
RepoID: 1,
Type: project_model.TypeRepository,
TemplateType: project_model.TemplateTypeNone,
}
require.NoError(t, project_model.NewProject(t.Context(), &project2))
require.NoError(t, project_model.NewColumn(t.Context(), &project_model.Column{
Title: "other column",
ProjectID: project2.ID,
}))
columns, err := project_model.GetProjectColumns(t.Context(), project2.ID, db.ListOptionsAll)
require.NoError(t, err)
require.NotEmpty(t, columns)
req := NewRequestWithValues(t, "POST", "/user2/repo1/issues/projects/column", map[string]string{
"issue_id": "1",
"id": strconv.FormatInt(columns[0].ID, 10),
})
sess.MakeRequest(t, req, http.StatusNotFound)
})
}
func TestIssueSidebarProjectColumn(t *testing.T) {
defer tests.PrepareTestEnv(t)()
// fixture: issue 5 (index=4) is in project 1 of repo user2/repo1, column "Done" (id=3)
sess := loginUser(t, "user2")
req := NewRequest(t, "GET", "/user2/repo1/issues/4")
resp := sess.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
cards := htmlDoc.Find(".sidebar-project-card")
assert.Equal(t, 1, cards.Length())
title := cards.Find(".sidebar-project-card a.suppressed .gt-ellipsis")
assert.Contains(t, strings.TrimSpace(title.Text()), "First project")
columnCombo := cards.Find(".sidebar-project-column-combo")
assert.Equal(t, 1, columnCombo.Length())
defaultItem := columnCombo.Find(`.menu .item[data-value="1"]`)
assert.Equal(t, 1, defaultItem.Length())
inProgressItem := columnCombo.Find(`.menu .item[data-value="2"]`)
assert.Equal(t, 1, inProgressItem.Length())
doneItem := columnCombo.Find(`.menu .item[data-value="3"]`)
assert.Equal(t, 1, doneItem.Length())
comboVal, exists := columnCombo.Find("input.combo-value").Attr("value")
assert.True(t, exists)
assert.Equal(t, "3", comboVal)
req = NewRequestWithValues(t, "POST", "/user2/repo1/issues/projects?issue_ids=5", map[string]string{
"id": "0",
})
sess.MakeRequest(t, req, http.StatusOK)
req = NewRequest(t, "GET", "/user2/repo1/issues/4")
resp = sess.MakeRequest(t, req, http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
cards = htmlDoc.Find(".sidebar-project-card")
assert.Equal(t, 0, cards.Length())
}
// getProjectIssueIDs returns the set of issue IDs rendered as cards on the project board page.
func getProjectIssueIDs(t *testing.T, htmlDoc *HTMLDoc) map[int64]struct{} {
t.Helper()