diff --git a/models/issues/close_reason.go b/models/issues/close_reason.go new file mode 100644 index 0000000000000..0430ec8123c51 --- /dev/null +++ b/models/issues/close_reason.go @@ -0,0 +1,64 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "fmt" +) + +type IssueCloseReason int64 + +const ( + IssueCloseReasonNone IssueCloseReason = iota + IssueCloseReasonCompleted + IssueCloseReasonCompletedByCommit + IssueCloseReasonCompletedByPull + IssueCloseReasonAnswered + IssueCloseReasonDuplicate + IssueCloseReasonNotPlanned +) + +func (r IssueCloseReason) String() string { + switch r { + case IssueCloseReasonCompleted: + return "completed" + case IssueCloseReasonCompletedByCommit: + return "completed_by_commit" + case IssueCloseReasonCompletedByPull: + return "completed_by_pull" + case IssueCloseReasonAnswered: + return "answered" + case IssueCloseReasonDuplicate: + return "duplicate" + case IssueCloseReasonNotPlanned: + return "not_planned" + default: + return "" + } +} + +func (r IssueCloseReason) IsValid() bool { + return r >= IssueCloseReasonNone && r <= IssueCloseReasonNotPlanned +} + +func ParseIssueCloseReason(reason string) (IssueCloseReason, error) { + switch reason { + case "": + return IssueCloseReasonNone, nil + case "completed": + return IssueCloseReasonCompleted, nil + case "completed_by_commit": + return IssueCloseReasonCompletedByCommit, nil + case "completed_by_pull": + return IssueCloseReasonCompletedByPull, nil + case "answered": + return IssueCloseReasonAnswered, nil + case "duplicate": + return IssueCloseReasonDuplicate, nil + case "not_planned": + return IssueCloseReasonNotPlanned, nil + default: + return IssueCloseReasonNone, fmt.Errorf("unknown close reason %q", reason) + } +} diff --git a/models/issues/close_reason_display.go b/models/issues/close_reason_display.go new file mode 100644 index 0000000000000..82ad70c65599e --- /dev/null +++ b/models/issues/close_reason_display.go @@ -0,0 +1,84 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import "code.gitea.io/gitea/modules/json" + +type closeReasonParam struct { + IssueIndex int64 `json:"issue_index"` + CommentID int64 `json:"comment_id"` + CommitHash string `json:"commit_hash"` + PullIndex int64 `json:"pull_index"` +} + +func parseCloseReasonParam(param string) closeReasonParam { + if param == "" { + return closeReasonParam{} + } + var p closeReasonParam + _ = json.Unmarshal([]byte(param), &p) + return p +} + +func normalizeCloseReason(isClosed bool, reason IssueCloseReason) string { + if isClosed && reason == IssueCloseReasonNone { + return IssueCloseReasonCompleted.String() + } + return reason.String() +} + +func (issue *Issue) CloseReasonForDisplay() string { + return normalizeCloseReason(issue.IsClosed, issue.CloseReason) +} + +func (issue *Issue) CloseReasonDuplicateIssueIndex() int64 { + return parseCloseReasonParam(issue.CloseReasonParam).IssueIndex +} + +func (issue *Issue) CloseReasonAnsweredCommentID() int64 { + return parseCloseReasonParam(issue.CloseReasonParam).CommentID +} + +func (issue *Issue) CloseReasonCommitHash() string { + return parseCloseReasonParam(issue.CloseReasonParam).CommitHash +} + +func (issue *Issue) CloseReasonPullIndex() int64 { + return parseCloseReasonParam(issue.CloseReasonParam).PullIndex +} + +func (c *Comment) CloseReasonForDisplay() string { + if c.CommentMetaData == nil { + return "" + } + return normalizeCloseReason(true, c.CommentMetaData.CloseReason) +} + +func (c *Comment) CloseReasonDuplicateIssueIndex() int64 { + if c.CommentMetaData == nil { + return 0 + } + return parseCloseReasonParam(c.CommentMetaData.CloseReasonParam).IssueIndex +} + +func (c *Comment) CloseReasonAnsweredCommentID() int64 { + if c.CommentMetaData == nil { + return 0 + } + return parseCloseReasonParam(c.CommentMetaData.CloseReasonParam).CommentID +} + +func (c *Comment) CloseReasonCommitHash() string { + if c.CommentMetaData == nil { + return "" + } + return parseCloseReasonParam(c.CommentMetaData.CloseReasonParam).CommitHash +} + +func (c *Comment) CloseReasonPullIndex() int64 { + if c.CommentMetaData == nil { + return 0 + } + return parseCloseReasonParam(c.CommentMetaData.CloseReasonParam).PullIndex +} diff --git a/models/issues/comment.go b/models/issues/comment.go index 25e74c01eab54..9ec2e65c91715 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -116,6 +116,8 @@ const ( CommentTypeUnpin // 37 unpin Issue/PullRequest CommentTypeChangeTimeEstimate // 38 Change time estimate + + CommentTypeCloseWithReason // 39 Close an issue with a structured close reason ) var commentStrings = []string{ @@ -158,6 +160,7 @@ var commentStrings = []string{ "pin", "unpin", "change_time_estimate", + "close_with_reason", } func (t CommentType) String() string { @@ -191,7 +194,7 @@ func (t CommentType) HasAttachmentSupport() bool { func (t CommentType) HasMailReplySupport() bool { switch t { - case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeReopen, CommentTypeClose, CommentTypeMergePull, CommentTypeAssignees: + case CommentTypeComment, CommentTypeCode, CommentTypeReview, CommentTypeDismissReview, CommentTypeReopen, CommentTypeClose, CommentTypeCloseWithReason, CommentTypeMergePull, CommentTypeAssignees: return true } return false @@ -240,9 +243,11 @@ const SpecialDoerNameCodeOwners SpecialDoerNameType = "CODEOWNERS" // CommentMetaData stores metadata for a comment, these data will not be changed once inserted into database type CommentMetaData struct { - ProjectColumnID int64 `json:"project_column_id,omitempty"` - ProjectColumnTitle string `json:"project_column_title,omitempty"` - ProjectTitle string `json:"project_title,omitempty"` + ProjectColumnID int64 `json:"project_column_id,omitempty"` + ProjectColumnTitle string `json:"project_column_title,omitempty"` + ProjectTitle string `json:"project_title,omitempty"` + CloseReason IssueCloseReason `json:"close_reason,omitempty"` + CloseReasonParam string `json:"close_reason_param,omitempty"` SpecialDoerName SpecialDoerNameType `json:"special_doer_name,omitempty"` // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests } @@ -823,6 +828,12 @@ func CreateComment(ctx context.Context, opts *CreateCommentOptions) (_ *Comment, SpecialDoerName: opts.SpecialDoerName, } } + if opts.CloseReason != IssueCloseReasonNone { + commentMetaData = &CommentMetaData{ + CloseReason: opts.CloseReason, + CloseReasonParam: opts.CloseReasonParam, + } + } comment := &Comment{ Type: opts.Type, @@ -1020,6 +1031,8 @@ type CreateCommentOptions struct { IsForcePush bool Invalidated bool SpecialDoerName SpecialDoerNameType // e.g. "CODEOWNERS" for CODEOWNERS-triggered review requests + CloseReason IssueCloseReason // for CommentTypeCloseWithReason + CloseReasonParam string // JSON-serialized param for the close reason } // GetCommentByID returns the comment by given ID. diff --git a/models/issues/dependency_test.go b/models/issues/dependency_test.go index 0e6a870ff9e96..2671abdde0fde 100644 --- a/models/issues/dependency_test.go +++ b/models/issues/dependency_test.go @@ -48,7 +48,7 @@ func TestCreateIssueDependency(t *testing.T) { assert.False(t, left) // Close #2 and check again - _, err = issues_model.CloseIssue(t.Context(), issue2, user1) + _, err = issues_model.CloseIssue(t.Context(), issue2, user1, issues_model.IssueCloseOptions{}) assert.NoError(t, err) issue2Closed, err := issues_model.GetIssueByID(t.Context(), 2) diff --git a/models/issues/issue.go b/models/issues/issue.go index 655cdebdfc6b9..9cee85a63e29f 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -122,6 +122,9 @@ type Issue struct { // Time estimate TimeEstimate int64 `xorm:"NOT NULL DEFAULT 0"` + + CloseReason IssueCloseReason `xorm:"INDEX DEFAULT 0"` + CloseReasonParam string `xorm:"TEXT"` } var ( diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 01a3eb9a2afc3..202f0c8cd0fa8 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -30,6 +30,17 @@ func UpdateIssueCols(ctx context.Context, issue *Issue, cols ...string) error { return err } +// IssueCloseOptions carries the close-reason payload for SetIssueAsClosed / CloseIssue. +// CloseReason and CloseReasonParam follow the same schema as Issue.CloseReason / +// Issue.CloseReasonParam (see services/issue/close_reason.go for the full set of +// constants). IsMergePull is a legacy flag preserved for the PR-merge path; it will +// be removed once step 7 migrates that path to use CloseReasonCompletedByPull. +type IssueCloseOptions struct { + CloseReason IssueCloseReason + CloseReasonParam string + IsMergePull bool // when true and CloseReason is empty, use CommentTypeMergePull +} + // ErrIssueIsClosed is used when close a closed issue type ErrIssueIsClosed struct { ID int64 @@ -48,7 +59,7 @@ func (err ErrIssueIsClosed) Error() string { return fmt.Sprintf("%s [id: %d, repo_id: %d, index: %d] is already closed", util.Iif(err.IsPull, "Pull Request", "Issue"), err.ID, err.RepoID, err.Index) } -func SetIssueAsClosed(ctx context.Context, issue *Issue, doer *user_model.User, isMergePull bool) (*Comment, error) { +func SetIssueAsClosed(ctx context.Context, issue *Issue, doer *user_model.User, opts IssueCloseOptions) (*Comment, error) { if issue.IsClosed { return nil, ErrIssueIsClosed{ ID: issue.ID, @@ -73,8 +84,10 @@ func SetIssueAsClosed(ctx context.Context, issue *Issue, doer *user_model.User, issue.IsClosed = true issue.ClosedUnix = timeutil.TimeStampNow() + issue.CloseReason = opts.CloseReason + issue.CloseReasonParam = opts.CloseReasonParam - if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix"). + if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix", "close_reason", "close_reason_param"). Where("is_closed = ?", false). Update(issue); err != nil { return nil, err @@ -82,7 +95,16 @@ func SetIssueAsClosed(ctx context.Context, issue *Issue, doer *user_model.User, return nil, ErrIssueAlreadyChanged } - return updateIssueNumbers(ctx, issue, doer, util.Iif(isMergePull, CommentTypeMergePull, CommentTypeClose)) + var cmtType CommentType + switch { + case opts.CloseReason != IssueCloseReasonNone: + cmtType = CommentTypeCloseWithReason + case opts.IsMergePull: + cmtType = CommentTypeMergePull + default: + cmtType = CommentTypeClose + } + return updateIssueNumbers(ctx, issue, doer, cmtType, opts.CloseReason, opts.CloseReasonParam) } // ErrIssueIsOpen is used when reopen an opened issue @@ -115,8 +137,10 @@ func setIssueAsReopen(ctx context.Context, issue *Issue, doer *user_model.User) issue.IsClosed = false issue.ClosedUnix = 0 + issue.CloseReason = IssueCloseReasonNone + issue.CloseReasonParam = "" - if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix"). + if cnt, err := db.GetEngine(ctx).ID(issue.ID).Cols("is_closed", "closed_unix", "close_reason", "close_reason_param"). Where("is_closed = ?", true). Update(issue); err != nil { return nil, err @@ -124,10 +148,10 @@ func setIssueAsReopen(ctx context.Context, issue *Issue, doer *user_model.User) return nil, ErrIssueAlreadyChanged } - return updateIssueNumbers(ctx, issue, doer, CommentTypeReopen) + return updateIssueNumbers(ctx, issue, doer, CommentTypeReopen, IssueCloseReasonNone, "") } -func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User, cmtType CommentType) (*Comment, error) { +func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User, cmtType CommentType, closeReason IssueCloseReason, closeReasonParam string) (*Comment, error) { // Update issue count of labels if err := issue.LoadLabels(ctx); err != nil { return nil, err @@ -147,7 +171,7 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User // update repository's issue closed number switch cmtType { - case CommentTypeClose, CommentTypeMergePull: + case CommentTypeClose, CommentTypeMergePull, CommentTypeCloseWithReason: // only increase closed count if err := IncrRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { return nil, err @@ -162,15 +186,17 @@ func updateIssueNumbers(ctx context.Context, issue *Issue, doer *user_model.User } return CreateComment(ctx, &CreateCommentOptions{ - Type: cmtType, - Doer: doer, - Repo: issue.Repo, - Issue: issue, + Type: cmtType, + Doer: doer, + Repo: issue.Repo, + Issue: issue, + CloseReason: closeReason, + CloseReasonParam: closeReasonParam, }) } // CloseIssue changes issue status to closed. -func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comment, error) { +func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User, opts IssueCloseOptions) (*Comment, error) { if err := issue.LoadRepo(ctx); err != nil { return nil, err } @@ -179,7 +205,7 @@ func CloseIssue(ctx context.Context, issue *Issue, doer *user_model.User) (*Comm } return db.WithTx2(ctx, func(ctx context.Context) (*Comment, error) { - return SetIssueAsClosed(ctx, issue, doer, false) + return SetIssueAsClosed(ctx, issue, doer, opts) }) } diff --git a/models/issues/issue_xref_test.go b/models/issues/issue_xref_test.go index b25a704bec28f..8565c565a942d 100644 --- a/models/issues/issue_xref_test.go +++ b/models/issues/issue_xref_test.go @@ -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.CloseIssue(t.Context(), i3, d) + _, err := issues_model.CloseIssue(t.Context(), i3, d, issues_model.IssueCloseOptions{}) assert.NoError(t, err) pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index)) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index cad4156dee862..792aa873fb690 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -405,6 +405,7 @@ func prepareMigrationTasks() []*migration { newMigration(328, "Add TokenPermissions column to ActionRunJob", v1_26.AddTokenPermissionsToActionRunJob), newMigration(329, "Add unique constraint for user badge", v1_26.AddUniqueIndexForUserBadge), newMigration(330, "Add name column to webhook", v1_26.AddNameToWebhook), + newMigration(331, "Add close reason columns to issue", v1_26.AddCloseReasonColumnsToIssue), } return preparedMigrations } diff --git a/models/migrations/v1_26/v331.go b/models/migrations/v1_26/v331.go new file mode 100644 index 0000000000000..f2062ae0fc0f0 --- /dev/null +++ b/models/migrations/v1_26/v331.go @@ -0,0 +1,16 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_26 + +import "xorm.io/xorm" + +func AddCloseReasonColumnsToIssue(x *xorm.Engine) error { + type Issue struct { + CloseReason int64 `xorm:"INDEX DEFAULT 0"` + CloseReasonParam string `xorm:"TEXT"` + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(Issue)) + return err +} diff --git a/modules/structs/issue.go b/modules/structs/issue.go index fd29727a4365e..68d760f2ac61d 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -77,6 +77,10 @@ type Issue struct { TimeEstimate int64 `json:"time_estimate"` + // StateReason is the reason why the issue is open/closed (for closed issues) + StateReason string `json:"state_reason,omitempty"` + StateReasonParam any `json:"state_reason_param,omitempty"` + PullRequest *PullRequestMeta `json:"pull_request"` Repo *RepositoryMeta `json:"repository"` @@ -109,10 +113,12 @@ type EditIssueOption struct { Body *string `json:"body"` Ref *string `json:"ref"` // deprecated - Assignee *string `json:"assignee"` - Assignees []string `json:"assignees"` - Milestone *int64 `json:"milestone"` - State *string `json:"state"` + Assignee *string `json:"assignee"` + Assignees []string `json:"assignees"` + Milestone *int64 `json:"milestone"` + State *string `json:"state"` + StateReason *string `json:"state_reason"` + StateReasonParam *string `json:"state_reason_param,omitempty"` // swagger:strfmt date-time Deadline *time.Time `json:"due_date"` RemoveDeadline *bool `json:"unset_due_date"` diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 7cd1fa024517f..e04feb6a1edc9 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -1535,11 +1535,50 @@ "repo.issues.comment_pull_merged_at": "merged commit %[1]s into %[2]s %[3]s", "repo.issues.comment_manually_pull_merged_at": "manually merged commit %[1]s into %[2]s %[3]s", "repo.issues.close_comment_issue": "Close with Comment", + "repo.issues.close_reason.completed_action": "Close as completed", + "repo.issues.close_reason.completed_action_with_comment": "Close as completed with comment", + "repo.issues.close_reason.completed_desc": "Done, closed, fixed, resolved", + "repo.issues.close_reason.not_planned_action": "Close as not planned", + "repo.issues.close_reason.not_planned_action_with_comment": "Close as not planned with comment", + "repo.issues.close_reason.not_planned_desc": "Won't fix, can't repro, stale", + "repo.issues.close_reason.duplicate_action": "Close as duplicate", + "repo.issues.close_reason.duplicate_action_label": "Close as duplicate with ...", + "repo.issues.close_reason.duplicate_action_with_comment": "Close as duplicate with comment", + "repo.issues.close_reason.duplicate_desc": "Duplicate of another issue", + "repo.issues.close_reason.duplicate_action_of": "Close as duplicate of #%s", + "repo.issues.close_reason.duplicate_action_of_with_comment": "Close as duplicate of #%s with comment", + "repo.issues.close_reason.duplicate_placeholder": "Issue number (e.g. 123)", + "repo.issues.close_reason.duplicate_modal_title": "Close as duplicate", + "repo.issues.close_reason.duplicate_modal_desc": "Select the issue that this issue duplicates.", + "repo.issues.close_reason.duplicate_no_results": "No matching issues found", + "repo.issues.close_reason.answered_action": "Close as answered", + "repo.issues.close_reason.answered_action_with_comment": "Close as answered with comment", + "repo.issues.close_reason.answered_desc": "Resolved/answered by this comment", + "repo.issues.close_reason.answered_placeholder": "Comment ID", + "repo.issues.close_reason.answered_requires_comment": "Close as answered requires a comment", + "repo.issues.close_reason.system_only": "This close reason is system-only", + "repo.issues.close_reason.closed_as_duplicate": "Closed as duplicate of %s", + "repo.issues.close_reason.closed_as_answered_by_comment": "Closed as answered by %s", + "repo.issues.close_reason.closed_by_commit": "Closed by commit %s", + "repo.issues.close_reason.closed_by_pull": "Completed by pull request %s", + "repo.issues.close_reason.closed_as_completed": "Closed as completed", + "repo.issues.close_reason.closed_as_not_planned": "Closed as not planned", + "repo.issues.close_reason.closed_as_answered": "Closed as answered", + "repo.issues.close_reason.closed_as_duplicate_generic": "Closed as duplicate", + "repo.issues.close_reason.as_completed": "as completed", + "repo.issues.close_reason.as_not_planned": "as not planned", + "repo.issues.close_reason.as_duplicate": "as duplicate", + "repo.issues.close_reason.as_duplicate_of": "as duplicate of %s", + "repo.issues.close_reason.as_answered": "as answered", + "repo.issues.close_reason.as_answered_by_comment": "as answered by %s", + "repo.issues.close_reason.via_commit": "via commit %s", + "repo.issues.close_reason.via_pull": "via pull request %s", "repo.issues.reopen_issue": "Reopen Issue", "repo.issues.reopen_comment_issue": "Reopen with Comment", "repo.issues.create_comment": "Comment", "repo.issues.comment.blocked_user": "Cannot create or edit comment because you are blocked by the poster or repository owner.", "repo.issues.closed_at": "closed this issue %[2]s", + "repo.issues.closed_at_with_reason": "closed this issue %[1]s %[3]s", "repo.issues.reopened_at": "reopened this issue %[2]s", "repo.issues.commit_ref_at": "referenced this issue from a commit %[2]s", "repo.issues.ref_issue_from": "referenced this issue %[4]s %[2]s", diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 20ccd099a478e..b75ba9e091746 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -702,7 +702,7 @@ func CreateIssue(ctx *context.APIContext) { } if form.Closed { - if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { + if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", issue_service.CloseOptionsCompleted()); err != nil { if issues_model.IsErrDependenciesLeft(err) { ctx.APIError(http.StatusPreconditionFailed, "cannot close this issue because it still has open dependencies") return @@ -907,7 +907,28 @@ func EditIssue(ctx *context.APIContext) { } state := api.StateType(*form.State) - closeOrReopenIssue(ctx, issue, state) + opts := issue_service.CloseOptions{} + if state == api.StateClosed && form.StateReason != nil { + reason, err := issues_model.ParseIssueCloseReason(*form.StateReason) + if err != nil { + ctx.APIError(http.StatusBadRequest, err) + return + } + opts.Reason = reason + if form.StateReasonParam != nil { + opts.ReasonParam = *form.StateReasonParam + } + opts.Normalize() + if opts.IsSystemOnly() { + ctx.APIError(http.StatusBadRequest, ctx.Locale.TrString("repo.issues.close_reason.system_only")) + return + } + if err := opts.Validate(ctx, issue); err != nil { + ctx.APIError(http.StatusBadRequest, err) + return + } + } + closeOrReopenIssue(ctx, issue, state, opts) if ctx.Written() { return } @@ -1034,14 +1055,14 @@ func UpdateIssueDeadline(ctx *context.APIContext) { ctx.JSON(http.StatusCreated, api.IssueDeadline{Deadline: deadlineUnix.AsTimePtr()}) } -func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, state api.StateType) { +func closeOrReopenIssue(ctx *context.APIContext, issue *issues_model.Issue, state api.StateType, opts issue_service.CloseOptions) { if state != api.StateOpen && state != api.StateClosed { ctx.APIError(http.StatusPreconditionFailed, fmt.Sprintf("unknown state: %s", state)) return } if state == api.StateClosed && !issue.IsClosed { - if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { + if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", opts); err != nil { if issues_model.IsErrDependenciesLeft(err) { ctx.APIError(http.StatusPreconditionFailed, "cannot close this issue or pull request because it still has open dependencies") return diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index ef86f413b7050..880b311610323 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -773,7 +773,7 @@ func EditPullRequest(ctx *context.APIContext) { } state := api.StateType(*form.State) - closeOrReopenIssue(ctx, issue, state) + closeOrReopenIssue(ctx, issue, state, issue_service.CloseOptions{}) if ctx.Written() { return } diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index 7f8cc23a3f909..74d17031064df 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -16,6 +16,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" repo_module "code.gitea.io/gitea/modules/repository" @@ -157,7 +158,39 @@ func NewComment(ctx *context.Context) { ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index)) } else { if form.Status == "close" && !issue.IsClosed { - if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { + opts := issue_service.CloseOptions{} + if form.StateReason != nil { + reason, err := issues_model.ParseIssueCloseReason(*form.StateReason) + if err != nil { + ctx.JSONError(err.Error()) + return + } + opts.Reason = reason + opts.ReasonParam = form.StateReasonParam + if opts.Reason == issue_service.CloseReasonAnswered && opts.ReasonParam == "" { + if comment == nil { + ctx.JSONError(ctx.Tr("repo.issues.close_reason.answered_requires_comment")) + return + } + b, err := json.Marshal(issue_service.CloseReasonAnsweredParam{CommentID: comment.ID}) + if err != nil { + ctx.ServerError("Marshal answered close reason param", err) + return + } + opts.ReasonParam = string(b) + } + } + opts.Normalize() + if opts.IsSystemOnly() { + ctx.JSONError(ctx.Tr("repo.issues.close_reason.system_only")) + return + } + if err := opts.Validate(ctx, issue); err != nil { + log.Error("CloseIssue opts.Validate: %v", err) + ctx.JSONError(err.Error()) + return + } + if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", opts); err != nil { log.Error("CloseIssue: %v", err) if issues_model.IsErrDependenciesLeft(err) { if issue.IsPull { diff --git a/routers/web/repo/issue_list.go b/routers/web/repo/issue_list.go index 83ef515bde5ad..0d40a86b37be1 100644 --- a/routers/web/repo/issue_list.go +++ b/routers/web/repo/issue_list.go @@ -419,7 +419,7 @@ func UpdateIssueStatus(ctx *context.Context) { continue } if action == "close" && !issue.IsClosed { - if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil { + if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, "", issue_service.CloseOptionsCompleted()); err != nil { if issues_model.IsErrDependenciesLeft(err) { ctx.JSON(http.StatusPreconditionFailed, map[string]any{ "error": ctx.Tr("repo.issues.dependency.issue_batch_close_blocked", issue.Index), diff --git a/services/convert/issue.go b/services/convert/issue.go index 61f11d8f191bf..f4af4ade1c1ba 100644 --- a/services/convert/issue.go +++ b/services/convert/issue.go @@ -14,6 +14,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/label" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -124,6 +125,20 @@ func toIssue(ctx context.Context, doer *user_model.User, issue *issues_model.Iss apiIssue.Deadline = issue.DeadlineUnix.AsTimePtr() } + // Set state_reason: if issue is closed but close_reason is empty, treat as "completed" + if issue.IsClosed { + if issue.CloseReason == issues_model.IssueCloseReasonNone { + apiIssue.StateReason = "completed" + } else { + apiIssue.StateReason = issue.CloseReason.String() + } + if issue.CloseReasonParam != "" { + var param any + _ = json.Unmarshal([]byte(issue.CloseReasonParam), ¶m) + apiIssue.StateReasonParam = param + } + } + return apiIssue } diff --git a/services/convert/issue_test.go b/services/convert/issue_test.go index 109bf63e7db6b..13187d0db9cc1 100644 --- a/services/convert/issue_test.go +++ b/services/convert/issue_test.go @@ -13,6 +13,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" @@ -21,6 +22,40 @@ import ( "github.com/stretchr/testify/require" ) +func TestToAPIIssue_StateReason_DefaultCompleted(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 4}) + require.True(t, issue.IsClosed) + // Fixture closed issue has no close_reason persisted; API should expose "completed". + assert.Empty(t, issue.CloseReason) + + apiIssue := ToAPIIssue(t.Context(), nil, issue) + assert.Equal(t, api.StateClosed, apiIssue.State) + assert.Equal(t, "completed", apiIssue.StateReason) + assert.Nil(t, apiIssue.StateReasonParam) +} + +func TestToAPIIssue_StateReason_WithParam(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + issue.IsClosed = true + issue.CloseReason = issues_model.IssueCloseReasonDuplicate + + b, err := json.Marshal(map[string]int64{"issue_index": 4}) + require.NoError(t, err) + issue.CloseReasonParam = string(b) + + apiIssue := ToAPIIssue(t.Context(), nil, issue) + assert.Equal(t, api.StateClosed, apiIssue.State) + assert.Equal(t, "duplicate", apiIssue.StateReason) + + param, ok := apiIssue.StateReasonParam.(map[string]any) + require.True(t, ok) + assert.InDelta(t, 4, param["issue_index"], 0) +} + func TestLabel_ToLabel(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) label := unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: 1}) diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 3792190a76acb..b0afe9171173c 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -429,9 +429,11 @@ 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)"` + Files []string + StateReason *string `form:"state_reason"` + StateReasonParam string `form:"state_reason_param"` } // Validate validates the fields diff --git a/services/issue/close_reason.go b/services/issue/close_reason.go new file mode 100644 index 0000000000000..93cf7415034c8 --- /dev/null +++ b/services/issue/close_reason.go @@ -0,0 +1,190 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issue + +import ( + "context" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/util" +) + +const ( + CloseReasonNone = issues_model.IssueCloseReasonNone + CloseReasonCompleted = issues_model.IssueCloseReasonCompleted + CloseReasonCompletedByCommit = issues_model.IssueCloseReasonCompletedByCommit + CloseReasonCompletedByPull = issues_model.IssueCloseReasonCompletedByPull + CloseReasonAnswered = issues_model.IssueCloseReasonAnswered + CloseReasonDuplicate = issues_model.IssueCloseReasonDuplicate + CloseReasonNotPlanned = issues_model.IssueCloseReasonNotPlanned +) + +// systemOnlyCloseReasons maps reasons that must not be written by external +// API or Web requests directly from users. +var systemOnlyCloseReasons = map[issues_model.IssueCloseReason]bool{ + CloseReasonCompletedByCommit: true, + CloseReasonCompletedByPull: true, +} + +// CloseReasonDuplicateParam is the JSON param for the "duplicate" close reason. +type CloseReasonDuplicateParam struct { + IssueIndex int64 `json:"issue_index"` // same-repo, non-PR issue index +} + +// CloseReasonAnsweredParam is the JSON param for the "answered" close reason. +type CloseReasonAnsweredParam struct { + CommentID int64 `json:"comment_id"` // comment ID belonging to the current issue +} + +// CloseReasonCommitParam is the JSON param for the "completed_by_commit" close reason. +type CloseReasonCommitParam struct { + CommitHash string `json:"commit_hash"` // full or abbreviated commit hash that triggered the close +} + +// CloseReasonPullParam is the JSON param for the "completed_by_pull" close reason. +type CloseReasonPullParam struct { + PullIndex int64 `json:"pull_index"` // pull request index that triggered the close +} + +// CloseOptions carries the close reason and its serialized param. +// ReasonParam is a JSON string whose schema depends on Reason: +// - completed / not_planned: empty +// - duplicate: CloseReasonDuplicateParam +// - answered: CloseReasonAnsweredParam +// - completed_by_commit: CloseReasonCommitParam +// - completed_by_pull: CloseReasonPullParam +type CloseOptions struct { + Reason issues_model.IssueCloseReason + ReasonParam string // JSON-serialized param, empty when no param is needed +} + +// IsSystemOnly returns true when this reason must only be written by internal +// code and must not be accepted from external API or Web requests. +func (o CloseOptions) IsSystemOnly() bool { + return systemOnlyCloseReasons[o.Reason] +} + +// Normalize fills in the default close reason ("completed") when Reason is empty. +func (o *CloseOptions) Normalize() { + if o.Reason == CloseReasonNone { + o.Reason = CloseReasonCompleted + } +} + +// Validate checks that Reason is known, that the serialized param is valid for +// the given reason, and that any referenced issue or comment actually exists +// in the repository and satisfies the required constraints. +func (o *CloseOptions) Validate(ctx context.Context, issue *issues_model.Issue) error { + switch o.Reason { + case CloseReasonCompleted, CloseReasonNotPlanned: + if o.ReasonParam != "" { + return util.NewInvalidArgumentErrorf("%s close reason does not accept a param", o.Reason.String()) + } + + case CloseReasonDuplicate: + var p CloseReasonDuplicateParam + if err := unmarshalParam(o.ReasonParam, &p); err != nil { + return util.NewInvalidArgumentErrorf("duplicate close reason param is invalid: %v", err) + } + if p.IssueIndex <= 0 { + return util.NewInvalidArgumentErrorf("duplicate close reason requires a valid issue_index") + } + if p.IssueIndex == issue.Index { + return util.NewInvalidArgumentErrorf("duplicate close reason cannot reference the current issue itself") + } + target, err := issues_model.GetIssueByIndex(ctx, issue.RepoID, p.IssueIndex) + if err != nil { + if issues_model.IsErrIssueNotExist(err) { + return util.NewInvalidArgumentErrorf("duplicate target issue #%d does not exist in this repository", p.IssueIndex) + } + return err + } + if target.IsPull { + return util.NewInvalidArgumentErrorf("duplicate close reason cannot reference a pull request (#%d)", p.IssueIndex) + } + + case CloseReasonAnswered: + var p CloseReasonAnsweredParam + if err := unmarshalParam(o.ReasonParam, &p); err != nil { + return util.NewInvalidArgumentErrorf("answered close reason param is invalid: %v", err) + } + if p.CommentID <= 0 { + return util.NewInvalidArgumentErrorf("answered close reason requires a valid comment_id") + } + comment, err := issues_model.GetCommentByID(ctx, p.CommentID) + if err != nil { + if issues_model.IsErrCommentNotExist(err) { + return util.NewInvalidArgumentErrorf("answered comment #%d does not exist", p.CommentID) + } + return err + } + if comment.IssueID != issue.ID { + return util.NewInvalidArgumentErrorf("answered comment #%d does not belong to this issue", p.CommentID) + } + + case CloseReasonCompletedByCommit: + var p CloseReasonCommitParam + if err := unmarshalParam(o.ReasonParam, &p); err != nil { + return util.NewInvalidArgumentErrorf("completed_by_commit param is invalid: %v", err) + } + if p.CommitHash == "" { + return util.NewInvalidArgumentErrorf("completed_by_commit requires a non-empty commit_hash") + } + + case CloseReasonCompletedByPull: + var p CloseReasonPullParam + if err := unmarshalParam(o.ReasonParam, &p); err != nil { + return util.NewInvalidArgumentErrorf("completed_by_pull param is invalid: %v", err) + } + if p.PullIndex <= 0 { + return util.NewInvalidArgumentErrorf("completed_by_pull requires a valid pull_index") + } + + default: + return util.NewInvalidArgumentErrorf("unknown close reason %d", o.Reason) + } + return nil +} + +// unmarshalParam is a small helper that rejects an empty param string for +// reasons that actually require a param (callers catch the zero-value case +// themselves after unmarshalling). +func unmarshalParam(param string, dst any) error { + if param == "" { + // Unmarshal into zero-value struct; callers validate individual fields. + return nil + } + return json.Unmarshal([]byte(param), dst) +} + +// Constructor helpers — use these instead of building CloseOptions by hand. + +func CloseOptionsCompleted() CloseOptions { + return CloseOptions{Reason: CloseReasonCompleted} +} + +func CloseOptionsNotPlanned() CloseOptions { + return CloseOptions{Reason: CloseReasonNotPlanned} +} + +func CloseOptionsDuplicate(issueIndex int64) CloseOptions { + b, _ := json.Marshal(CloseReasonDuplicateParam{IssueIndex: issueIndex}) + return CloseOptions{Reason: CloseReasonDuplicate, ReasonParam: string(b)} +} + +func CloseOptionsAnswered(commentID int64) CloseOptions { + b, _ := json.Marshal(CloseReasonAnsweredParam{CommentID: commentID}) + return CloseOptions{Reason: CloseReasonAnswered, ReasonParam: string(b)} +} + +func CloseOptionsCompletedByCommit(commitHash string) CloseOptions { + b, _ := json.Marshal(CloseReasonCommitParam{CommitHash: commitHash}) + return CloseOptions{Reason: CloseReasonCompletedByCommit, ReasonParam: string(b)} +} + +func CloseOptionsCompletedByPull(pullIndex int64) CloseOptions { + b, _ := json.Marshal(CloseReasonPullParam{PullIndex: pullIndex}) + return CloseOptions{Reason: CloseReasonCompletedByPull, ReasonParam: string(b)} +} diff --git a/services/issue/close_reason_test.go b/services/issue/close_reason_test.go new file mode 100644 index 0000000000000..b7518e3d44130 --- /dev/null +++ b/services/issue/close_reason_test.go @@ -0,0 +1,309 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issue + +import ( + "testing" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/util" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --------------------------------------------------------------------------- +// Normalize +// --------------------------------------------------------------------------- + +func TestCloseOptions_Normalize(t *testing.T) { + t.Run("empty reason becomes completed", func(t *testing.T) { + o := CloseOptions{} + o.Normalize() + assert.Equal(t, CloseReasonCompleted, o.Reason) + }) + + t.Run("non-empty reason is unchanged", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonNotPlanned} + o.Normalize() + assert.Equal(t, CloseReasonNotPlanned, o.Reason) + }) + + t.Run("system reason is unchanged", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByCommit} + o.Normalize() + assert.Equal(t, CloseReasonCompletedByCommit, o.Reason) + }) +} + +// --------------------------------------------------------------------------- +// IsSystemOnly +// --------------------------------------------------------------------------- + +func TestCloseOptions_IsSystemOnly(t *testing.T) { + cases := []struct { + reason issues_model.IssueCloseReason + expected bool + }{ + {CloseReasonCompleted, false}, + {CloseReasonNotPlanned, false}, + {CloseReasonDuplicate, false}, + {CloseReasonAnswered, false}, + {CloseReasonCompletedByCommit, true}, + {CloseReasonCompletedByPull, true}, + } + for _, c := range cases { + o := CloseOptions{Reason: c.reason} + assert.Equal(t, c.expected, o.IsSystemOnly(), "reason=%s", c.reason) + } +} + +// --------------------------------------------------------------------------- +// Constructor helpers +// --------------------------------------------------------------------------- + +func TestCloseOptionsConstructors(t *testing.T) { + t.Run("CloseOptionsCompleted", func(t *testing.T) { + o := CloseOptionsCompleted() + assert.Equal(t, CloseReasonCompleted, o.Reason) + assert.Empty(t, o.ReasonParam) + }) + + t.Run("CloseOptionsNotPlanned", func(t *testing.T) { + o := CloseOptionsNotPlanned() + assert.Equal(t, CloseReasonNotPlanned, o.Reason) + assert.Empty(t, o.ReasonParam) + }) + + t.Run("CloseOptionsDuplicate", func(t *testing.T) { + o := CloseOptionsDuplicate(42) + assert.Equal(t, CloseReasonDuplicate, o.Reason) + var p CloseReasonDuplicateParam + require.NoError(t, json.Unmarshal([]byte(o.ReasonParam), &p)) + assert.Equal(t, int64(42), p.IssueIndex) + }) + + t.Run("CloseOptionsAnswered", func(t *testing.T) { + o := CloseOptionsAnswered(99) + assert.Equal(t, CloseReasonAnswered, o.Reason) + var p CloseReasonAnsweredParam + require.NoError(t, json.Unmarshal([]byte(o.ReasonParam), &p)) + assert.Equal(t, int64(99), p.CommentID) + }) + + t.Run("CloseOptionsCompletedByCommit", func(t *testing.T) { + o := CloseOptionsCompletedByCommit("deadbeef") + assert.Equal(t, CloseReasonCompletedByCommit, o.Reason) + var p CloseReasonCommitParam + require.NoError(t, json.Unmarshal([]byte(o.ReasonParam), &p)) + assert.Equal(t, "deadbeef", p.CommitHash) + }) + + t.Run("CloseOptionsCompletedByPull", func(t *testing.T) { + o := CloseOptionsCompletedByPull(7) + assert.Equal(t, CloseReasonCompletedByPull, o.Reason) + var p CloseReasonPullParam + require.NoError(t, json.Unmarshal([]byte(o.ReasonParam), &p)) + assert.Equal(t, int64(7), p.PullIndex) + }) +} + +// --------------------------------------------------------------------------- +// Validate +// --------------------------------------------------------------------------- + +var issueForValidate = &issues_model.Issue{ID: 1, RepoID: 1, Index: 1} + +func TestValidate(t *testing.T) { + t.Run("no-DB cases", func(t *testing.T) { + t.Run("no-param reasons", func(t *testing.T) { + for _, reason := range []issues_model.IssueCloseReason{CloseReasonCompleted, CloseReasonNotPlanned} { + o := CloseOptions{Reason: reason} + assert.NoError(t, o.Validate(t.Context(), issueForValidate), "reason=%s", reason.String()) + } + }) + + t.Run("no-param reasons reject param", func(t *testing.T) { + for _, reason := range []issues_model.IssueCloseReason{CloseReasonCompleted, CloseReasonNotPlanned} { + o := CloseOptions{Reason: reason, ReasonParam: `{"unexpected":true}`} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + assert.Contains(t, err.Error(), "does not accept a param") + } + }) + + t.Run("unknown reason", func(t *testing.T) { + o := CloseOptions{Reason: issues_model.IssueCloseReason(999)} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("completed by commit", func(t *testing.T) { + t.Run("valid hash", func(t *testing.T) { + o := CloseOptionsCompletedByCommit("abc123") + assert.NoError(t, o.Validate(t.Context(), issueForValidate)) + }) + + t.Run("empty hash is rejected", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByCommit, ReasonParam: `{"commit_hash":""}`} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("missing param treated as zero-value", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByCommit, ReasonParam: ""} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("invalid JSON param", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByCommit, ReasonParam: "not-json"} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + + t.Run("completed by pull", func(t *testing.T) { + t.Run("valid pull index", func(t *testing.T) { + o := CloseOptionsCompletedByPull(3) + assert.NoError(t, o.Validate(t.Context(), issueForValidate)) + }) + + t.Run("zero index is rejected", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByPull, ReasonParam: `{"pull_index":0}`} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("missing param treated as zero-value", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonCompletedByPull, ReasonParam: ""} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + + t.Run("duplicate", func(t *testing.T) { + t.Run("zero index is rejected without DB call", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonDuplicate, ReasonParam: `{"issue_index":0}`} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("self-reference is rejected without DB call", func(t *testing.T) { + // issueForValidate has Index=1; duplicate param also 1 + o := CloseOptionsDuplicate(1) + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("missing param treated as zero-value", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonDuplicate, ReasonParam: ""} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("invalid JSON param", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonDuplicate, ReasonParam: "not-json"} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + + t.Run("answered", func(t *testing.T) { + t.Run("zero comment id is rejected without DB call", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonAnswered, ReasonParam: `{"comment_id":0}`} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("missing param treated as zero-value", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonAnswered, ReasonParam: ""} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("invalid JSON param", func(t *testing.T) { + o := CloseOptions{Reason: CloseReasonAnswered, ReasonParam: "not-json"} + err := o.Validate(t.Context(), issueForValidate) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + }) + + // Fixtures used: + // issue (repo_id=1, id=1, index=1, is_pull=false) ← "current issue" + // issue (repo_id=1, id=5, index=4, is_pull=false) ← valid duplicate target + // issue (repo_id=1, id=2, index=2, is_pull=true) ← PR, must be rejected + // comment (id=2, type=0, issue_id=1) ← valid answered comment + // comment (id=4, type=21, issue_id=2) ← belongs to different issue + t.Run("DB cases", func(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // Reload current issue from DB so all fields are populated. + currentIssue, err := issues_model.GetIssueByIndex(t.Context(), 1, 1) + require.NoError(t, err) + + t.Run("duplicate", func(t *testing.T) { + t.Run("valid non-PR same-repo issue", func(t *testing.T) { + // index=4 in repo_id=1 is a regular issue (id=5) + o := CloseOptionsDuplicate(4) + assert.NoError(t, o.Validate(t.Context(), currentIssue)) + }) + + t.Run("target is a pull request", func(t *testing.T) { + // index=2 in repo_id=1 is is_pull=true + o := CloseOptionsDuplicate(2) + err := o.Validate(t.Context(), currentIssue) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("target issue does not exist", func(t *testing.T) { + o := CloseOptionsDuplicate(99999) + err := o.Validate(t.Context(), currentIssue) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + + t.Run("answered", func(t *testing.T) { + t.Run("valid comment belonging to this issue", func(t *testing.T) { + // comment id=2 has issue_id=1 + o := CloseOptionsAnswered(2) + assert.NoError(t, o.Validate(t.Context(), currentIssue)) + }) + + t.Run("comment belongs to a different issue", func(t *testing.T) { + // comment id=4 has issue_id=2 + o := CloseOptionsAnswered(4) + err := o.Validate(t.Context(), currentIssue) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + + t.Run("comment does not exist", func(t *testing.T) { + o := CloseOptionsAnswered(99999) + err := o.Validate(t.Context(), currentIssue) + require.Error(t, err) + assert.ErrorIs(t, err, util.ErrInvalidArgument) + }) + }) + }) +} diff --git a/services/issue/commit.go b/services/issue/commit.go index 68ccc906b66e7..23f40020eb2d4 100644 --- a/services/issue/commit.go +++ b/services/issue/commit.go @@ -219,7 +219,7 @@ func UpdateIssuesCommit(ctx context.Context, doer *user_model.User, repo *repo_m return err } } - if err := CloseIssue(ctx, refIssue, doer, c.Sha1); err != nil { + if err := CloseIssue(ctx, refIssue, doer, c.Sha1, CloseOptionsCompletedByCommit(c.Sha1)); err != nil { return err } } else if ref.Action == references.XRefActionReopens && refIssue.IsClosed { diff --git a/services/issue/status.go b/services/issue/status.go index fa59df93ba107..b3da2405e1015 100644 --- a/services/issue/status.go +++ b/services/issue/status.go @@ -14,11 +14,22 @@ import ( ) // CloseIssue close an issue. -func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string) error { +func CloseIssue(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, commitID string, opts CloseOptions) error { + // Pull requests are excluded from the close-reason feature by design. + // Reset any reason that may have been passed, so the model layer falls back + // to the plain CommentTypeClose path. + if issue.IsPull { + opts = CloseOptions{} + } else { + opts.Normalize() + } var comment *issues_model.Comment if err := db.WithTx(ctx, func(ctx context.Context) error { var err error - comment, err = issues_model.CloseIssue(ctx, issue, doer) + comment, err = issues_model.CloseIssue(ctx, issue, doer, issues_model.IssueCloseOptions{ + CloseReason: opts.Reason, + CloseReasonParam: opts.ReasonParam, + }) if err != nil { if issues_model.IsErrDependenciesLeft(err) { if _, err := issues_model.FinishIssueStopwatch(ctx, doer, issue); err != nil { diff --git a/services/issue/status_test.go b/services/issue/status_test.go new file mode 100644 index 0000000000000..239d2d73b8cfa --- /dev/null +++ b/services/issue/status_test.go @@ -0,0 +1,222 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issue + +import ( + "testing" + + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// loadOpenNonPRIssue returns issue ID=1 (repo_id=1, is_closed=false, is_pull=false) from fixtures. +func loadOpenNonPRIssue(t *testing.T) *issues_model.Issue { + t.Helper() + issue, err := issues_model.GetIssueByID(t.Context(), 1) + require.NoError(t, err) + require.False(t, issue.IsClosed) + require.False(t, issue.IsPull) + return issue +} + +// loadRepoOwnerDoer returns user_id=2 (owner of repo_id=1) from fixtures. +func loadRepoOwnerDoer(t *testing.T) *user_model.User { + t.Helper() + u, err := user_model.GetUserByID(t.Context(), 2) + require.NoError(t, err) + return u +} + +func TestCloseIssue_Completed(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsCompleted())) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonCompleted, reloaded.CloseReason) + assert.Empty(t, reloaded.CloseReasonParam) + + // The close comment must use CommentTypeCloseWithReason and carry the reason in its metadata. + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonCompleted, cmt.CommentMetaData.CloseReason) +} + +func TestCloseIssue_NotPlanned(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsNotPlanned())) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonNotPlanned, reloaded.CloseReason) + assert.Empty(t, reloaded.CloseReasonParam) + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonNotPlanned, cmt.CommentMetaData.CloseReason) +} + +func TestCloseIssue_Duplicate(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsDuplicate(4))) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonDuplicate, reloaded.CloseReason) + assert.Contains(t, reloaded.CloseReasonParam, `"issue_index":4`) + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonDuplicate, cmt.CommentMetaData.CloseReason) + assert.Contains(t, cmt.CommentMetaData.CloseReasonParam, `"issue_index":4`) +} + +func TestCloseIssue_Answered(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + + // comment id=2 in fixtures belongs to issue id=1. + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsAnswered(2))) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonAnswered, reloaded.CloseReason) + assert.Contains(t, reloaded.CloseReasonParam, `"comment_id":2`) + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonAnswered, cmt.CommentMetaData.CloseReason) + assert.Contains(t, cmt.CommentMetaData.CloseReasonParam, `"comment_id":2`) +} + +func TestCloseIssue_CompletedByCommit(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + commitHash := "deadbeef1234abcd" + + require.NoError(t, CloseIssue(t.Context(), issue, doer, commitHash, CloseOptionsCompletedByCommit(commitHash))) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonCompletedByCommit, reloaded.CloseReason) + assert.Contains(t, reloaded.CloseReasonParam, commitHash) + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonCompletedByCommit, cmt.CommentMetaData.CloseReason) + assert.Contains(t, cmt.CommentMetaData.CloseReasonParam, commitHash) +} + +func TestCloseIssue_CompletedByPull(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + const pullIndex = int64(42) + + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsCompletedByPull(pullIndex))) + + reloaded, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + assert.Equal(t, CloseReasonCompletedByPull, reloaded.CloseReason) + assert.Contains(t, reloaded.CloseReasonParam, "42") + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ + IssueID: issue.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) + require.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, CloseReasonCompletedByPull, cmt.CommentMetaData.CloseReason) + assert.Contains(t, cmt.CommentMetaData.CloseReasonParam, "42") +} + +// TestReopenIssue_ClearsCloseReason verifies that reopening an issue clears its +// close reason and param so reopened issues never leak stale reason data. +func TestReopenIssue_ClearsCloseReason(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + issue := loadOpenNonPRIssue(t) + doer := loadRepoOwnerDoer(t) + + // Close with a reason first. + require.NoError(t, CloseIssue(t.Context(), issue, doer, "", CloseOptionsNotPlanned())) + closed, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + require.True(t, closed.IsClosed) + require.Equal(t, CloseReasonNotPlanned, closed.CloseReason) + + // Reopen — reason must be cleared in the database. + require.NoError(t, ReopenIssue(t.Context(), closed, doer, "")) + + reopened, err := issues_model.GetIssueByID(t.Context(), issue.ID) + require.NoError(t, err) + assert.False(t, reopened.IsClosed) + assert.Empty(t, reopened.CloseReason) + assert.Empty(t, reopened.CloseReasonParam) +} + +// TestCloseIssue_PullRequest_NoReasonStored verifies that pull requests are +// excluded from the close-reason feature: even if a reason is passed, it must +// not be stored, and the close comment must be plain CommentTypeClose (not the +// new CommentTypeCloseWithReason). +func TestCloseIssue_PullRequest_NoReasonStored(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + // Issue ID=2 in repo_id=1 is a pull request (is_pull=true). + pr, err := issues_model.GetIssueByID(t.Context(), 2) + require.NoError(t, err) + require.True(t, pr.IsPull) + require.False(t, pr.IsClosed) + + doer := loadRepoOwnerDoer(t) + + // Pass a reason — the service layer should strip it for PRs. + require.NoError(t, CloseIssue(t.Context(), pr, doer, "", CloseOptionsNotPlanned())) + + reloaded, err := issues_model.GetIssueByID(t.Context(), pr.ID) + require.NoError(t, err) + assert.True(t, reloaded.IsClosed) + // PR must have no close reason stored. + assert.Empty(t, reloaded.CloseReason) + assert.Empty(t, reloaded.CloseReasonParam) + + // There must be NO CommentTypeCloseWithReason comment for this PR. + unittest.AssertNotExistsBean(t, &issues_model.Comment{ + IssueID: pr.ID, + Type: issues_model.CommentTypeCloseWithReason, + }) +} diff --git a/services/pull/merge.go b/services/pull/merge.go index 4925302797b1f..a0741bc2afc52 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -315,7 +315,7 @@ func handleCloseCrossReferences(ctx context.Context, pr *issues_model.PullReques return err } if ref.RefAction == references.XRefActionCloses && !ref.Issue.IsClosed { - if err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID); err != nil { + if err = issue_service.CloseIssue(ctx, ref.Issue, doer, pr.MergedCommitID, issue_service.CloseOptionsCompletedByPull(pr.Index)); err != nil { // Allow ErrDependenciesLeft if !issues_model.IsErrDependenciesLeft(err) { return err @@ -713,7 +713,7 @@ func SetMerged(ctx context.Context, pr *issues_model.PullRequest, mergedCommitID } // Set issue as closed - if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, true); err != nil { + if _, err := issues_model.SetIssueAsClosed(ctx, pr.Issue, pr.Merger, issues_model.IssueCloseOptions{IsMergePull: true}); err != nil { return false, fmt.Errorf("ChangeIssueStatus: %w", err) } diff --git a/services/pull/pull.go b/services/pull/pull.go index 779d2b13e1fcc..b9b8f34d176bc 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -681,7 +681,7 @@ func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User var errs []error for _, pr := range prs { - if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) { + if err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", issue_service.CloseOptions{}); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) { errs = append(errs, err) } if err == nil { @@ -721,7 +721,7 @@ func AdjustPullsCausedByBranchDeleted(ctx context.Context, doer *user_model.User errs = append(errs, err) } if err == nil { - if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) { + if err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", issue_service.CloseOptions{}); err != nil && !issues_model.IsErrIssueIsClosed(err) && !issues_model.IsErrDependenciesLeft(err) { errs = append(errs, err) } } @@ -753,7 +753,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re if pr.BaseRepoID == repo.ID { continue } - if err = issue_service.CloseIssue(ctx, pr.Issue, doer, ""); err != nil && !issues_model.IsErrIssueIsClosed(err) { + if err = issue_service.CloseIssue(ctx, pr.Issue, doer, "", issue_service.CloseOptions{}); err != nil && !issues_model.IsErrIssueIsClosed(err) { errs = append(errs, err) } } diff --git a/templates/repo/issue/view_content.tmpl b/templates/repo/issue/view_content.tmpl index 275dd47a76b25..05f39e4da7533 100644 --- a/templates/repo/issue/view_content.tmpl +++ b/templates/repo/issue/view_content.tmpl @@ -82,27 +82,78 @@
{{template "repo/issue/comment_tab" .}} + {{if and (not .Issue.IsClosed) (not .Issue.IsPull) (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .DisableStatusChange)}} + + {{end}}
-
+
{{if and (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .DisableStatusChange)}} - {{$btnIconColor := ""}}{{$btnIcon := ""}}{{$btnTextNoComment := ""}}{{$btnTextWithComment := ""}}{{$btnValue := ""}} {{if .Issue.IsClosed}} - {{$btnValue = "reopen"}} - {{$btnIconColor = "tw-text-green"}} - {{$btnIcon = Iif .Issue.IsPull "octicon-git-pull-request" "octicon-issue-reopened"}} - {{$btnTextNoComment = ctx.Locale.Tr (Iif .Issue.IsPull "repo.pulls.reopen" "repo.issues.reopen_issue")}} - {{$btnTextWithComment = ctx.Locale.Tr "repo.issues.reopen_comment_issue"}}{{/* general: Reopen with Comment */}} + {{$btnTextNoComment := ctx.Locale.Tr (Iif .Issue.IsPull "repo.pulls.reopen" "repo.issues.reopen_issue")}} + {{$btnTextWithComment := ctx.Locale.Tr "repo.issues.reopen_comment_issue"}} + + {{else if .Issue.IsPull}} + {{$btnTextNoComment := ctx.Locale.Tr "repo.pulls.close"}} + {{$btnTextWithComment := ctx.Locale.Tr "repo.issues.close_comment_issue"}} + {{else}} - {{$btnValue = "close"}} - {{$btnIconColor = "tw-text-red"}} - {{$btnIcon = Iif .Issue.IsPull "octicon-git-pull-request-closed" "octicon-issue-closed"}} - {{$btnTextNoComment = ctx.Locale.Tr (Iif .Issue.IsPull "repo.pulls.close" "repo.issues.close")}} - {{$btnTextWithComment = ctx.Locale.Tr "repo.issues.close_comment_issue"}}{{/* general: Close with Comment */}} + {{$btnTextNoComment := ctx.Locale.Tr "repo.issues.close_reason.completed_action"}} + {{$btnTextWithComment := ctx.Locale.Tr "repo.issues.close_comment_issue"}} + +
+ + +
{{end}} - {{end}}
+ {{if and (not .Issue.IsClosed) (not .Issue.IsPull) (or .HasIssuesOrPullsWritePermission .IsIssuePoster) (not .DisableStatusChange)}} + +
{{ctx.Locale.Tr "repo.issues.close_reason.duplicate_modal_title"}}
+
+

{{ctx.Locale.Tr "repo.issues.close_reason.duplicate_modal_desc"}}

+
+ + +
+
{{ctx.Locale.Tr "loading"}}
+
{{ctx.Locale.Tr "repo.issues.close_reason.duplicate_no_results"}}
+
+
+
+ + +
+ + {{end}}
diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 8c08f1c1f1c8d..ce95058a20022 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -94,9 +94,18 @@ {{end}} - {{else if eq .Type 2}} + {{else if or (eq .Type 2) (eq .Type 39)}}
- {{svg "octicon-issue-closed"}} + {{$closeBadgeClass := "badge tw-bg-red tw-text-white"}} + {{$closeBadgeIcon := "octicon-issue-closed"}} + {{$closeReason := ""}} + {{if and (eq .Type 39) (not .Issue.IsPull)}} + {{$closeReason = .CloseReasonForDisplay}} + {{if or (eq $closeReason "duplicate") (eq $closeReason "not_planned")}} + {{$closeBadgeIcon = "octicon-skip"}} + {{end}} + {{end}} + {{svg $closeBadgeIcon}} {{if not .OriginalAuthor}} {{template "shared/user/avatarlink" dict "user" .Poster}} {{end}} @@ -105,7 +114,52 @@ {{if .Issue.IsPull}} {{ctx.Locale.Tr "repo.pulls.closed_at" .EventTag $createdStr}} {{else}} - {{ctx.Locale.Tr "repo.issues.closed_at" .EventTag $createdStr}} + {{if eq .Type 39}} + {{$reasonPhrase := ""}} + {{if not $closeReason}} + {{$closeReason = .CloseReasonForDisplay}} + {{end}} + {{if eq $closeReason "completed"}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_completed"}} + {{else if eq $closeReason "not_planned"}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_not_planned"}} + {{else if eq $closeReason "duplicate"}} + {{$dupIdx := .CloseReasonDuplicateIssueIndex}} + {{if gt $dupIdx 0}} + {{$dupLink := HTMLFormat `#%d` $.RepoLink $dupIdx $dupIdx}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_duplicate_of" $dupLink}} + {{else}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_duplicate"}} + {{end}} + {{else if eq $closeReason "answered"}} + {{$commentID := .CloseReasonAnsweredCommentID}} + {{if gt $commentID 0}} + {{$answerLink := HTMLFormat `comment #%d` $commentID $commentID}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_answered_by_comment" $answerLink}} + {{else}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.as_answered"}} + {{end}} + {{else if eq $closeReason "completed_by_commit"}} + {{$hash := .CloseReasonCommitHash}} + {{if $hash}} + {{$commitLink := HTMLFormat `%[3]s` $.RepoLink $hash (ShortSha $hash)}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.via_commit" $commitLink}} + {{end}} + {{else if eq $closeReason "completed_by_pull"}} + {{$pullIdx := .CloseReasonPullIndex}} + {{if gt $pullIdx 0}} + {{$pullLink := HTMLFormat `#%d` $.RepoLink $pullIdx $pullIdx}} + {{$reasonPhrase = ctx.Locale.Tr "repo.issues.close_reason.via_pull" $pullLink}} + {{end}} + {{end}} + {{if $reasonPhrase}} + {{ctx.Locale.Tr "repo.issues.closed_at_with_reason" $reasonPhrase .EventTag $createdStr}} + {{else}} + {{ctx.Locale.Tr "repo.issues.closed_at" .EventTag $createdStr}} + {{end}} + {{else}} + {{ctx.Locale.Tr "repo.issues.closed_at" .EventTag $createdStr}} + {{end}} {{end}}
diff --git a/templates/repo/issue/view_title.tmpl b/templates/repo/issue/view_title.tmpl index 103fa5de530a9..6e219eac489b7 100644 --- a/templates/repo/issue/view_title.tmpl +++ b/templates/repo/issue/view_title.tmpl @@ -42,7 +42,56 @@ {{if .HasMerged}}
{{svg "octicon-git-merge" 16 "tw-mr-1"}} {{if eq .Issue.PullRequest.Status 3}}{{ctx.Locale.Tr "repo.pulls.manually_merged"}}{{else}}{{ctx.Locale.Tr "repo.pulls.merged"}}{{end}}
{{else if .Issue.IsClosed}} -
{{svg (Iif .Issue.IsPull "octicon-git-pull-request-closed" "octicon-issue-closed")}} {{ctx.Locale.Tr "repo.issues.closed_title"}}
+ {{$statusText := ctx.Locale.Tr "repo.issues.closed_title"}} + {{$statusIcon := Iif .Issue.IsPull "octicon-git-pull-request-closed" "octicon-issue-closed"}} + {{$statusLabelClass := "ui red label issue-state-label"}} + {{if not .Issue.IsPull}} + {{$reasonText := ""}} + {{$closeReason := .Issue.CloseReasonForDisplay}} + {{if eq $closeReason "completed"}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_completed"}} + {{else if eq $closeReason "not_planned"}} + {{$statusIcon = "octicon-skip"}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_not_planned"}} + {{else if eq $closeReason "duplicate"}} + {{$statusIcon = "octicon-skip"}} + {{$dupIdx := .Issue.CloseReasonDuplicateIssueIndex}} + {{if gt $dupIdx 0}} + {{$dupLink := HTMLFormat `#%d` $.RepoLink $dupIdx $dupIdx}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_duplicate" $dupLink}} + {{else}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_duplicate_generic"}} + {{end}} + {{else if eq $closeReason "answered"}} + {{$commentID := .Issue.CloseReasonAnsweredCommentID}} + {{if gt $commentID 0}} + {{$answerLink := HTMLFormat `comment #%d` $commentID $commentID}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_answered_by_comment" $answerLink}} + {{else}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_answered"}} + {{end}} + {{else if eq $closeReason "completed_by_commit"}} + {{$hash := .Issue.CloseReasonCommitHash}} + {{if $hash}} + {{$commitLink := HTMLFormat `%[3]s` $.RepoLink $hash (ShortSha $hash)}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_by_commit" $commitLink}} + {{else}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_completed"}} + {{end}} + {{else if eq $closeReason "completed_by_pull"}} + {{$pullIdx := .Issue.CloseReasonPullIndex}} + {{if gt $pullIdx 0}} + {{$pullLink := HTMLFormat `#%d` $.RepoLink $pullIdx $pullIdx}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_by_pull" $pullLink}} + {{else}} + {{$reasonText = ctx.Locale.Tr "repo.issues.close_reason.closed_as_completed"}} + {{end}} + {{end}} + {{if $reasonText}} + {{$statusText = $reasonText}} + {{end}} + {{end}} +
{{svg $statusIcon}} {{$statusText}}
{{else if .Issue.IsPull}} {{if .IsPullWorkInProgress}}
{{svg "octicon-git-pull-request-draft"}} {{ctx.Locale.Tr "repo.issues.draft_title"}}
diff --git a/templates/shared/issueicon.tmpl b/templates/shared/issueicon.tmpl index 53e077c95161f..a35d58113f915 100644 --- a/templates/shared/issueicon.tmpl +++ b/templates/shared/issueicon.tmpl @@ -19,7 +19,11 @@ {{- end -}} {{- else -}} {{- if .IsClosed -}} - {{- svg "octicon-issue-closed" 16 "tw-text-red" -}} + {{- if or (eq .CloseReasonForDisplay "duplicate") (eq .CloseReasonForDisplay "not_planned") -}} + {{- svg "octicon-skip" 16 "tw-text-red" -}} + {{- else -}} + {{- svg "octicon-issue-closed" 16 "tw-text-red" -}} + {{- end -}} {{- else -}} {{- svg "octicon-issue-opened" 16 "tw-text-green" -}} {{- end -}} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index b4247e261243d..4fa1b267b659d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24801,6 +24801,14 @@ "type": "string", "x-go-name": "State" }, + "state_reason": { + "type": "string", + "x-go-name": "StateReason" + }, + "state_reason_param": { + "type": "string", + "x-go-name": "StateReasonParam" + }, "title": { "type": "string", "x-go-name": "Title" @@ -26328,6 +26336,14 @@ "x-go-enum-desc": "open StateOpen StateOpen pr is opened\nclosed StateClosed StateClosed pr is closed", "x-go-name": "State" }, + "state_reason": { + "description": "StateReason is the reason why the issue is open/closed (for closed issues)", + "type": "string", + "x-go-name": "StateReason" + }, + "state_reason_param": { + "x-go-name": "StateReasonParam" + }, "time_estimate": { "type": "integer", "format": "int64", diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index e63e1b26a482c..98d207c9b8619 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -563,7 +563,7 @@ jobs: checkCommitStatusAndInsertFakeStatus(t, repo, sha) // closed - err = issue_service.CloseIssue(t.Context(), pullIssue, user2, "") + err = issue_service.CloseIssue(t.Context(), pullIssue, user2, "", issue_service.CloseOptions{}) assert.NoError(t, err) checkCommitStatusAndInsertFakeStatus(t, repo, sha) diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 8dc9e31cfa72d..91b83067bdc43 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -32,6 +32,8 @@ func TestAPIIssue(t *testing.T) { t.Run("SearchIssues", testAPISearchIssues) t.Run("SearchIssuesWithLabels", testAPISearchIssuesWithLabels) t.Run("EditIssue", testAPIEditIssue) + t.Run("EditIssueRejectSystemOnlyReason", testAPIEditIssueRejectSystemOnlyReason) + t.Run("EditIssueRejectParamForNotPlannedReason", testAPIEditIssueRejectParamForNotPlannedReason) t.Run("IssueContentVersion", testAPIIssueContentVersion) t.Run("CreateIssue", testAPICreateIssue) t.Run("CreateIssueParallel", testAPICreateIssueParallel) @@ -215,18 +217,31 @@ func testAPIEditIssue(t *testing.T) { // update values of issue issueState := "closed" + stateReason := "duplicate" removeDeadline := true milestone := int64(4) body := "new content!" title := "new title from api set" + duplicateTargetTitle := "duplicate target issue" + duplicateTargetBody := "duplicate target body" + + createReq := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/issues", owner.Name, repoBefore.Name), &api.CreateIssueOption{ + Title: duplicateTargetTitle, + Body: duplicateTargetBody, + }).AddTokenAuth(token) + createResp := MakeRequest(t, createReq, http.StatusCreated) + duplicateTarget := DecodeJSON(t, createResp, &api.Issue{}) + stateReasonParam := fmt.Sprintf(`{"issue_index":%d}`, duplicateTarget.Index) urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d", owner.Name, repoBefore.Name, issueBefore.Index) req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditIssueOption{ - State: &issueState, - RemoveDeadline: &removeDeadline, - Milestone: &milestone, - Body: &body, - Title: title, + State: &issueState, + StateReason: &stateReason, + StateReasonParam: &stateReasonParam, + RemoveDeadline: &removeDeadline, + Milestone: &milestone, + Body: &body, + Title: title, // ToDo change more }).AddTokenAuth(token) @@ -253,19 +268,78 @@ func testAPIEditIssue(t *testing.T) { // API response assert.Equal(t, api.StateClosed, apiIssue.State) + assert.Equal(t, stateReason, apiIssue.StateReason) assert.Equal(t, milestone, apiIssue.Milestone.ID) assert.Equal(t, body, apiIssue.Body) assert.Nil(t, apiIssue.Deadline) assert.Equal(t, title, apiIssue.Title) + param, ok := apiIssue.StateReasonParam.(map[string]any) + if assert.True(t, ok) { + assert.InDelta(t, duplicateTarget.Index, param["issue_index"], 0) + } // in database assert.Equal(t, api.StateClosed, issueAfter.State()) + assert.Equal(t, issues_model.IssueCloseReasonDuplicate, issueAfter.CloseReason) + assert.JSONEq(t, stateReasonParam, issueAfter.CloseReasonParam) assert.Equal(t, milestone, issueAfter.MilestoneID) assert.Equal(t, int64(0), int64(issueAfter.DeadlineUnix)) assert.Equal(t, body, issueAfter.Content) assert.Equal(t, title, issueAfter.Title) } +func testAPIEditIssueRejectSystemOnlyReason(t *testing.T) { + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 10}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + issueState := "closed" + stateReason := "completed_by_commit" + stateReasonParam := `{"commit_hash":"deadbeef"}` + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d", owner.Name, repo.Name, issue.Index) + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditIssueOption{ + State: &issueState, + StateReason: &stateReason, + StateReasonParam: &stateReasonParam, + }).AddTokenAuth(token) + + resp := MakeRequest(t, req, http.StatusBadRequest) + var apiError struct { + Message string `json:"message"` + } + DecodeJSON(t, resp, &apiError) + assert.Equal(t, "This close reason is system-only", apiError.Message) +} + +func testAPIEditIssueRejectParamForNotPlannedReason(t *testing.T) { + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 10}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + issueState := "closed" + stateReason := "not_planned" + stateReasonParam := `{"unexpected":true}` + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d", owner.Name, repo.Name, issue.Index) + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditIssueOption{ + State: &issueState, + StateReason: &stateReason, + StateReasonParam: &stateReasonParam, + }).AddTokenAuth(token) + + resp := MakeRequest(t, req, http.StatusBadRequest) + var apiError struct { + Message string `json:"message"` + } + DecodeJSON(t, resp, &apiError) + assert.Equal(t, "not_planned close reason does not accept a param", apiError.Message) +} + func testAPISearchIssues(t *testing.T) { defer test.MockVariableValue(&setting.API.DefaultPagingNum, 20)() expectedIssueCount := 20 // 20 is from the fixtures diff --git a/tests/integration/issue_test.go b/tests/integration/issue_test.go index a9101d7b7efe2..377780235c2a2 100644 --- a/tests/integration/issue_test.go +++ b/tests/integration/issue_test.go @@ -248,6 +248,122 @@ func TestIssueCommentClose(t *testing.T) { assert.Equal(t, "Description", val) } +func TestIssueCommentCloseWithDuplicateReason(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Close duplicate", "Description") + + getReq := NewRequest(t, "GET", issueURL) + getResp := session.MakeRequest(t, getReq, http.StatusOK) + htmlDoc := NewHTMLParser(t, getResp.Body) + formAction, exists := htmlDoc.doc.Find("#comment-form").Attr("action") + assert.True(t, exists, "The template has changed") + + postReq := NewRequestWithValues(t, "POST", formAction, map[string]string{ + "content": "close as duplicate", + "status": "close", + "state_reason": "duplicate", + "state_reason_param": `{"issue_index":4}`, + }) + postResp := session.MakeRequest(t, postReq, http.StatusOK) + + redirectReq := NewRequest(t, "GET", test.RedirectURL(postResp)) + session.MakeRequest(t, redirectReq, http.StatusOK) + + indexStr := issueURL[strings.LastIndexByte(issueURL, '/')+1:] + issueIndex, err := strconv.ParseInt(indexStr, 10, 64) + assert.NoError(t, err) + + issue, err := issues_model.GetIssueByIndex(t.Context(), 1, issueIndex) + assert.NoError(t, err) + assert.True(t, issue.IsClosed) + assert.Equal(t, issues_model.IssueCloseReasonDuplicate, issue.CloseReason) + assert.Contains(t, issue.CloseReasonParam, "\"issue_index\":4") + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{IssueID: issue.ID, Type: issues_model.CommentTypeCloseWithReason}) + assert.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, issues_model.IssueCloseReasonDuplicate, cmt.CommentMetaData.CloseReason) +} + +func TestIssueCommentCloseAsAnsweredAutoBind(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Close answered", "Description") + + getReq := NewRequest(t, "GET", issueURL) + getResp := session.MakeRequest(t, getReq, http.StatusOK) + htmlDoc := NewHTMLParser(t, getResp.Body) + formAction, exists := htmlDoc.doc.Find("#comment-form").Attr("action") + assert.True(t, exists, "The template has changed") + + postReq := NewRequestWithValues(t, "POST", formAction, map[string]string{ + "content": "final answer in this closing comment", + "status": "close", + "state_reason": "answered", + }) + postResp := session.MakeRequest(t, postReq, http.StatusOK) + + redirectReq := NewRequest(t, "GET", test.RedirectURL(postResp)) + session.MakeRequest(t, redirectReq, http.StatusOK) + + indexStr := issueURL[strings.LastIndexByte(issueURL, '/')+1:] + issueIndex, err := strconv.ParseInt(indexStr, 10, 64) + assert.NoError(t, err) + + issue, err := issues_model.GetIssueByIndex(t.Context(), 1, issueIndex) + assert.NoError(t, err) + assert.True(t, issue.IsClosed) + assert.Equal(t, issues_model.IssueCloseReasonAnswered, issue.CloseReason) + assert.Contains(t, issue.CloseReasonParam, "comment_id") + + cmt := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{IssueID: issue.ID, Type: issues_model.CommentTypeCloseWithReason}) + assert.NotNil(t, cmt.CommentMetaData) + assert.Equal(t, issues_model.IssueCloseReasonAnswered, cmt.CommentMetaData.CloseReason) + assert.Contains(t, cmt.CommentMetaData.CloseReasonParam, "comment_id") +} + +func TestIssueCommentCloseRejectSystemOnlyReason(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Reject system-only reason", "Description") + + getReq := NewRequest(t, "GET", issueURL) + getResp := session.MakeRequest(t, getReq, http.StatusOK) + htmlDoc := NewHTMLParser(t, getResp.Body) + formAction, exists := htmlDoc.doc.Find("#comment-form").Attr("action") + assert.True(t, exists, "The template has changed") + + postReq := NewRequestWithValues(t, "POST", formAction, map[string]string{ + "content": "attempt invalid reason", + "status": "close", + "state_reason": "completed_by_commit", + "state_reason_param": `{"commit_hash":"deadbeef"}`, + }) + resp := session.MakeRequest(t, postReq, http.StatusBadRequest) + assert.Equal(t, "This close reason is system-only", test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) +} + +func TestIssueCommentCloseRejectParamForCompletedReason(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + issueURL := testNewIssue(t, session, "user2", "repo1", "Reject param for completed reason", "Description") + + getReq := NewRequest(t, "GET", issueURL) + getResp := session.MakeRequest(t, getReq, http.StatusOK) + htmlDoc := NewHTMLParser(t, getResp.Body) + formAction, exists := htmlDoc.doc.Find("#comment-form").Attr("action") + assert.True(t, exists, "The template has changed") + + postReq := NewRequestWithValues(t, "POST", formAction, map[string]string{ + "content": "attempt invalid param", + "status": "close", + "state_reason": "completed", + "state_reason_param": `{"unexpected":true}`, + }) + resp := session.MakeRequest(t, postReq, http.StatusBadRequest) + assert.Equal(t, "completed close reason does not accept a param", test.ParseJSONError(resp.Body.Bytes()).ErrorMessage) +} + func TestIssueCommentDelete(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 1dd530133842d..0a4775fa35824 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -594,6 +594,21 @@ td .commit-summary { width: 100%; margin: 0; } + + .repository.view.issue .comment-list .comment .content .form #close-with-reason-group { + width: 100%; + display: flex; + } + + .repository.view.issue .comment-list .comment .content .form #close-with-reason-group #status-button { + width: auto; + flex: 1; + } + + .repository.view.issue .comment-list .comment .content .form #close-with-reason-group #close-reason-dropdown-btn { + width: auto; + flex: 0 0 auto; + } } .repository.view.issue .comment-list .comment .merge-section { @@ -614,6 +629,123 @@ td .commit-summary { width: calc(100% + 2rem); } +#close-with-reason-group { + position: relative; +} + +.close-reason-duplicate-modal { + width: min(560px, 92vw); + border: 1px solid var(--color-secondary); + border-radius: var(--border-radius); + background: var(--color-box-body); + color: var(--color-text); + padding: 0; +} + +.close-reason-duplicate-modal::backdrop { + background: rgb(0 0 0 / 40%); +} + +.close-reason-duplicate-modal .header { + padding: 12px 16px; + border-bottom: 1px solid var(--color-secondary); + font-weight: var(--font-weight-semibold); +} + +.close-reason-duplicate-modal .content { + padding: 16px; +} + +.close-reason-duplicate-modal .actions { + display: flex; + justify-content: flex-end; + gap: 8px; + padding: 12px 16px; + border-top: 1px solid var(--color-secondary); +} + +#duplicate-issue-list { + max-height: 220px; + overflow: auto; + border: 1px solid var(--color-secondary); + border-radius: var(--border-radius); +} + +.close-reason-duplicate-item { + width: 100%; + display: flex; + justify-content: space-between; + align-items: center; + gap: 12px; + padding: 10px 12px; + border: 0; + border-bottom: 1px solid var(--color-secondary); + background: transparent; + color: var(--color-text); + cursor: pointer; + text-align: left; +} + +.close-reason-duplicate-item:last-child { + border-bottom: 0; +} + +.close-reason-duplicate-item:hover { + background: var(--color-hover); +} + +.close-reason-duplicate-item-main { + min-width: 0; + overflow-wrap: anywhere; +} + +.close-reason-duplicate-item-state { + color: var(--color-text-light-2); + font-size: 12px; + flex-shrink: 0; +} + +.close-reason-menu-item-content { + display: flex; + flex-direction: column; + min-width: 0; +} + +.close-reason-menu-item-icon { + width: 16px; + height: 16px; + display: inline-flex; + align-items: center; + justify-content: center; + flex-shrink: 0; +} + +.close-reason-menu-item-title { + font-size: 13px; + line-height: 1.4; +} + +.close-reason-menu-item-desc { + font-size: 11px; + color: var(--color-text-light-2); + line-height: 1.3; +} + +#close-reason-menu .item { + display: flex; + align-items: center; + gap: 8px; +} + +#close-reason-menu .item:hover, +#close-reason-menu .item:focus-visible { + background: var(--color-error-bg); +} + +.close-reason-duplicate-item.is-selected { + background: var(--color-error-bg); +} + .merge-section-info code { border: 1px solid var(--color-light-border); border-radius: var(--border-radius); @@ -634,6 +766,10 @@ td .commit-summary { overflow: hidden; } +.repository.view.issue .comment-list #comment-form > .field.footer { + overflow: visible; +} + .repository.view.issue .comment-list .comment .ui.form .field .tab.markup { min-height: 5rem; } diff --git a/web_src/js/features/issue.ts b/web_src/js/features/issue.ts index 74fec71742e35..4841905188717 100644 --- a/web_src/js/features/issue.ts +++ b/web_src/js/features/issue.ts @@ -18,6 +18,12 @@ export function getIssueIcon(issue: Issue) { if (issue.state === 'open') { return 'octicon-issue-opened'; // Open Issue } + + const closeReason = issue.state_reason || 'completed'; + if (closeReason === 'duplicate' || closeReason === 'not_planned') { + return 'octicon-skip'; // Closed Issue (duplicate/not planned) + } + return 'octicon-issue-closed'; // Closed Issue } diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 03b0c93d85fcd..73ab5d20f5f2d 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -1,4 +1,4 @@ -import {htmlEscape} from '../utils/html.ts'; +import {html, htmlEscape} from '../utils/html.ts'; import {createTippy} from '../modules/tippy.ts'; import { addDelegatedEventListener, @@ -10,7 +10,7 @@ import { } from '../utils/dom.ts'; import {setFileFolding} from './file-fold.ts'; import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} from './comp/ComboMarkdownEditor.ts'; -import {toAbsoluteUrl} from '../utils.ts'; +import {parseIssuePageInfo, toAbsoluteUrl} from '../utils.ts'; import {GET, POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; import {initRepoIssueSidebar} from './repo-issue-sidebar.ts'; @@ -450,19 +450,264 @@ async function initSingleCommentEditor(commentForm: HTMLFormElement) { const editor = await initComboMarkdownEditor(commentForm.querySelector('.combo-markdown-editor')!); const statusButton = document.querySelector('#status-button'); const commentButton = document.querySelector('#comment-button'); + const issuePageInfo = parseIssuePageInfo(); + const closeReasonHidden = document.querySelector('#close-reason-hidden'); + const closeReasonParamHidden = document.querySelector('#close-reason-param-hidden'); + const closeReasonDropdownBtn = document.querySelector('#close-reason-dropdown-btn'); + const closeReasonMenu = document.querySelector('#close-reason-menu'); + const closeReasonAnsweredItem = document.querySelector('#close-reason-answered-item'); + const issueCommentForm = document.querySelector('#comment-form') ?? commentForm; + const duplicateModal = document.querySelector('#close-reason-duplicate-modal'); + const duplicateInput = document.querySelector('#duplicate-issue-index-input'); + const duplicateLoading = document.querySelector('#duplicate-issue-loading'); + const duplicateEmpty = document.querySelector('#duplicate-issue-empty'); + const duplicateList = document.querySelector('#duplicate-issue-list'); + const duplicateConfirmBtn = document.querySelector('#duplicate-modal-confirm'); + const duplicateCancelBtn = document.querySelector('#duplicate-modal-cancel'); + + const defaultStatusText = statusButton?.getAttribute('data-status') ?? ''; + const defaultStatusAndCommentText = statusButton?.getAttribute('data-status-and-comment') ?? ''; + let selectedDuplicateIssueIndex = 0; + let duplicateSearchTimer: ReturnType | undefined; + let menuShown = false; + let duplicateModalReturnFocus: HTMLElement | null = null; + + const hideMenu = () => { + if (!menuShown || !closeReasonMenu) return; + menuShown = false; + closeReasonMenu.classList.remove('show'); + closeReasonMenu.style.display = 'none'; + }; + + const hasComment = () => editor.value().trim().length > 0; + + const clearCloseReasonFields = () => { + if (closeReasonHidden) closeReasonHidden.value = ''; + if (closeReasonParamHidden) closeReasonParamHidden.value = ''; + }; + + const syncCloseReasonMenuLabels = (withComment: boolean) => { + if (!closeReasonMenu) return; + for (const item of closeReasonMenu.querySelectorAll('.item[data-reason]')) { + item.querySelector('.close-reason-menu-item-title')!.textContent = item.getAttribute(withComment ? 'data-text-with-comment' : 'data-text-no-comment') ?? ''; + } + }; + + const submitCloseWithReason = (reason: string, reasonParam = '') => { + if (!statusButton || statusButton.disabled) return; + if (closeReasonHidden) closeReasonHidden.value = reason; + if (closeReasonParamHidden) closeReasonParamHidden.value = reasonParam; + hideMenu(); + issueCommentForm.requestSubmit(statusButton); + }; + const syncUiState = () => { - const editorText = editor.value().trim(), isUploading = editor.isUploading(); + const editorText = editor.value().trim(); + const isUploading = editor.isUploading(); if (statusButton) { - const statusText = statusButton.getAttribute(editorText ? 'data-status-and-comment' : 'data-status'); - statusButton.querySelector('.status-button-text')!.textContent = statusText; + statusButton.querySelector('.status-button-text')!.textContent = editorText ? defaultStatusAndCommentText : defaultStatusText; statusButton.disabled = isUploading; } + if (closeReasonAnsweredItem) { + toggleElem(closeReasonAnsweredItem, editorText.length > 0); + } + syncCloseReasonMenuLabels(editorText.length > 0); if (commentButton) { commentButton.disabled = !editorText || isUploading; } }; + + const normalizeIssueIndex = (value: string): number => { + const m = /^#?(\d+)$/.exec(value.trim()); + if (!m) return 0; + return parseInt(m[1], 10) || 0; + }; + + const renderDuplicateList = (issues: Array>) => { + if (!duplicateList) return; + duplicateList.innerHTML = issues.map((issue) => { + const stateText = issue.state === 'closed' ? 'closed' : 'open'; + return html` + + `; + }).join(''); + + for (const item of duplicateList.querySelectorAll('.close-reason-duplicate-item')) { + item.addEventListener('click', () => { + selectedDuplicateIssueIndex = parseInt(item.getAttribute('data-index') || '0', 10); + for (const sibling of duplicateList.querySelectorAll('.close-reason-duplicate-item')) { + sibling.classList.toggle('is-selected', sibling === item); + } + if (duplicateInput) duplicateInput.value = `#${selectedDuplicateIssueIndex}`; + if (duplicateConfirmBtn) duplicateConfirmBtn.disabled = selectedDuplicateIssueIndex <= 0; + }); + } + }; + + const closeDuplicateModal = () => { + if (!duplicateModal) return; + if (typeof duplicateModal.close === 'function' && duplicateModal.open) { + duplicateModal.close(); + } + duplicateModal.removeAttribute('open'); + duplicateModalReturnFocus?.focus(); + duplicateModalReturnFocus = null; + }; + + const searchDuplicateIssues = async (query: string) => { + const normalized = normalizeIssueIndex(query); + if (!duplicateList || !duplicateLoading || !duplicateEmpty) return; + + selectedDuplicateIssueIndex = 0; + if (duplicateConfirmBtn) duplicateConfirmBtn.disabled = true; + duplicateList.innerHTML = ''; + + if (normalized <= 0) { + hideElem(duplicateLoading); + hideElem(duplicateEmpty); + return; + } + + showElem(duplicateLoading); + hideElem(duplicateEmpty); + try { + const url = `${issuePageInfo.repoLink}/issues/search?q=${encodeURIComponent(String(normalized))}&type=issues`; + const resp = await GET(url); + if (!resp.ok) { + throw new Error(`Failed to search issues: ${resp.status}`); + } + const response = await resp.json() as Array>; + const issues = response.filter((issue) => !issue.pull_request && issue.number !== issuePageInfo.issueNumber); + hideElem(duplicateLoading); + toggleElem(duplicateEmpty, issues.length === 0); + renderDuplicateList(issues); + } catch (error: any) { + hideElem(duplicateLoading); + showElem(duplicateEmpty); + showErrorToast(error?.message || 'Failed to search issues'); + } + }; + + const openDuplicateModal = () => { + if (!duplicateModal) return; + duplicateModalReturnFocus = closeReasonDropdownBtn; + selectedDuplicateIssueIndex = 0; + if (duplicateConfirmBtn) duplicateConfirmBtn.disabled = true; + if (duplicateInput) duplicateInput.value = ''; + if (duplicateList) duplicateList.innerHTML = ''; + hideElem(duplicateLoading || []); + hideElem(duplicateEmpty || []); + + if (typeof duplicateModal.showModal === 'function') { + duplicateModal.showModal(); + } else { + duplicateModal.setAttribute('open', 'open'); + } + duplicateInput?.focus(); + }; + + // Split button dropdown logic for close reason (only present for open, non-PR issues) + if (closeReasonDropdownBtn && closeReasonMenu) { + closeReasonDropdownBtn.addEventListener('click', (e) => { + e.stopPropagation(); + menuShown = !menuShown; + closeReasonMenu.classList.toggle('show', menuShown); + closeReasonMenu.style.display = menuShown ? 'block' : 'none'; + }); + + closeReasonDropdownBtn.addEventListener('keydown', (e) => { + if (e.key === 'Escape') { + hideMenu(); + } + }); + + closeReasonMenu.addEventListener('keydown', (e) => { + if (e.key === 'Escape') { + hideMenu(); + closeReasonDropdownBtn.focus(); + } + }); + + document.addEventListener('mouseup', hideMenu); + + for (const item of closeReasonMenu.querySelectorAll('.item')) { + item.addEventListener('click', (e) => { + e.stopPropagation(); + const reason = item.getAttribute('data-reason') ?? ''; + + if (reason === 'duplicate') { + hideMenu(); + openDuplicateModal(); + return; + } + + if (reason === 'answered' && !hasComment()) { + hideMenu(); + return; + } + + submitCloseWithReason(reason); + }); + } + } + + if (duplicateInput) { + duplicateInput.addEventListener('input', () => { + if (duplicateSearchTimer) clearTimeout(duplicateSearchTimer); + duplicateSearchTimer = setTimeout(() => { + searchDuplicateIssues(duplicateInput.value); + }, 250); + }); + } + + if (duplicateCancelBtn) { + duplicateCancelBtn.addEventListener('click', () => { + closeDuplicateModal(); + }); + } + + if (duplicateModal) { + duplicateModal.addEventListener('cancel', (e) => { + e.preventDefault(); + closeDuplicateModal(); + }); + } + + if (duplicateConfirmBtn) { + duplicateConfirmBtn.addEventListener('click', () => { + if (selectedDuplicateIssueIndex <= 0) return; + closeDuplicateModal(); + submitCloseWithReason('duplicate', JSON.stringify({issue_index: selectedDuplicateIssueIndex})); + }); + } + + // Serialize reason param as JSON on form submit + issueCommentForm.addEventListener('submit', (e) => { + const reason = closeReasonHidden?.value ?? ''; + + if (reason === 'duplicate' && !closeReasonParamHidden?.value) { + e.preventDefault(); + openDuplicateModal(); + return; + } + + if (reason === 'answered' && !hasComment()) { + e.preventDefault(); + showErrorToast('Close as answered requires a comment'); + return; + } + + if (reason !== 'duplicate' && closeReasonParamHidden) { + closeReasonParamHidden.value = ''; + } + }); + editor.container.addEventListener(ComboMarkdownEditor.EventUploadStateChanged, syncUiState); editor.container.addEventListener(ComboMarkdownEditor.EventEditorContentChanged, syncUiState); + clearCloseReasonFields(); syncUiState(); } diff --git a/web_src/js/types.ts b/web_src/js/types.ts index d233eba24f3ed..d974869326588 100644 --- a/web_src/js/types.ts +++ b/web_src/js/types.ts @@ -41,6 +41,7 @@ export type Issue = { title: string, body: string, state: 'open' | 'closed', + state_reason?: string, created_at: string, pull_request?: { draft: boolean;