Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaofficial/main'
Browse files Browse the repository at this point in the history
* giteaofficial/main:
  Do not render truncated links in markdown (go-gitea#32980)
  Use `CloseIssue` and `ReopenIssue` instead of `ChangeStatus` (go-gitea#32467)
  • Loading branch information
zjjhot committed Dec 26, 2024
2 parents 7da99b0 + 594edad commit 0fb6f40
Show file tree
Hide file tree
Showing 16 changed files with 207 additions and 112 deletions.
13 changes: 12 additions & 1 deletion models/issues/dependency_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,25 @@ func TestCreateIssueDependency(t *testing.T) {
assert.False(t, left)

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

issue2Closed, err := issues_model.GetIssueByID(db.DefaultContext, 2)
assert.NoError(t, err)
assert.True(t, issue2Closed.IsClosed)

left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err)
assert.True(t, left)

// Test removing the dependency
err = issues_model.RemoveIssueDependency(db.DefaultContext, user1, issue1, issue2, issues_model.DependencyTypeBlockedBy)
assert.NoError(t, err)

_, err = issues_model.ReopenIssue(db.DefaultContext, issue2, user1)
assert.NoError(t, err)

issue2Reopened, err := issues_model.GetIssueByID(db.DefaultContext, 2)
assert.NoError(t, err)
assert.False(t, issue2Reopened.IsClosed)
}
44 changes: 41 additions & 3 deletions models/issues/issue_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,54 @@ func doChangeIssueStatus(ctx context.Context, issue *Issue, doer *user_model.Use
})
}

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

return changeIssueStatus(ctx, issue, doer, isClosed, false)
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
defer committer.Close()

comment, err := changeIssueStatus(ctx, issue, doer, true, false)
if err != nil {
return nil, err
}
if err := committer.Commit(); err != nil {
return nil, err
}
return comment, nil
}

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

ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
defer committer.Close()

comment, err := changeIssueStatus(ctx, issue, doer, false, false)
if err != nil {
return nil, err
}
if err := committer.Commit(); err != nil {
return nil, err
}
return comment, nil
}

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

pr := testCreatePR(t, 1, 2, "titlepr", fmt.Sprintf("closes #%d", i1.Index))
Expand Down
5 changes: 5 additions & 0 deletions modules/markup/html_link.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"code.gitea.io/gitea/modules/markup/common"
"code.gitea.io/gitea/modules/util"

"golang.org/x/net/html"
"golang.org/x/net/html/atom"
Expand Down Expand Up @@ -171,6 +172,10 @@ func linkProcessor(ctx *RenderContext, node *html.Node) {
}

uri := node.Data[m[0]:m[1]]
remaining := node.Data[m[1]:]
if util.IsLikelySplitLeftPart(remaining) {
return
}
replaceContent(node, m[0], m[1], createLink(ctx, uri, uri, "" /*link*/))
node = node.NextSibling.NextSibling
}
Expand Down
10 changes: 10 additions & 0 deletions modules/markup/html_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,16 @@ func TestRender_links(t *testing.T) {
test(
"ftps://gitea.com",
`<p>ftps://gitea.com</p>`)

t.Run("LinkSplit", func(t *testing.T) {
input, _ := util.SplitStringAtByteN("http://10.1.2.3", 12)
assert.Equal(t, "http://10…", input)
test(input, "<p>http://10…</p>")

input, _ = util.SplitStringAtByteN("http://10.1.2.3", 13)
assert.Equal(t, "http://10.…", input)
test(input, "<p>http://10.…</p>")
})
}

