Skip to content

Implement the feature that close issuse as archived/merged/resolved #23522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
57cb23d
button & dropdown
sillyguodong Mar 16, 2023
13a5e3d
fix style of button & dropdown
sillyguodong Mar 16, 2023
1eb8b18
dropdown item add click event
sillyguodong Mar 16, 2023
b7ab794
dropdown js
sillyguodong Mar 16, 2023
96cca04
dropdown use fomanticUI component
sillyguodong Mar 17, 2023
9516184
complete ui
sillyguodong Mar 17, 2023
4a9e672
implement basically
sillyguodong Mar 17, 2023
4af0bcd
fix lint
sillyguodong Mar 17, 2023
ba8b08b
Merge branch 'main' into feature/issue_22793
sillyguodong Mar 17, 2023
71c03a5
keep active
sillyguodong Mar 17, 2023
baf2ab7
mark the comobox with classname 'js-aria-comobox'
sillyguodong Mar 17, 2023
a12c386
Merge branch 'main' into feature/issue_22793
sillyguodong Mar 20, 2023
7b68c3f
use svg instead of fomantic icon
sillyguodong Mar 20, 2023
b625d55
use selected
sillyguodong Mar 20, 2023
2de28aa
use selected in dropdown
sillyguodong Mar 20, 2023
1c6365e
demo
wxiaoguang Mar 20, 2023
d7db875
fix
sillyguodong Mar 20, 2023
f1ff475
fix
sillyguodong Mar 20, 2023
d0ce23f
fix lint
sillyguodong Mar 20, 2023
42cc712
fix
sillyguodong Mar 20, 2023
466fcb3
fix lint
sillyguodong Mar 20, 2023
69d46a9
rename closed status
sillyguodong Mar 21, 2023
4c092df
Merge branch 'main' into feature/issue_22793
sillyguodong Mar 21, 2023
cb2accb
add migration
sillyguodong Mar 21, 2023
f117aa0
fix and add tooltip
sillyguodong Mar 21, 2023
099fd27
Merge branch 'main' into feature/issue_22793
sillyguodong Mar 22, 2023
04e98a0
lint
sillyguodong Mar 22, 2023
3ce31c8
Merge branch 'main' into feature/issue_22793
sillyguodong Mar 22, 2023
f8b80f0
fix css format
sillyguodong Mar 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,12 @@ const (
CommentTypePRScheduledToAutoMerge
// 35 pr was un scheduled to auto merge when checks succeed
CommentTypePRUnScheduledToAutoMerge
// 36
CommentTypeCloseAsArchived
// 37
CommentTypeCloseAsResolved
// 38
CommentTypeCloseAsMerged
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's better to extend the CommentTypeClose , instead of adding too many new comment types.

)

