From 09699c15069eaefa771503c8fc1eb315a513a5d7 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Fri, 13 Jun 2025 12:41:34 +0200 Subject: [PATCH] feat: always publish the link to the commit status (#8177) See https://codeberg.org/forgejo/forgejo/pulls/4801#issuecomment-5094525 and #8152 for more context. The current implementation is limited to self-hosted actions and buggy as soon as multiple repos are involved, like for the homepage (because each permission must be fetched individually). Ideally this feature should work for all kind of status (with some setting indicating which collaborator can access with status). Probably inside the `git_model.ParseCommitsWithStatus` function. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8177 Reviewed-by: Earl Warren Co-authored-by: oliverpool Co-committed-by: oliverpool --- models/git/commit_status.go | 70 +++++++------------------------- models/git/commit_status_test.go | 25 ------------ models/issues/comment.go | 2 +- routers/web/repo/branch.go | 5 --- routers/web/repo/commit.go | 24 ++--------- routers/web/repo/compare.go | 2 +- routers/web/repo/issue.go | 14 ------- routers/web/repo/pull.go | 11 +---- routers/web/repo/repo.go | 3 -- routers/web/repo/view.go | 3 -- routers/web/repo/wiki.go | 2 +- routers/web/user/home.go | 6 --- routers/web/user/notification.go | 7 ---- 13 files changed, 21 insertions(+), 153 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index a679703ffd..60a0aa5a4f 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -179,25 +179,6 @@ func (status *CommitStatus) LocaleString(lang translation.Locale) string { return lang.TrString("repo.commitstatus." + status.State.String()) } -// HideActionsURL set `TargetURL` to an empty string if the status comes from Gitea Actions -func (status *CommitStatus) HideActionsURL(ctx context.Context) { - if status.RepoID == 0 { - return - } - - if status.Repo == nil { - if err := status.loadRepository(ctx); err != nil { - log.Error("loadRepository: %v", err) - return - } - } - - prefix := fmt.Sprintf("%s/actions", status.Repo.Link()) - if strings.HasPrefix(status.TargetURL, prefix) { - status.TargetURL = "" - } -} - // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc func CalcCommitStatus(statuses []*CommitStatus) *CommitStatus { if len(statuses) == 0 { @@ -453,11 +434,19 @@ type SignCommitWithStatuses struct { *asymkey_model.SignCommit } -// ParseCommitsWithStatus checks commits latest statuses and calculates its worst status state -func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.SignCommit, repo *repo_model.Repository) []*SignCommitWithStatuses { - newCommits := make([]*SignCommitWithStatuses, 0, len(oldCommits)) +// ParseCommitsWithStatus converts git commits into SignCommitWithStatuses (checks signature and calculates its worst status state) +func ParseCommitsWithStatus(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses { + commitsWithSignature := asymkey_model.ParseCommitsWithSignature( + ctx, + user_model.ValidateCommitsWithEmails(ctx, commits), + repo.GetTrustModel(), + func(user *user_model.User) (bool, error) { + return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID) + }, + ) - for _, c := range oldCommits { + commitsWithStatus := make([]*SignCommitWithStatuses, 0, len(commitsWithSignature)) + for _, c := range commitsWithSignature { commit := &SignCommitWithStatuses{ SignCommit: c, } @@ -469,43 +458,12 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig commit.Status = CalcCommitStatus(statuses) } - newCommits = append(newCommits, commit) + commitsWithStatus = append(commitsWithStatus, commit) } - return newCommits + return commitsWithStatus } // hashCommitStatusContext hash context func hashCommitStatusContext(context string) string { return fmt.Sprintf("%x", sha1.Sum([]byte(context))) } - -// ConvertFromGitCommit converts git commits into SignCommitWithStatuses -func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) []*SignCommitWithStatuses { - return ParseCommitsWithStatus(ctx, - asymkey_model.ParseCommitsWithSignature( - ctx, - user_model.ValidateCommitsWithEmails(ctx, commits), - repo.GetTrustModel(), - func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID) - }, - ), - repo, - ) -} - -// CommitStatusesHideActionsURL hide Gitea Actions urls -func CommitStatusesHideActionsURL(ctx context.Context, statuses []*CommitStatus) { - idToRepos := make(map[int64]*repo_model.Repository) - for _, status := range statuses { - if status == nil { - continue - } - - if status.Repo == nil { - status.Repo = idToRepos[status.RepoID] - } - status.HideActionsURL(ctx) - idToRepos[status.RepoID] = status.Repo - } -} diff --git a/models/git/commit_status_test.go b/models/git/commit_status_test.go index c062bbbbb9..ce6c0d4673 100644 --- a/models/git/commit_status_test.go +++ b/models/git/commit_status_test.go @@ -4,11 +4,9 @@ package git_test import ( - "fmt" "testing" "time" - actions_model "forgejo.org/models/actions" "forgejo.org/models/db" git_model "forgejo.org/models/git" repo_model "forgejo.org/models/repo" @@ -246,26 +244,3 @@ func TestFindRepoRecentCommitStatusContexts(t *testing.T) { assert.Equal(t, "compliance/lint-backend", contexts[0]) } } - -func TestCommitStatusesHideActionsURL(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) - run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: 791, RepoID: repo.ID}) - require.NoError(t, run.LoadAttributes(db.DefaultContext)) - - statuses := []*git_model.CommitStatus{ - { - RepoID: repo.ID, - TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), run.Index), - }, - { - RepoID: repo.ID, - TargetURL: "https://mycicd.org/1", - }, - } - - git_model.CommitStatusesHideActionsURL(db.DefaultContext, statuses) - assert.Empty(t, statuses[0].TargetURL) - assert.Equal(t, "https://mycicd.org/1", statuses[1].TargetURL) -} diff --git a/models/issues/comment.go b/models/issues/comment.go index c44f65e29c..a81221caf4 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -802,7 +802,7 @@ func (c *Comment) LoadPushCommits(ctx context.Context) (err error) { } defer closer.Close() - c.Commits = git_model.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) + c.Commits = git_model.ParseCommitsWithStatus(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo) c.CommitsNum = int64(len(c.Commits)) } diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index af8a838fc9..0fe52bfb48 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -70,11 +70,6 @@ func Branches(ctx *context.Context) { ctx.ServerError("LoadBranches", err) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - for key := range commitStatuses { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) - } - } commitStatus := make(map[string]*git_model.CommitStatus) for commitID, cs := range commitStatuses { diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 89463d9d03..f3192266ad 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -16,7 +16,6 @@ import ( "forgejo.org/models/db" git_model "forgejo.org/models/git" repo_model "forgejo.org/models/repo" - unit_model "forgejo.org/models/unit" user_model "forgejo.org/models/user" "forgejo.org/modules/base" "forgejo.org/modules/charset" @@ -84,7 +83,7 @@ func Commits(ctx *context.Context) { ctx.ServerError("CommitsByRange", err) return } - ctx.Data["Commits"] = processGitCommits(ctx, commits) + ctx.Data["Commits"] = git_model.ParseCommitsWithStatus(ctx, commits, ctx.Repo.Repository) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name @@ -202,7 +201,7 @@ func SearchCommits(ctx *context.Context) { return } ctx.Data["CommitCount"] = len(commits) - ctx.Data["Commits"] = processGitCommits(ctx, commits) + ctx.Data["Commits"] = git_model.ParseCommitsWithStatus(ctx, commits, ctx.Repo.Repository) ctx.Data["Keyword"] = query if all { @@ -267,7 +266,7 @@ func FileHistory(ctx *context.Context) { } } - ctx.Data["Commits"] = processGitCommits(ctx, commits) + ctx.Data["Commits"] = git_model.ParseCommitsWithStatus(ctx, commits, ctx.Repo.Repository) ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name @@ -375,9 +374,6 @@ func Diff(ctx *context.Context) { if err != nil { log.Error("GetLatestCommitStatus: %v", err) } - if !ctx.Repo.CanRead(unit_model.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, statuses) - } ctx.Data["CommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["CommitStatuses"] = statuses @@ -456,20 +452,6 @@ func RawDiff(ctx *context.Context) { } } -func processGitCommits(ctx *context.Context, gitCommits []*git.Commit) []*git_model.SignCommitWithStatuses { - commits := git_model.ConvertFromGitCommit(ctx, gitCommits, ctx.Repo.Repository) - if !ctx.Repo.CanRead(unit_model.TypeActions) { - for _, commit := range commits { - if commit.Status == nil { - continue - } - commit.Status.HideActionsURL(ctx) - git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) - } - } - return commits -} - func SetCommitNotes(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CommitNotesForm) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index f5826cf249..de2e29ab9f 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -654,7 +654,7 @@ func PrepareCompareDiff( return false } - commits := processGitCommits(ctx, ci.CompareInfo.Commits) + commits := git_model.ParseCommitsWithStatus(ctx, ci.CompareInfo.Commits, ctx.Repo.Repository) ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index b97c268ae2..d8f3bd8d9f 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -348,11 +348,6 @@ func issues(ctx *context.Context, milestoneID, projectID int64, isPullOption opt ctx.ServerError("GetIssuesAllCommitStatus", err) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - for key := range commitStatuses { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) - } - } if err := issues.LoadAttributes(ctx); err != nil { ctx.ServerError("issues.LoadAttributes", err) @@ -1799,15 +1794,6 @@ func ViewIssue(ctx *context.Context) { ctx.ServerError("LoadPushCommits", err) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - for _, commit := range comment.Commits { - if commit.Status == nil { - continue - } - commit.Status.HideActionsURL(ctx) - git_model.CommitStatusesHideActionsURL(ctx, commit.Statuses) - } - } } else if comment.Type == issues_model.CommentTypeAddTimeManual || comment.Type == issues_model.CommentTypeStopTracking || comment.Type == issues_model.CommentTypeDeleteTimeManual { diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index bb89e30d54..2db982d3b6 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -515,9 +515,6 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *issues_model.Issue) ctx.ServerError("GetLatestCommitStatus", err) return nil } - if !ctx.Repo.CanRead(unit.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) - } if len(commitStatuses) != 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses @@ -581,9 +578,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError("GetLatestCommitStatus", err) return nil } - if !ctx.Repo.CanRead(unit.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) - } if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses @@ -677,9 +671,6 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.ServerError("GetLatestCommitStatus", err) return nil } - if !ctx.Repo.CanRead(unit.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses) - } if len(commitStatuses) > 0 { ctx.Data["LatestCommitStatuses"] = commitStatuses @@ -847,7 +838,7 @@ func ViewPullCommits(ctx *context.Context) { ctx.Data["Username"] = ctx.Repo.Owner.Name ctx.Data["Reponame"] = ctx.Repo.Repository.Name - commits := processGitCommits(ctx, prInfo.Commits) + commits := git_model.ParseCommitsWithStatus(ctx, prInfo.Commits, ctx.Repo.Repository) ctx.Data["Commits"] = commits ctx.Data["CommitCount"] = len(commits) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 3c923c2c5e..493787ad8b 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -693,9 +693,6 @@ func SearchRepo(ctx *context.Context) { ctx.JSON(http.StatusInternalServerError, nil) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, latestCommitStatuses) - } results := make([]*repo_service.WebSearchRepository, len(repos)) for i, repo := range repos { diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index de508509dc..c7cc715fc1 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -369,9 +369,6 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { if err != nil { log.Error("GetLatestCommitStatus: %v", err) } - if !ctx.Repo.CanRead(unit_model.TypeActions) { - git_model.CommitStatusesHideActionsURL(ctx, statuses) - } ctx.Data["LatestCommitStatus"] = git_model.CalcCommitStatus(statuses) ctx.Data["LatestCommitStatuses"] = statuses diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 9a21ac21a3..1b5265978a 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -393,7 +393,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) ctx.ServerError("CommitsByFileAndRange", err) return nil, nil } - ctx.Data["Commits"] = git_model.ConvertFromGitCommit(ctx, commitsHistory, ctx.Repo.Repository) + ctx.Data["Commits"] = git_model.ParseCommitsWithStatus(ctx, commitsHistory, ctx.Repo.Repository) pager := context.NewPagination(int(commitsCount), setting.Git.CommitsRangeSize, page, 5) pager.SetDefaultParams(ctx) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 2a93221c8f..d980fa393a 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -16,7 +16,6 @@ import ( activities_model "forgejo.org/models/activities" asymkey_model "forgejo.org/models/asymkey" "forgejo.org/models/db" - git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" "forgejo.org/models/organization" repo_model "forgejo.org/models/repo" @@ -611,11 +610,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) { ctx.ServerError("GetIssuesLastCommitStatus", err) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - for key := range commitStatuses { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) - } - } // ------------------------------- // Fill stats to post to ctx.Data. diff --git a/routers/web/user/notification.go b/routers/web/user/notification.go index 9fa71add57..fdca1a2fdd 100644 --- a/routers/web/user/notification.go +++ b/routers/web/user/notification.go @@ -13,10 +13,8 @@ import ( activities_model "forgejo.org/models/activities" "forgejo.org/models/db" - git_model "forgejo.org/models/git" issues_model "forgejo.org/models/issues" repo_model "forgejo.org/models/repo" - "forgejo.org/models/unit" "forgejo.org/modules/base" "forgejo.org/modules/log" "forgejo.org/modules/optional" @@ -311,11 +309,6 @@ func NotificationSubscriptions(ctx *context.Context) { ctx.ServerError("GetIssuesAllCommitStatus", err) return } - if !ctx.Repo.CanRead(unit.TypeActions) { - for key := range commitStatuses { - git_model.CommitStatusesHideActionsURL(ctx, commitStatuses[key]) - } - } ctx.Data["CommitLastStatus"] = lastStatus ctx.Data["CommitStatuses"] = commitStatuses ctx.Data["Issues"] = issues