From 9a93b1816e0bc65101e7ad7ca66786fb38a8e628 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 13 Mar 2024 07:04:07 +0100 Subject: [PATCH 01/14] Refactor label.IsArchived() (#29750) just some missed nits --- models/issues/label.go | 12 ++++++------ modules/templates/util_render.go | 3 +-- routers/web/repo/issue_label.go | 12 +++++------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/models/issues/label.go b/models/issues/label.go index f6ecc68cd180..2397a29e357e 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -116,12 +116,17 @@ func (l *Label) CalOpenIssues() { func (l *Label) SetArchived(isArchived bool) { if !isArchived { l.ArchivedUnix = timeutil.TimeStamp(0) - } else if isArchived && l.ArchivedUnix.IsZero() { + } else if isArchived && !l.IsArchived() { // Only change the date when it is newly archived. l.ArchivedUnix = timeutil.TimeStampNow() } } +// IsArchived returns true if label is an archived +func (l *Label) IsArchived() bool { + return !l.ArchivedUnix.IsZero() +} + // CalOpenOrgIssues calculates the open issues of a label for a specific repo func (l *Label) CalOpenOrgIssues(ctx context.Context, repoID, labelID int64) { counts, _ := CountIssuesByRepo(ctx, &IssuesOptions{ @@ -166,11 +171,6 @@ func (l *Label) BelongsToOrg() bool { return l.OrgID > 0 } -// IsArchived returns true if label is an archived -func (l *Label) IsArchived() bool { - return l.ArchivedUnix > 0 -} - // BelongsToRepo returns true if label is a repository label func (l *Label) BelongsToRepo() bool { return l.RepoID > 0 diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index ba9b050e02aa..7ed3a8b9b49d 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -124,7 +124,6 @@ func RenderLabel(ctx context.Context, locale translation.Locale, label *issues_m var ( archivedCSSClass string textColor = "#111" - isArchived = !label.ArchivedUnix.IsZero() labelScope = label.ExclusiveScope() ) @@ -136,7 +135,7 @@ func RenderLabel(ctx context.Context, locale translation.Locale, label *issues_m description := emoji.ReplaceAliases(template.HTMLEscapeString(label.Description)) - if isArchived { + if label.IsArchived() { archivedCSSClass = "archived-label" description = fmt.Sprintf("(%s) %s", locale.TrString("archived"), description) } diff --git a/routers/web/repo/issue_label.go b/routers/web/repo/issue_label.go index 9dedaefa4beb..81bee4dbb531 100644 --- a/routers/web/repo/issue_label.go +++ b/routers/web/repo/issue_label.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/modules/label" "code.gitea.io/gitea/modules/log" repo_module "code.gitea.io/gitea/modules/repository" - "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/forms" @@ -112,12 +111,11 @@ func NewLabel(ctx *context.Context) { } l := &issues_model.Label{ - RepoID: ctx.Repo.Repository.ID, - Name: form.Title, - Exclusive: form.Exclusive, - Description: form.Description, - Color: form.Color, - ArchivedUnix: timeutil.TimeStamp(0), + RepoID: ctx.Repo.Repository.ID, + Name: form.Title, + Exclusive: form.Exclusive, + Description: form.Description, + Color: form.Color, } if err := issues_model.NewLabel(ctx, l); err != nil { ctx.ServerError("NewLabel", err) From 67e9f0d49828f62a942ac6db04153207f88e82ee Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 13 Mar 2024 14:57:30 +0800 Subject: [PATCH 02/14] Fix user router possbile panic (#29751) regression from #28023 --- routers/web/user/home.go | 7 +++++-- tests/integration/user_test.go | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index caa71152593e..e731a2a9b767 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -714,12 +714,16 @@ func UsernameSubRoute(ctx *context.Context) { reloadParam := func(suffix string) (success bool) { ctx.SetParams("username", strings.TrimSuffix(username, suffix)) context.UserAssignmentWeb()(ctx) + if ctx.Written() { + return false + } + // check view permissions if !user_model.IsUserVisibleToViewer(ctx, ctx.ContextUser, ctx.Doer) { ctx.NotFound("user", fmt.Errorf(ctx.ContextUser.Name)) return false } - return !ctx.Written() + return true } switch { case strings.HasSuffix(username, ".png"): @@ -740,7 +744,6 @@ func UsernameSubRoute(ctx *context.Context) { return } if reloadParam(".rss") { - context.UserAssignmentWeb()(ctx) feed.ShowUserFeedRSS(ctx) } case strings.HasSuffix(username, ".atom"): diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index c30733b1b07c..c4544f37aa3a 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -243,6 +243,8 @@ func testExportUserGPGKeys(t *testing.T, user, expected string) { } func TestGetUserRss(t *testing.T) { + defer tests.PrepareTestEnv(t)() + user34 := "the_34-user.with.all.allowedChars" req := NewRequestf(t, "GET", "/%s.rss", user34) resp := MakeRequest(t, req, http.StatusOK) @@ -253,6 +255,13 @@ func TestGetUserRss(t *testing.T) { description, _ := rssDoc.ChildrenFiltered("description").Html() assert.EqualValues(t, "<p dir="auto">some <a href="https://commonmark.org/" rel="nofollow">commonmark</a>!</p>\n", description) } + + req = NewRequestf(t, "GET", "/non-existent-user.rss") + MakeRequest(t, req, http.StatusNotFound) + + session := loginUser(t, "user2") + req = NewRequestf(t, "GET", "/non-existent-user.rss") + session.MakeRequest(t, req, http.StatusNotFound) } func TestListStopWatches(t *testing.T) { From 7fd0a5b276aadcf88dcc012fcd364fe160a58810 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 13 Mar 2024 09:25:53 +0100 Subject: [PATCH 03/14] Refactor to use optional.Option for issue index search option (#29739) Signed-off-by: 6543 <6543@obermui.de> --- modules/indexer/internal/bleve/query.go | 12 ++-- modules/indexer/issues/bleve/bleve.go | 39 ++++++----- modules/indexer/issues/db/options.go | 32 ++++----- modules/indexer/issues/dboptions.go | 12 ++-- .../issues/elasticsearch/elasticsearch.go | 42 ++++++------ modules/indexer/issues/indexer_test.go | 65 ++++++++----------- modules/indexer/issues/internal/model.go | 20 +++--- .../indexer/issues/internal/tests/tests.go | 65 ++++--------------- .../indexer/issues/meilisearch/meilisearch.go | 40 ++++++------ routers/api/v1/repo/issue.go | 24 +++---- routers/web/repo/issue.go | 32 ++++----- routers/web/user/home.go | 20 +++--- 12 files changed, 178 insertions(+), 225 deletions(-) diff --git a/modules/indexer/internal/bleve/query.go b/modules/indexer/internal/bleve/query.go index 2a427c402026..b96875343e5e 100644 --- a/modules/indexer/internal/bleve/query.go +++ b/modules/indexer/internal/bleve/query.go @@ -4,6 +4,8 @@ package bleve import ( + "code.gitea.io/gitea/modules/optional" + "github.com/blevesearch/bleve/v2" "github.com/blevesearch/bleve/v2/search/query" ) @@ -39,18 +41,18 @@ func BoolFieldQuery(value bool, field string) *query.BoolFieldQuery { return q } -func NumericRangeInclusiveQuery(min, max *int64, field string) *query.NumericRangeQuery { +func NumericRangeInclusiveQuery(min, max optional.Option[int64], field string) *query.NumericRangeQuery { var minF, maxF *float64 var minI, maxI *bool - if min != nil { + if min.Has() { minF = new(float64) - *minF = float64(*min) + *minF = float64(min.Value()) minI = new(bool) *minI = true } - if max != nil { + if max.Has() { maxF = new(float64) - *maxF = float64(*max) + *maxF = float64(max.Value()) maxI = new(bool) *maxI = true } diff --git a/modules/indexer/issues/bleve/bleve.go b/modules/indexer/issues/bleve/bleve.go index aaea854efa03..927ad58cd4c5 100644 --- a/modules/indexer/issues/bleve/bleve.go +++ b/modules/indexer/issues/bleve/bleve.go @@ -224,38 +224,41 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( queries = append(queries, bleve.NewDisjunctionQuery(milestoneQueries...)) } - if options.ProjectID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ProjectID, "project_id")) + if options.ProjectID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectID.Value(), "project_id")) } - if options.ProjectBoardID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ProjectBoardID, "project_board_id")) + if options.ProjectBoardID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.ProjectBoardID.Value(), "project_board_id")) } - if options.PosterID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.PosterID, "poster_id")) + if options.PosterID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.PosterID.Value(), "poster_id")) } - if options.AssigneeID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.AssigneeID, "assignee_id")) + if options.AssigneeID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.AssigneeID.Value(), "assignee_id")) } - if options.MentionID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.MentionID, "mention_ids")) + if options.MentionID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.MentionID.Value(), "mention_ids")) } - if options.ReviewedID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ReviewedID, "reviewed_ids")) + if options.ReviewedID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.ReviewedID.Value(), "reviewed_ids")) } - if options.ReviewRequestedID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.ReviewRequestedID, "review_requested_ids")) + if options.ReviewRequestedID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.ReviewRequestedID.Value(), "review_requested_ids")) } - if options.SubscriberID != nil { - queries = append(queries, inner_bleve.NumericEqualityQuery(*options.SubscriberID, "subscriber_ids")) + if options.SubscriberID.Has() { + queries = append(queries, inner_bleve.NumericEqualityQuery(options.SubscriberID.Value(), "subscriber_ids")) } - if options.UpdatedAfterUnix != nil || options.UpdatedBeforeUnix != nil { - queries = append(queries, inner_bleve.NumericRangeInclusiveQuery(options.UpdatedAfterUnix, options.UpdatedBeforeUnix, "updated_unix")) + if options.UpdatedAfterUnix.Has() || options.UpdatedBeforeUnix.Has() { + queries = append(queries, inner_bleve.NumericRangeInclusiveQuery( + options.UpdatedAfterUnix, + options.UpdatedBeforeUnix, + "updated_unix")) } var indexerQuery query.Query = bleve.NewConjunctionQuery(queries...) diff --git a/modules/indexer/issues/db/options.go b/modules/indexer/issues/db/options.go index 69146573a868..eeaf1696adaf 100644 --- a/modules/indexer/issues/db/options.go +++ b/modules/indexer/issues/db/options.go @@ -15,22 +15,6 @@ import ( ) func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_model.IssuesOptions, error) { - // See the comment of issues_model.SearchOptions for the reason why we need to convert - convertID := func(id *int64) int64 { - if id == nil { - return 0 - } - if *id == 0 { - return db.NoConditionID - } - return *id - } - convertInt64 := func(i *int64) int64 { - if i == nil { - return 0 - } - return *i - } var sortType string switch options.SortBy { case internal.SortByCreatedAsc: @@ -53,6 +37,18 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m sortType = "newest" } + // See the comment of issues_model.SearchOptions for the reason why we need to convert + convertID := func(id optional.Option[int64]) int64 { + if !id.Has() { + return 0 + } + value := id.Value() + if value == 0 { + return db.NoConditionID + } + return value + } + opts := &issue_model.IssuesOptions{ Paginator: options.Paginator, RepoIDs: options.RepoIDs, @@ -73,8 +69,8 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m IncludeMilestones: nil, SortType: sortType, IssueIDs: nil, - UpdatedAfterUnix: convertInt64(options.UpdatedAfterUnix), - UpdatedBeforeUnix: convertInt64(options.UpdatedBeforeUnix), + UpdatedAfterUnix: options.UpdatedAfterUnix.Value(), + UpdatedBeforeUnix: options.UpdatedBeforeUnix.Value(), PriorityRepoID: 0, IsArchived: optional.None[bool](), Org: nil, diff --git a/modules/indexer/issues/dboptions.go b/modules/indexer/issues/dboptions.go index 80e233e29a35..4a98b4588a3d 100644 --- a/modules/indexer/issues/dboptions.go +++ b/modules/indexer/issues/dboptions.go @@ -6,6 +6,7 @@ package issues import ( "code.gitea.io/gitea/models/db" issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/modules/optional" ) func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOptions { @@ -38,13 +39,12 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp } // See the comment of issues_model.SearchOptions for the reason why we need to convert - convertID := func(id int64) *int64 { + convertID := func(id int64) optional.Option[int64] { if id > 0 { - return &id + return optional.Some(id) } if id == db.NoConditionID { - var zero int64 - return &zero + return optional.None[int64]() } return nil } @@ -59,10 +59,10 @@ func ToSearchOptions(keyword string, opts *issues_model.IssuesOptions) *SearchOp searchOpt.SubscriberID = convertID(opts.SubscriberID) if opts.UpdatedAfterUnix > 0 { - searchOpt.UpdatedAfterUnix = &opts.UpdatedAfterUnix + searchOpt.UpdatedAfterUnix = optional.Some(opts.UpdatedAfterUnix) } if opts.UpdatedBeforeUnix > 0 { - searchOpt.UpdatedBeforeUnix = &opts.UpdatedBeforeUnix + searchOpt.UpdatedBeforeUnix = optional.Some(opts.UpdatedBeforeUnix) } searchOpt.Paginator = opts.Paginator diff --git a/modules/indexer/issues/elasticsearch/elasticsearch.go b/modules/indexer/issues/elasticsearch/elasticsearch.go index 0077da263a7c..53b383c8d5d7 100644 --- a/modules/indexer/issues/elasticsearch/elasticsearch.go +++ b/modules/indexer/issues/elasticsearch/elasticsearch.go @@ -195,43 +195,43 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( query.Must(elastic.NewTermsQuery("milestone_id", toAnySlice(options.MilestoneIDs)...)) } - if options.ProjectID != nil { - query.Must(elastic.NewTermQuery("project_id", *options.ProjectID)) + if options.ProjectID.Has() { + query.Must(elastic.NewTermQuery("project_id", options.ProjectID.Value())) } - if options.ProjectBoardID != nil { - query.Must(elastic.NewTermQuery("project_board_id", *options.ProjectBoardID)) + if options.ProjectBoardID.Has() { + query.Must(elastic.NewTermQuery("project_board_id", options.ProjectBoardID.Value())) } - if options.PosterID != nil { - query.Must(elastic.NewTermQuery("poster_id", *options.PosterID)) + if options.PosterID.Has() { + query.Must(elastic.NewTermQuery("poster_id", options.PosterID.Value())) } - if options.AssigneeID != nil { - query.Must(elastic.NewTermQuery("assignee_id", *options.AssigneeID)) + if options.AssigneeID.Has() { + query.Must(elastic.NewTermQuery("assignee_id", options.AssigneeID.Value())) } - if options.MentionID != nil { - query.Must(elastic.NewTermQuery("mention_ids", *options.MentionID)) + if options.MentionID.Has() { + query.Must(elastic.NewTermQuery("mention_ids", options.MentionID.Value())) } - if options.ReviewedID != nil { - query.Must(elastic.NewTermQuery("reviewed_ids", *options.ReviewedID)) + if options.ReviewedID.Has() { + query.Must(elastic.NewTermQuery("reviewed_ids", options.ReviewedID.Value())) } - if options.ReviewRequestedID != nil { - query.Must(elastic.NewTermQuery("review_requested_ids", *options.ReviewRequestedID)) + if options.ReviewRequestedID.Has() { + query.Must(elastic.NewTermQuery("review_requested_ids", options.ReviewRequestedID.Value())) } - if options.SubscriberID != nil { - query.Must(elastic.NewTermQuery("subscriber_ids", *options.SubscriberID)) + if options.SubscriberID.Has() { + query.Must(elastic.NewTermQuery("subscriber_ids", options.SubscriberID.Value())) } - if options.UpdatedAfterUnix != nil || options.UpdatedBeforeUnix != nil { + if options.UpdatedAfterUnix.Has() || options.UpdatedBeforeUnix.Has() { q := elastic.NewRangeQuery("updated_unix") - if options.UpdatedAfterUnix != nil { - q.Gte(*options.UpdatedAfterUnix) + if options.UpdatedAfterUnix.Has() { + q.Gte(options.UpdatedAfterUnix.Value()) } - if options.UpdatedBeforeUnix != nil { - q.Lte(*options.UpdatedBeforeUnix) + if options.UpdatedBeforeUnix.Has() { + q.Lte(options.UpdatedBeforeUnix.Value()) } query.Must(q) } diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index 10ffa7cbe6b6..0d0cfc851697 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -134,63 +134,60 @@ func searchIssueInRepo(t *testing.T) { } func searchIssueByID(t *testing.T) { - int64Pointer := func(x int64) *int64 { - return &x - } tests := []struct { opts SearchOptions expectedIDs []int64 }{ { - SearchOptions{ - PosterID: int64Pointer(1), + opts: SearchOptions{ + PosterID: optional.Some(int64(1)), }, - []int64{11, 6, 3, 2, 1}, + expectedIDs: []int64{11, 6, 3, 2, 1}, }, { - SearchOptions{ - AssigneeID: int64Pointer(1), + opts: SearchOptions{ + AssigneeID: optional.Some(int64(1)), }, - []int64{6, 1}, + expectedIDs: []int64{6, 1}, }, { - SearchOptions{ - MentionID: int64Pointer(4), + opts: SearchOptions{ + MentionID: optional.Some(int64(4)), }, - []int64{1}, + expectedIDs: []int64{1}, }, { - SearchOptions{ - ReviewedID: int64Pointer(1), + opts: SearchOptions{ + ReviewedID: optional.Some(int64(1)), }, - []int64{}, + expectedIDs: []int64{}, }, { - SearchOptions{ - ReviewRequestedID: int64Pointer(1), + opts: SearchOptions{ + ReviewRequestedID: optional.Some(int64(1)), }, - []int64{12}, + expectedIDs: []int64{12}, }, { - SearchOptions{ - SubscriberID: int64Pointer(1), + opts: SearchOptions{ + SubscriberID: optional.Some(int64(1)), }, - []int64{11, 6, 5, 3, 2, 1}, + expectedIDs: []int64{11, 6, 5, 3, 2, 1}, }, { // issue 20 request user 15 and team 5 which user 15 belongs to // the review request number of issue 20 should be 1 - SearchOptions{ - ReviewRequestedID: int64Pointer(15), + opts: SearchOptions{ + ReviewRequestedID: optional.Some(int64(15)), }, - []int64{12, 20}, + expectedIDs: []int64{12, 20}, }, { // user 20 approved the issue 20, so return nothing - SearchOptions{ - ReviewRequestedID: int64Pointer(20), + opts: SearchOptions{ + ReviewRequestedID: optional.Some(int64(20)), }, - []int64{}, + expectedIDs: []int64{}, }, } @@ -318,16 +315,13 @@ func searchIssueByLabelID(t *testing.T) { } func searchIssueByTime(t *testing.T) { - int64Pointer := func(i int64) *int64 { - return &i - } tests := []struct { opts SearchOptions expectedIDs []int64 }{ { SearchOptions{ - UpdatedAfterUnix: int64Pointer(0), + UpdatedAfterUnix: optional.Some(int64(0)), }, []int64{22, 21, 17, 16, 15, 14, 13, 12, 11, 20, 6, 5, 19, 18, 10, 7, 4, 9, 8, 3, 2, 1}, }, @@ -363,28 +357,25 @@ func searchIssueWithOrder(t *testing.T) { } func searchIssueInProject(t *testing.T) { - int64Pointer := func(i int64) *int64 { - return &i - } tests := []struct { opts SearchOptions expectedIDs []int64 }{ { SearchOptions{ - ProjectID: int64Pointer(1), + ProjectID: optional.Some(int64(1)), }, []int64{5, 3, 2, 1}, }, { SearchOptions{ - ProjectBoardID: int64Pointer(1), + ProjectBoardID: optional.Some(int64(1)), }, []int64{1}, }, { SearchOptions{ - ProjectBoardID: int64Pointer(0), // issue with in default board + ProjectBoardID: optional.Some(int64(0)), // issue with in default board }, []int64{2}, }, diff --git a/modules/indexer/issues/internal/model.go b/modules/indexer/issues/internal/model.go index d41fec4aba87..b7102c35af5a 100644 --- a/modules/indexer/issues/internal/model.go +++ b/modules/indexer/issues/internal/model.go @@ -89,22 +89,22 @@ type SearchOptions struct { MilestoneIDs []int64 // milestones the issues have - ProjectID *int64 // project the issues belong to - ProjectBoardID *int64 // project board the issues belong to + ProjectID optional.Option[int64] // project the issues belong to + ProjectBoardID optional.Option[int64] // project board the issues belong to - PosterID *int64 // poster of the issues + PosterID optional.Option[int64] // poster of the issues - AssigneeID *int64 // assignee of the issues, zero means no assignee + AssigneeID optional.Option[int64] // assignee of the issues, zero means no assignee - MentionID *int64 // mentioned user of the issues + MentionID optional.Option[int64] // mentioned user of the issues - ReviewedID *int64 // reviewer of the issues - ReviewRequestedID *int64 // requested reviewer of the issues + ReviewedID optional.Option[int64] // reviewer of the issues + ReviewRequestedID optional.Option[int64] // requested reviewer of the issues - SubscriberID *int64 // subscriber of the issues + SubscriberID optional.Option[int64] // subscriber of the issues - UpdatedAfterUnix *int64 - UpdatedBeforeUnix *int64 + UpdatedAfterUnix optional.Option[int64] + UpdatedBeforeUnix optional.Option[int64] db.Paginator diff --git a/modules/indexer/issues/internal/tests/tests.go b/modules/indexer/issues/internal/tests/tests.go index 67244715394d..91aafd589c76 100644 --- a/modules/indexer/issues/internal/tests/tests.go +++ b/modules/indexer/issues/internal/tests/tests.go @@ -300,10 +300,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectID: func() *int64 { - id := int64(1) - return &id - }(), + ProjectID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -321,10 +318,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectID: func() *int64 { - id := int64(0) - return &id - }(), + ProjectID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -342,10 +336,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectBoardID: func() *int64 { - id := int64(1) - return &id - }(), + ProjectBoardID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -363,10 +354,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ProjectBoardID: func() *int64 { - id := int64(0) - return &id - }(), + ProjectBoardID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -384,10 +372,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - PosterID: func() *int64 { - id := int64(1) - return &id - }(), + PosterID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -405,10 +390,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - AssigneeID: func() *int64 { - id := int64(1) - return &id - }(), + AssigneeID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -426,10 +408,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - AssigneeID: func() *int64 { - id := int64(0) - return &id - }(), + AssigneeID: optional.Some(int64(0)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -447,10 +426,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - MentionID: func() *int64 { - id := int64(1) - return &id - }(), + MentionID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -468,10 +444,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ReviewedID: func() *int64 { - id := int64(1) - return &id - }(), + ReviewedID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -489,10 +462,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - ReviewRequestedID: func() *int64 { - id := int64(1) - return &id - }(), + ReviewRequestedID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -510,10 +480,7 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - SubscriberID: func() *int64 { - id := int64(1) - return &id - }(), + SubscriberID: optional.Some(int64(1)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) @@ -531,14 +498,8 @@ var cases = []*testIndexerCase{ Paginator: &db.ListOptions{ PageSize: 5, }, - UpdatedAfterUnix: func() *int64 { - var t int64 = 20 - return &t - }(), - UpdatedBeforeUnix: func() *int64 { - var t int64 = 30 - return &t - }(), + UpdatedAfterUnix: optional.Some(int64(20)), + UpdatedBeforeUnix: optional.Some(int64(30)), }, Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) { assert.Equal(t, 5, len(result.Hits)) diff --git a/modules/indexer/issues/meilisearch/meilisearch.go b/modules/indexer/issues/meilisearch/meilisearch.go index c42992006530..34066bf559ae 100644 --- a/modules/indexer/issues/meilisearch/meilisearch.go +++ b/modules/indexer/issues/meilisearch/meilisearch.go @@ -170,41 +170,41 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) ( query.And(inner_meilisearch.NewFilterIn("milestone_id", options.MilestoneIDs...)) } - if options.ProjectID != nil { - query.And(inner_meilisearch.NewFilterEq("project_id", *options.ProjectID)) + if options.ProjectID.Has() { + query.And(inner_meilisearch.NewFilterEq("project_id", options.ProjectID.Value())) } - if options.ProjectBoardID != nil { - query.And(inner_meilisearch.NewFilterEq("project_board_id", *options.ProjectBoardID)) + if options.ProjectBoardID.Has() { + query.And(inner_meilisearch.NewFilterEq("project_board_id", options.ProjectBoardID.Value())) } - if options.PosterID != nil { - query.And(inner_meilisearch.NewFilterEq("poster_id", *options.PosterID)) + if options.PosterID.Has() { + query.And(inner_meilisearch.NewFilterEq("poster_id", options.PosterID.Value())) } - if options.AssigneeID != nil { - query.And(inner_meilisearch.NewFilterEq("assignee_id", *options.AssigneeID)) + if options.AssigneeID.Has() { + query.And(inner_meilisearch.NewFilterEq("assignee_id", options.AssigneeID.Value())) } - if options.MentionID != nil { - query.And(inner_meilisearch.NewFilterEq("mention_ids", *options.MentionID)) + if options.MentionID.Has() { + query.And(inner_meilisearch.NewFilterEq("mention_ids", options.MentionID.Value())) } - if options.ReviewedID != nil { - query.And(inner_meilisearch.NewFilterEq("reviewed_ids", *options.ReviewedID)) + if options.ReviewedID.Has() { + query.And(inner_meilisearch.NewFilterEq("reviewed_ids", options.ReviewedID.Value())) } - if options.ReviewRequestedID != nil { - query.And(inner_meilisearch.NewFilterEq("review_requested_ids", *options.ReviewRequestedID)) + if options.ReviewRequestedID.Has() { + query.And(inner_meilisearch.NewFilterEq("review_requested_ids", options.ReviewRequestedID.Value())) } - if options.SubscriberID != nil { - query.And(inner_meilisearch.NewFilterEq("subscriber_ids", *options.SubscriberID)) + if options.SubscriberID.Has() { + query.And(inner_meilisearch.NewFilterEq("subscriber_ids", options.SubscriberID.Value())) } - if options.UpdatedAfterUnix != nil { - query.And(inner_meilisearch.NewFilterGte("updated_unix", *options.UpdatedAfterUnix)) + if options.UpdatedAfterUnix.Has() { + query.And(inner_meilisearch.NewFilterGte("updated_unix", options.UpdatedAfterUnix.Value())) } - if options.UpdatedBeforeUnix != nil { - query.And(inner_meilisearch.NewFilterLte("updated_unix", *options.UpdatedBeforeUnix)) + if options.UpdatedBeforeUnix.Has() { + query.And(inner_meilisearch.NewFilterLte("updated_unix", options.UpdatedBeforeUnix.Value())) } if options.SortBy == "" { diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 09175fe0ac23..61a318baabab 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -269,28 +269,28 @@ func SearchIssues(ctx *context.APIContext) { } if since != 0 { - searchOpt.UpdatedAfterUnix = &since + searchOpt.UpdatedAfterUnix = optional.Some(since) } if before != 0 { - searchOpt.UpdatedBeforeUnix = &before + searchOpt.UpdatedBeforeUnix = optional.Some(before) } if ctx.IsSigned { ctxUserID := ctx.Doer.ID if ctx.FormBool("created") { - searchOpt.PosterID = &ctxUserID + searchOpt.PosterID = optional.Some(ctxUserID) } if ctx.FormBool("assigned") { - searchOpt.AssigneeID = &ctxUserID + searchOpt.AssigneeID = optional.Some(ctxUserID) } if ctx.FormBool("mentioned") { - searchOpt.MentionID = &ctxUserID + searchOpt.MentionID = optional.Some(ctxUserID) } if ctx.FormBool("review_requested") { - searchOpt.ReviewRequestedID = &ctxUserID + searchOpt.ReviewRequestedID = optional.Some(ctxUserID) } if ctx.FormBool("reviewed") { - searchOpt.ReviewedID = &ctxUserID + searchOpt.ReviewedID = optional.Some(ctxUserID) } } @@ -502,10 +502,10 @@ func ListIssues(ctx *context.APIContext) { SortBy: issue_indexer.SortByCreatedDesc, } if since != 0 { - searchOpt.UpdatedAfterUnix = &since + searchOpt.UpdatedAfterUnix = optional.Some(since) } if before != 0 { - searchOpt.UpdatedBeforeUnix = &before + searchOpt.UpdatedBeforeUnix = optional.Some(before) } if len(labelIDs) == 1 && labelIDs[0] == 0 { searchOpt.NoLabelOnly = true @@ -526,13 +526,13 @@ func ListIssues(ctx *context.APIContext) { } if createdByID > 0 { - searchOpt.PosterID = &createdByID + searchOpt.PosterID = optional.Some(createdByID) } if assignedByID > 0 { - searchOpt.AssigneeID = &assignedByID + searchOpt.AssigneeID = optional.Some(assignedByID) } if mentionedByID > 0 { - searchOpt.MentionID = &mentionedByID + searchOpt.MentionID = optional.Some(mentionedByID) } ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 8935cc80e2f7..d5fd8439f306 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -2634,9 +2634,9 @@ func SearchIssues(ctx *context.Context) { } } - var projectID *int64 + projectID := optional.None[int64]() if v := ctx.FormInt64("project"); v > 0 { - projectID = &v + projectID = optional.Some(v) } // this api is also used in UI, @@ -2665,28 +2665,28 @@ func SearchIssues(ctx *context.Context) { } if since != 0 { - searchOpt.UpdatedAfterUnix = &since + searchOpt.UpdatedAfterUnix = optional.Some(since) } if before != 0 { - searchOpt.UpdatedBeforeUnix = &before + searchOpt.UpdatedBeforeUnix = optional.Some(before) } if ctx.IsSigned { ctxUserID := ctx.Doer.ID if ctx.FormBool("created") { - searchOpt.PosterID = &ctxUserID + searchOpt.PosterID = optional.Some(ctxUserID) } if ctx.FormBool("assigned") { - searchOpt.AssigneeID = &ctxUserID + searchOpt.AssigneeID = optional.Some(ctxUserID) } if ctx.FormBool("mentioned") { - searchOpt.MentionID = &ctxUserID + searchOpt.MentionID = optional.Some(ctxUserID) } if ctx.FormBool("review_requested") { - searchOpt.ReviewRequestedID = &ctxUserID + searchOpt.ReviewRequestedID = optional.Some(ctxUserID) } if ctx.FormBool("reviewed") { - searchOpt.ReviewedID = &ctxUserID + searchOpt.ReviewedID = optional.Some(ctxUserID) } } @@ -2791,9 +2791,9 @@ func ListIssues(ctx *context.Context) { } } - var projectID *int64 + projectID := optional.None[int64]() if v := ctx.FormInt64("project"); v > 0 { - projectID = &v + projectID = optional.Some(v) } isPull := optional.None[bool]() @@ -2831,10 +2831,10 @@ func ListIssues(ctx *context.Context) { SortBy: issue_indexer.SortByCreatedDesc, } if since != 0 { - searchOpt.UpdatedAfterUnix = &since + searchOpt.UpdatedAfterUnix = optional.Some(since) } if before != 0 { - searchOpt.UpdatedBeforeUnix = &before + searchOpt.UpdatedBeforeUnix = optional.Some(before) } if len(labelIDs) == 1 && labelIDs[0] == 0 { searchOpt.NoLabelOnly = true @@ -2855,13 +2855,13 @@ func ListIssues(ctx *context.Context) { } if createdByID > 0 { - searchOpt.PosterID = &createdByID + searchOpt.PosterID = optional.Some(createdByID) } if assignedByID > 0 { - searchOpt.AssigneeID = &assignedByID + searchOpt.AssigneeID = optional.Some(assignedByID) } if mentionedByID > 0 { - searchOpt.MentionID = &mentionedByID + searchOpt.MentionID = optional.Some(mentionedByID) } ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index e731a2a9b767..4ec6f6be3f36 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -792,15 +792,15 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod case issues_model.FilterModeYourRepositories: openClosedOpts.AllPublic = false case issues_model.FilterModeAssign: - openClosedOpts.AssigneeID = &doerID + openClosedOpts.AssigneeID = optional.Some(doerID) case issues_model.FilterModeCreate: - openClosedOpts.PosterID = &doerID + openClosedOpts.PosterID = optional.Some(doerID) case issues_model.FilterModeMention: - openClosedOpts.MentionID = &doerID + openClosedOpts.MentionID = optional.Some(doerID) case issues_model.FilterModeReviewRequested: - openClosedOpts.ReviewRequestedID = &doerID + openClosedOpts.ReviewRequestedID = optional.Some(doerID) case issues_model.FilterModeReviewed: - openClosedOpts.ReviewedID = &doerID + openClosedOpts.ReviewedID = optional.Some(doerID) } openClosedOpts.IsClosed = optional.Some(false) ret.OpenCount, err = issue_indexer.CountIssues(ctx, openClosedOpts) @@ -818,23 +818,23 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod if err != nil { return nil, err } - ret.AssignCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AssigneeID = &doerID })) + ret.AssignCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.AssigneeID = optional.Some(doerID) })) if err != nil { return nil, err } - ret.CreateCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.PosterID = &doerID })) + ret.CreateCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.PosterID = optional.Some(doerID) })) if err != nil { return nil, err } - ret.MentionCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.MentionID = &doerID })) + ret.MentionCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.MentionID = optional.Some(doerID) })) if err != nil { return nil, err } - ret.ReviewRequestedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewRequestedID = &doerID })) + ret.ReviewRequestedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewRequestedID = optional.Some(doerID) })) if err != nil { return nil, err } - ret.ReviewedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewedID = &doerID })) + ret.ReviewedCount, err = issue_indexer.CountIssues(ctx, opts.Copy(func(o *issue_indexer.SearchOptions) { o.ReviewedID = optional.Some(doerID) })) if err != nil { return nil, err } From 9b1a8888fa754676073bc851b783b2b8f1adecfb Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 13 Mar 2024 09:43:58 +0100 Subject: [PATCH 04/14] Configure pinned JS dependencies via updates.config.js (#29696) Split out from https://github.com/go-gitea/gitea/pull/29684. This configures the [`updates`](https://github.com/silverwind/updates) module to exclude these modules for reasons stated in the comments. --- updates.config.js | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 updates.config.js diff --git a/updates.config.js b/updates.config.js new file mode 100644 index 000000000000..11908dea8e5b --- /dev/null +++ b/updates.config.js @@ -0,0 +1,6 @@ +export default { + exclude: [ + '@mcaptcha/vanilla-glue', // breaking changes in rc versions need to be handled + 'eslint-plugin-array-func', // need to migrate to eslint flat config first + ], +}; From 66edc888ee8b2f77a6f11139acd2d03c561ad5ef Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 13 Mar 2024 18:07:53 +0800 Subject: [PATCH 05/14] Move fork router functions to a standalone file (#29756) To reduce the pull.go file's size. --- routers/web/repo/fork.go | 238 +++++++++++++++++++++++++++++++++++++++ routers/web/repo/pull.go | 214 ----------------------------------- 2 files changed, 238 insertions(+), 214 deletions(-) create mode 100644 routers/web/repo/fork.go diff --git a/routers/web/repo/fork.go b/routers/web/repo/fork.go new file mode 100644 index 000000000000..60e37476eefd --- /dev/null +++ b/routers/web/repo/fork.go @@ -0,0 +1,238 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "errors" + "net/http" + "net/url" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + "code.gitea.io/gitea/models/organization" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/context" + "code.gitea.io/gitea/services/forms" + repo_service "code.gitea.io/gitea/services/repository" +) + +const ( + tplFork base.TplName = "repo/pulls/fork" +) + +func getForkRepository(ctx *context.Context) *repo_model.Repository { + forkRepo := ctx.Repo.Repository + if ctx.Written() { + return nil + } + + if forkRepo.IsEmpty { + log.Trace("Empty repository %-v", forkRepo) + ctx.NotFound("getForkRepository", nil) + return nil + } + + if err := forkRepo.LoadOwner(ctx); err != nil { + ctx.ServerError("LoadOwner", err) + return nil + } + + ctx.Data["repo_name"] = forkRepo.Name + ctx.Data["description"] = forkRepo.Description + ctx.Data["IsPrivate"] = forkRepo.IsPrivate || forkRepo.Owner.Visibility == structs.VisibleTypePrivate + canForkToUser := forkRepo.OwnerID != ctx.Doer.ID && !repo_model.HasForkedRepo(ctx, ctx.Doer.ID, forkRepo.ID) + + ctx.Data["ForkRepo"] = forkRepo + + ownedOrgs, err := organization.GetOrgsCanCreateRepoByUserID(ctx, ctx.Doer.ID) + if err != nil { + ctx.ServerError("GetOrgsCanCreateRepoByUserID", err) + return nil + } + var orgs []*organization.Organization + for _, org := range ownedOrgs { + if forkRepo.OwnerID != org.ID && !repo_model.HasForkedRepo(ctx, org.ID, forkRepo.ID) { + orgs = append(orgs, org) + } + } + + traverseParentRepo := forkRepo + for { + if ctx.Doer.ID == traverseParentRepo.OwnerID { + canForkToUser = false + } else { + for i, org := range orgs { + if org.ID == traverseParentRepo.OwnerID { + orgs = append(orgs[:i], orgs[i+1:]...) + break + } + } + } + + if !traverseParentRepo.IsFork { + break + } + traverseParentRepo, err = repo_model.GetRepositoryByID(ctx, traverseParentRepo.ForkID) + if err != nil { + ctx.ServerError("GetRepositoryByID", err) + return nil + } + } + + ctx.Data["CanForkToUser"] = canForkToUser + ctx.Data["Orgs"] = orgs + + if canForkToUser { + ctx.Data["ContextUser"] = ctx.Doer + } else if len(orgs) > 0 { + ctx.Data["ContextUser"] = orgs[0] + } else { + ctx.Data["CanForkRepo"] = false + ctx.Flash.Error(ctx.Tr("repo.fork_no_valid_owners"), true) + return nil + } + + branches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{ + RepoID: ctx.Repo.Repository.ID, + ListOptions: db.ListOptions{ + ListAll: true, + }, + IsDeletedBranch: optional.Some(false), + // Add it as the first option + ExcludeBranchNames: []string{ctx.Repo.Repository.DefaultBranch}, + }) + if err != nil { + ctx.ServerError("FindBranchNames", err) + return nil + } + ctx.Data["Branches"] = append([]string{ctx.Repo.Repository.DefaultBranch}, branches...) + + return forkRepo +} + +// Fork render repository fork page +func Fork(ctx *context.Context) { + ctx.Data["Title"] = ctx.Tr("new_fork") + + if ctx.Doer.CanForkRepo() { + ctx.Data["CanForkRepo"] = true + } else { + maxCreationLimit := ctx.Doer.MaxCreationLimit() + msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) + ctx.Flash.Error(msg, true) + } + + getForkRepository(ctx) + if ctx.Written() { + return + } + + ctx.HTML(http.StatusOK, tplFork) +} + +// ForkPost response for forking a repository +func ForkPost(ctx *context.Context) { + form := web.GetForm(ctx).(*forms.CreateRepoForm) + ctx.Data["Title"] = ctx.Tr("new_fork") + ctx.Data["CanForkRepo"] = true + + ctxUser := checkContextUser(ctx, form.UID) + if ctx.Written() { + return + } + + forkRepo := getForkRepository(ctx) + if ctx.Written() { + return + } + + ctx.Data["ContextUser"] = ctxUser + + if ctx.HasError() { + ctx.HTML(http.StatusOK, tplFork) + return + } + + var err error + traverseParentRepo := forkRepo + for { + if ctxUser.ID == traverseParentRepo.OwnerID { + ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) + return + } + repo := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID) + if repo != nil { + ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) + return + } + if !traverseParentRepo.IsFork { + break + } + traverseParentRepo, err = repo_model.GetRepositoryByID(ctx, traverseParentRepo.ForkID) + if err != nil { + ctx.ServerError("GetRepositoryByID", err) + return + } + } + + // Check if user is allowed to create repo's on the organization. + if ctxUser.IsOrganization() { + isAllowedToFork, err := organization.OrgFromUser(ctxUser).CanCreateOrgRepo(ctx, ctx.Doer.ID) + if err != nil { + ctx.ServerError("CanCreateOrgRepo", err) + return + } else if !isAllowedToFork { + ctx.Error(http.StatusForbidden) + return + } + } + + repo, err := repo_service.ForkRepository(ctx, ctx.Doer, ctxUser, repo_service.ForkRepoOptions{ + BaseRepo: forkRepo, + Name: form.RepoName, + Description: form.Description, + SingleBranch: form.ForkSingleBranch, + }) + if err != nil { + ctx.Data["Err_RepoName"] = true + switch { + case repo_model.IsErrReachLimitOfRepo(err): + maxCreationLimit := ctxUser.MaxCreationLimit() + msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) + ctx.RenderWithErr(msg, tplFork, &form) + case repo_model.IsErrRepoAlreadyExist(err): + ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) + case repo_model.IsErrRepoFilesAlreadyExist(err): + switch { + case ctx.IsUserSiteAdmin() || (setting.Repository.AllowAdoptionOfUnadoptedRepositories && setting.Repository.AllowDeleteOfUnadoptedRepositories): + ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt_or_delete"), tplFork, form) + case setting.Repository.AllowAdoptionOfUnadoptedRepositories: + ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt"), tplFork, form) + case setting.Repository.AllowDeleteOfUnadoptedRepositories: + ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.delete"), tplFork, form) + default: + ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist"), tplFork, form) + } + case db.IsErrNameReserved(err): + ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplFork, &form) + case db.IsErrNamePatternNotAllowed(err): + ctx.RenderWithErr(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern), tplFork, &form) + case errors.Is(err, user_model.ErrBlockedUser): + ctx.RenderWithErr(ctx.Tr("repo.fork.blocked_user"), tplFork, form) + default: + ctx.ServerError("ForkPost", err) + } + return + } + + log.Trace("Repository forked[%d]: %s/%s", forkRepo.ID, ctxUser.Name, repo.Name) + ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) +} diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index ed063715e50d..447781602d88 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -10,7 +10,6 @@ import ( "fmt" "html" "net/http" - "net/url" "strconv" "strings" "time" @@ -20,7 +19,6 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" issues_model "code.gitea.io/gitea/models/issues" - "code.gitea.io/gitea/models/organization" access_model "code.gitea.io/gitea/models/perm/access" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" @@ -32,9 +30,7 @@ import ( "code.gitea.io/gitea/modules/gitrepo" issue_template "code.gitea.io/gitea/modules/issue/template" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/utils" @@ -53,7 +49,6 @@ import ( ) const ( - tplFork base.TplName = "repo/pulls/fork" tplCompareDiff base.TplName = "repo/diff/compare" tplPullCommits base.TplName = "repo/pulls/commits" tplPullFiles base.TplName = "repo/pulls/files" @@ -112,215 +107,6 @@ func getRepository(ctx *context.Context, repoID int64) *repo_model.Repository { return repo } -func getForkRepository(ctx *context.Context) *repo_model.Repository { - forkRepo := ctx.Repo.Repository - if ctx.Written() { - return nil - } - - if forkRepo.IsEmpty { - log.Trace("Empty repository %-v", forkRepo) - ctx.NotFound("getForkRepository", nil) - return nil - } - - if err := forkRepo.LoadOwner(ctx); err != nil { - ctx.ServerError("LoadOwner", err) - return nil - } - - ctx.Data["repo_name"] = forkRepo.Name - ctx.Data["description"] = forkRepo.Description - ctx.Data["IsPrivate"] = forkRepo.IsPrivate || forkRepo.Owner.Visibility == structs.VisibleTypePrivate - canForkToUser := forkRepo.OwnerID != ctx.Doer.ID && !repo_model.HasForkedRepo(ctx, ctx.Doer.ID, forkRepo.ID) - - ctx.Data["ForkRepo"] = forkRepo - - ownedOrgs, err := organization.GetOrgsCanCreateRepoByUserID(ctx, ctx.Doer.ID) - if err != nil { - ctx.ServerError("GetOrgsCanCreateRepoByUserID", err) - return nil - } - var orgs []*organization.Organization - for _, org := range ownedOrgs { - if forkRepo.OwnerID != org.ID && !repo_model.HasForkedRepo(ctx, org.ID, forkRepo.ID) { - orgs = append(orgs, org) - } - } - - traverseParentRepo := forkRepo - for { - if ctx.Doer.ID == traverseParentRepo.OwnerID { - canForkToUser = false - } else { - for i, org := range orgs { - if org.ID == traverseParentRepo.OwnerID { - orgs = append(orgs[:i], orgs[i+1:]...) - break - } - } - } - - if !traverseParentRepo.IsFork { - break - } - traverseParentRepo, err = repo_model.GetRepositoryByID(ctx, traverseParentRepo.ForkID) - if err != nil { - ctx.ServerError("GetRepositoryByID", err) - return nil - } - } - - ctx.Data["CanForkToUser"] = canForkToUser - ctx.Data["Orgs"] = orgs - - if canForkToUser { - ctx.Data["ContextUser"] = ctx.Doer - } else if len(orgs) > 0 { - ctx.Data["ContextUser"] = orgs[0] - } else { - ctx.Data["CanForkRepo"] = false - ctx.Flash.Error(ctx.Tr("repo.fork_no_valid_owners"), true) - return nil - } - - branches, err := git_model.FindBranchNames(ctx, git_model.FindBranchOptions{ - RepoID: ctx.Repo.Repository.ID, - ListOptions: db.ListOptions{ - ListAll: true, - }, - IsDeletedBranch: optional.Some(false), - // Add it as the first option - ExcludeBranchNames: []string{ctx.Repo.Repository.DefaultBranch}, - }) - if err != nil { - ctx.ServerError("FindBranchNames", err) - return nil - } - ctx.Data["Branches"] = append([]string{ctx.Repo.Repository.DefaultBranch}, branches...) - - return forkRepo -} - -// Fork render repository fork page -func Fork(ctx *context.Context) { - ctx.Data["Title"] = ctx.Tr("new_fork") - - if ctx.Doer.CanForkRepo() { - ctx.Data["CanForkRepo"] = true - } else { - maxCreationLimit := ctx.Doer.MaxCreationLimit() - msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) - ctx.Flash.Error(msg, true) - } - - getForkRepository(ctx) - if ctx.Written() { - return - } - - ctx.HTML(http.StatusOK, tplFork) -} - -// ForkPost response for forking a repository -func ForkPost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.CreateRepoForm) - ctx.Data["Title"] = ctx.Tr("new_fork") - ctx.Data["CanForkRepo"] = true - - ctxUser := checkContextUser(ctx, form.UID) - if ctx.Written() { - return - } - - forkRepo := getForkRepository(ctx) - if ctx.Written() { - return - } - - ctx.Data["ContextUser"] = ctxUser - - if ctx.HasError() { - ctx.HTML(http.StatusOK, tplFork) - return - } - - var err error - traverseParentRepo := forkRepo - for { - if ctxUser.ID == traverseParentRepo.OwnerID { - ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) - return - } - repo := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID) - if repo != nil { - ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) - return - } - if !traverseParentRepo.IsFork { - break - } - traverseParentRepo, err = repo_model.GetRepositoryByID(ctx, traverseParentRepo.ForkID) - if err != nil { - ctx.ServerError("GetRepositoryByID", err) - return - } - } - - // Check if user is allowed to create repo's on the organization. - if ctxUser.IsOrganization() { - isAllowedToFork, err := organization.OrgFromUser(ctxUser).CanCreateOrgRepo(ctx, ctx.Doer.ID) - if err != nil { - ctx.ServerError("CanCreateOrgRepo", err) - return - } else if !isAllowedToFork { - ctx.Error(http.StatusForbidden) - return - } - } - - repo, err := repo_service.ForkRepository(ctx, ctx.Doer, ctxUser, repo_service.ForkRepoOptions{ - BaseRepo: forkRepo, - Name: form.RepoName, - Description: form.Description, - SingleBranch: form.ForkSingleBranch, - }) - if err != nil { - ctx.Data["Err_RepoName"] = true - switch { - case repo_model.IsErrReachLimitOfRepo(err): - maxCreationLimit := ctxUser.MaxCreationLimit() - msg := ctx.TrN(maxCreationLimit, "repo.form.reach_limit_of_creation_1", "repo.form.reach_limit_of_creation_n", maxCreationLimit) - ctx.RenderWithErr(msg, tplFork, &form) - case repo_model.IsErrRepoAlreadyExist(err): - ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form) - case repo_model.IsErrRepoFilesAlreadyExist(err): - switch { - case ctx.IsUserSiteAdmin() || (setting.Repository.AllowAdoptionOfUnadoptedRepositories && setting.Repository.AllowDeleteOfUnadoptedRepositories): - ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt_or_delete"), tplFork, form) - case setting.Repository.AllowAdoptionOfUnadoptedRepositories: - ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.adopt"), tplFork, form) - case setting.Repository.AllowDeleteOfUnadoptedRepositories: - ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist.delete"), tplFork, form) - default: - ctx.RenderWithErr(ctx.Tr("form.repository_files_already_exist"), tplFork, form) - } - case db.IsErrNameReserved(err): - ctx.RenderWithErr(ctx.Tr("repo.form.name_reserved", err.(db.ErrNameReserved).Name), tplFork, &form) - case db.IsErrNamePatternNotAllowed(err): - ctx.RenderWithErr(ctx.Tr("repo.form.name_pattern_not_allowed", err.(db.ErrNamePatternNotAllowed).Pattern), tplFork, &form) - case errors.Is(err, user_model.ErrBlockedUser): - ctx.RenderWithErr(ctx.Tr("repo.fork.blocked_user"), tplFork, form) - default: - ctx.ServerError("ForkPost", err) - } - return - } - - log.Trace("Repository forked[%d]: %s/%s", forkRepo.ID, ctxUser.Name, repo.Name) - ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name)) -} - func getPullInfo(ctx *context.Context) (issue *issues_model.Issue, ok bool) { issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) if err != nil { From 85c59d6c21e10ef9d3ccf11713548f50e47e920f Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Wed, 13 Mar 2024 11:34:58 +0100 Subject: [PATCH 06/14] Use relative links for commits, mentions, and issues in markdown (#29427) Fixes #29404 Use relative links for - commits - mentions - issues --------- Co-authored-by: silverwind --- modules/markup/html.go | 12 +++++------ modules/markup/html_internal_test.go | 1 + modules/markup/html_test.go | 9 +++++--- modules/markup/markdown/markdown_test.go | 6 +++--- modules/markup/renderer.go | 14 ++++++++++--- modules/templates/util_render_test.go | 14 ++++++------- routers/common/markup.go | 6 ++++-- services/mailer/mail.go | 3 ++- services/mailer/mail_test.go | 26 +++++++++++++++++++++++- 9 files changed, 65 insertions(+), 26 deletions(-) diff --git a/modules/markup/html.go b/modules/markup/html.go index 56e1a1c54eda..21bd6206e0eb 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -609,7 +609,7 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { if ok && strings.Contains(mention, "/") { mentionOrgAndTeam := strings.Split(mention, "/") if mentionOrgAndTeam[0][1:] == ctx.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) node = node.NextSibling.NextSibling start = 0 continue @@ -620,7 +620,7 @@ func mentionProcessor(ctx *RenderContext, node *html.Node) { mentionedUsername := mention[1:] if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, mentionedUsername), mention, "mention")) + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "mention")) node = node.NextSibling.NextSibling } else { node = node.NextSibling @@ -898,9 +898,9 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { path = "pulls" } if ref.Owner == "" { - link = createLink(util.URLJoin(setting.AppURL, ctx.Metas["user"], ctx.Metas["repo"], path, ref.Issue), reftext, "ref-issue") + link = createLink(util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], path, ref.Issue), reftext, "ref-issue") } else { - link = createLink(util.URLJoin(setting.AppURL, ref.Owner, ref.Name, path, ref.Issue), reftext, "ref-issue") + link = createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, path, ref.Issue), reftext, "ref-issue") } } @@ -939,7 +939,7 @@ func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { } reftext := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) - link := createLink(util.URLJoin(setting.AppSubURL, ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") + link := createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) node = node.NextSibling.NextSibling @@ -1166,7 +1166,7 @@ func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { continue } - link := util.URLJoin(setting.AppURL, ctx.Metas["user"], ctx.Metas["repo"], "commit", hash) + link := util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], "commit", hash) replaceContent(node, m[2], m[3], createCodeLink(link, base.ShortSha(hash), "commit")) start = 0 node = node.NextSibling.NextSibling diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 93ba9d7667c4..e313be7040c7 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -287,6 +287,7 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) { } func testRenderIssueIndexPattern(t *testing.T, input, expected string, ctx *RenderContext) { + ctx.Links.AbsolutePrefix = true if ctx.Links.Base == "" { ctx.Links.Base = TestRepoURL } diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index ccb63c6baba8..55de65d196dc 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -43,7 +43,8 @@ func TestRender_Commits(t *testing.T) { Ctx: git.DefaultContext, RelativePath: ".md", Links: markup.Links{ - Base: markup.TestRepoURL, + AbsolutePrefix: true, + Base: markup.TestRepoURL, }, Metas: localMetas, }, input) @@ -96,7 +97,8 @@ func TestRender_CrossReferences(t *testing.T) { Ctx: git.DefaultContext, RelativePath: "a.md", Links: markup.Links{ - Base: setting.AppSubURL, + AbsolutePrefix: true, + Base: setting.AppSubURL, }, Metas: localMetas, }, input) @@ -588,7 +590,8 @@ func TestPostProcess_RenderDocument(t *testing.T) { err := markup.PostProcess(&markup.RenderContext{ Ctx: git.DefaultContext, Links: markup.Links{ - Base: "https://example.com", + AbsolutePrefix: true, + Base: "https://example.com", }, Metas: localMetas, }, strings.NewReader(input), &res) diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index dbf95e5e625d..a12bd4f9e7c2 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -130,11 +130,11 @@ func testAnswers(baseURLContent, baseURLImages string) []string {
  • Links, Language bindings, Engine bindings
  • Tips
    • -

    See commit 65f1bf27bc

    +

    See commit 65f1bf27bc

    Ideas and codes

      -
    • Bezier widget (by @r-lyeh) ocornut/imgui#786
      • -
    • Bezier widget (by @r-lyeh) #786
      • +
    • Bezier widget (by @r-lyeh) ocornut/imgui#786
      • +
    • Bezier widget (by @r-lyeh) #786
    • Node graph editors https://github.com/ocornut/imgui/issues/306
    • Memory Editor
    • Plot var helper
      • diff --git a/modules/markup/renderer.go b/modules/markup/renderer.go index 5a7adcc55322..0f0bf557403e 100644 --- a/modules/markup/renderer.go +++ b/modules/markup/renderer.go @@ -82,9 +82,17 @@ type RenderContext struct { } type Links struct { - Base string - BranchPath string - TreePath string + AbsolutePrefix bool + Base string + BranchPath string + TreePath string +} + +func (l *Links) Prefix() string { + if l.AbsolutePrefix { + return setting.AppURL + } + return setting.AppSubURL } func (l *Links) HasBranchInfo() bool { diff --git a/modules/templates/util_render_test.go b/modules/templates/util_render_test.go index 8648967d3861..15aee8912d1e 100644 --- a/modules/templates/util_render_test.go +++ b/modules/templates/util_render_test.go @@ -117,21 +117,21 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a582 com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit 👍 mail@domain.com -@mention-user test -#123 +@mention-user test +#123 space` assert.EqualValues(t, expected, RenderCommitBody(context.Background(), testInput, testMetas)) } func TestRenderCommitMessage(t *testing.T) { - expected := `space @mention-user ` + expected := `space @mention-user ` assert.EqualValues(t, expected, RenderCommitMessage(context.Background(), testInput, testMetas)) } func TestRenderCommitMessageLinkSubject(t *testing.T) { - expected := `space @mention-user` + expected := `space @mention-user` assert.EqualValues(t, expected, RenderCommitMessageLinkSubject(context.Background(), testInput, "https://example.com/link", testMetas)) } @@ -155,14 +155,14 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit 👍 mail@domain.com @mention-user test -#123 +#123 space ` assert.EqualValues(t, expected, RenderIssueTitle(context.Background(), testInput, testMetas)) } func TestRenderMarkdownToHtml(t *testing.T) { - expected := `

      space @mention-user
      + expected := `

      space @mention-user
      /just/a/path.bin https://example.com/file.bin local link @@ -179,7 +179,7 @@ com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb...12fc37a3c0a4dda553bdcfc80c178a582 com 88fc37a3c0a4dda553bdcfc80c178a58247f42fb mit 👍 mail@domain.com -@mention-user test +@mention-user test #123 space

      ` diff --git a/routers/common/markup.go b/routers/common/markup.go index 7819ee72276f..2d5638ef6123 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -34,7 +34,8 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPr if err := markdown.RenderRaw(&markup.RenderContext{ Ctx: ctx, Links: markup.Links{ - Base: urlPrefix, + AbsolutePrefix: true, + Base: urlPrefix, }, }, strings.NewReader(text), ctx.Resp); err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) @@ -79,7 +80,8 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPr if err := markup.Render(&markup.RenderContext{ Ctx: ctx, Links: markup.Links{ - Base: urlPrefix, + AbsolutePrefix: true, + Base: urlPrefix, }, Metas: meta, IsWiki: wiki, diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 38973ea9352f..a63ba7a52af0 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -222,7 +222,8 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient body, err := markdown.RenderString(&markup.RenderContext{ Ctx: ctx, Links: markup.Links{ - Base: ctx.Issue.Repo.HTMLURL(), + AbsolutePrefix: true, + Base: ctx.Issue.Repo.HTMLURL(), }, Metas: ctx.Issue.Repo.ComposeMetas(ctx), }, ctx.Content) diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index e300aeccb0c1..d87c57ffe750 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -8,6 +8,8 @@ import ( "context" "fmt" "html/template" + "io" + "mime/quotedprintable" "regexp" "strings" "testing" @@ -19,6 +21,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/markup" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" @@ -67,6 +70,12 @@ func prepareMailerTest(t *testing.T) (doer *user_model.User, repo *repo_model.Re func TestComposeIssueCommentMessage(t *testing.T) { doer, _, issue, comment := prepareMailerTest(t) + markup.Init(&markup.ProcessorHelper{ + IsUsernameMentionable: func(ctx context.Context, username string) bool { + return username == doer.Name + }, + }) + setting.IncomingEmail.Enabled = true defer func() { setting.IncomingEmail.Enabled = false }() @@ -77,7 +86,8 @@ func TestComposeIssueCommentMessage(t *testing.T) { msgs, err := composeIssueCommentMessages(&mailCommentContext{ Context: context.TODO(), // TODO: use a correct context Issue: issue, Doer: doer, ActionType: activities_model.ActionCommentIssue, - Content: "test body", Comment: comment, + Content: fmt.Sprintf("test @%s %s#%d body", doer.Name, issue.Repo.FullName(), issue.Index), + Comment: comment, }, "en-US", recipients, false, "issue comment") assert.NoError(t, err) assert.Len(t, msgs, 2) @@ -96,6 +106,20 @@ func TestComposeIssueCommentMessage(t *testing.T) { assert.Equal(t, "", gomailMsg.GetHeader("Message-ID")[0], "Message-ID header doesn't match") assert.Equal(t, "", gomailMsg.GetHeader("List-Post")[0]) assert.Len(t, gomailMsg.GetHeader("List-Unsubscribe"), 2) // url + mailto + + var buf bytes.Buffer + gomailMsg.WriteTo(&buf) + + b, err := io.ReadAll(quotedprintable.NewReader(&buf)) + assert.NoError(t, err) + + // text/plain + assert.Contains(t, string(b), fmt.Sprintf(`( %s )`, doer.HTMLURL())) + assert.Contains(t, string(b), fmt.Sprintf(`( %s )`, issue.HTMLURL())) + + // text/html + assert.Contains(t, string(b), fmt.Sprintf(`href="%s"`, doer.HTMLURL())) + assert.Contains(t, string(b), fmt.Sprintf(`href="%s"`, issue.HTMLURL())) } func TestComposeIssueMessage(t *testing.T) { From 3e94ac5c7c6751919453fdb66ba3472e2793759e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 13 Mar 2024 21:32:30 +0800 Subject: [PATCH 07/14] Improve QueryEscape helper function (#29768) Make it return "template.URL" to follow Golang template's context auto-escaping. --- modules/templates/helper.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 0997239a55ad..24520647499a 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -38,7 +38,7 @@ func NewFuncMap() template.FuncMap { "SafeHTML": SafeHTML, "HTMLFormat": HTMLFormat, "HTMLEscape": HTMLEscape, - "QueryEscape": url.QueryEscape, + "QueryEscape": QueryEscape, "JSEscape": JSEscapeSafe, "SanitizeHTML": SanitizeHTML, "URLJoin": util.URLJoin, @@ -226,6 +226,10 @@ func JSEscapeSafe(s string) template.HTML { return template.HTML(template.JSEscapeString(s)) } +func QueryEscape(s string) template.URL { + return template.URL(url.QueryEscape(s)) +} + // DotEscape wraps a dots in names with ZWJ [U+200D] in order to prevent autolinkers from detecting these as urls func DotEscape(raw string) string { return strings.ReplaceAll(raw, ".", "\u200d.\u200d") From e01b0014de5b732181ac42c03a77c21219f88c6a Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 13 Mar 2024 21:44:46 +0800 Subject: [PATCH 08/14] Improve a11y document and dropdown item (#29753) Co-authored-by: silverwind --- docs/content/contributing/guidelines-frontend.zh-cn.md | 2 +- web_src/js/modules/fomantic/aria.md | 10 +++++----- web_src/js/modules/fomantic/dropdown.js | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/content/contributing/guidelines-frontend.zh-cn.md b/docs/content/contributing/guidelines-frontend.zh-cn.md index ace0d97f497c..b5fb8964b342 100644 --- a/docs/content/contributing/guidelines-frontend.zh-cn.md +++ b/docs/content/contributing/guidelines-frontend.zh-cn.md @@ -53,7 +53,7 @@ HTML 页面由[Go HTML Template](https://pkg.go.dev/html/template)渲染。 ### 可访问性 / ARIA 在历史上，Gitea大量使用了可访问性不友好的框架 Fomantic UI。 -Gitea使用一些补丁使Fomantic UI更具可访问性（参见`aria.js`和`aria.md`）， +Gitea 使用一些补丁使 Fomantic UI 更具可访问性（参见 `aria.md`）， 但仍然存在许多问题需要大量的工作和时间来修复。 ### 框架使用 diff --git a/web_src/js/modules/fomantic/aria.md b/web_src/js/modules/fomantic/aria.md index a32d15f46fe8..f639233346c2 100644 --- a/web_src/js/modules/fomantic/aria.md +++ b/web_src/js/modules/fomantic/aria.md @@ -2,10 +2,10 @@ This document is used as aria/accessibility(a11y) reference for future developers. -There are a lot of a11y problems in the Fomantic UI library. This `aria.js` is used -as a workaround to make the UI more accessible. +There are a lot of a11y problems in the Fomantic UI library. Files in +`web_src/js/modules/fomantic/` are used as a workaround to make the UI more accessible. -The `aria.js` is designed to avoid touching the official Fomantic UI library, +The aria-related code is designed to avoid touching the official Fomantic UI library, and to be as independent as possible, so it can be easily modified/removed in the future. To test the aria/accessibility with screen readers, developers can use the following steps: @@ -14,7 +14,7 @@ To test the aria/accessibility with screen readers, developers can use the follo * Press `Command + F5` to turn on VoiceOver. * Try to operate the UI with keyboard-only. * Use Tab/Shift+Tab to switch focus between elements. - * Arrow keys to navigate between menu/combobox items (only aria-active, not really focused). + * Arrow keys (Option+Up/Down) to navigate between menu/combobox items (only aria-active, not really focused). * Press Enter to trigger the aria-active element. * On Android, you can use TalkBack. * Go to Settings -> Accessibility -> TalkBack, turn it on. @@ -75,7 +75,7 @@ Fomantic Dropdown is designed to be used for many purposes: Fomantic Dropdown requires that the focus must be on its primary element. If the focus changes, it hides or panics. -At the moment, `aria.js` only tries to partially resolve the a11y problems for dropdowns with items. +At the moment, the aria-related code only tries to partially resolve the a11y problems for dropdowns with items. There are different solutions: diff --git a/web_src/js/modules/fomantic/dropdown.js b/web_src/js/modules/fomantic/dropdown.js index c053256dd5f0..caba8a2f282d 100644 --- a/web_src/js/modules/fomantic/dropdown.js +++ b/web_src/js/modules/fomantic/dropdown.js @@ -38,7 +38,7 @@ function updateMenuItem(dropdown, item) { if (!item.id) item.id = generateAriaId(); item.setAttribute('role', dropdown[ariaPatchKey].listItemRole); item.setAttribute('tabindex', '-1'); - for (const a of item.querySelectorAll('a')) a.setAttribute('tabindex', '-1'); + for (const el of item.querySelectorAll('a, input, button')) el.setAttribute('tabindex', '-1'); } // make the label item and its "delete icon" has correct aria attributes From df60dbfb9918081962614d063485337fb42e0ee7 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Mar 2024 00:24:34 +0800 Subject: [PATCH 09/14] Fix incorrect locale Tr for gpg command (#29754) --- options/locale/locale_en-US.ini | 1 - templates/user/settings/keys_gpg.tmpl | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index afd613af590d..836703a480ef 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -804,7 +804,6 @@ gpg_invalid_token_signature = The provided GPG key, signature and token do not m gpg_token_required = You must provide a signature for the below token gpg_token = Token gpg_token_help = You can generate a signature using: -gpg_token_code = echo "%s" | gpg -a --default-key %s --detach-sig gpg_token_signature = Armored GPG signature key_signature_gpg_placeholder = Begins with '-----BEGIN PGP SIGNATURE-----' verify_gpg_key_success = GPG key "%s" has been verified. diff --git a/templates/user/settings/keys_gpg.tmpl b/templates/user/settings/keys_gpg.tmpl index 0dd0059511c5..e57658b19733 100644 --- a/templates/user/settings/keys_gpg.tmpl +++ b/templates/user/settings/keys_gpg.tmpl @@ -22,7 +22,7 @@

      {{ctx.Locale.Tr "settings.gpg_token_help"}}

      -

      {{ctx.Locale.Tr "settings.gpg_token_code" .TokenToSign .PaddedKeyID}}

      +

      {{printf `echo "%s" | gpg -a --default-key %s --detach-sig` .TokenToSign .PaddedKeyID}}

      @@ -90,7 +90,7 @@

      {{ctx.Locale.Tr "settings.gpg_token_help"}}

      -

      {{ctx.Locale.Tr "settings.gpg_token_code" $.TokenToSign .PaddedKeyID}}

      +

      {{printf `echo "%s" | gpg -a --default-key %s --detach-sig` $.TokenToSign .PaddedKeyID}}


      From 712e19fa6fbf2f1a5b0a471782d38a7d91e538ae Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 13 Mar 2024 19:00:38 +0100 Subject: [PATCH 10/14] fix missed RenderLabel change in card template (#29772) regression of #29680 close #29770 PS: it would be nice to have a linter that is able to check template helpers ... --- templates/repo/issue/card.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/issue/card.tmpl b/templates/repo/issue/card.tmpl index ff635c736a5b..b461c5fc9835 100644 --- a/templates/repo/issue/card.tmpl +++ b/templates/repo/issue/card.tmpl @@ -61,7 +61,7 @@ {{if or .Labels .Assignees}}
      {{range .Labels}} - {{RenderLabel ctx .}} + {{RenderLabel ctx ctx.Locale .}} {{end}}
      {{range .Assignees}} From 83ba882bab7e1545fe02cd41f554ae41b83a6040 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Mar 2024 03:46:15 +0800 Subject: [PATCH 11/14] Fix possible NPE in ToPullReviewList (#29759) Co-authored-by: wxiaoguang --- services/convert/pull_review.go | 2 +- services/convert/pull_review_test.go | 52 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 services/convert/pull_review_test.go diff --git a/services/convert/pull_review.go b/services/convert/pull_review.go index aa7ad68a4785..29a5ab7466b1 100644 --- a/services/convert/pull_review.go +++ b/services/convert/pull_review.go @@ -66,7 +66,7 @@ func ToPullReviewList(ctx context.Context, rl []*issues_model.Review, doer *user result := make([]*api.PullReview, 0, len(rl)) for i := range rl { // show pending reviews only for the user who created them - if rl[i].Type == issues_model.ReviewTypePending && !(doer.IsAdmin || doer.ID == rl[i].ReviewerID) { + if rl[i].Type == issues_model.ReviewTypePending && (doer == nil || (!doer.IsAdmin && doer.ID != rl[i].ReviewerID)) { continue } r, err := ToPullReview(ctx, rl[i], doer) diff --git a/services/convert/pull_review_test.go b/services/convert/pull_review_test.go new file mode 100644 index 000000000000..68869502802b --- /dev/null +++ b/services/convert/pull_review_test.go @@ -0,0 +1,52 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package convert + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + 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" +) + +func Test_ToPullReview(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + reviewer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + review := unittest.AssertExistsAndLoadBean(t, &issues_model.Review{ID: 6}) + assert.EqualValues(t, reviewer.ID, review.ReviewerID) + assert.EqualValues(t, issues_model.ReviewTypePending, review.Type) + + reviewList := []*issues_model.Review{review} + + t.Run("Anonymous User", func(t *testing.T) { + prList, err := ToPullReviewList(db.DefaultContext, reviewList, nil) + assert.NoError(t, err) + assert.Empty(t, prList) + }) + + t.Run("Reviewer Himself", func(t *testing.T) { + prList, err := ToPullReviewList(db.DefaultContext, reviewList, reviewer) + assert.NoError(t, err) + assert.Len(t, prList, 1) + }) + + t.Run("Other User", func(t *testing.T) { + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + prList, err := ToPullReviewList(db.DefaultContext, reviewList, user4) + assert.NoError(t, err) + assert.Len(t, prList, 0) + }) + + t.Run("Admin User", func(t *testing.T) { + adminUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + prList, err := ToPullReviewList(db.DefaultContext, reviewList, adminUser) + assert.NoError(t, err) + assert.Len(t, prList, 1) + }) +} From 43de021ac1ca017212ec75fd88a8a80a9db27c4c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Mar 2024 09:10:51 +0800 Subject: [PATCH 12/14] Add test for webhook (#29755) Follow #29690 --- modules/util/util.go | 9 ++++ services/webhook/deliver_test.go | 78 ++++++++++++-------------------- 2 files changed, 38 insertions(+), 49 deletions(-) diff --git a/modules/util/util.go b/modules/util/util.go index 5c751581965c..c94fb910471c 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -212,3 +212,12 @@ func ToFloat64(number any) (float64, error) { func ToPointer[T any](val T) *T { return &val } + +// IfZero returns "def" if "v" is a zero value, otherwise "v" +func IfZero[T comparable](v, def T) T { + var zero T + if v == zero { + return def + } + return v +} diff --git a/services/webhook/deliver_test.go b/services/webhook/deliver_test.go index 24924ab214e0..85de1f99047e 100644 --- a/services/webhook/deliver_test.go +++ b/services/webhook/deliver_test.go @@ -18,6 +18,7 @@ import ( webhook_model "code.gitea.io/gitea/models/webhook" "code.gitea.io/gitea/modules/hostmatcher" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" webhook_module "code.gitea.io/gitea/modules/webhook" "github.com/stretchr/testify/assert" @@ -226,49 +227,29 @@ func TestWebhookDeliverSpecificTypes(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) type hookCase struct { - gotBody chan []byte + gotBody chan []byte + httpMethod string // default to POST } - cases := map[string]hookCase{ - webhook_module.SLACK: { - gotBody: make(chan []byte, 1), - }, - webhook_module.DISCORD: { - gotBody: make(chan []byte, 1), - }, - webhook_module.DINGTALK: { - gotBody: make(chan []byte, 1), - }, - webhook_module.TELEGRAM: { - gotBody: make(chan []byte, 1), - }, - webhook_module.MSTEAMS: { - gotBody: make(chan []byte, 1), - }, - webhook_module.FEISHU: { - gotBody: make(chan []byte, 1), - }, - webhook_module.MATRIX: { - gotBody: make(chan []byte, 1), - }, - webhook_module.WECHATWORK: { - gotBody: make(chan []byte, 1), - }, - webhook_module.PACKAGIST: { - gotBody: make(chan []byte, 1), - }, + cases := map[string]*hookCase{ + webhook_module.SLACK: {}, + webhook_module.DISCORD: {}, + webhook_module.DINGTALK: {}, + webhook_module.TELEGRAM: {}, + webhook_module.MSTEAMS: {}, + webhook_module.FEISHU: {}, + webhook_module.MATRIX: {httpMethod: "PUT"}, + webhook_module.WECHATWORK: {}, + webhook_module.PACKAGIST: {}, } s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + typ := strings.Split(r.URL.Path, "/")[1] // URL: "/{webhook_type}/other-path" assert.Equal(t, "application/json", r.Header.Get("Content-Type"), r.URL.Path) - - typ := strings.Split(r.URL.Path, "/")[1] // take first segment (after skipping leading slash) - hc := cases[typ] - require.NotNil(t, hc.gotBody, r.URL.Path) - body, err := io.ReadAll(r.Body) - assert.NoError(t, err) - w.WriteHeader(200) - hc.gotBody <- body + assert.Equal(t, util.IfZero(cases[typ].httpMethod, "POST"), r.Method, "webhook test request %q", r.URL.Path) + body, _ := io.ReadAll(r.Body) // read request and send it back to the test by testcase's chan + cases[typ].gotBody <- body + w.WriteHeader(http.StatusNoContent) })) t.Cleanup(s.Close) @@ -276,19 +257,17 @@ func TestWebhookDeliverSpecificTypes(t *testing.T) { data, err := p.JSONPayload() assert.NoError(t, err) - for typ, hc := range cases { - typ := typ - hc := hc + for typ := range cases { + cases[typ].gotBody = make(chan []byte, 1) + typ := typ // TODO: remove this workaround when Go >= 1.22 t.Run(typ, func(t *testing.T) { t.Parallel() hook := &webhook_model.Webhook{ - RepoID: 3, - IsActive: true, - Type: typ, - URL: s.URL + "/" + typ, - HTTPMethod: "POST", - ContentType: 0, // set to 0 so that falling back to default request fails with "invalid content type" - Meta: "{}", + RepoID: 3, + IsActive: true, + Type: typ, + URL: s.URL + "/" + typ, + Meta: "{}", } assert.NoError(t, webhook_model.CreateWebhook(db.DefaultContext, hook)) @@ -304,10 +283,11 @@ func TestWebhookDeliverSpecificTypes(t *testing.T) { assert.NotNil(t, hookTask) assert.NoError(t, Deliver(context.Background(), hookTask)) + select { - case gotBody := <-hc.gotBody: + case gotBody := <-cases[typ].gotBody: assert.NotEqual(t, string(data), string(gotBody), "request body must be different from the event payload") - assert.Equal(t, hookTask.RequestInfo.Body, string(gotBody), "request body was not saved") + assert.Equal(t, hookTask.RequestInfo.Body, string(gotBody), "delivered webhook payload doesn't match saved request") case <-time.After(5 * time.Second): t.Fatal("waited to long for request to happen") } From 2da13675c0cfdc531044553636c3b74f2fda3eb4 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Thu, 14 Mar 2024 10:37:15 +0900 Subject: [PATCH 13/14] Fix incorrect menu/link on webhook edit page (#29709) Fix #29699 --------- Co-authored-by: silverwind --- routers/web/repo/setting/webhook.go | 1 + tests/integration/repo_webhook_test.go | 41 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/integration/repo_webhook_test.go diff --git a/routers/web/repo/setting/webhook.go b/routers/web/repo/setting/webhook.go index c8e621fac8cd..1a3549fea411 100644 --- a/routers/web/repo/setting/webhook.go +++ b/routers/web/repo/setting/webhook.go @@ -588,6 +588,7 @@ func checkWebhook(ctx *context.Context) (*ownerRepoCtx, *webhook.Webhook) { return nil, nil } ctx.Data["BaseLink"] = orCtx.Link + ctx.Data["BaseLinkNew"] = orCtx.LinkNew var w *webhook.Webhook if orCtx.RepoID > 0 { diff --git a/tests/integration/repo_webhook_test.go b/tests/integration/repo_webhook_test.go new file mode 100644 index 000000000000..ef44a9e2d0fb --- /dev/null +++ b/tests/integration/repo_webhook_test.go @@ -0,0 +1,41 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "strings" + "testing" + + "code.gitea.io/gitea/tests" + + "github.com/PuerkitoBio/goquery" + "github.com/stretchr/testify/assert" +) + +func TestNewWebHookLink(t *testing.T) { + defer tests.PrepareTestEnv(t)() + session := loginUser(t, "user2") + + baseurl := "/user2/repo1/settings/hooks" + tests := []string{ + // webhook list page + baseurl, + // new webhook page + baseurl + "/gitea/new", + // edit webhook page + baseurl + "/1", + } + + for _, url := range tests { + resp := session.MakeRequest(t, NewRequest(t, "GET", url), http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + menus := htmlDoc.doc.Find(".ui.top.attached.header .ui.dropdown .menu a") + menus.Each(func(i int, menu *goquery.Selection) { + url, exist := menu.Attr("href") + assert.True(t, exist) + assert.True(t, strings.HasPrefix(url, baseurl)) + }) + } +} From bbef5fc5c304d8e9cef110bffa44a0e4bcf028fb Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 14 Mar 2024 03:23:58 +0100 Subject: [PATCH 14/14] Fix `make generate-swagger` in go 1.22 (#29780) Fixes: https://github.com/go-gitea/gitea/issues/29664. No release available for https://github.com/go-swagger/go-swagger/issues/3070 so let's depend on latest commit hash. Output is the same as before for me. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5ab8655c2fc3..cedc4b419836 100644 --- a/Makefile +++ b/Makefile @@ -31,7 +31,7 @@ GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.6.0 GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56.1 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.11 MISSPELL_PACKAGE ?= github.com/golangci/misspell/cmd/misspell@v0.4.1 -SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.30.5 +SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@db51e79a0e37c572d8b59ae0c58bf2bbbbe53285 XGO_PACKAGE ?= src.techknowlogick.com/xgo@latest GO_LICENSES_PACKAGE ?= github.com/google/go-licenses@v1.6.0 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1.0.3