var commentStrings = []string{
Expand Down Expand Up @@ -169,6 +175,9 @@ var commentStrings = []string{
"change_issue_ref",
"pull_scheduled_merge",
"pull_cancel_scheduled_merge",
"close_as_archived",
"close_as_resolved",
"close_as_merged",
}

func (t CommentType) String() string {
Expand Down
2 changes: 1 addition & 1 deletion models/issues/dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestCreateIssueDependency(t *testing.T) {
assert.False(t, left)

// Close #2 and check again
_, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue2, user1, true)
_, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue2, user1, true, issues_model.IssueClosedStatusCommonClosed)
assert.NoError(t, err)

left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
Expand Down
56 changes: 47 additions & 9 deletions models/issues/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ type Issue struct {
IsRead bool `xorm:"-"`
IsPull bool `xorm:"INDEX"` // Indicates whether is a pull request or not.
PullRequest *PullRequest `xorm:"-"`
ClosedStatus IssueClosedStatus
NumComments int
Ref string

Expand Down Expand Up @@ -643,15 +644,15 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error {
return nil
}

func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool) (*Comment, error) {
func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed, isMergePull bool, status IssueClosedStatus) (*Comment, error) {
// Reload the issue
currentIssue, err := GetIssueByID(ctx, issue.ID)
if err != nil {
return nil, err
}

// Nothing should be performed if current status is same as target status
if currentIssue.IsClosed == isClosed {
if currentIssue.IsClosed == isClosed && (!issue.IsPull && issue.ClosedStatus == status) {
if !issue.IsPull {
return nil, ErrIssueWasClosed{
ID: issue.ID,
Expand All @@ -663,6 +664,9 @@ func changeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User,
}

issue.IsClosed = isClosed
if !issue.IsPull {
issue.ClosedStatus = status
}
return doChangeIssueStatus(ctx, issue, doer, isMergePull)
}

Expand All @@ -686,7 +690,7 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
issue.ClosedUnix = 0
}

if err := UpdateIssueCols(ctx, issue, "is_closed", "closed_unix"); err != nil {
if err := UpdateIssueCols(ctx, issue, "is_closed", "closed_status", "closed_unix"); err != nil {
return nil, err
}

Expand All @@ -713,11 +717,24 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
}

// New action comment
cmtType := CommentTypeClose
if !issue.IsClosed {
cmtType = CommentTypeReopen
} else if isMergePull {
cmtType := CommentTypeUnknown
if isMergePull {
cmtType = CommentTypeMergePull
} else if !issue.IsClosed {
cmtType = CommentTypeReopen
} else if issue.IsPull {
cmtType = CommentTypeClose
} else {
switch issue.ClosedStatus {
case IssueClosedStatusCommonClosed:
cmtType = CommentTypeClose
case IssueClosedStatusArchived:
cmtType = CommentTypeCloseAsArchived
case IssueClosedStatusResolved:
cmtType = CommentTypeCloseAsResolved
case IssueClosedStatusMerged:
cmtType = CommentTypeCloseAsMerged
}
}

return CreateComment(ctx, &CreateCommentOptions{
Expand All @@ -729,15 +746,15 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
}

// ChangeIssueStatus changes issue status to open or closed.
func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool) (*Comment, error) {
func ChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.User, isClosed bool, status IssueClosedStatus) (*Comment, error) {
if err := issue.LoadRepo(ctx); err != nil {
return nil, err
}
if err := issue.LoadPoster(ctx); err != nil {
return nil, err
}

return changeIssueStatus(ctx, issue, doer, isClosed, false)
return changeIssueStatus(ctx, issue, doer, isClosed, false, status)
}

// ChangeIssueTitle changes the title of this issue, as the given user.
Expand Down Expand Up @@ -2051,6 +2068,10 @@ func UpdateIssueByAPI(issue *Issue, doer *user_model.User) (statusChangeComment
}

if currentIssue.IsClosed != issue.IsClosed {
issue.ClosedStatus = IssueClosedStatusOpen
if issue.IsClosed {
issue.ClosedStatus = IssueClosedStatusCommonClosed
}
statusChangeComment, err = doChangeIssueStatus(ctx, issue, doer, false)
if err != nil {
return nil, false, err
Expand Down Expand Up @@ -2498,3 +2519,20 @@ func DeleteOrphanedIssues(ctx context.Context) error {
func (issue *Issue) HasOriginalAuthor() bool {
return issue.OriginalAuthor != "" && issue.OriginalAuthorID != 0
}

type IssueClosedStatus int64

const IssueClosedStatusUndefined = -1

const (
// IssueClosedStatusOpen
IssueClosedStatusOpen IssueClosedStatus = iota
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks meanless, can check it by Issue.IsClosed

// IssueClosedStatusCommonClosed
IssueClosedStatusCommonClosed
// IssueClosedStatusArchived
IssueClosedStatusArchived
// IssueClosedStatusResolved
IssueClosedStatusResolved
// IssueClosedStatusMerged
IssueClosedStatusMerged
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about add duplicate and stale status?

)
2 changes: 1 addition & 1 deletion models/issues/issue_xref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestXRef_ResolveCrossReferences(t *testing.T) {
i1 := testCreateIssue(t, 1, 2, "title1", "content1", false)
i2 := testCreateIssue(t, 1, 2, "title2", "content2", false)
i3 := testCreateIssue(t, 1, 2, "title3", "content3", false)
_, err := issues_model.ChangeIssueStatus(db.DefaultContext, i3, d, true)
_, err := issues_model.ChangeIssueStatus(db.DefaultContext, i3, d, true, issues_model.IssueClosedStatusCommonClosed)
assert.NoError(t, err)

pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
Expand Down
2 changes: 1 addition & 1 deletion models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
return false, err
}

if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true); err != nil {
if _, err := changeIssueStatus(ctx, pr.Issue, pr.Merger, true, true, IssueClosedStatusCommonClosed); err != nil {
return false, fmt.Errorf("Issue.changeStatus: %w", err)
}

Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ var migrations = []Migration{
NewMigration("Fix incorrect project type", v1_20.FixIncorrectProjectType),
// v248 -> v249
NewMigration("Add version column to action_runner table", v1_20.AddVersionToActionRunner),
// v249 -> v250
NewMigration("Add closed_status column to issue table", v1_20.AddClosedStatusToIssue),
}

// GetCurrentDBVersion returns the current db version
Expand Down
26 changes: 26 additions & 0 deletions models/migrations/v1_20/v249.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_20 //nolint

import (
"xorm.io/xorm"
)

func AddClosedStatusToIssue(x *xorm.Engine) error {
type Issue struct {
ClosedStatus int8
}

if err := x.Sync(new(Issue)); err != nil {
return err
}

// TODO: TBD Whether to use issues_model.IssueClosedStatusUndefined (-1)
if _, err := x.Exec("UPDATE issue SET closed_status = ? WHERE closed_status IS NULL and is_pull = false AND is_closed = true", 1); err != nil {
return err
}

_, err := x.Exec("UPDATE issue SET closed_status = ? WHERE closed_status IS NULL and is_pull = false AND is_closed = false", 0)
return err
}
16 changes: 16 additions & 0 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,22 @@ func NewFuncMap() []template.FuncMap {
"RefShortName": func(ref string) string {
return git.RefName(ref).ShortName()
},
"ParseIssueClosedStatus2TranslateStr": func(cs issues_model.IssueClosedStatus) string {
switch cs {
case issues_model.IssueClosedStatusOpen:
return ""
case issues_model.IssueClosedStatusCommonClosed:
return "repo.issues.close_as.common"
case issues_model.IssueClosedStatusArchived:
return "repo.issues.close_as.archived"
case issues_model.IssueClosedStatusResolved:
return "repo.issues.close_as.resolved"
case issues_model.IssueClosedStatusMerged:
return "repo.issues.close_as.merged"
default:
return ""
}
},
}}
}

Expand Down
13 changes: 13 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,9 @@ issues.reopen_issue = Reopen
issues.reopen_comment_issue = Comment and Reopen
issues.create_comment = Comment
issues.closed_at = `closed this issue <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.closed_as_archived_at = `closed this issue as archived <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.closed_as_resolved_at = `closed this issue as resolved <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.closed_as_merged_at = `closed this issue as merged <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.reopened_at = `reopened this issue <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.commit_ref_at = `referenced this issue from a commit <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.ref_issue_from = `<a href="%[3]s">referenced this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>`
Expand All @@ -1387,6 +1390,16 @@ issues.ref_reopening_from = `<a href="%[3]s">referenced a pull request %[4]s tha
issues.ref_closed_from = `<a href="%[3]s">closed this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.ref_reopened_from = `<a href="%[3]s">reopened this issue %[4]s</a> <a id="%[1]s" href="#%[1]s">%[2]s</a>`
issues.ref_from = `from %[1]s`
issues.close_as.common = Close Issue
issues.close_as.archived = Close as archived
issues.close_as.resolved = Close as resolved
issues.close_as.merged = Close as merged
issues.close_as.reopen = Reopen
issues.comment_and_close_as.common = Comment and Close Issue
issues.comment_and_close_as.archived = Comment and Close as archived
issues.comment_and_close_as.resolved = Comment and Close as resolved
issues.comment_and_close_as.merged = Comment and Close as merged
issues.comment_and_close_as.reopen = Comment and Reopen
issues.poster = Poster
issues.collaborator = Collaborator
issues.owner = Owner
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ func CreateIssue(ctx *context.APIContext) {
}

if form.Closed {
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", true); err != nil {
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", true, issues_model.IssueClosedStatusCommonClosed); err != nil {
if issues_model.IsErrDependenciesLeft(err) {
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
return
Expand Down
7 changes: 5 additions & 2 deletions routers/web/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -2619,11 +2619,14 @@ func UpdateIssueStatus(ctx *context.Context) {
}

var isClosed bool
var status issues_model.IssueClosedStatus
switch action := ctx.FormString("action"); action {
case "open":
isClosed = false
status = issues_model.IssueClosedStatusOpen
case "close":
isClosed = true
status = issues_model.IssueClosedStatusCommonClosed
default:
log.Warn("Unrecognized action: %s", action)
}
Expand All @@ -2634,7 +2637,7 @@ func UpdateIssueStatus(ctx *context.Context) {
}
for _, issue := range issues {
if issue.IsClosed != isClosed {
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed); err != nil {
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed, status); err != nil {
if issues_model.IsErrDependenciesLeft(err) {
ctx.JSON(http.StatusPreconditionFailed, map[string]interface{}{
"error": "cannot close this issue because it still has open dependencies",
Expand Down Expand Up @@ -2731,7 +2734,7 @@ func NewComment(ctx *context.Context) {
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
} else {
isClosed := form.Status == "close"
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed); err != nil {
if err := issue_service.ChangeStatus(issue, ctx.Doer, "", isClosed, form.ClosedStatus); err != nil {
log.Error("ChangeStatus: %v", err)

if issues_model.IsErrDependenciesLeft(err) {
Expand Down
7 changes: 4 additions & 3 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,10 @@ func (f *CreateIssueForm) Validate(req *http.Request, errs binding.Errors) bindi

// CreateCommentForm form for creating comment
type CreateCommentForm struct {
Content string
Status string `binding:"OmitEmpty;In(reopen,close)"`
Files []string
Content string
Status string `binding:"OmitEmpty;In(reopen,close)"`
ClosedStatus issues_model.IssueClosedStatus
Files []string
}

// Validate validates the fields
Expand Down
6 changes: 5 additions & 1 deletion services/issue/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ func UpdateIssuesCommit(doer *user_model.User, repo *repo_model.Repository, comm
}
if close != refIssue.IsClosed {
refIssue.Repo = refRepo
if err := ChangeStatus(refIssue, doer, c.Sha1, close); err != nil {
status := issues_model.IssueClosedStatusOpen
if close {
status = issues_model.IssueClosedStatusCommonClosed
}
if err := ChangeStatus(refIssue, doer, c.Sha1, close, status); err != nil {
return err
}
}
Expand Down
8 changes: 4 additions & 4 deletions services/issue/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@ import (
)

// ChangeStatus changes issue status to open or closed.
func ChangeStatus(issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
return changeStatusCtx(db.DefaultContext, issue, doer, commitID, closed)
func ChangeStatus(issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool, status issues_model.IssueClosedStatus) error {
return changeStatusCtx(db.DefaultContext, issue, doer, commitID, closed, status)
}

// changeStatusCtx changes issue status to open or closed.
// TODO: if context is not db.DefaultContext we get a deadlock!!!
func changeStatusCtx(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool) error {
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed)
func changeStatusCtx(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, closed bool, status issues_model.IssueClosedStatus) error {
comment, err := issues_model.ChangeIssueStatus(ctx, issue, doer, closed, status)
if err != nil {
if issues_model.IsErrDependenciesLeft(err) && closed {
if err := issues_model.FinishIssueStopwatchIfPossible(ctx, doer, issue); err != nil {
Expand Down
6 changes: 5 additions & 1 deletion services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,11 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U
}
close := ref.RefAction == references.XRefActionCloses
if close != ref.Issue.IsClosed {
if err = issue_service.ChangeStatus(ref.Issue, doer, pr.MergedCommitID, close); err != nil {
status := issues_model.IssueClosedStatusOpen
if close {
status = issues_model.IssueClosedStatusCommonClosed
}
if err = issue_service.ChangeStatus(ref.Issue, doer, pr.MergedCommitID, close, status); err != nil {
// Allow ErrDependenciesLeft
if !issues_model.IsErrDependenciesLeft(err) {
return err
Expand Down
4 changes: 2 additions & 2 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error

var errs errlist
for _, pr := range prs {
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true, issues_model.IssueClosedStatusCommonClosed); err != nil && !issues_model.IsErrPullWasClosed(err) && !issues_model.IsErrDependenciesLeft(err) {
errs = append(errs, err)
}
}
Expand Down Expand Up @@ -550,7 +550,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re
if pr.BaseRepoID == repo.ID {
continue
}
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true); err != nil && !issues_model.IsErrPullWasClosed(err) {
if err = issue_service.ChangeStatus(pr.Issue, doer, "", true, issues_model.IssueClosedStatusCommonClosed); err != nil && !issues_model.IsErrPullWasClosed(err) {
errs = append(errs, err)
}
}
Expand Down
Loading