From 4bb44f6ef774bcf41663b67039b712a38a33b2ae Mon Sep 17 00:00:00 2001 From: Zafer Cesur Date: Tue, 21 Jan 2025 15:34:19 +0300 Subject: [PATCH 1/3] fix: parse commands into keyword list to allow multiple commands with same name --- lib/algora/integrations/github/command.ex | 7 +-- test/algora/github/command_test.exs | 61 ++++++++++------------- 2 files changed, 27 insertions(+), 41 deletions(-) diff --git a/lib/algora/integrations/github/command.ex b/lib/algora/integrations/github/command.ex index 627b81fff..69f6cf4fe 100644 --- a/lib/algora/integrations/github/command.ex +++ b/lib/algora/integrations/github/command.ex @@ -76,13 +76,10 @@ defmodule Algora.Github.Command do def parse(input) when is_binary(input) do case parse_raw(input) do {:ok, [], _, _, _, _} -> - {:ok, %{}} + {:ok, []} {:ok, parsed, _, _, _, _} -> - {:ok, - parsed - |> Enum.reject(&is_nil/1) - |> Map.new()} + {:ok, Enum.reject(parsed, &is_nil/1)} {:error, reason, _, _, _, _} -> {:error, reason} diff --git a/test/algora/github/command_test.exs b/test/algora/github/command_test.exs index 91bdf548b..ebb8c7b11 100644 --- a/test/algora/github/command_test.exs +++ b/test/algora/github/command_test.exs @@ -7,95 +7,92 @@ defmodule Algora.Github.CommandTest do describe "parse/1 with bounty command" do test "parses simple bounty amount" do - assert {:ok, %{bounty: [{:amount, ~M[1000]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1000]usd}]]} == Command.parse("/bounty 1000") end test "parses bounty with EN thousand separators and decimal points" do - assert {:ok, %{bounty: [{:amount, ~M[1000.5]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1000.5]usd}]]} == Command.parse("/bounty 1,000.5") end test "parses bounty with EN thousand separators" do - assert {:ok, %{bounty: [{:amount, ~M[1_000_000]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1_000_000]usd}]]} == Command.parse("/bounty 1,000,000") end test "parses bounty with EN decimal points" do - assert {:ok, %{bounty: [{:amount, ~M[1000.50]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1000.50]usd}]]} == Command.parse("/bounty 1000.50") end test "parses bounty with DE thousand separators and decimal points" do - assert {:ok, %{bounty: [{:amount, ~M[1000.5]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1000.5]usd}]]} == Command.parse("/bounty 1.000,5") end test "parses bounty with DE thousand separators" do - assert {:ok, %{bounty: [{:amount, ~M[1_000_000]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1_000_000]usd}]]} == Command.parse("/bounty 1.000.000") end test "parses bounty with DE decimal points" do - assert {:ok, %{bounty: [{:amount, ~M[1000.50]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[1000.50]usd}]]} == Command.parse("/bounty 1000,50") end test "parses bounty with dollar sign" do - assert {:ok, %{bounty: [{:amount, ~M[50]usd}]}} == + assert {:ok, [bounty: [{:amount, ~M[50]usd}]]} == Command.parse("/bounty $50") end end describe "parse/1 with tip command" do test "parses tip with amount first" do - assert {:ok, %{tip: [{:amount, ~M[100]usd}, {:recipient, "user"}]}} == + assert {:ok, [tip: [{:amount, ~M[100]usd}, {:recipient, "user"}]]} == Command.parse("/tip 100 @user") end test "parses tip with recipient first" do - assert {:ok, %{tip: [{:recipient, "user"}, {:amount, ~M[100]usd}]}} == + assert {:ok, [tip: [{:recipient, "user"}, {:amount, ~M[100]usd}]]} == Command.parse("/tip @user 100") end test "parses tip with only amount" do - assert {:ok, %{tip: [{:amount, ~M[100]usd}]}} == + assert {:ok, [tip: [{:amount, ~M[100]usd}]]} == Command.parse("/tip 100") end test "parses tip with only recipient" do - assert {:ok, %{tip: [{:recipient, "user"}]}} == + assert {:ok, [tip: [{:recipient, "user"}]]} == Command.parse("/tip @user") end end describe "parse/1 with claim command" do test "parses simple issue number" do - assert {:ok, %{claim: [{:ticket_ref, [number: 123]}]}} == + assert {:ok, [claim: [{:ticket_ref, [number: 123]}]]} == Command.parse("/claim 123") end test "parses issue with hash" do - assert {:ok, %{claim: [{:ticket_ref, [number: 123]}]}} == + assert {:ok, [claim: [{:ticket_ref, [number: 123]}]]} == Command.parse("/claim #123") end test "parses repo issue reference" do - assert {:ok, %{claim: [{:ticket_ref, [repo: "repo", number: 123]}]}} == + assert {:ok, [claim: [{:ticket_ref, [repo: "repo", number: 123]}]]} == Command.parse("/claim repo#123") end test "parses full repo path" do - assert {:ok, %{claim: [{:ticket_ref, [owner: "owner", repo: "repo", number: 123]}]}} == + assert {:ok, [claim: [{:ticket_ref, [owner: "owner", repo: "repo", number: 123]}]]} == Command.parse("/claim owner/repo#123") end test "parses full GitHub URL for issues" do expected = - {:ok, - %{ - claim: [{:ticket_ref, [owner: "owner", repo: "repo", type: "issues", number: 123]}] - }} + {:ok, [claim: [{:ticket_ref, [owner: "owner", repo: "repo", type: "issues", number: 123]}]]} assert expected == Command.parse("/claim github.com/owner/repo/issues/123") assert expected == Command.parse("/claim http://github.com/owner/repo/issues/123") @@ -103,20 +100,12 @@ defmodule Algora.Github.CommandTest do end test "parses full GitHub URL for pull requests" do - assert {:ok, - %{ - claim: [{:ticket_ref, [owner: "owner", repo: "repo", type: "pull", number: 123]}] - }} == + assert {:ok, [claim: [{:ticket_ref, [owner: "owner", repo: "repo", type: "pull", number: 123]}]]} == Command.parse("/claim https://github.com/owner/repo/pull/123") end test "parses full GitHub URL for discussions" do - assert {:ok, - %{ - claim: [ - {:ticket_ref, [owner: "owner", repo: "repo", type: "discussions", number: 123]} - ] - }} == + assert {:ok, [claim: [{:ticket_ref, [owner: "owner", repo: "repo", type: "discussions", number: 123]}]]} == Command.parse("/claim https://github.com/owner/repo/discussions/123") end end @@ -124,28 +113,28 @@ defmodule Algora.Github.CommandTest do describe "parse/1 with multiple commands" do test "parses multiple commands in sequence" do assert {:ok, - %{ + [ bounty: [{:amount, Money.new!(100, :USD)}], tip: [{:amount, Money.new!(50, :USD)}, {:recipient, "user"}] - }} == Command.parse("/bounty 100 /tip 50 @user") + ]} == Command.parse("/bounty 100 /tip 50 @user") end test "handles text between commands" do assert {:ok, - %{ + [ bounty: [{:amount, Money.new!(100, :USD)}], tip: [{:recipient, "user"}, {:amount, Money.new!(50, :USD)}] - }} == Command.parse("Hello /bounty 100 world /tip @user 50") + ]} == Command.parse("Hello /bounty 100 world /tip @user 50") end end describe "parse/1 with invalid input" do test "returns empty list for invalid commands" do - assert {:ok, %{}} == Command.parse("/invalid") + assert {:ok, []} == Command.parse("/invalid") end test "returns empty list for no commands" do - assert {:ok, %{}} == Command.parse("just some text") + assert {:ok, []} == Command.parse("just some text") end test "returns error for malformed amounts" do From f97d3adda65d8c4496b12def704fead3c98f13e7 Mon Sep 17 00:00:00 2001 From: zafer Date: Tue, 21 Jan 2025 15:57:49 +0300 Subject: [PATCH 2/3] simplify --- lib/algora/integrations/github/command.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/algora/integrations/github/command.ex b/lib/algora/integrations/github/command.ex index 69f6cf4fe..0801f93b5 100644 --- a/lib/algora/integrations/github/command.ex +++ b/lib/algora/integrations/github/command.ex @@ -75,9 +75,6 @@ defmodule Algora.Github.Command do def parse(input) when is_binary(input) do case parse_raw(input) do - {:ok, [], _, _, _, _} -> - {:ok, []} - {:ok, parsed, _, _, _, _} -> {:ok, Enum.reject(parsed, &is_nil/1)} From 1fc426d5628faec5bf816fb61a709d78910623dd Mon Sep 17 00:00:00 2001 From: zafer Date: Tue, 21 Jan 2025 15:58:44 +0300 Subject: [PATCH 3/3] fix edge case --- lib/algora/integrations/github/command.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/algora/integrations/github/command.ex b/lib/algora/integrations/github/command.ex index 0801f93b5..d399f59f1 100644 --- a/lib/algora/integrations/github/command.ex +++ b/lib/algora/integrations/github/command.ex @@ -71,7 +71,7 @@ defmodule Algora.Github.Command do defparsec(:parse_raw, Helper.commands()) - def parse(nil), do: {:ok, %{}} + def parse(nil), do: {:ok, []} def parse(input) when is_binary(input) do case parse_raw(input) do