From ee0b5c08f3ef88623dfe97be7a5b8ea3d10b669c Mon Sep 17 00:00:00 2001 From: zafer Date: Sat, 11 Jan 2025 17:49:07 +0300 Subject: [PATCH 1/2] feat: handle bounty creation via GitHub app --- lib/algora/bounties/bounties.ex | 59 ++++++++++----- lib/algora/bounties/jobs/notify_bounty.ex | 33 ++++++++- lib/algora/integrations/github/client.ex | 6 +- lib/algora/workspace/workspace.ex | 6 ++ .../controllers/webhooks/github_controller.ex | 71 ++++++++----------- 5 files changed, 112 insertions(+), 63 deletions(-) diff --git a/lib/algora/bounties/bounties.ex b/lib/algora/bounties/bounties.ex index cebcbae5e..444e4981a 100644 --- a/lib/algora/bounties/bounties.ex +++ b/lib/algora/bounties/bounties.ex @@ -10,6 +10,7 @@ defmodule Algora.Bounties do alias Algora.Bounties.Jobs alias Algora.Bounties.Tip alias Algora.FeeTier + alias Algora.Github alias Algora.MoneyUtils alias Algora.Organizations.Member alias Algora.Payments @@ -50,7 +51,6 @@ defmodule Algora.Bounties do case Repo.insert(changeset) do {:ok, bounty} -> - broadcast() {:ok, bounty} {:error, %{errors: [ticket_id: {_, [constraint: :unique, constraint_name: _]}]}} -> @@ -61,24 +61,39 @@ defmodule Algora.Bounties do end end - @spec create_bounty(%{ - creator: User.t(), - owner: User.t(), - amount: Money.t(), - ticket_ref: %{owner: String.t(), repo: String.t(), number: integer()} - }) :: + @spec create_bounty( + %{ + creator: User.t(), + owner: User.t(), + amount: Money.t(), + ticket_ref: %{owner: String.t(), repo: String.t(), number: integer()} + }, + opts :: [installation_id: integer()] + ) :: {:ok, Bounty.t()} | {:error, atom()} - def create_bounty(%{ - creator: creator, - owner: owner, - amount: amount, - ticket_ref: %{owner: repo_owner, repo: repo_name, number: number} = ticket_ref - }) do + def create_bounty( + %{ + creator: creator, + owner: owner, + amount: amount, + ticket_ref: %{owner: repo_owner, repo: repo_name, number: number} = ticket_ref + }, + opts \\ [] + ) do + installation_id = opts[:installation_id] + + token_res = + if installation_id, + do: Github.get_installation_token(installation_id), + else: Accounts.get_access_token(creator) + Repo.transact(fn -> - with {:ok, token} <- Accounts.get_access_token(creator), + with {:ok, token} <- token_res, {:ok, ticket} <- Workspace.ensure_ticket(token, repo_owner, repo_name, number), {:ok, bounty} <- do_create_bounty(%{creator: creator, owner: owner, amount: amount, ticket: ticket}), - {:ok, _job} <- notify_bounty(%{owner: owner, bounty: bounty, ticket_ref: ticket_ref}) do + {:ok, _job} <- + notify_bounty(%{owner: owner, bounty: bounty, ticket_ref: ticket_ref}, installation_id: installation_id) do + broadcast() {:ok, bounty} else {:error, _reason} = error -> error @@ -86,11 +101,21 @@ defmodule Algora.Bounties do end) end - def notify_bounty(%{owner: owner, bounty: bounty, ticket_ref: ticket_ref}) do + @spec notify_bounty( + %{ + owner: User.t(), + bounty: Bounty.t(), + ticket_ref: %{owner: String.t(), repo: String.t(), number: integer()} + }, + opts :: [installation_id: integer()] + ) :: + {:ok, Oban.Job.t()} | {:error, atom()} + def notify_bounty(%{owner: owner, bounty: bounty, ticket_ref: ticket_ref}, opts \\ []) do %{ owner_login: owner.provider_login, amount: Money.to_string!(bounty.amount, no_fraction_if_integer: true), - ticket_ref: %{owner: ticket_ref.owner, repo: ticket_ref.repo, number: ticket_ref.number} + ticket_ref: %{owner: ticket_ref.owner, repo: ticket_ref.repo, number: ticket_ref.number}, + installation_id: opts[:installation_id] } |> Jobs.NotifyBounty.new() |> Oban.insert() diff --git a/lib/algora/bounties/jobs/notify_bounty.ex b/lib/algora/bounties/jobs/notify_bounty.ex index 71c867a72..cebde955b 100644 --- a/lib/algora/bounties/jobs/notify_bounty.ex +++ b/lib/algora/bounties/jobs/notify_bounty.ex @@ -2,12 +2,17 @@ defmodule Algora.Bounties.Jobs.NotifyBounty do @moduledoc false use Oban.Worker, queue: :notify_bounty + alias Algora.Accounts + alias Algora.Accounts.User alias Algora.Github + alias Algora.Workspace require Logger @impl Oban.Worker - def perform(%Oban.Job{args: %{"owner_login" => owner_login, "amount" => amount, "ticket_ref" => ticket_ref}}) do + def perform(%Oban.Job{ + args: %{"owner_login" => owner_login, "amount" => amount, "ticket_ref" => ticket_ref, "installation_id" => nil} + }) do body = """ 💎 **#{owner_login}** is offering a **#{amount}** bounty for this issue @@ -31,4 +36,30 @@ defmodule Algora.Bounties.Jobs.NotifyBounty do """) end end + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"amount" => amount, "ticket_ref" => ticket_ref, "installation_id" => installation_id}}) do + with {:ok, token} <- Github.get_installation_token(installation_id), + {:ok, installation} <- + Workspace.fetch_installation_by(provider: "github", provider_id: to_string(installation_id)), + {:ok, owner} <- Accounts.fetch_user_by(id: installation.connected_user_id) do + body = """ + ## 💎 #{amount} bounty [• #{owner.name}](#{User.url(owner)}) + ### Steps to solve: + 1. **Start working**: Comment `/attempt ##{ticket_ref["number"]}` with your implementation plan + 2. **Submit work**: Create a pull request including `/claim ##{ticket_ref["number"]}` in the PR body to claim the bounty + 3. **Receive payment**: 100% of the bounty is received 2-5 days post-reward. [Make sure you are eligible for payouts](https://docs.algora.io/bounties/payments#supported-countries-regions) + + Thank you for contributing to #{ticket_ref["owner"]}/#{ticket_ref["repo"]}! + """ + + Github.create_issue_comment( + token, + ticket_ref["owner"], + ticket_ref["repo"], + ticket_ref["number"], + body + ) + end + end end diff --git a/lib/algora/integrations/github/client.ex b/lib/algora/integrations/github/client.ex index c91937785..3703f72db 100644 --- a/lib/algora/integrations/github/client.ex +++ b/lib/algora/integrations/github/client.ex @@ -155,9 +155,9 @@ defmodule Algora.Github.Client do def get_installation_token(installation_id) do path = "/app/installations/#{installation_id}/access_tokens" - case Crypto.generate_jwt() do - {:ok, jwt, _claims} -> fetch(jwt, path, "POST") - error -> error + with {:ok, jwt, _claims} <- Crypto.generate_jwt(), + {:ok, %{"token" => token}} <- fetch(jwt, path, "POST") do + {:ok, token} end end diff --git a/lib/algora/workspace/workspace.ex b/lib/algora/workspace/workspace.ex index f7aa8feba..134476c60 100644 --- a/lib/algora/workspace/workspace.ex +++ b/lib/algora/workspace/workspace.ex @@ -126,6 +126,12 @@ defmodule Algora.Workspace do end end + @spec fetch_installation_by(clauses :: Keyword.t() | map()) :: + {:ok, Installation.t()} | {:error, :not_found} + def fetch_installation_by(clauses) do + Repo.fetch_by(Installation, clauses) + end + def get_installation_by(fields), do: Repo.get_by(Installation, fields) def get_installation_by!(fields), do: Repo.get_by!(Installation, fields) diff --git a/lib/algora_web/controllers/webhooks/github_controller.ex b/lib/algora_web/controllers/webhooks/github_controller.ex index 0be25d935..6bcf05f79 100644 --- a/lib/algora_web/controllers/webhooks/github_controller.ex +++ b/lib/algora_web/controllers/webhooks/github_controller.ex @@ -1,8 +1,11 @@ defmodule AlgoraWeb.Webhooks.GithubController do use AlgoraWeb, :controller + alias Algora.Accounts + alias Algora.Bounties alias Algora.Github alias Algora.Github.Webhook + alias Algora.Workspace require Logger @@ -32,8 +35,9 @@ defmodule AlgoraWeb.Webhooks.GithubController do end # TODO: cache installation tokens + # TODO: check org permissions on algora defp get_permissions(author, %{"repository" => repository, "installation" => installation}) do - with {:ok, %{"token" => access_token}} <- Github.get_installation_token(installation["id"]), + with {:ok, access_token} <- Github.get_installation_token(installation["id"]), {:ok, %{"permission" => permission}} <- Github.get_repository_permissions( access_token, @@ -48,49 +52,32 @@ defmodule AlgoraWeb.Webhooks.GithubController do defp get_permissions(_author, _params), do: {:error, :invalid_params} defp execute_command({:bounty, args}, author, params) do - amount = Keyword.fetch!(args, :amount) - - case get_permissions(author, params) do - {:ok, "admin"} -> - # Get repository and issue details from params - repo = params["repository"] - issue = params["issue"] - - # Construct the bounty message - message = """ - ## 💎 $#{amount} bounty [• #{repo["owner"]["login"]}](https://console.algora.io/org/#{repo["owner"]["login"]}) - ### Steps to solve: - 1. **Start working**: Comment `/attempt ##{issue["number"]}` with your implementation plan - 2. **Submit work**: Create a pull request including `/claim ##{issue["number"]}` in the PR body to claim the bounty - 3. **Receive payment**: 100% of the bounty is received 2-5 days post-reward. [Make sure you are eligible for payouts](https://docs.algora.io/bounties/payments#supported-countries-regions) - - Thank you for contributing to #{repo["full_name"]}! - - **[Add a bounty](https://console.algora.io/org/#{repo["owner"]["login"]}/bounties/community?fund=#{repo["full_name"]}%23#{issue["number"]})** • **[Share on socials](https://twitter.com/intent/tweet?text=%24#{amount}+bounty%21+%F0%9F%92%8E+#{issue["html_url"]}&related=algoraio)** - - Attempt | Started (GMT+0) | Solution - --------|----------------|---------- - """ - - # Post comment to the issue - with {:ok, %{"token" => token}} <- - Github.get_installation_token(params["installation"]["id"]) do - Github.create_issue_comment( - token, - repo["owner"]["login"], - repo["name"], - issue["number"], - message - ) - end - - {:ok, amount} - + amount = args[:amount] + repo = params["repository"] + issue = params["issue"] + installation_id = params["installation"]["id"] + + with {:ok, "admin"} <- get_permissions(author, params), + {:ok, token} <- Github.get_installation_token(installation_id), + {:ok, installation} <- + Workspace.fetch_installation_by(provider: "github", provider_id: to_string(installation_id)), + {:ok, owner} <- Accounts.fetch_user_by(id: installation.connected_user_id), + {:ok, creator} <- Workspace.ensure_user(token, repo["owner"]["login"]) do + Bounties.create_bounty( + %{ + creator: creator, + owner: owner, + amount: amount, + ticket_ref: %{owner: repo["owner"]["login"], repo: repo["name"], number: issue["number"]} + }, + installation_id: installation_id + ) + else {:ok, _permission} -> {:error, :unauthorized} - {:error, error} -> - {:error, error} + {:error, _reason} = error -> + error end end @@ -113,7 +100,7 @@ defmodule AlgoraWeb.Webhooks.GithubController do do: Logger.info("Unhandled command: #{command} #{inspect(args)}") def process_commands(body, author, params) when is_binary(body) do - case Algora.Github.Command.parse(body) do + case Github.Command.parse(body) do {:ok, commands} -> Enum.map(commands, &execute_command(&1, author, params)) # TODO: handle errors {:error, error} -> Logger.error("Error parsing commands: #{inspect(error)}") From 7afd8f480f17dc5a66a3f11cab8d3104800d01e0 Mon Sep 17 00:00:00 2001 From: zafer Date: Sat, 11 Jan 2025 18:20:42 +0300 Subject: [PATCH 2/2] fix tests --- .../webhooks/github_controller_test.exs | 89 ++++++++++++++++--- test/support/factory.ex | 18 +++- 2 files changed, 93 insertions(+), 14 deletions(-) diff --git a/test/algora_web/controllers/webhooks/github_controller_test.exs b/test/algora_web/controllers/webhooks/github_controller_test.exs index 35c9dfbfd..6b3070230 100644 --- a/test/algora_web/controllers/webhooks/github_controller_test.exs +++ b/test/algora_web/controllers/webhooks/github_controller_test.exs @@ -1,6 +1,7 @@ defmodule AlgoraWeb.Webhooks.GithubControllerTest do use AlgoraWeb.ConnCase + import Algora.Factory import Money.Sigil import Mox @@ -19,65 +20,84 @@ defmodule AlgoraWeb.Webhooks.GithubControllerTest do "owner" => %{"login" => @repo_owner}, "name" => @repo_name }, + "issue" => %{ + "number" => 123 + }, "installation" => %{ "id" => @installation_id } } + setup do + admin = insert!(:user, handle: @admin_user) + org = insert!(:organization, handle: @repo_owner) + + installation = + insert!(:installation, %{ + owner: admin, + connected_user: org, + provider_id: to_string(@installation_id) + }) + + %{admin: admin, org: org, installation: installation} + end + describe "bounty command" do setup [:setup_github_mocks] @tag user: @unauthorized_user test "handles bounty command with unauthorized user", %{user: user} do - assert [error: :unauthorized] == process_bounty_command("/bounty $100", user) + assert process_bounty_command("/bounty $100", user)[:ok] == nil + assert process_bounty_command("/bounty $100", user)[:error] == :unauthorized end test "handles bounty command without amount" do - assert [] == process_bounty_command("/bounty") + assert process_bounty_command("/bounty")[:ok] == nil + assert process_bounty_command("/bounty")[:error] == nil end test "handles valid bounty command with $ prefix" do - assert [ok: ~M[100]usd] == process_bounty_command("/bounty $100") + assert process_bounty_command("/bounty $100")[:ok].amount == ~M[100]usd end test "handles invalid bounty command with $ suffix" do - assert [ok: ~M[100]usd] == process_bounty_command("/bounty 100$") + assert process_bounty_command("/bounty 100$")[:ok].amount == ~M[100]usd end test "handles bounty command without $ symbol" do - assert [ok: ~M[100]usd] == process_bounty_command("/bounty 100") + assert process_bounty_command("/bounty 100")[:ok].amount == ~M[100]usd end test "handles bounty command with decimal amount" do - assert [ok: ~M[100.50]usd] == process_bounty_command("/bounty 100.50") + assert process_bounty_command("/bounty 100.50")[:ok].amount == ~M[100.50]usd end test "handles bounty command with partial decimal amount" do - assert [ok: ~M[100.5]usd] == process_bounty_command("/bounty 100.5") + assert process_bounty_command("/bounty 100.5")[:ok].amount == ~M[100.5]usd end test "handles bounty command with decimal amount and $ prefix" do - assert [ok: ~M[100.50]usd] == process_bounty_command("/bounty $100.50") + assert process_bounty_command("/bounty $100.50")[:ok].amount == ~M[100.50]usd end test "handles bounty command with partial decimal amount and $ prefix" do - assert [ok: ~M[100.5]usd] == process_bounty_command("/bounty $100.5") + assert process_bounty_command("/bounty $100.5")[:ok].amount == ~M[100.5]usd end test "handles bounty command with decimal amount and $ suffix" do - assert [ok: ~M[100.50]usd] == process_bounty_command("/bounty 100.50$") + assert process_bounty_command("/bounty 100.50$")[:ok].amount == ~M[100.50]usd end test "handles bounty command with partial decimal amount and $ suffix" do - assert [ok: ~M[100.5]usd] == process_bounty_command("/bounty 100.5$") + assert process_bounty_command("/bounty 100.5$")[:ok].amount == ~M[100.5]usd end test "handles bounty command with comma separator" do - assert [ok: ~M[1000]usd] == process_bounty_command("/bounty 1,000") + assert process_bounty_command("/bounty 1,000")[:ok].amount == ~M[1000]usd end test "handles bounty command with comma separator and decimal amount" do - assert [ok: ~M[1000.50]usd] == process_bounty_command("/bounty 1,000.50") + assert process_bounty_command("/bounty 1,000.50")[:ok].amount == ~M[1000.50]usd end end @@ -85,6 +105,9 @@ defmodule AlgoraWeb.Webhooks.GithubControllerTest do setup_installation_token() setup_repository_permissions(context[:user] || @admin_user) setup_create_issue_comment() + setup_get_user_by_username() + setup_get_issue() + setup_get_repository() :ok end @@ -120,6 +143,46 @@ defmodule AlgoraWeb.Webhooks.GithubControllerTest do ) end + defp setup_get_user_by_username do + stub( + Algora.GithubMock, + :get_user_by_username, + fn _token, username -> {:ok, %{"id" => 123, "login" => username}} end + ) + end + + defp setup_get_issue do + stub( + Algora.GithubMock, + :get_issue, + fn _token, owner, repo, issue_number -> + {:ok, + %{ + "id" => 123, + "number" => issue_number, + "title" => "Test Issue", + "body" => "Test body", + "html_url" => "https://github.com/#{owner}/#{repo}/issues/#{issue_number}" + }} + end + ) + end + + defp setup_get_repository do + stub( + Algora.GithubMock, + :get_repository, + fn _token, owner, repo -> + {:ok, + %{ + "id" => 123, + "name" => repo, + "html_url" => "https://github.com/#{owner}/#{repo}" + }} + end + ) + end + # Helper function to process bounty commands defp process_bounty_command(body, author \\ @admin_user) do full_body = """ diff --git a/test/support/factory.ex b/test/support/factory.ex index 02c0eb9a7..0a8dda7f6 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -4,6 +4,7 @@ defmodule Algora.Factory do alias Algora.Accounts.User alias Algora.Reviews.Review + alias Algora.Workspace.Installation def identity_factory do %Algora.Accounts.Identity{ @@ -197,7 +198,7 @@ defmodule Algora.Factory do %Algora.Bounties.Claim{ id: Nanoid.generate(), provider: "github", - provider_id: sequence(:privider_id, &"#{&1}"), + provider_id: sequence(:provider_id, &"#{&1}"), type: :code, status: :pending, title: "Implemented compression optimization", @@ -215,6 +216,21 @@ defmodule Algora.Factory do } end + def installation_factory do + %Installation{ + id: Nanoid.generate(), + provider: "github", + provider_id: sequence(:provider_id, &"#{&1}"), + provider_user_id: sequence(:provider_user_id, &"#{&1}"), + provider_meta: %{ + "account" => %{"avatar_url" => "https://algora.io/asset/storage/v1/object/public/mock/piedpiper-logo.png"}, + "repository_selection" => "selected" + }, + avatar_url: "https://algora.io/asset/storage/v1/object/public/mock/piedpiper-logo.png", + repository_selection: "selected" + } + end + # Convenience API def insert!(factory_name, attributes \\ []) do insert(factory_name, attributes)