From cd7d0166f22003d928316f3c0744ae2f505bb6a7 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sun, 11 Aug 2024 22:48:20 +0800 Subject: [PATCH 1/6] Show lock owner instead of repo owner on LFS setting page (#31788) Fix #31784. Before: image After: image (cherry picked from commit 0470646d46f90c20f40fde718be6ef8d8c84ee2c) (cherry picked from commit 7f1db1df3ee8d620f997b8e70a40c2f48ae96c0f) --- models/git/lfs_lock.go | 45 ++++++++++++++++--- models/git/lfs_lock_list.go | 54 ++++++++++++++++++++++ routers/web/repo/setting/lfs.go | 5 +++ templates/repo/settings/lfs_locks.tmpl | 6 +-- tests/integration/lfs_view_test.go | 62 ++++++++++++++++++++++++++ 5 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 models/git/lfs_lock_list.go diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 2f65833fe3..07ce7d4abf 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -6,6 +6,7 @@ package git import ( "context" "errors" + "fmt" "strings" "time" @@ -21,11 +22,12 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX NOT NULL"` - OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"TEXT"` - Created time.Time `xorm:"created"` + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX NOT NULL"` + OwnerID int64 `xorm:"INDEX NOT NULL"` + Owner *user_model.User `xorm:"-"` + Path string `xorm:"TEXT"` + Created time.Time `xorm:"created"` } func init() { @@ -37,6 +39,35 @@ func (l *LFSLock) BeforeInsert() { l.Path = util.PathJoinRel(l.Path) } +// LoadAttributes loads attributes of the lock. +func (l *LFSLock) LoadAttributes(ctx context.Context) error { + // Load owner + if err := l.LoadOwner(ctx); err != nil { + return fmt.Errorf("load owner: %w", err) + } + + return nil +} + +// LoadOwner loads owner of the lock. +func (l *LFSLock) LoadOwner(ctx context.Context) error { + if l.Owner != nil { + return nil + } + + owner, err := user_model.GetUserByID(ctx, l.OwnerID) + if err != nil { + if user_model.IsErrUserNotExist(err) { + l.Owner = user_model.NewGhostUser() + return nil + } + return err + } + l.Owner = owner + + return nil +} + // CreateLFSLock creates a new lock. func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) { dbCtx, committer, err := db.TxContext(ctx) @@ -94,7 +125,7 @@ func GetLFSLockByID(ctx context.Context, id int64) (*LFSLock, error) { } // GetLFSLockByRepoID returns a list of locks of repository. -func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) ([]*LFSLock, error) { +func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) (LFSLockList, error) { e := db.GetEngine(ctx) if page >= 0 && pageSize > 0 { start := 0 @@ -103,7 +134,7 @@ func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) ( } e.Limit(pageSize, start) } - lfsLocks := make([]*LFSLock, 0, pageSize) + lfsLocks := make(LFSLockList, 0, pageSize) return lfsLocks, e.Find(&lfsLocks, &LFSLock{RepoID: repoID}) } diff --git a/models/git/lfs_lock_list.go b/models/git/lfs_lock_list.go new file mode 100644 index 0000000000..cab1e61cab --- /dev/null +++ b/models/git/lfs_lock_list.go @@ -0,0 +1,54 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "context" + "fmt" + + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" +) + +// LFSLockList is a list of LFSLock +type LFSLockList []*LFSLock + +// LoadAttributes loads the attributes for the given locks +func (locks LFSLockList) LoadAttributes(ctx context.Context) error { + if len(locks) == 0 { + return nil + } + + if err := locks.LoadOwner(ctx); err != nil { + return fmt.Errorf("load owner: %w", err) + } + + return nil +} + +// LoadOwner loads the owner of the locks +func (locks LFSLockList) LoadOwner(ctx context.Context) error { + if len(locks) == 0 { + return nil + } + + usersIDs := container.FilterSlice(locks, func(lock *LFSLock) (int64, bool) { + return lock.OwnerID, true + }) + users := make(map[int64]*user_model.User, len(usersIDs)) + if err := db.GetEngine(ctx). + In("id", usersIDs). + Find(&users); err != nil { + return fmt.Errorf("find users: %w", err) + } + for _, v := range locks { + v.Owner = users[v.OwnerID] + if v.Owner == nil { // not exist + v.Owner = user_model.NewGhostUser() + } + } + + return nil +} diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index c18cb6a8c8..7e3634375a 100644 --- a/routers/web/repo/setting/lfs.go +++ b/routers/web/repo/setting/lfs.go @@ -95,6 +95,11 @@ func LFSLocks(ctx *context.Context) { ctx.ServerError("LFSLocks", err) return } + if err := lfsLocks.LoadAttributes(ctx); err != nil { + ctx.ServerError("LFSLocks", err) + return + } + ctx.Data["LFSLocks"] = lfsLocks if len(lfsLocks) == 0 { diff --git a/templates/repo/settings/lfs_locks.tmpl b/templates/repo/settings/lfs_locks.tmpl index 98f0e49097..9a18f525e9 100644 --- a/templates/repo/settings/lfs_locks.tmpl +++ b/templates/repo/settings/lfs_locks.tmpl @@ -30,9 +30,9 @@ {{end}} - - {{ctx.AvatarUtils.Avatar $.Owner}} - {{$.Owner.DisplayName}} + + {{ctx.AvatarUtils.Avatar $lock.Owner}} + {{$lock.Owner.DisplayName}} {{TimeSince .Created ctx.Locale}} diff --git a/tests/integration/lfs_view_test.go b/tests/integration/lfs_view_test.go index 8817986b79..791eeac877 100644 --- a/tests/integration/lfs_view_test.go +++ b/tests/integration/lfs_view_test.go @@ -4,12 +4,21 @@ package integration import ( + "context" + "fmt" "net/http" + "strings" "testing" + 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/lfs" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // check that files stored in LFS render properly in the web UI @@ -103,3 +112,56 @@ func TestLFSRender(t *testing.T) { assert.Equal(t, 1, doc.Find(`.sha.label[href="/user2/lfs/commit/73cf03db6ece34e12bf91e8853dc58f678f2f82d"]`).Length(), "could not find link to commit") }) } + +// TestLFSLockView tests the LFS lock view on settings page of repositories +func TestLFSLockView(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // in org 3 + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // own by org 3 + session := loginUser(t, user2.Name) + + // create a lock + lockPath := "test_lfs_lock_view.zip" + lockID := "" + { + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks", repo3.FullName()), map[string]string{"path": lockPath}) + req.Header.Set("Accept", lfs.AcceptHeader) + req.Header.Set("Content-Type", lfs.MediaType) + resp := session.MakeRequest(t, req, http.StatusCreated) + lockResp := &api.LFSLockResponse{} + DecodeJSON(t, resp, lockResp) + lockID = lockResp.Lock.ID + } + defer func() { + // release the lock + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/%s/unlock", repo3.FullName(), lockID), map[string]string{}) + req.Header.Set("Accept", lfs.AcceptHeader) + req.Header.Set("Content-Type", lfs.MediaType) + session.MakeRequest(t, req, http.StatusOK) + }() + + t.Run("owner name", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + // make sure the display names are different, or the test is meaningless + require.NoError(t, repo3.LoadOwner(context.Background())) + require.NotEqual(t, user2.DisplayName(), repo3.Owner.DisplayName()) + + req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings/lfs/locks", repo3.FullName())) + resp := session.MakeRequest(t, req, http.StatusOK) + + doc := NewHTMLParser(t, resp.Body).doc + + tr := doc.Find("table#lfs-files-locks-table tbody tr") + require.Equal(t, 1, tr.Length()) + + td := tr.First().Find("td") + require.Equal(t, 4, td.Length()) + + // path + assert.Equal(t, lockPath, strings.TrimSpace(td.Eq(0).Text())) + // owner name + assert.Equal(t, user2.DisplayName(), strings.TrimSpace(td.Eq(1).Text())) + }) +} From e442e71e03c5fb83d505874e7cc5fd5db22fe529 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 15 Aug 2024 23:59:01 +0800 Subject: [PATCH 2/6] Fix panic of ssh public key page after deletion of auth source (#31829) Fix #31730 This PR rewrote the function `PublicKeysAreExternallyManaged` with a simple test. The new function removed the loop to make it more readable. (cherry picked from commit b491b2104f83ee8fc4956c099c427b339291b3be) (cherry picked from commit d5500422c926764ce44becdda5d2b70cea0cbbf6) --- models/asymkey/ssh_key.go | 23 +++++++---------------- models/asymkey/ssh_key_test.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index a409d8e841..7a18732c32 100644 --- a/models/asymkey/ssh_key.go +++ b/models/asymkey/ssh_key.go @@ -229,35 +229,26 @@ func UpdatePublicKeyUpdated(ctx context.Context, id int64) error { // PublicKeysAreExternallyManaged returns whether the provided KeyID represents an externally managed Key func PublicKeysAreExternallyManaged(ctx context.Context, keys []*PublicKey) ([]bool, error) { - sources := make([]*auth.Source, 0, 5) + sourceCache := make(map[int64]*auth.Source, len(keys)) externals := make([]bool, len(keys)) -keyloop: + for i, key := range keys { if key.LoginSourceID == 0 { externals[i] = false - continue keyloop + continue } - var source *auth.Source - - sourceloop: - for _, s := range sources { - if s.ID == key.LoginSourceID { - source = s - break sourceloop - } - } - - if source == nil { + source, ok := sourceCache[key.LoginSourceID] + if !ok { var err error source, err = auth.GetSourceByID(ctx, key.LoginSourceID) if err != nil { if auth.IsErrSourceNotExist(err) { externals[i] = false - sources[i] = &auth.Source{ + sourceCache[key.LoginSourceID] = &auth.Source{ ID: key.LoginSourceID, } - continue keyloop + continue } return nil, err } diff --git a/models/asymkey/ssh_key_test.go b/models/asymkey/ssh_key_test.go index e4e81f51c4..2625d6ac91 100644 --- a/models/asymkey/ssh_key_test.go +++ b/models/asymkey/ssh_key_test.go @@ -12,6 +12,8 @@ import ( "strings" "testing" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/setting" "github.com/42wim/sshsig" @@ -501,3 +503,11 @@ func runErr(t *testing.T, stdin []byte, args ...string) { t.Fatal("expected error") } } + +func Test_PublicKeysAreExternallyManaged(t *testing.T) { + key1 := unittest.AssertExistsAndLoadBean(t, &PublicKey{ID: 1}) + externals, err := PublicKeysAreExternallyManaged(db.DefaultContext, []*PublicKey{key1}) + require.NoError(t, err) + assert.Len(t, externals, 1) + assert.False(t, externals[0]) +} From d5fec46de7d8682152312291783889206428328a Mon Sep 17 00:00:00 2001 From: Jason Song Date: Sat, 17 Aug 2024 01:04:54 +0800 Subject: [PATCH 3/6] Avoid returning without written ctx when posting PR (#31843) Fix #31625. If `pull_service.NewPullRequest` return an error which misses each `if` check, `CompareAndPullRequestPost` will return immediately, since it doesn't write the HTTP response, a 200 response with empty body will be sent to clients. ```go if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) } else if git.IsErrPushRejected(err) { // ... ctx.JSONError(flashError) } else if errors.Is(err, user_model.ErrBlockedUser) { // ... ctx.JSONError(flashError) } else if errors.Is(err, issues_model.ErrMustCollaborator) { // ... ctx.JSONError(flashError) } return } ``` Not sure what kind of error can cause it to happen, so this PR just expose it. And we can fix it when users report that creating PRs failed with error responses. It's all my guess since I cannot reproduce the problem, but even if it's not related, the code here needs to be improved. (cherry picked from commit acd7053e9d4968e8b9812ab379be9027ac8e7771) Conflicts: routers/web/repo/pull.go trivial context conflict (cherry picked from commit 385718dd78bf0b398091565494828aef2ec85f44) --- routers/web/repo/pull.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 5c56a6f5ad..21353bae9a 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -1507,13 +1507,12 @@ func CompareAndPullRequestPost(ctx *context.Context) { // instead of 500. if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil { - if errors.Is(err, user_model.ErrBlockedByUser) { + switch { + case errors.Is(err, user_model.ErrBlockedByUser): ctx.JSONError(ctx.Tr("repo.pulls.blocked_by_user")) - return - } else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) { + case repo_model.IsErrUserDoesNotHaveAccessToRepo(err): ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error()) - return - } else if git.IsErrPushRejected(err) { + case git.IsErrPushRejected(err): pushrejErr := err.(*git.ErrPushRejected) message := pushrejErr.Message if len(message) == 0 { @@ -1530,7 +1529,11 @@ func CompareAndPullRequestPost(ctx *context.Context) { return } ctx.JSONError(flashError) - return + default: + // It's an unexpected error. + // If it happens, we should add another case to handle it. + log.Error("Unexpected error of NewPullRequest: %T %s", err, err) + ctx.ServerError("CompareAndPullRequest", err) } ctx.ServerError("NewPullRequest", err) return From 9a142e3fd18190e1af7047de4378f8e0b09191a7 Mon Sep 17 00:00:00 2001 From: Adrian Hirt <13788379+Adrian-Hirt@users.noreply.github.com> Date: Sat, 17 Aug 2024 05:34:27 +0200 Subject: [PATCH 4/6] Fix overflowing content in action run log (#31842) When a long line with characters such as dots is returned by a step in an action (e.g. by the output of the Ruby on Rails test runner), it overflows the log container, causing the page to scroll sideways. This PR adds the CSS `overflow-wrap: anywhere;` to the `.job-step-section .job-step-logs .job-log-line .log-msg` selector, which causes such lines to wrap as well (cherry picked from commit 61aaf3440142d225802e3e9ce3db28bcf71f5a7e) (cherry picked from commit 1dfa11551c1c179bb5b9b4e0257178190a4d331f) --- web_src/js/components/RepoActionView.vue | 1 + 1 file changed, 1 insertion(+) diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 4f2af3ac6d..fae4500bde 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -878,6 +878,7 @@ export function initRepositoryActionView() { word-break: break-all; white-space: break-spaces; margin-left: 10px; + overflow-wrap: anywhere; } /* selectors here are intentionally exact to only match fullscreen */ From c5acd4114684e3431bc839fa24034dac4685fd88 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Fri, 16 Aug 2024 01:34:24 +0900 Subject: [PATCH 5/6] Add missing repository type filter parameters to pager (#31832) Fix #31807 ps: the newly added params's value will be changed. When the first time you selected the filter, the values of params will be `0` or `1` But in pager it will be `true` or `false`. So do we have `boolToInt` function? (cherry picked from commit 7092402a2db255ecde2c20574b973fb632c16d2e) Conflicts: routers/web/org/home.go trivial conflict s/pager.AddParam/pager.AddParamString/ (cherry picked from commit a8e25e907c66140961f28ba92403176c816dfb60) --- routers/web/explore/repo.go | 15 +++++++++++++++ routers/web/org/home.go | 18 +++++++++++++++++- routers/web/user/notification.go | 15 +++++++++++++++ routers/web/user/profile.go | 15 +++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/routers/web/explore/repo.go b/routers/web/explore/repo.go index 4e880660b1..e978385b66 100644 --- a/routers/web/explore/repo.go +++ b/routers/web/explore/repo.go @@ -144,6 +144,21 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) { pager.AddParam(ctx, "topic", "TopicOnly") pager.AddParam(ctx, "language", "Language") pager.AddParamString(relevantReposOnlyParam, fmt.Sprint(opts.OnlyShowRelevant)) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.HTML(http.StatusOK, opts.TplName) diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 71d10f3a43..1e04b72cbb 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -4,6 +4,7 @@ package org import ( + "fmt" "net/http" "path" "strings" @@ -154,7 +155,22 @@ func Home(ctx *context.Context) { pager := context.NewPagination(int(count), setting.UI.User.RepoPagingNum, page, 5) pager.SetDefaultParams(ctx) - pager.AddParam(ctx, "language", "Language") + pager.AddParamString("language", language) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.Data["ShowMemberAndTeamTab"] = ctx.Org.IsMember || len(members) > 0 diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index 2105cfe5c5..c12cec85b5 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -439,6 +439,21 @@ func NotificationWatching(ctx *context.Context) { // redirect to last page if request page is more than total pages pager := context.NewPagination(total, setting.UI.User.RepoPagingNum, page, 5) pager.SetDefaultParams(ctx) + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager ctx.Data["Status"] = 2 diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index a83d7f7333..72aac7e15d 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -335,6 +335,21 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb if tab == "activity" { pager.AddParam(ctx, "date", "Date") } + if archived.Has() { + pager.AddParamString("archived", fmt.Sprint(archived.Value())) + } + if fork.Has() { + pager.AddParamString("fork", fmt.Sprint(fork.Value())) + } + if mirror.Has() { + pager.AddParamString("mirror", fmt.Sprint(mirror.Value())) + } + if template.Has() { + pager.AddParamString("template", fmt.Sprint(template.Value())) + } + if private.Has() { + pager.AddParamString("private", fmt.Sprint(private.Value())) + } ctx.Data["Page"] = pager } From 4e5774fef7b6c0f3900e17b20fdf4ee01ed6838a Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 18 Aug 2024 06:30:08 +0200 Subject: [PATCH 6/6] chore(release-notes): weekly cherry-pick week 2024-34-v8.0 --- release-notes/5050.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 release-notes/5050.md diff --git a/release-notes/5050.md b/release-notes/5050.md new file mode 100644 index 0000000000..02a64161cd --- /dev/null +++ b/release-notes/5050.md @@ -0,0 +1,3 @@ +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/cd7d0166f22003d928316f3c0744ae2f505bb6a7) Show lock owner instead of repo owner on LFS setting page. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/bd0cb143b117ad2218151a98618406534e551c03) Fix panic of ssh public key page after deletion of auth source. +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/787a08d007c07c92aa28a744e2d701e0933611b5) Add missing repository type filter parameters to pager.