From eb1478791f2bdf77f54853e25878e3c5371a80d3 Mon Sep 17 00:00:00 2001 From: Nanguan Lin <70063547+lng2020@users.noreply.github.com> Date: Fri, 20 Oct 2023 20:01:25 +0800 Subject: [PATCH] Clean some functions about project issue (#27705) 1. remove unused function `MoveIssueAcrossProjectBoards` 2. extract the project board condition into a function 3. use db.NoCondition instead of -1. (BTW, the usage of db.NoCondition is too confusing. Is there any way to avoid that?) 4. remove the unnecessary comment since the ctx refactor is completed. 5. Change `b.ID != 0` to `b.ID > 0`. It's more intuitive but I think they're the same since board ID can't be negative. --- models/issues/issue_project.go | 31 ++-------------------------- models/issues/issue_search.go | 19 ++++++++++------- modules/indexer/issues/db/options.go | 1 - 3 files changed, 14 insertions(+), 37 deletions(-) diff --git a/models/issues/issue_project.go b/models/issues/issue_project.go index 7906c3eac..cc7ffb356 100644 --- a/models/issues/issue_project.go +++ b/models/issues/issue_project.go @@ -51,7 +51,7 @@ func (issue *Issue) ProjectBoardID(ctx context.Context) int64 { func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList, error) { issueList := make(IssueList, 0, 10) - if b.ID != 0 { + if b.ID > 0 { issues, err := Issues(ctx, &IssuesOptions{ ProjectBoardID: b.ID, ProjectID: b.ProjectID, @@ -65,7 +65,7 @@ func LoadIssuesFromBoard(ctx context.Context, b *project_model.Board) (IssueList if b.Default { issues, err := Issues(ctx, &IssuesOptions{ - ProjectBoardID: -1, // Issues without ProjectBoardID + ProjectBoardID: db.NoConditionID, ProjectID: b.ProjectID, SortType: "project-column-sorting", }) @@ -150,30 +150,3 @@ func addUpdateIssueProject(ctx context.Context, issue *Issue, doer *user_model.U ProjectID: newProjectID, }) } - -// MoveIssueAcrossProjectBoards move a card from one board to another -func MoveIssueAcrossProjectBoards(ctx context.Context, issue *Issue, board *project_model.Board) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - - var pis project_model.ProjectIssue - has, err := sess.Where("issue_id=?", issue.ID).Get(&pis) - if err != nil { - return err - } - - if !has { - return fmt.Errorf("issue has to be added to a project first") - } - - pis.ProjectBoardID = board.ID - if _, err := sess.ID(pis.ID).Cols("project_board_id").Update(&pis); err != nil { - return err - } - - return committer.Commit() -} diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 3e878ff1e..9b6bf117b 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -180,6 +180,17 @@ func applyProjectCondition(sess *xorm.Session, opts *IssuesOptions) *xorm.Sessio return sess } +func applyProjectBoardCondition(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { + // opts.ProjectBoardID == 0 means all project boards, + // do not need to apply any condition + if opts.ProjectBoardID > 0 { + sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": opts.ProjectBoardID})) + } else if opts.ProjectBoardID == db.NoConditionID { + sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Neq{"project_board_id": 0})) + } + return sess +} + func applyRepoConditions(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { if len(opts.RepoIDs) == 1 { opts.RepoCond = builder.Eq{"issue.repo_id": opts.RepoIDs[0]} @@ -240,13 +251,7 @@ func applyConditions(sess *xorm.Session, opts *IssuesOptions) *xorm.Session { applyProjectCondition(sess, opts) - if opts.ProjectBoardID != 0 { - if opts.ProjectBoardID > 0 { - sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": opts.ProjectBoardID})) - } else { - sess.In("issue.id", builder.Select("issue_id").From("project_issue").Where(builder.Eq{"project_board_id": 0})) - } - } + applyProjectBoardCondition(sess, opts) switch opts.IsPull { case util.OptionalBoolTrue: diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index 4d3fa44ca..e14906649 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -96,7 +96,6 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m } if len(options.IncludedLabelIDs) == 0 && len(options.IncludedAnyLabelIDs) > 0 { - _ = ctx // issue_model.GetLabelsByIDs should be called with ctx, this line can be removed when it's done. labels, err := issue_model.GetLabelsByIDs(ctx, options.IncludedAnyLabelIDs, "name") if err != nil { return nil, fmt.Errorf("GetLabelsByIDs: %v", err)