mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-16 23:27:40 +02:00
fix(api): address review feedback on project board API
Three issues raised by @lunny in review of #36008 are addressed: 1. Duplicate permission checks removed The /projects route group is already wrapped with reqRepoReader and reqRepoWriter in api.go. The inline CanRead/CanWrite checks at the top of all 10 handlers were unreachable dead code. 2. AddOrUpdateIssueToColumn replaced with IssueAssignOrRemoveProject The custom function introduced in #36008 was missing a db.WithTx transaction wrapper, the CommentTypeProject audit comment written by the UI, and the CanBeAccessedByOwnerRepo cross-repo ownership guard. AddIssueToProjectColumn now delegates to the existing IssueAssignOrRemoveProject which provides all three. 3. ListProjectColumns pagination implemented correctly Added CountColumns and GetColumnsPaginated (using db.SetSessionPagination) to the project model. The handler uses utils.GetListOptions and sets X-Total-Count via ctx.SetTotalCountHeader per API contribution guidelines. Integration tests cover full list, page 1, page 2, and 404. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
4338a2b72f
commit
f59ebbcfe2
@ -185,7 +185,7 @@ func deleteColumnByID(ctx context.Context, columnID int64) error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
if err = moveIssuesToAnotherColumn(ctx, column, defaultColumn); err != nil {
|
if err = column.moveIssuesToAnotherColumn(ctx, defaultColumn); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -257,12 +257,26 @@ func (p *Project) GetColumns(ctx context.Context) (ColumnList, error) {
|
|||||||
return columns, nil
|
return columns, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// getDefaultColumnWithFallback return default column if one exists
|
// CountColumns returns the total number of columns for a project
|
||||||
// otherwise return the first column by sorting and set it as default column
|
func (p *Project) CountColumns(ctx context.Context) (int64, error) {
|
||||||
func (p *Project) getDefaultColumnWithFallback(ctx context.Context) (*Column, error) {
|
return db.GetEngine(ctx).Where("project_id=?", p.ID).Count(&Column{})
|
||||||
var column Column
|
}
|
||||||
|
|
||||||
// try to find a column "default=true"
|
// GetColumnsPaginated fetches a page of columns for a project
|
||||||
|
func (p *Project) GetColumnsPaginated(ctx context.Context, opts db.ListOptions) (ColumnList, error) {
|
||||||
|
columns := make([]*Column, 0, opts.PageSize)
|
||||||
|
if err := db.SetSessionPagination(db.GetEngine(ctx), &opts).
|
||||||
|
Where("project_id=?", p.ID).
|
||||||
|
OrderBy("sorting, id").
|
||||||
|
Find(&columns); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
return columns, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// getDefaultColumn return default column and ensure only one exists
|
||||||
|
func (p *Project) getDefaultColumn(ctx context.Context) (*Column, error) {
|
||||||
|
var column Column
|
||||||
has, err := db.GetEngine(ctx).
|
has, err := db.GetEngine(ctx).
|
||||||
Where("project_id=? AND `default` = ?", p.ID, true).
|
Where("project_id=? AND `default` = ?", p.ID, true).
|
||||||
Desc("id").Get(&column)
|
Desc("id").Get(&column)
|
||||||
@ -273,9 +287,23 @@ func (p *Project) getDefaultColumnWithFallback(ctx context.Context) (*Column, er
|
|||||||
if has {
|
if has {
|
||||||
return &column, nil
|
return &column, nil
|
||||||
}
|
}
|
||||||
|
return nil, ErrProjectColumnNotExist{ColumnID: 0}
|
||||||
|
}
|
||||||
|
|
||||||
// try to find the first column by sorting
|
// MustDefaultColumn returns the default column for a project.
|
||||||
has, err = db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Get(&column)
|
// If one exists, it is returned
|
||||||
|
// If none exists, the first column will be elevated to the default column of this project
|
||||||
|
func (p *Project) MustDefaultColumn(ctx context.Context) (*Column, error) {
|
||||||
|
c, err := p.getDefaultColumn(ctx)
|
||||||
|
if err != nil && !IsErrProjectColumnNotExist(err) {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
if c != nil {
|
||||||
|
return c, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
var column Column
|
||||||
|
has, err := db.GetEngine(ctx).Where("project_id=?", p.ID).OrderBy("sorting, id").Get(&column)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
@ -287,24 +315,8 @@ func (p *Project) getDefaultColumnWithFallback(ctx context.Context) (*Column, er
|
|||||||
return &column, nil
|
return &column, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil, ErrProjectColumnNotExist{ColumnID: 0}
|
|
||||||
}
|
|
||||||
|
|
||||||
// MustDefaultColumn returns the default column for a project.
|
|
||||||
// If one exists, it is returned
|
|
||||||
// If none exists, the first column will be elevated to the default column of this project
|
|
||||||
// If there is no column, it creates a default column and returns it
|
|
||||||
func (p *Project) MustDefaultColumn(ctx context.Context) (*Column, error) {
|
|
||||||
c, err := p.getDefaultColumnWithFallback(ctx)
|
|
||||||
if err != nil && !IsErrProjectColumnNotExist(err) {
|
|
||||||
return nil, err
|
|
||||||
}
|
|
||||||
if c != nil {
|
|
||||||
return c, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// create a default column if none is found
|
// create a default column if none is found
|
||||||
column := Column{
|
column = Column{
|
||||||
ProjectID: p.ID,
|
ProjectID: p.ID,
|
||||||
Default: true,
|
Default: true,
|
||||||
Title: "Uncategorized",
|
Title: "Uncategorized",
|
||||||
@ -337,6 +349,20 @@ func SetDefaultColumn(ctx context.Context, projectID, columnID int64) error {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// UpdateColumnSorting update project column sorting
|
||||||
|
func UpdateColumnSorting(ctx context.Context, cl ColumnList) error {
|
||||||
|
return db.WithTx(ctx, func(ctx context.Context) error {
|
||||||
|
for i := range cl {
|
||||||
|
if _, err := db.GetEngine(ctx).ID(cl[i].ID).Cols(
|
||||||
|
"sorting",
|
||||||
|
).Update(cl[i]); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (ColumnList, error) {
|
func GetColumnsByIDs(ctx context.Context, projectID int64, columnsIDs []int64) (ColumnList, error) {
|
||||||
columns := make([]*Column, 0, 5)
|
columns := make([]*Column, 0, 5)
|
||||||
if len(columnsIDs) == 0 {
|
if len(columnsIDs) == 0 {
|
||||||
|
|||||||
@ -7,6 +7,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/models/db"
|
||||||
"code.gitea.io/gitea/models/unittest"
|
"code.gitea.io/gitea/models/unittest"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
@ -123,3 +124,39 @@ func Test_NewColumn(t *testing.T) {
|
|||||||
assert.Error(t, err)
|
assert.Error(t, err)
|
||||||
assert.Contains(t, err.Error(), "maximum number of columns reached")
|
assert.Contains(t, err.Error(), "maximum number of columns reached")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCountColumns(t *testing.T) {
|
||||||
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
|
||||||
|
project, err := GetProjectByID(t.Context(), 1)
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
count, err := project.CountColumns(t.Context())
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.EqualValues(t, 3, count)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestGetColumnsPaginated(t *testing.T) {
|
||||||
|
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||||
|
|
||||||
|
project, err := GetProjectByID(t.Context(), 1)
|
||||||
|
assert.NoError(t, err)
|
||||||
|
|
||||||
|
// Page 1, limit 2 — returns first 2 columns
|
||||||
|
page1, err := project.GetColumnsPaginated(t.Context(), db.ListOptions{Page: 1, PageSize: 2})
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.Len(t, page1, 2)
|
||||||
|
|
||||||
|
// Page 2, limit 2 — returns remaining column
|
||||||
|
page2, err := project.GetColumnsPaginated(t.Context(), db.ListOptions{Page: 2, PageSize: 2})
|
||||||
|
assert.NoError(t, err)
|
||||||
|
assert.Len(t, page2, 1)
|
||||||
|
|
||||||
|
// Page 1 and page 2 together cover all columns with no overlap
|
||||||
|
allIDs := make(map[int64]bool)
|
||||||
|
for _, c := range append(page1, page2...) {
|
||||||
|
assert.False(t, allIDs[c.ID], "duplicate column ID %d across pages", c.ID)
|
||||||
|
allIDs[c.ID] = true
|
||||||
|
}
|
||||||
|
assert.Len(t, allIDs, 3)
|
||||||
|
}
|
||||||
|
|||||||
@ -13,10 +13,10 @@ import (
|
|||||||
"code.gitea.io/gitea/modules/setting"
|
"code.gitea.io/gitea/modules/setting"
|
||||||
api "code.gitea.io/gitea/modules/structs"
|
api "code.gitea.io/gitea/modules/structs"
|
||||||
"code.gitea.io/gitea/modules/web"
|
"code.gitea.io/gitea/modules/web"
|
||||||
|
"code.gitea.io/gitea/routers/api/v1/utils"
|
||||||
"code.gitea.io/gitea/services/context"
|
"code.gitea.io/gitea/services/context"
|
||||||
"code.gitea.io/gitea/services/convert"
|
"code.gitea.io/gitea/services/convert"
|
||||||
project_service "code.gitea.io/gitea/services/projects"
|
project_service "code.gitea.io/gitea/services/projects"
|
||||||
"code.gitea.io/gitea/routers/api/v1/utils"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// ListProjects lists all projects in a repository
|
// ListProjects lists all projects in a repository
|
||||||
@ -398,6 +398,7 @@ func ListProjectColumns(ctx *context.APIContext) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
ctx.SetLinkHeader(int(total), listOptions.PageSize)
|
||||||
ctx.SetTotalCountHeader(total)
|
ctx.SetTotalCountHeader(total)
|
||||||
ctx.JSON(http.StatusOK, convert.ToProjectColumnList(ctx, columns))
|
ctx.JSON(http.StatusOK, convert.ToProjectColumnList(ctx, columns))
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user