Skip to content

Commit f7959bc

Browse files
wxiaoguangwxiaoguang
committed
improve comments and tests
1 parent f878b14 commit f7959bc

File tree

3 files changed

+57
-72
lines changed

3 files changed

+57
-72
lines changed

‎models/issues/issue_lock.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,14 @@ import (
1212

1313
// IssueLockOptions defines options for locking and/or unlocking an issue/PR
1414
type IssueLockOptions struct {
15-
Doer *user_model.User
16-
Issue *Issue
15+
Doer *user_model.User
16+
Issue *Issue
17+
18+
// Reason is the doer-provided comment message for the locked issue
19+
// GitHub doesn't support changing the "reasons" by config file, so GitHub has pre-defined "reason" enum values.
20+
// Gitea is not like GitHub, it allows site admin to define customized "reasons" in the config file.
21+
// So the API caller might not know what kind of "reasons" are valid, and the customized reasons are not translatable.
22+
// To make things clear and simple: doer have the chance to use any reason they like, we do not do validation.
1723
Reason string
1824
}
1925

‎routers/api/v1/repo/issue_lock.go‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,11 @@ package repo
66
import (
77
"errors"
88
"net/http"
9-
"strings"
109

1110
issues_model "code.gitea.io/gitea/models/issues"
1211
api "code.gitea.io/gitea/modules/structs"
1312
"code.gitea.io/gitea/modules/web"
1413
"code.gitea.io/gitea/services/context"
15-
16-
"golang.org/x/text/cases"
17-
"golang.org/x/text/language"
1814
)
1915

2016
// LockIssue lock an issue
@@ -55,13 +51,7 @@ func LockIssue(ctx *context.APIContext) {
5551
// "404":
5652
// "$ref": "#/responses/notFound"
5753

58-
caser := cases.Title(language.English)
5954
reason := web.GetForm(ctx).(*api.LockIssueOption).Reason
60-
reason = strings.ToLower(reason)
61-
reasonParts := strings.Split(reason, " ")
62-
reasonParts[0] = caser.String(reasonParts[0])
63-
reason = strings.Join(reasonParts, " ")
64-
6555
issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("index"))
6656
if err != nil {
6757
if issues_model.IsErrIssueNotExist(err) {

‎tests/integration/api_issue_lock_test.go‎

Lines changed: 49 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -22,64 +22,53 @@ import (
2222
func TestAPILockIssue(t *testing.T) {
2323
defer tests.PrepareTestEnv(t)()
2424

25-
issueBefore := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
26-
assert.False(t, issueBefore.IsLocked)
27-
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID})
28-
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
29-
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/lock", owner.Name, repo.Name, issueBefore.Index)
30-
31-
session := loginUser(t, owner.Name)
32-
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
33-
34-
// check lock issue
35-
req := NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token)
36-
MakeRequest(t, req, http.StatusNoContent)
37-
issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
38-
assert.True(t, issueAfter.IsLocked)
39-
40-
// check reason is case insensitive
41-
unlockReq := NewRequest(t, "DELETE", urlStr).AddTokenAuth(token)
42-
MakeRequest(t, unlockReq, http.StatusNoContent)
43-
issueAfter = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
44-
assert.False(t, issueAfter.IsLocked)
45-
46-
req = NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "too heated"}).AddTokenAuth(token)
47-
MakeRequest(t, req, http.StatusNoContent)
48-
issueAfter = unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
49-
assert.True(t, issueAfter.IsLocked)
50-
51-
// check with other user
52-
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})
53-
session34 := loginUser(t, user34.Name)
54-
token34 := getTokenForLoggedInUser(t, session34, auth_model.AccessTokenScopeAll)
55-
req = NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token34)
56-
MakeRequest(t, req, http.StatusForbidden)
57-
}
58-
59-
func TestAPIUnlockIssue(t *testing.T) {
60-
defer tests.PrepareTestEnv(t)()
61-
62-
issueBefore := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
63-
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID})
64-
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
65-
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/lock", owner.Name, repo.Name, issueBefore.Index)
66-
67-
session := loginUser(t, owner.Name)
68-
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
69-
70-
lockReq := NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token)
71-
MakeRequest(t, lockReq, http.StatusNoContent)
72-
73-
// check unlock issue
74-
req := NewRequest(t, "DELETE", urlStr).AddTokenAuth(token)
75-
MakeRequest(t, req, http.StatusNoContent)
76-
issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
77-
assert.False(t, issueAfter.IsLocked)
78-
79-
// check with other user
80-
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})
81-
session34 := loginUser(t, user34.Name)
82-
token34 := getTokenForLoggedInUser(t, session34, auth_model.AccessTokenScopeAll)
83-
req = NewRequest(t, "DELETE", urlStr).AddTokenAuth(token34)
84-
MakeRequest(t, req, http.StatusForbidden)
25+
t.Run("Lock", func(t *testing.T) {
26+
issueBefore := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
27+
assert.False(t, issueBefore.IsLocked)
28+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID})
29+
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
30+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/lock", owner.Name, repo.Name, issueBefore.Index)
31+
32+
session := loginUser(t, owner.Name)
33+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
34+
35+
// check lock issue
36+
req := NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token)
37+
MakeRequest(t, req, http.StatusNoContent)
38+
issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
39+
assert.True(t, issueAfter.IsLocked)
40+
41+
// check with other user
42+
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})
43+
session34 := loginUser(t, user34.Name)
44+
token34 := getTokenForLoggedInUser(t, session34, auth_model.AccessTokenScopeAll)
45+
req = NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token34)
46+
MakeRequest(t, req, http.StatusForbidden)
47+
})
48+
49+
t.Run("Unlock", func(t *testing.T) {
50+
issueBefore := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
51+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issueBefore.RepoID})
52+
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
53+
urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/lock", owner.Name, repo.Name, issueBefore.Index)
54+
55+
session := loginUser(t, owner.Name)
56+
token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
57+
58+
lockReq := NewRequestWithJSON(t, "PUT", urlStr, api.LockIssueOption{Reason: "Spam"}).AddTokenAuth(token)
59+
MakeRequest(t, lockReq, http.StatusNoContent)
60+
61+
// check unlock issue
62+
req := NewRequest(t, "DELETE", urlStr).AddTokenAuth(token)
63+
MakeRequest(t, req, http.StatusNoContent)
64+
issueAfter := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
65+
assert.False(t, issueAfter.IsLocked)
66+
67+
// check with other user
68+
user34 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 34})
69+
session34 := loginUser(t, user34.Name)
70+
token34 := getTokenForLoggedInUser(t, session34, auth_model.AccessTokenScopeAll)
71+
req = NewRequest(t, "DELETE", urlStr).AddTokenAuth(token34)
72+
MakeRequest(t, req, http.StatusForbidden)
73+
})
8574
}

0 commit comments

Comments
 (0)