Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore: fix some trivial problems and TODOs #33473

Merged
merged 3 commits into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion models/issues/issue_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func GetIssueStats(ctx context.Context, opts *IssuesOptions) (*IssueStats, error
accum.YourRepositoriesCount += stats.YourRepositoriesCount
accum.AssignCount += stats.AssignCount
accum.CreateCount += stats.CreateCount
accum.OpenCount += stats.MentionCount
accum.MentionCount += stats.MentionCount
accum.ReviewRequestedCount += stats.ReviewRequestedCount
accum.ReviewedCount += stats.ReviewedCount
i = chunk
Expand Down
2 changes: 0 additions & 2 deletions models/system/notice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ func TestCreateRepositoryNotice(t *testing.T) {
unittest.AssertExistsAndLoadBean(t, noticeBean)
}

// TODO TestRemoveAllWithNotice

func TestCountNotices(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
assert.Equal(t, int64(3), system.CountNotices(db.DefaultContext))
Expand Down
9 changes: 2 additions & 7 deletions modules/base/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"time"

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"

Expand Down Expand Up @@ -64,10 +63,7 @@ func VerifyTimeLimitCode(now time.Time, data string, minutes int, code string) b
// check code
retCode := CreateTimeLimitCode(data, aliveTime, startTimeStr, nil)
if subtle.ConstantTimeCompare([]byte(retCode), []byte(code)) != 1 {
retCode = CreateTimeLimitCode(data, aliveTime, startTimeStr, sha1.New()) // TODO: this is only for the support of legacy codes, remove this in/after 1.23
if subtle.ConstantTimeCompare([]byte(retCode), []byte(code)) != 1 {
return false
}
return false
}

// check time is expired or not: startTime <= now && now < startTime + minutes
Expand Down Expand Up @@ -144,13 +140,12 @@ func Int64sToStrings(ints []int64) []string {
return strs
}

// EntryIcon returns the octicon class for displaying files/directories
// EntryIcon returns the octicon name for displaying files/directories
func EntryIcon(entry *git.TreeEntry) string {
switch {
case entry.IsLink():
te, err := entry.FollowLink()
if err != nil {
log.Debug(err.Error())
return "file-symlink-file"
}
if te.IsDir() {
Expand Down
11 changes: 3 additions & 8 deletions modules/base/tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,10 @@ JWT_SECRET = %s
verifyDataCode := func(c string) bool {
return VerifyTimeLimitCode(now, "data", 2, c)
}
code1 := CreateTimeLimitCode("data", 2, now, sha1.New())
code2 := CreateTimeLimitCode("data", 2, now, nil)
assert.True(t, verifyDataCode(code1))
assert.True(t, verifyDataCode(code2))
code := CreateTimeLimitCode("data", 2, now, nil)
assert.True(t, verifyDataCode(code))
initGeneralSecret("000_QLUd4fYVyxetjxC4eZkrBgWM2SndOOWDNtgUUko")
assert.False(t, verifyDataCode(code1))
assert.False(t, verifyDataCode(code2))
assert.False(t, verifyDataCode(code))
})
}

Expand Down Expand Up @@ -137,5 +134,3 @@ func TestInt64sToStrings(t *testing.T) {
Int64sToStrings([]int64{1, 4, 16, 64, 256}),
)
}

// TODO: Test EntryIcon
2 changes: 1 addition & 1 deletion modules/markup/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var globalVars = sync.OnceValue(func() *globalVarsType {
// NOTE: All below regex matching do not perform any extra validation.
// Thus a link is produced even if the linked entity does not exist.
// While fast, this is also incorrect and lead to false positives.
// TODO: fix invalid linking issue
// TODO: fix invalid linking issue (update: stale TODO, what issues? maybe no TODO anymore)

// valid chars in encoded path and parameter: [-+~_%.a-zA-Z0-9/]

Expand Down
2 changes: 1 addition & 1 deletion modules/web/middleware/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func GetInclude(field reflect.StructField) string {
return getRuleBody(field, "Include(")
}

// Validate validate TODO:
// Validate validate
func Validate(errs binding.Errors, data map[string]any, f Form, l translation.Locale) binding.Errors {
if errs.Len() == 0 {
return errs
Expand Down
5 changes: 0 additions & 5 deletions routers/web/repo/actions/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -628,11 +628,6 @@ func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions
}

func ArtifactsDeleteView(ctx *context_module.Context) {
if !ctx.Repo.CanWrite(unit.TypeActions) {
ctx.Error(http.StatusForbidden, "no permission")
return
}

runIndex := getRunIndex(ctx)
artifactName := ctx.PathParam("artifact_name")

Expand Down
4 changes: 2 additions & 2 deletions routers/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func webAuth(authMethod auth_service.Method) func(*context.Context) {
ar, err := common.AuthShared(ctx.Base, ctx.Session, authMethod)
if err != nil {
log.Error("Failed to verify user: %v", err)
ctx.Error(http.StatusUnauthorized, "Verify")
ctx.Error(http.StatusUnauthorized, "Failed to authenticate user")
return
}
ctx.Doer = ar.Doer
Expand Down Expand Up @@ -1430,7 +1430,7 @@ func registerRoutes(m *web.Router) {
m.Post("/cancel", reqRepoActionsWriter, actions.Cancel)
m.Post("/approve", reqRepoActionsWriter, actions.Approve)
m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView)
m.Delete("/artifacts/{artifact_name}", actions.ArtifactsDeleteView)
m.Delete("/artifacts/{artifact_name}", reqRepoActionsWriter, actions.ArtifactsDeleteView)
m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
})
m.Group("/workflows/{workflow_name}", func() {
Expand Down
16 changes: 8 additions & 8 deletions services/mailer/mail_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestComposeIssueCommentMessage(t *testing.T) {

recipients := []*user_model.User{{Name: "Test", Email: "[email protected]"}, {Name: "Test2", Email: "[email protected]"}}
msgs, err := composeIssueCommentMessages(&mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: activities_model.ActionCommentIssue,
Content: fmt.Sprintf("test @%s %s#%d body", doer.Name, issue.Repo.FullName(), issue.Index),
Comment: comment,
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestComposeIssueMessage(t *testing.T) {

recipients := []*user_model.User{{Name: "Test", Email: "[email protected]"}, {Name: "Test2", Email: "[email protected]"}}
msgs, err := composeIssueCommentMessages(&mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: activities_model.ActionCreateIssue,
Content: "test body",
}, "en-US", recipients, false, "issue create")
Expand Down Expand Up @@ -178,14 +178,14 @@ func TestTemplateSelection(t *testing.T) {
}

msg := testComposeIssueCommentMessage(t, &mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: activities_model.ActionCreateIssue,
Content: "test body",
}, recipients, false, "TestTemplateSelection")
expect(t, msg, "issue/new/subject", "issue/new/body")

msg = testComposeIssueCommentMessage(t, &mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: activities_model.ActionCommentIssue,
Content: "test body", Comment: comment,
}, recipients, false, "TestTemplateSelection")
Expand All @@ -194,14 +194,14 @@ func TestTemplateSelection(t *testing.T) {
pull := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2, Repo: repo, Poster: doer})
comment = unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 4, Issue: pull})
msg = testComposeIssueCommentMessage(t, &mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: pull, Doer: doer, ActionType: activities_model.ActionCommentPull,
Content: "test body", Comment: comment,
}, recipients, false, "TestTemplateSelection")
expect(t, msg, "pull/comment/subject", "pull/comment/body")

msg = testComposeIssueCommentMessage(t, &mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: activities_model.ActionCloseIssue,
Content: "test body", Comment: comment,
}, recipients, false, "TestTemplateSelection")
Expand All @@ -220,7 +220,7 @@ func TestTemplateServices(t *testing.T) {

recipients := []*user_model.User{{Name: "Test", Email: "[email protected]"}}
msg := testComposeIssueCommentMessage(t, &mailCommentContext{
Context: context.TODO(), // TODO: use a correct context
Context: context.TODO(),
Issue: issue, Doer: doer, ActionType: actionType,
Content: "test body", Comment: comment,
}, recipients, fromMention, "TestTemplateServices")
Expand Down Expand Up @@ -263,7 +263,7 @@ func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, recip
func TestGenerateAdditionalHeaders(t *testing.T) {
doer, _, issue, _ := prepareMailerTest(t)

ctx := &mailCommentContext{Context: context.TODO() /* TODO: use a correct context */, Issue: issue, Doer: doer}
ctx := &mailCommentContext{Context: context.TODO(), Issue: issue, Doer: doer}
recipient := &user_model.User{Name: "test", Email: "[email protected]"}

headers := generateAdditionalHeaders(ctx, "dummy-reason", recipient)
Expand Down
69 changes: 0 additions & 69 deletions tests/integration/benchmarks_test.go

This file was deleted.