fix(github_hooks): populate payloads with info about commit author (#260)

## 📝 Description

This update enhances the payloads to include additional information
required to populate `SEMAPHORE_GIT_COMMIT_AUTHOR` when a workflow is
triggered via the API.

It addresses [this
issue](renderedtext/tasks#7744) and is needed
for [this
task](renderedtext/on-prem-overview#235).

## ✅ Checklist
- [x] I have tested this change
- [x] This change requires documentation update - N/A

Author: dexyk

github_hooks/lib/internal_api/repo_proxy/branch_payload.rb

@@ -12,19 +12,23 @@ def call(project, user)
 
       reference = repo_host.reference(project.repo_owner_and_name, ref.delete_prefix("refs/"))
 
-      commit = repo_host.commit(project.repo_owner_and_name, commit_sha(sha, reference))
+      branch_commit = repo_host.commit(project.repo_owner_and_name, commit_sha(sha, reference))
 
-      repo_url = commit[:html_url].split("/").first(5).join("/")
+      repo_url = branch_commit[:html_url].split("/").first(5).join("/")
       author_name  = user.github_repo_host_account.name
       author_email = user.email
       github_uid = user.github_repo_host_account.github_uid
       avatar = ::Avatar.avatar_url(github_uid)
 
       commit = {
-        "message" => commit.commit.message,
-        "id" => commit.sha,
-        "url" => commit.html_url,
-        "author" => { "name" => "", "email" => "" },
+        "message" => branch_commit.commit.message,
+        "id" => branch_commit.sha,
+        "url" => branch_commit.html_url,
+        "author" => {
+          "name" => branch_commit.commit.author&.name || "",
+          "email" => branch_commit.commit.author&.email || "",
+          "username" => branch_commit.author&.login
+        },
         "timestamp" => ""
       }
 

github_hooks/lib/internal_api/repo_proxy/pr_payload.rb

@@ -19,6 +19,21 @@ def call(project, user)
         raise PrNotMergeableError, "Pull Request ##{number} is not mergeable (#{pr[:html_url]})"
       end
 
+      repo_host = ::RepoHost::Factory.create_from_project(project)
+      pr_commit = repo_host.commit(project.repo_owner_and_name, pr[:head][:sha])
+
+      commit = {
+        "message" => pr_commit.commit.message,
+        "id" => pr_commit.sha,
+        "url" => pr_commit.html_url,
+        "author" => {
+          "name" => pr_commit.commit.author&.name || "",
+          "email" => pr_commit.commit.author&.email || "",
+          "username" => pr_commit.author&.login
+        },
+        "timestamp" => ""
+      }
+
       repo_url = pr[:html_url].split("/").first(5).join("/")
       author_name  = user.github_repo_host_account.name
       author_email = user.email
@@ -38,6 +53,7 @@ def call(project, user)
           "base" => pr["base"].to_h,
           "head" => pr["head"].to_h
         },
+        "commits" => [commit],
         "single" => true,
         "created" => true,
         "after" => "",

github_hooks/lib/internal_api/repo_proxy/tag_payload.rb

@@ -9,19 +9,23 @@ def call(project, user)
 
       reference = repo_host.reference(project.repo_owner_and_name, ref.delete_prefix("refs/"))
 
-      commit = repo_host.commit(project.repo_owner_and_name, commit_sha(reference, repo_host, project))
+      tag_commit = repo_host.commit(project.repo_owner_and_name, commit_sha(reference, repo_host, project))
 
-      repo_url = commit[:html_url].split("/").first(5).join("/")
+      repo_url = tag_commit[:html_url].split("/").first(5).join("/")
       author_name  = user.github_repo_host_account.name
       author_email = user.email
       github_uid = user.github_repo_host_account.github_uid
       avatar = ::Avatar.avatar_url(github_uid)
 
       commit = {
-        "message" => commit.commit.message,
-        "id" => commit.sha,
-        "url" => commit.html_url,
-        "author" => { "name" => "", "email" => "" },
+        "message" => tag_commit.commit.message,
+        "id" => tag_commit.sha,
+        "url" => tag_commit.html_url,
+        "author" => {
+          "name" => tag_commit.commit.author&.name || "",
+          "email" => tag_commit.commit.author&.email || "",
+          "username" => tag_commit.author&.login
+        },
         "timestamp" => ""
       }
 

github_hooks/spec/lib/internal_api/repo_proxy/branch_payload_spec.rb

@@ -0,0 +1,108 @@
+require "spec_helper"
+
+GitAuthor = Struct.new(:name, :email, :login)
+GitCommitData = Struct.new(:message, :author)
+GitCommit = Struct.new(:sha, :html_url, :commit, :author) do
+  def [](key)
+    html_url if key == :html_url
+  end
+end
+
+BranchRef = Struct.new(:ref, :object) do
+  def [](key)
+    return ref    if key == :ref
+    return object if key == :object
+
+    nil
+  end
+end
+
+RepoHostBranchStub = Struct.new(:branch_commit) do
+  def reference(*_args)
+    BranchRef.new("heads/main", { sha: branch_commit.sha, type: "commit" })
+  end
+
+  def commit(*_args)
+    branch_commit
+  end
+end
+
+RSpec.describe InternalApi::RepoProxy::BranchPayload do
+  let(:ref)    { "refs/heads/main" }
+  let(:sha)    { "abc123" }
+  let(:project) { instance_double(Project, repo_owner_and_name: "owner/repo") }
+  let(:user) do
+    host_account = Struct.new(:name, :github_uid, :login)
+                         .new("Alice", 123, "alice")
+    Struct.new(:github_repo_host_account, :email)
+          .new(host_account, "[email protected]")
+  end
+
+  let(:branch_commit) do
+    author      = GitAuthor.new("Alice", "[email protected]", "alice")
+    commit_data = GitCommitData.new("Branch commit", author)
+    GitCommit.new(
+      sha,
+      "https://github.com/owner/repo/commit/#{sha}",
+      commit_data,
+      author
+    )
+  end
+
+  let(:repo_host) { RepoHostBranchStub.new(branch_commit) }
+
+  before do
+    allow(::RepoHost::Factory).to receive(:create_from_project).and_return(repo_host)
+    allow(::Avatar).to receive(:avatar_url).and_return("http://avatar.url")
+  end
+
+  describe "#call" do
+    subject(:payload) { described_class.new(ref, sha).call(project, user) }
+
+    it "returns a payload hash with expected keys" do
+      expect(payload).to include(
+        "ref" => "heads/main",
+        "single" => true,
+        "created" => true,
+        "head_commit" => a_kind_of(Hash),
+        "commits" => [a_kind_of(Hash)],
+        "repository" => hash_including("html_url", "full_name"),
+        "pusher" => hash_including("name", "email"),
+        "sender" => hash_including("id", "avatar_url")
+      )
+    end
+
+    it "sets author and commit details correctly" do
+      commit = payload["commits"].last
+      expect(commit["author"]["name"]).to eq("Alice")
+      expect(commit["author"]["email"]).to eq("[email protected]")
+      expect(commit["author"]["username"]).to eq("alice")
+      expect(commit["id"]).to eq(sha)
+      expect(commit["message"]).to eq("Branch commit")
+    end
+  end
+
+  describe "#commit_sha" do
+    it "returns sha if it matches SHA_REGEXP" do
+      obj = described_class.new(ref, sha)
+      result = obj.send(:commit_sha, sha, BranchRef.new("heads/main", { sha: sha, type: "commit" }))
+      expect(result).to eq(sha)
+    end
+
+    it "returns reference sha if type is commit and sha is not valid" do
+      bad_sha = "notasha"
+      ref_obj = { ref: "heads/main", object: { sha: "def456", type: "commit" } }
+      obj = described_class.new(ref, bad_sha)
+      result = obj.send(:commit_sha, bad_sha, ref_obj)
+      expect(result).to eq("def456")
+    end
+
+    it "returns nil if neither condition matches" do
+      bad_sha = "notasha"
+      ref_obj = { ref: "heads/main", object: { sha: "def456", type: "tag" } }
+      obj = described_class.new(ref, bad_sha)
+      result = obj.send(:commit_sha, bad_sha, ref_obj)
+      expect(result).to be_nil
+    end
+  end
+end

github_hooks/spec/lib/internal_api/repo_proxy/pr_payload_spec.rb

@@ -0,0 +1,114 @@
+require "spec_helper"
+
+GitAuthor = Struct.new(:name, :email, :login)
+GitCommitData = Struct.new(:message, :author)
+GitCommit = Struct.new(:sha, :html_url, :commit, :author) do
+  def [](key)
+    html_url if key == :html_url
+  end
+end
+
+RepoHostStub = Struct.new(:pr_commit) do
+  def commit(*_args)
+    pr_commit
+  end
+end
+
+RSpec.describe InternalApi::RepoProxy::PrPayload do
+  let(:ref)      { "refs/pull/1/head" }
+  let(:number)   { 1 }
+  let(:project)  { instance_double(Project, repo_owner_and_name: "owner/repo") }
+  let(:user) do
+    host_account = Struct.new(:name, :github_uid, :login).new("Alice", 123, "alice")
+    Struct.new(:github_repo_host_account, :email)
+          .new(host_account, "[email protected]")
+  end
+
+  let(:pr_commit) do
+    author      = GitAuthor.new("Alice", "[email protected]", "alice")
+    commit_data = GitCommitData.new("PR commit", author)
+    GitCommit.new(
+      "abc123",
+      "https://github.com/owner/repo/commit/abc123",
+      commit_data,
+      author
+    )
+  end
+
+  let(:repo_host) { RepoHostStub.new(pr_commit) }
+
+  let(:pr) do
+    {
+      "number" => 1,
+      head: { sha: "abc123", ref: "feature-branch", repo: { full_name: "owner/repo" } },
+      base: { ref: "main", repo: { full_name: "owner/repo" } },
+      title: "A PR",
+      commits_url: "https://api.github.com/repos/owner/repo/pulls/1/commits",
+      html_url: "https://github.com/owner/repo/pull/1"
+    }
+  end
+  let(:meta) { { pr: pr, ref: ref, merge_commit_sha: "abc123", commit_author: "Alice" } }
+
+  before do
+    allow(::RepoHost::Factory).to receive(:create_from_project).and_return(repo_host)
+    allow(::Avatar).to receive(:avatar_url).and_return("http://avatar.url")
+  end
+
+  describe "#call" do
+    subject(:payload) { described_class.new(ref, number).call(project, user) }
+
+    before do
+      allow(Semaphore::RepoHost::Hooks::Handler)
+        .to receive(:update_pr_data)
+        .and_return([:ok, meta, ""])
+    end
+
+    it "returns a payload hash with expected keys" do
+      expect(payload).to include(
+        "number" => 1,
+        "pull_request" => a_kind_of(Hash),
+        "commits" => [a_kind_of(Hash)],
+        "repository" => hash_including("html_url", "full_name"),
+        "pusher" => hash_including("name", "email"),
+        "sender" => hash_including("id", "avatar_url", "login")
+      )
+    end
+
+    it "sets author and commit details correctly" do
+      commit = payload["commits"].last
+      expect(commit["author"]["name"]).to eq("Alice")
+      expect(commit["author"]["email"]).to eq("[email protected]")
+      expect(commit["author"]["username"]).to eq("alice")
+      expect(commit["id"]).to eq("abc123")
+      expect(commit["message"]).to eq("PR commit")
+    end
+  end
+
+  context "when PR is not found" do
+    before do
+      allow(Semaphore::RepoHost::Hooks::Handler)
+        .to receive(:update_pr_data)
+        .and_return([:not_found, {}, "not found"])
+    end
+
+    it "raises PrNotMergeableError" do
+      expect do
+        described_class.new(ref, number).call(project, user)
+      end.to raise_error(described_class::PrNotMergeableError, /not found/i)
+    end
+  end
+
+  context "when PR is not mergeable" do
+    before do
+      allow(Semaphore::RepoHost::Hooks::Handler)
+        .to receive(:update_pr_data)
+        .and_return([:non_mergeable, { pr: pr }, "not mergeable"])
+    end
+
+    it "raises PrNotMergeableError" do
+      expect do
+        described_class.new(ref, number).call(project, user)
+      end.to raise_error(described_class::PrNotMergeableError, /not mergeable/i)
+    end
+  end
+end