diff --git a/models/project/project.go b/models/project/project.go index 18c647c8ac..7d507b358d 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -309,6 +309,18 @@ func GetProjectForRepoByID(ctx context.Context, repoID, id int64) (*Project, err return p, nil } +// GetProjectForUserByID returns the project by id that belongs to the specified user. +func GetProjectForUserByID(ctx context.Context, uid, id int64) (*Project, error) { + p := new(Project) + has, err := db.GetEngine(ctx).Where("id=? AND owner_id=?", id, uid).Get(p) + if err != nil { + return nil, err + } else if !has { + return nil, ErrProjectNotExist{ID: id} + } + return p, nil +} + // UpdateProject updates project properties func UpdateProject(ctx context.Context, p *Project) error { if !IsCardTypeValid(p.CardType) { @@ -346,42 +358,26 @@ func updateRepositoryProjectCount(ctx context.Context, repoID int64) error { return nil } -// ChangeProjectStatusByRepoIDAndID toggles a project between opened and closed -func ChangeProjectStatusByRepoIDAndID(ctx context.Context, repoID, projectID int64, isClosed bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - - p := new(Project) - - has, err := db.GetEngine(ctx).ID(projectID).Where("repo_id = ?", repoID).Get(p) - if err != nil { - return err - } else if !has { - return ErrProjectNotExist{ID: projectID, RepoID: repoID} - } - - if err := changeProjectStatus(ctx, p, isClosed); err != nil { - return err - } - - return committer.Commit() -} - -func changeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { - p.IsClosed = isClosed - p.ClosedDateUnix = timeutil.TimeStampNow() - count, err := db.GetEngine(ctx).ID(p.ID).Where("repo_id = ? AND is_closed = ?", p.RepoID, !isClosed).Cols("is_closed", "closed_date_unix").Update(p) - if err != nil { - return err - } - if count < 1 { +// ChangeProjectStatus changes the status of the specified project to the state +// specified via the `isClosed` argument. +func ChangeProjectStatus(ctx context.Context, p *Project, isClosed bool) error { + if p.IsClosed == isClosed { return nil } - return updateRepositoryProjectCount(ctx, p.RepoID) + return db.WithTx(ctx, func(ctx context.Context) error { + p.IsClosed = isClosed + p.ClosedDateUnix = timeutil.TimeStampNow() + count, err := db.GetEngine(ctx).ID(p.ID).Cols("is_closed", "closed_date_unix").Update(p) + if err != nil { + return err + } + if count < 1 { + return nil + } + + return updateRepositoryProjectCount(ctx, p.RepoID) + }) } // DeleteProjectByID deletes a project from a repository. if it's not in a database diff --git a/models/project/project_test.go b/models/project/project_test.go index b6f7c7db27..73b8f01f00 100644 --- a/models/project/project_test.go +++ b/models/project/project_test.go @@ -8,7 +8,6 @@ import ( "forgejo.org/models/db" "forgejo.org/models/unittest" - "forgejo.org/modules/timeutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -48,42 +47,6 @@ func TestGetProjects(t *testing.T) { assert.Len(t, projects, 1) } -func TestProject(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - project := &Project{ - Type: TypeRepository, - TemplateType: TemplateTypeBasicKanban, - CardType: CardTypeTextOnly, - Title: "New Project", - RepoID: 1, - CreatedUnix: timeutil.TimeStampNow(), - CreatorID: 2, - } - - require.NoError(t, NewProject(db.DefaultContext, project)) - - _, err := GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - // Update project - project.Title = "Updated title" - require.NoError(t, UpdateProject(db.DefaultContext, project)) - - projectFromDB, err := GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - assert.Equal(t, project.Title, projectFromDB.Title) - - require.NoError(t, ChangeProjectStatusByRepoIDAndID(db.DefaultContext, project.RepoID, project.ID, true)) - - // Retrieve from DB afresh to check if it is truly closed - projectFromDB, err = GetProjectByID(db.DefaultContext, project.ID) - require.NoError(t, err) - - assert.True(t, projectFromDB.IsClosed) -} - func TestProjectsSort(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index 66c560f55e..a492d85d84 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -218,8 +218,13 @@ func ChangeProjectStatus(ctx *context.Context) { } id := ctx.ParamsInt64(":id") - if err := project_model.ChangeProjectStatusByRepoIDAndID(ctx, 0, id, toClose); err != nil { - ctx.NotFoundOrServerError("ChangeProjectStatusByRepoIDAndID", project_model.IsErrProjectNotExist, err) + project, err := project_model.GetProjectForUserByID(ctx, ctx.ContextUser.ID, id) + if err != nil { + ctx.NotFoundOrServerError("GetProjectForUserByID", project_model.IsErrProjectNotExist, err) + return + } + if err := project_model.ChangeProjectStatus(ctx, project, toClose); err != nil { + ctx.ServerError("ChangeProjectStatus", err) return } ctx.JSONRedirect(project_model.ProjectLinkForOrg(ctx.ContextUser, id)) diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index ecc1cc89d9..e3e9ce0eb7 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -192,8 +192,13 @@ func ChangeProjectStatus(ctx *context.Context) { } id := ctx.ParamsInt64(":id") - if err := project_model.ChangeProjectStatusByRepoIDAndID(ctx, ctx.Repo.Repository.ID, id, toClose); err != nil { - ctx.NotFoundOrServerError("ChangeProjectStatusByRepoIDAndID", project_model.IsErrProjectNotExist, err) + project, err := project_model.GetProjectForRepoByID(ctx, ctx.Repo.Repository.ID, id) + if err != nil { + ctx.NotFoundOrServerError("GetProjectForRepoByID", project_model.IsErrProjectNotExist, err) + return + } + if err := project_model.ChangeProjectStatus(ctx, project, toClose); err != nil { + ctx.ServerError("ChangeProjectStatus", err) return } ctx.JSONRedirect(project_model.ProjectLinkForRepo(ctx.Repo.Repository, id))