Skip to content

Commit ef98b16

Browse files
lunnylunny
authored
Move UpdateIssuesCommit from models to repofiles (#9276)
1 parent 9cb418e commit ef98b16

File tree

4 files changed

+333
-334
lines changed

4 files changed

+333
-334
lines changed

‎models/action.go

Lines changed: 0 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package models
77

88
import (
99
"fmt"
10-
"html"
1110
"path"
1211
"strconv"
1312
"strings"
@@ -16,7 +15,6 @@ import (
1615
"code.gitea.io/gitea/modules/base"
1716
"code.gitea.io/gitea/modules/git"
1817
"code.gitea.io/gitea/modules/log"
19-
"code.gitea.io/gitea/modules/references"
2018
"code.gitea.io/gitea/modules/setting"
2119
api "code.gitea.io/gitea/modules/structs"
2220
"code.gitea.io/gitea/modules/timeutil"
@@ -409,132 +407,6 @@ func (pc *PushCommits) AvatarLink(email string) string {
409407
return pc.avatars[email]
410408
}
411409

412-
// getIssueFromRef returns the issue referenced by a ref. Returns a nil *Issue
413-
// if the provided ref references a non-existent issue.
414-
func getIssueFromRef(repo *Repository, index int64) (*Issue, error) {
415-
issue, err := GetIssueByIndex(repo.ID, index)
416-
if err != nil {
417-
if IsErrIssueNotExist(err) {
418-
return nil, nil
419-
}
420-
return nil, err
421-
}
422-
return issue, nil
423-
}
424-
425-
func changeIssueStatus(repo *Repository, issue *Issue, doer *User, status bool) error {
426-
427-
stopTimerIfAvailable := func(doer *User, issue *Issue) error {
428-
429-
if StopwatchExists(doer.ID, issue.ID) {
430-
if err := CreateOrStopIssueStopwatch(doer, issue); err != nil {
431-
return err
432-
}
433-
}
434-
435-
return nil
436-
}
437-
438-
issue.Repo = repo
439-
if err := issue.ChangeStatus(doer, status); err != nil {
440-
// Don't return an error when dependencies are open as this would let the push fail
441-
if IsErrDependenciesLeft(err) {
442-
return stopTimerIfAvailable(doer, issue)
443-
}
444-
return err
445-
}
446-
447-
return stopTimerIfAvailable(doer, issue)
448-
}
449-
450-
// UpdateIssuesCommit checks if issues are manipulated by commit message.
451-
func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit, branchName string) error {
452-
// Commits are appended in the reverse order.
453-
for i := len(commits) - 1; i >= 0; i-- {
454-
c := commits[i]
455-
456-
type markKey struct {
457-
ID int64
458-
Action references.XRefAction
459-
}
460-
461-
refMarked := make(map[markKey]bool)
462-
var refRepo *Repository
463-
var refIssue *Issue
464-
var err error
465-
for _, ref := range references.FindAllIssueReferences(c.Message) {
466-
467-
// issue is from another repo
468-
if len(ref.Owner) > 0 && len(ref.Name) > 0 {
469-
refRepo, err = GetRepositoryFromMatch(ref.Owner, ref.Name)
470-
if err != nil {
471-
continue
472-
}
473-
} else {
474-
refRepo = repo
475-
}
476-
if refIssue, err = getIssueFromRef(refRepo, ref.Index); err != nil {
477-
return err
478-
}
479-
if refIssue == nil {
480-
continue
481-
}
482-
483-
perm, err := GetUserRepoPermission(refRepo, doer)
484-
if err != nil {
485-
return err
486-
}
487-
488-
key := markKey{ID: refIssue.ID, Action: ref.Action}
489-
if refMarked[key] {
490-
continue
491-
}
492-
refMarked[key] = true
493-
494-
// FIXME: this kind of condition is all over the code, it should be consolidated in a single place
495-
canclose := perm.IsAdmin() || perm.IsOwner() || perm.CanWrite(UnitTypeIssues) || refIssue.PosterID == doer.ID
496-
cancomment := canclose || perm.CanRead(UnitTypeIssues)
497-
498-
// Don't proceed if the user can't comment
499-
if !cancomment {
500-
continue
501-
}
502-
503-
message := fmt.Sprintf(`<a href="%s/commit/%s">%s</a>`, repo.Link(), c.Sha1, html.EscapeString(c.Message))
504-
if err = CreateRefComment(doer, refRepo, refIssue, message, c.Sha1); err != nil {
505-
return err
506-
}
507-
508-
// Only issues can be closed/reopened this way, and user needs the correct permissions
509-
if refIssue.IsPull || !canclose {
510-
continue
511-
}
512-
513-
// Only process closing/reopening keywords
514-
if ref.Action != references.XRefActionCloses && ref.Action != references.XRefActionReopens {
515-
continue
516-
}
517-
518-
if !repo.CloseIssuesViaCommitInAnyBranch {
519-
// If the issue was specified to be in a particular branch, don't allow commits in other branches to close it
520-
if refIssue.Ref != "" {
521-
if branchName != refIssue.Ref {
522-
continue
523-
}
524-
// Otherwise, only process commits to the default branch
525-
} else if branchName != repo.DefaultBranch {
526-
continue
527-
}
528-
}
529-
530-
if err := changeIssueStatus(refRepo, refIssue, doer, ref.Action == references.XRefActionCloses); err != nil {
531-
return err
532-
}
533-
}
534-
}
535-
return nil
536-
}
537-
538410
// GetFeedsOptions options for retrieving feeds
539411
type GetFeedsOptions struct {
540412
RequestedUser *User

‎models/action_test.go

Lines changed: 0 additions & 205 deletions
Original file line numberDiff line numberDiff line change
@@ -127,211 +127,6 @@ func TestPushCommits_AvatarLink(t *testing.T) {
127127
pushCommits.AvatarLink("[email protected]"))
128128
}
129129

130-
func TestUpdateIssuesCommit(t *testing.T) {
131-
assert.NoError(t, PrepareTestDatabase())
132-
pushCommits := []*PushCommit{
133-
{
134-
Sha1: "abcdef1",
135-
CommitterEmail: "[email protected]",
136-
CommitterName: "User Two",
137-
AuthorEmail: "[email protected]",
138-
AuthorName: "User Four",
139-
Message: "start working on #FST-1, #1",
140-
},
141-
{
142-
Sha1: "abcdef2",
143-
CommitterEmail: "[email protected]",
144-
CommitterName: "User Two",
145-
AuthorEmail: "[email protected]",
146-
AuthorName: "User Two",
147-
Message: "a plain message",
148-
},
149-
{
150-
Sha1: "abcdef2",
151-
CommitterEmail: "[email protected]",
152-
CommitterName: "User Two",
153-
AuthorEmail: "[email protected]",
154-
AuthorName: "User Two",
155-
Message: "close #2",
156-
},
157-
}
158-
159-
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
160-
repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
161-
repo.Owner = user
162-
163-
commentBean := &Comment{
164-
Type: CommentTypeCommitRef,
165-
CommitSHA: "abcdef1",
166-
PosterID: user.ID,
167-
IssueID: 1,
168-
}
169-
issueBean := &Issue{RepoID: repo.ID, Index: 4}
170-
171-
AssertNotExistsBean(t, commentBean)
172-
AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1")
173-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
174-
AssertExistsAndLoadBean(t, commentBean)
175-
AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
176-
CheckConsistencyFor(t, &Action{})
177-
178-
// Test that push to a non-default branch closes no issue.
179-
pushCommits = []*PushCommit{
180-
{
181-
Sha1: "abcdef1",
182-
CommitterEmail: "[email protected]",
183-
CommitterName: "User Two",
184-
AuthorEmail: "[email protected]",
185-
AuthorName: "User Four",
186-
Message: "close #1",
187-
},
188-
}
189-
repo = AssertExistsAndLoadBean(t, &Repository{ID: 3}).(*Repository)
190-
commentBean = &Comment{
191-
Type: CommentTypeCommitRef,
192-
CommitSHA: "abcdef1",
193-
PosterID: user.ID,
194-
IssueID: 6,
195-
}
196-
issueBean = &Issue{RepoID: repo.ID, Index: 1}
197-
198-
AssertNotExistsBean(t, commentBean)
199-
AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 1}, "is_closed=1")
200-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, "non-existing-branch"))
201-
AssertExistsAndLoadBean(t, commentBean)
202-
AssertNotExistsBean(t, issueBean, "is_closed=1")
203-
CheckConsistencyFor(t, &Action{})
204-
}
205-
206-
func TestUpdateIssuesCommit_Colon(t *testing.T) {
207-
assert.NoError(t, PrepareTestDatabase())
208-
pushCommits := []*PushCommit{
209-
{
210-
Sha1: "abcdef2",
211-
CommitterEmail: "[email protected]",
212-
CommitterName: "User Two",
213-
AuthorEmail: "[email protected]",
214-
AuthorName: "User Two",
215-
Message: "close: #2",
216-
},
217-
}
218-
219-
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
220-
repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
221-
repo.Owner = user
222-
223-
issueBean := &Issue{RepoID: repo.ID, Index: 4}
224-
225-
AssertNotExistsBean(t, &Issue{RepoID: repo.ID, Index: 2}, "is_closed=1")
226-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
227-
AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
228-
CheckConsistencyFor(t, &Action{})
229-
}
230-
231-
func TestUpdateIssuesCommit_Issue5957(t *testing.T) {
232-
assert.NoError(t, PrepareTestDatabase())
233-
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
234-
235-
// Test that push to a non-default branch closes an issue.
236-
pushCommits := []*PushCommit{
237-
{
238-
Sha1: "abcdef1",
239-
CommitterEmail: "[email protected]",
240-
CommitterName: "User Two",
241-
AuthorEmail: "[email protected]",
242-
AuthorName: "User Four",
243-
Message: "close #2",
244-
},
245-
}
246-
247-
repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
248-
commentBean := &Comment{
249-
Type: CommentTypeCommitRef,
250-
CommitSHA: "abcdef1",
251-
PosterID: user.ID,
252-
IssueID: 7,
253-
}
254-
255-
issueBean := &Issue{RepoID: repo.ID, Index: 2, ID: 7}
256-
257-
AssertNotExistsBean(t, commentBean)
258-
AssertNotExistsBean(t, issueBean, "is_closed=1")
259-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, "non-existing-branch"))
260-
AssertExistsAndLoadBean(t, commentBean)
261-
AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
262-
CheckConsistencyFor(t, &Action{})
263-
}
264-
265-
func TestUpdateIssuesCommit_AnotherRepo(t *testing.T) {
266-
assert.NoError(t, PrepareTestDatabase())
267-
user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User)
268-
269-
// Test that a push to default branch closes issue in another repo
270-
// If the user also has push permissions to that repo
271-
pushCommits := []*PushCommit{
272-
{
273-
Sha1: "abcdef1",
274-
CommitterEmail: "[email protected]",
275-
CommitterName: "User Two",
276-
AuthorEmail: "[email protected]",
277-
AuthorName: "User Two",
278-
Message: "close user2/repo1#1",
279-
},
280-
}
281-
282-
repo := AssertExistsAndLoadBean(t, &Repository{ID: 2}).(*Repository)
283-
commentBean := &Comment{
284-
Type: CommentTypeCommitRef,
285-
CommitSHA: "abcdef1",
286-
PosterID: user.ID,
287-
IssueID: 1,
288-
}
289-
290-
issueBean := &Issue{RepoID: 1, Index: 1, ID: 1}
291-
292-
AssertNotExistsBean(t, commentBean)
293-
AssertNotExistsBean(t, issueBean, "is_closed=1")
294-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
295-
AssertExistsAndLoadBean(t, commentBean)
296-
AssertExistsAndLoadBean(t, issueBean, "is_closed=1")
297-
CheckConsistencyFor(t, &Action{})
298-
}
299-
300-
func TestUpdateIssuesCommit_AnotherRepoNoPermission(t *testing.T) {
301-
assert.NoError(t, PrepareTestDatabase())
302-
user := AssertExistsAndLoadBean(t, &User{ID: 10}).(*User)
303-
304-
// Test that a push with close reference *can not* close issue
305-
// If the commiter doesn't have push rights in that repo
306-
pushCommits := []*PushCommit{
307-
{
308-
Sha1: "abcdef3",
309-
CommitterEmail: "[email protected]",
310-
CommitterName: "User Ten",
311-
AuthorEmail: "[email protected]",
312-
AuthorName: "User Ten",
313-
Message: "close user3/repo3#1",
314-
},
315-
}
316-
317-
repo := AssertExistsAndLoadBean(t, &Repository{ID: 6}).(*Repository)
318-
commentBean := &Comment{
319-
Type: CommentTypeCommitRef,
320-
CommitSHA: "abcdef3",
321-
PosterID: user.ID,
322-
IssueID: 6,
323-
}
324-
325-
issueBean := &Issue{RepoID: 3, Index: 1, ID: 6}
326-
327-
AssertNotExistsBean(t, commentBean)
328-
AssertNotExistsBean(t, issueBean, "is_closed=1")
329-
assert.NoError(t, UpdateIssuesCommit(user, repo, pushCommits, repo.DefaultBranch))
330-
AssertNotExistsBean(t, commentBean)
331-
AssertNotExistsBean(t, issueBean, "is_closed=1")
332-
CheckConsistencyFor(t, &Action{})
333-
}
334-
335130
func TestGetFeeds(t *testing.T) {
336131
// test with an individual user
337132
assert.NoError(t, PrepareTestDatabase())

0 commit comments

Comments
 (0)