From e1521ef4607d41b7fb9be2ea3dda574fba62a53f Mon Sep 17 00:00:00 2001
From: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Date: Sun, 11 Feb 2024 10:44:50 +0100
Subject: [PATCH] Set correct link for commit (#3368)

Closes https://github.com/woodpecker-ci/woodpecker/issues/2657
Closes https://github.com/woodpecker-ci/woodpecker/issues/906
---
 server/api/hook.go                         | 12 ++++++++++--
 server/api/pipeline.go                     | 11 +++++------
 server/cron/cron.go                        |  4 ++--
 server/cron/cron_test.go                   | 18 +++++++++++-------
 server/forge/bitbucket/bitbucket.go        | 11 +++++++++--
 server/forge/bitbucket/bitbucket_test.go   |  3 ++-
 server/forge/bitbucket/fixtures/handler.go |  7 ++++++-
 server/forge/bitbucket/internal/client.go  |  8 ++++----
 server/forge/bitbucket/internal/types.go   |  7 ++++++-
 server/forge/forge.go                      |  2 +-
 server/forge/gitea/gitea.go                | 11 +++++++----
 server/forge/github/github.go              |  9 ++++++---
 server/forge/gitlab/gitlab.go              | 13 ++++++++-----
 server/forge/mocks/forge.go                | 14 ++++++++------
 server/model/commit.go                     |  6 ++++++
 server/store/mocks/store.go                |  2 +-
 16 files changed, 92 insertions(+), 46 deletions(-)
 create mode 100644 server/model/commit.go

diff --git a/server/api/hook.go b/server/api/hook.go
index 91abc3971..4318ae480 100644
--- a/server/api/hook.go
+++ b/server/api/hook.go
@@ -27,6 +27,7 @@ import (
 	"github.com/rs/zerolog/log"
 
 	"go.woodpecker-ci.org/woodpecker/v2/server"
+	"go.woodpecker-ci.org/woodpecker/v2/server/forge"
 	"go.woodpecker-ci.org/woodpecker/v2/server/forge/types"
 	"go.woodpecker-ci.org/woodpecker/v2/server/model"
 	"go.woodpecker-ci.org/woodpecker/v2/server/pipeline"
@@ -103,13 +104,13 @@ func BlockTilQueueHasRunningItem(c *gin.Context) {
 //	@Param		hook	body	object	true	"the webhook payload; forge is automatically detected"
 func PostHook(c *gin.Context) {
 	_store := store.FromContext(c)
-	forge := server.Config.Services.Forge
+	_forge := server.Config.Services.Forge
 
 	//
 	// 1. Parse webhook
 	//
 
-	tmpRepo, tmpPipeline, err := forge.Hook(c, c.Request)
+	tmpRepo, tmpPipeline, err := _forge.Hook(c, c.Request)
 	if err != nil {
 		if errors.Is(err, &types.ErrIgnoreEvent{}) {
 			msg := fmt.Sprintf("forge driver: %s", err)
@@ -160,6 +161,13 @@ func PostHook(c *gin.Context) {
 		return
 	}
 
+	user, err := _store.GetUser(repo.UserID)
+	if err != nil {
+		handleDBError(c, err)
+		return
+	}
+	forge.Refresh(c, _forge, _store, user)
+
 	//
 	// 3. Check if the webhook is a valid and authorized one
 	//
diff --git a/server/api/pipeline.go b/server/api/pipeline.go
index b5ed9a11b..5fbfaa09b 100644
--- a/server/api/pipeline.go
+++ b/server/api/pipeline.go
@@ -61,9 +61,9 @@ func CreatePipeline(c *gin.Context) {
 
 	lastCommit, _ := server.Config.Services.Forge.BranchHead(c, user, repo, opts.Branch)
 
-	tmpBuild := createTmpPipeline(model.EventManual, lastCommit, repo, user, &opts)
+	tmpPipeline := createTmpPipeline(model.EventManual, lastCommit, user, &opts)
 
-	pl, err := pipeline.Create(c, _store, repo, tmpBuild)
+	pl, err := pipeline.Create(c, _store, repo, tmpPipeline)
 	if err != nil {
 		handlePipelineErr(c, err)
 	} else {
@@ -71,10 +71,10 @@ func CreatePipeline(c *gin.Context) {
 	}
 }
 
-func createTmpPipeline(event model.WebhookEvent, commitSHA string, repo *model.Repo, user *model.User, opts *model.PipelineOptions) *model.Pipeline {
+func createTmpPipeline(event model.WebhookEvent, commit *model.Commit, user *model.User, opts *model.PipelineOptions) *model.Pipeline {
 	return &model.Pipeline{
 		Event:     event,
-		Commit:    commitSHA,
+		Commit:    commit.SHA,
 		Branch:    opts.Branch,
 		Timestamp: time.Now().UTC().Unix(),
 
@@ -87,8 +87,7 @@ func createTmpPipeline(event model.WebhookEvent, commitSHA string, repo *model.R
 		Author: user.Login,
 		Email:  user.Email,
 
-		// TODO: Generate proper URL to commit
-		ForgeURL: repo.ForgeURL,
+		ForgeURL: commit.ForgeURL,
 	}
 }
 
diff --git a/server/cron/cron.go b/server/cron/cron.go
index ac3ae33bf..bce8374d5 100644
--- a/server/cron/cron.go
+++ b/server/cron/cron.go
@@ -133,12 +133,12 @@ func CreatePipeline(ctx context.Context, store store.Store, f forge.Forge, cron
 
 	return repo, &model.Pipeline{
 		Event:     model.EventCron,
-		Commit:    commit,
+		Commit:    commit.SHA,
 		Ref:       "refs/heads/" + cron.Branch,
 		Branch:    cron.Branch,
 		Message:   cron.Name,
 		Timestamp: cron.NextExec,
 		Sender:    cron.Name,
-		ForgeURL:  repo.ForgeURL,
+		ForgeURL:  commit.ForgeURL,
 	}, nil
 }
diff --git a/server/cron/cron_test.go b/server/cron/cron_test.go
index 450831bdb..0b170cfe9 100644
--- a/server/cron/cron_test.go
+++ b/server/cron/cron_test.go
@@ -47,19 +47,23 @@ func TestCreateBuild(t *testing.T) {
 	// mock things
 	store.On("GetRepo", mock.Anything).Return(repo1, nil)
 	store.On("GetUser", mock.Anything).Return(creator, nil)
-	forge.On("BranchHead", mock.Anything, creator, repo1, "default").Return("sha1", nil)
+	forge.On("BranchHead", mock.Anything, creator, repo1, "default").Return(&model.Commit{
+		ForgeURL: "https://example.com/sha1",
+		SHA:      "sha1",
+	}, nil)
 
 	_, pipeline, err := CreatePipeline(ctx, store, forge, &model.Cron{
 		Name: "test",
 	})
 	assert.NoError(t, err)
 	assert.EqualValues(t, &model.Pipeline{
-		Event:   "cron",
-		Commit:  "sha1",
-		Branch:  "default",
-		Ref:     "refs/heads/default",
-		Message: "test",
-		Sender:  "test",
+		Branch:   "default",
+		Commit:   "sha1",
+		Event:    "cron",
+		ForgeURL: "https://example.com/sha1",
+		Message:  "test",
+		Ref:      "refs/heads/default",
+		Sender:   "test",
 	}, pipeline)
 }
 
diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go
index 9247de60c..13b0d43e5 100644
--- a/server/forge/bitbucket/bitbucket.go
+++ b/server/forge/bitbucket/bitbucket.go
@@ -363,8 +363,15 @@ func (c *config) Branches(ctx context.Context, u *model.User, r *model.Repo, p *
 }
 
 // BranchHead returns the sha of the head (latest commit) of the specified branch
-func (c *config) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error) {
-	return c.newClient(ctx, u).GetBranchHead(r.Owner, r.Name, branch)
+func (c *config) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error) {
+	commit, err := c.newClient(ctx, u).GetBranchHead(r.Owner, r.Name, branch)
+	if err != nil {
+		return nil, err
+	}
+	return &model.Commit{
+		SHA:      commit.Hash,
+		ForgeURL: commit.Links.HTML.Href,
+	}, nil
 }
 
 // PullRequests returns the pull requests of the named repository.
diff --git a/server/forge/bitbucket/bitbucket_test.go b/server/forge/bitbucket/bitbucket_test.go
index 2afde4789..8488cc6aa 100644
--- a/server/forge/bitbucket/bitbucket_test.go
+++ b/server/forge/bitbucket/bitbucket_test.go
@@ -184,7 +184,8 @@ func Test_bitbucket(t *testing.T) {
 			g.It("Should return the details", func() {
 				branchHead, err := c.BranchHead(ctx, fakeUser, fakeRepo, "branch_name")
 				g.Assert(err).IsNil()
-				g.Assert(branchHead).Equal("branch_head_name")
+				g.Assert(branchHead.SHA).Equal("branch_head_name")
+				g.Assert(branchHead.ForgeURL).Equal("https://bitbucket.org/commitlink")
 			})
 			g.It("Should handle not found errors", func() {
 				_, err := c.BranchHead(ctx, fakeUser, fakeRepo, "branch_not_found")
diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go
index dd586fd12..a2a42d8aa 100644
--- a/server/forge/bitbucket/fixtures/handler.go
+++ b/server/forge/bitbucket/fixtures/handler.go
@@ -268,7 +268,12 @@ const branchCommitsPayload = `
 {
     "values": [
         {
-            "hash": "branch_head_name"
+            "hash": "branch_head_name",
+						"links": {
+							"html": {
+								"href": "https://bitbucket.org/commitlink"
+							}
+						}
         },
         {
             "hash": "random1"
diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go
index c7ca81b01..779029ca9 100644
--- a/server/forge/bitbucket/internal/client.go
+++ b/server/forge/bitbucket/internal/client.go
@@ -198,17 +198,17 @@ func (c *Client) ListBranches(owner, name string, opts *ListOpts) ([]*Branch, er
 	return out.Values, err
 }
 
-func (c *Client) GetBranchHead(owner, name, branch string) (string, error) {
+func (c *Client) GetBranchHead(owner, name, branch string) (*Commit, error) {
 	out := new(CommitsResp)
 	uri := fmt.Sprintf(pathBranchCommits, c.base, owner, name, branch)
 	_, err := c.do(uri, get, nil, out)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 	if len(out.Values) == 0 {
-		return "", fmt.Errorf("no commits in branch %s", branch)
+		return nil, fmt.Errorf("no commits in branch %s", branch)
 	}
-	return out.Values[0].Hash, nil
+	return out.Values[0], nil
 }
 
 func (c *Client) GetUserWorkspaceMembership(workspace, user string) (string, error) {
diff --git a/server/forge/bitbucket/internal/types.go b/server/forge/bitbucket/internal/types.go
index fa3376a74..e9d7ecc87 100644
--- a/server/forge/bitbucket/internal/types.go
+++ b/server/forge/bitbucket/internal/types.go
@@ -282,7 +282,12 @@ type CommitsResp struct {
 }
 
 type Commit struct {
-	Hash string `json:"hash"`
+	Hash  string `json:"hash"`
+	Links struct {
+		HTML struct {
+			Href string `json:"href"`
+		} `json:"html"`
+	} `json:"links"`
 }
 
 type DirResp struct {
diff --git a/server/forge/forge.go b/server/forge/forge.go
index a9f163bb5..a6fb65e3b 100644
--- a/server/forge/forge.go
+++ b/server/forge/forge.go
@@ -78,7 +78,7 @@ type Forge interface {
 	Branches(ctx context.Context, u *model.User, r *model.Repo, p *model.ListOptions) ([]string, error)
 
 	// BranchHead returns the sha of the head (latest commit) of the specified branch
-	BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error)
+	BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error)
 
 	// PullRequests returns all pull requests for the named repository.
 	PullRequests(ctx context.Context, u *model.User, r *model.Repo, p *model.ListOptions) ([]*model.PullRequest, error)
diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go
index 30673e4d3..f378e9783 100644
--- a/server/forge/gitea/gitea.go
+++ b/server/forge/gitea/gitea.go
@@ -463,18 +463,21 @@ func (c *Gitea) Branches(ctx context.Context, u *model.User, r *model.Repo, p *m
 }
 
 // BranchHead returns the sha of the head (latest commit) of the specified branch
-func (c *Gitea) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error) {
+func (c *Gitea) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error) {
 	token := common.UserToken(ctx, r, u)
 	client, err := c.newClientToken(ctx, token)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	b, _, err := client.GetRepoBranch(r.Owner, r.Name, branch)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return b.Commit.ID, nil
+	return &model.Commit{
+		SHA:      b.Commit.ID,
+		ForgeURL: b.Commit.URL,
+	}, nil
 }
 
 func (c *Gitea) PullRequests(ctx context.Context, u *model.User, r *model.Repo, p *model.ListOptions) ([]*model.PullRequest, error) {
diff --git a/server/forge/github/github.go b/server/forge/github/github.go
index fb89a7448..a05e6b3ab 100644
--- a/server/forge/github/github.go
+++ b/server/forge/github/github.go
@@ -565,13 +565,16 @@ func (c *client) Branches(ctx context.Context, u *model.User, r *model.Repo, p *
 }
 
 // BranchHead returns the sha of the head (latest commit) of the specified branch
-func (c *client) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error) {
+func (c *client) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error) {
 	token := common.UserToken(ctx, r, u)
 	b, _, err := c.newClientToken(ctx, token).Repositories.GetBranch(ctx, r.Owner, r.Name, branch, 1)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return b.GetCommit().GetSHA(), nil
+	return &model.Commit{
+		SHA:      b.GetCommit().GetSHA(),
+		ForgeURL: b.GetCommit().GetHTMLURL(),
+	}, nil
 }
 
 // Hook parses the post-commit hook from the Request body
diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go
index eb13aedce..8a59c08b1 100644
--- a/server/forge/gitlab/gitlab.go
+++ b/server/forge/gitlab/gitlab.go
@@ -607,24 +607,27 @@ func (g *GitLab) Branches(ctx context.Context, user *model.User, repo *model.Rep
 }
 
 // BranchHead returns the sha of the head (latest commit) of the specified branch
-func (g *GitLab) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error) {
+func (g *GitLab) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error) {
 	token := common.UserToken(ctx, r, u)
 	client, err := newClient(g.url, token, g.SkipVerify)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	_repo, err := g.getProject(ctx, client, r.ForgeRemoteID, r.Owner, r.Name)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
 	b, _, err := client.Branches.GetBranch(_repo.ID, branch, gitlab.WithContext(ctx))
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
-	return b.Commit.ID, nil
+	return &model.Commit{
+		SHA:      b.Commit.ID,
+		ForgeURL: b.Commit.WebURL,
+	}, nil
 }
 
 // Hook parses the post-commit hook from the Request body
diff --git a/server/forge/mocks/forge.go b/server/forge/mocks/forge.go
index 6fe891fd8..5d4627130 100644
--- a/server/forge/mocks/forge.go
+++ b/server/forge/mocks/forge.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.40.1. DO NOT EDIT.
+// Code generated by mockery v2.40.3. DO NOT EDIT.
 
 package mocks
 
@@ -66,22 +66,24 @@ func (_m *Forge) Auth(ctx context.Context, token string, secret string) (string,
 }
 
 // BranchHead provides a mock function with given fields: ctx, u, r, branch
-func (_m *Forge) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (string, error) {
+func (_m *Forge) BranchHead(ctx context.Context, u *model.User, r *model.Repo, branch string) (*model.Commit, error) {
 	ret := _m.Called(ctx, u, r, branch)
 
 	if len(ret) == 0 {
 		panic("no return value specified for BranchHead")
 	}
 
-	var r0 string
+	var r0 *model.Commit
 	var r1 error
-	if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo, string) (string, error)); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo, string) (*model.Commit, error)); ok {
 		return rf(ctx, u, r, branch)
 	}
-	if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo, string) string); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *model.User, *model.Repo, string) *model.Commit); ok {
 		r0 = rf(ctx, u, r, branch)
 	} else {
-		r0 = ret.Get(0).(string)
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*model.Commit)
+		}
 	}
 
 	if rf, ok := ret.Get(1).(func(context.Context, *model.User, *model.Repo, string) error); ok {
diff --git a/server/model/commit.go b/server/model/commit.go
new file mode 100644
index 000000000..2de74fc44
--- /dev/null
+++ b/server/model/commit.go
@@ -0,0 +1,6 @@
+package model
+
+type Commit struct {
+	SHA      string
+	ForgeURL string
+}
diff --git a/server/store/mocks/store.go b/server/store/mocks/store.go
index f4c60c593..aa3315925 100644
--- a/server/store/mocks/store.go
+++ b/server/store/mocks/store.go
@@ -1,4 +1,4 @@
-// Code generated by mockery v2.40.1. DO NOT EDIT.
+// Code generated by mockery v2.40.3. DO NOT EDIT.
 
 package mocks