func TestRender_email(t *testing.T) {
Expand Down
17 changes: 16 additions & 1 deletion modules/util/string.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@

package util

import "unsafe"
import (
"strings"
"unsafe"
)

func isSnakeCaseUpper(c byte) bool {
return 'A' <= c && c <= 'Z'
Expand Down Expand Up @@ -95,3 +98,15 @@ func UnsafeBytesToString(b []byte) string {
func UnsafeStringToBytes(s string) []byte {
return unsafe.Slice(unsafe.StringData(s), len(s))
}

// SplitTrimSpace splits the string at given separator and trims leading and trailing space
func SplitTrimSpace(input, sep string) []string {
input = strings.TrimSpace(input)
var stringList []string
for _, s := range strings.Split(input, sep) {
if s = strings.TrimSpace(s); s != "" {
stringList = append(stringList, s)
}
}
return stringList
}
5 changes: 5 additions & 0 deletions modules/util/string_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,8 @@ func TestToSnakeCase(t *testing.T) {
assert.Equal(t, expected, ToSnakeCase(input))
}
}

func TestSplitTrimSpace(t *testing.T) {
assert.Equal(t, []string{"a", "b", "c"}, SplitTrimSpace("a\nb\nc", "\n"))
assert.Equal(t, []string{"a", "b"}, SplitTrimSpace("\r\na\n\r\nb\n\n", "\n"))
}
20 changes: 4 additions & 16 deletions modules/util/truncate.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const (
asciiEllipsis = "..."
)

func IsLikelySplitLeftPart(s string) bool {
return strings.HasSuffix(s, utf8Ellipsis) || strings.HasSuffix(s, asciiEllipsis)
}

// SplitStringAtByteN splits a string at byte n accounting for rune boundaries. (Combining characters are not accounted for.)
func SplitStringAtByteN(input string, n int) (left, right string) {
if len(input) <= n {
Expand All @@ -38,19 +42,3 @@ func SplitStringAtByteN(input string, n int) (left, right string) {

return input[:end] + utf8Ellipsis, utf8Ellipsis + input[end:]
}

// SplitTrimSpace splits the string at given separator and trims leading and trailing space
func SplitTrimSpace(input, sep string) []string {
// Trim initial leading & trailing space
input = strings.TrimSpace(input)
// replace CRLF with LF
input = strings.ReplaceAll(input, "\r\n", "\n")

var stringList []string
for _, s := range strings.Split(input, sep) {
// trim leading and trailing space
stringList = append(stringList, strings.TrimSpace(s))
}

return stringList
}
47 changes: 27 additions & 20 deletions routers/api/v1/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ func CreateIssue(ctx *context.APIContext) {
}

if form.Closed {
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", true); err != nil {
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
if issues_model.IsErrDependenciesLeft(err) {
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
return
Expand Down Expand Up @@ -912,27 +912,11 @@ func EditIssue(ctx *context.APIContext) {
}
}

var isClosed bool
switch state := api.StateType(*form.State); state {
case api.StateOpen:
isClosed = false
case api.StateClosed:
isClosed = true
default:
ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", fmt.Sprintf("unknown state: %s", state))
state := api.StateType(*form.State)
closeOrReopenIssue(ctx, issue, state)
if ctx.Written() {
return
}

if issue.IsClosed != isClosed {
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
if issues_model.IsErrDependenciesLeft(err) {
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue because it still has open dependencies")
return
}
ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
return
}
}
}

// Refetch from database to assign some automatic values
Expand Down Expand Up @@ -1055,3 +1039,26 @@ 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) {
if state != api.StateOpen && state != api.StateClosed {
ctx.Error(http.StatusPreconditionFailed, "UnknownIssueStateError", 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 issues_model.IsErrDependenciesLeft(err) {
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this issue or pull request because it still has open dependencies")
return
}
ctx.Error(http.StatusInternalServerError, "CloseIssue", err)
return
}
} else if state == api.StateOpen && issue.IsClosed {
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
ctx.Error(http.StatusInternalServerError, "ReopenIssue", err)
return
}
}
}
22 changes: 3 additions & 19 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,27 +728,11 @@ func EditPullRequest(ctx *context.APIContext) {
return
}

var isClosed bool
switch state := api.StateType(*form.State); state {
case api.StateOpen:
isClosed = false
case api.StateClosed:
isClosed = true
default:
ctx.Error(http.StatusPreconditionFailed, "UnknownPRStateError", fmt.Sprintf("unknown state: %s", state))
state := api.StateType(*form.State)
closeOrReopenIssue(ctx, issue, state)
if ctx.Written() {
return
}

if issue.IsClosed != isClosed {
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
if issues_model.IsErrDependenciesLeft(err) {
ctx.Error(http.StatusPreconditionFailed, "DependenciesLeft", "cannot close this pull request because it still has open dependencies")
return
}
ctx.Error(http.StatusInternalServerError, "ChangeStatus", err)
return
}
}
}

// change pull target branch
Expand Down
35 changes: 19 additions & 16 deletions routers/web/repo/issue_comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,25 +154,28 @@ func NewComment(ctx *context.Context) {
if pr != nil {
ctx.Flash.Info(ctx.Tr("repo.pulls.open_unmerged_pull_exists", pr.Index))
} else {
isClosed := form.Status == "close"
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
log.Error("ChangeStatus: %v", err)

if issues_model.IsErrDependenciesLeft(err) {
if issue.IsPull {
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
} else {
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
if form.Status == "close" && !issue.IsClosed {
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); err != nil {
log.Error("CloseIssue: %v", err)
if issues_model.IsErrDependenciesLeft(err) {
if issue.IsPull {
ctx.JSONError(ctx.Tr("repo.issues.dependency.pr_close_blocked"))
} else {
ctx.JSONError(ctx.Tr("repo.issues.dependency.issue_close_blocked"))
}
return
}
return
} else {
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
ctx.ServerError("stopTimerIfAvailable", err)
return
}
log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
}
} else {
if err := stopTimerIfAvailable(ctx, ctx.Doer, issue); err != nil {
ctx.ServerError("CreateOrStopIssueStopwatch", err)
return
} else if form.Status == "reopen" && issue.IsClosed {
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
log.Error("ReopenIssue: %v", err)
}

log.Trace("Issue [%d] status changed to closed: %v", issue.ID, issue.IsClosed)
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions routers/web/repo/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,14 +418,11 @@ func UpdateIssueStatus(ctx *context.Context) {
return
}

var isClosed bool
switch action := ctx.FormString("action"); action {
case "open":
isClosed = false
case "close":
isClosed = true
default:
action := ctx.FormString("action")
if action != "open" && action != "close" {
log.Warn("Unrecognized action: %s", action)
ctx.JSONOK()
return
}

if _, err := issues.LoadRepositories(ctx); err != nil {
Expand All @@ -441,15 +438,20 @@ func UpdateIssueStatus(ctx *context.Context) {
if issue.IsPull && issue.PullRequest.HasMerged {
continue
}
if issue.IsClosed != isClosed {
if err := issue_service.ChangeStatus(ctx, issue, ctx.Doer, "", isClosed); err != nil {
if action == "close" && !issue.IsClosed {
if err := issue_service.CloseIssue(ctx, issue, ctx.Doer, ""); 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),
})
return
}
ctx.ServerError("ChangeStatus", err)
ctx.ServerError("CloseIssue", err)
return
}
} else if action == "open" && issue.IsClosed {
if err := issue_service.ReopenIssue(ctx, issue, ctx.Doer, ""); err != nil {
ctx.ServerError("ReopenIssue", err)
return
}
}
Expand Down
Loading

0 comments on commit 0fb6f40

Please sign in to comment.