Skip to content

Commit 805df2e

Browse files
committed
handle various claim lifecycle scenarios
1 parent 58b60d4 commit 805df2e

File tree

5 files changed

+395
-72
lines changed

5 files changed

+395
-72
lines changed

lib/algora/bounties/bounties.ex

Lines changed: 113 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -229,26 +229,41 @@ defmodule Algora.Bounties do
229229
status: status,
230230
type: type
231231
}) do
232-
with {:ok, user} <- Workspace.ensure_user(token, provider_login),
233-
activity_attrs = %{type: :claim_submitted, notify_users: [user.id]},
234-
{:ok, claim} <-
235-
Repo.insert_with_activity(
236-
Claim.changeset(%Claim{}, %{
237-
target_id: target.id,
238-
source_id: source.id,
239-
user_id: user.id,
240-
type: type,
241-
status: status,
242-
url: source.url,
243-
group_id: group_id,
244-
group_share: group_share
245-
}),
246-
activity_attrs
247-
) do
248-
{:ok, claim}
249-
else
250-
{:error, %{errors: [target_id: {_, [constraint: :unique, constraint_name: _]}]}} ->
251-
{:error, :already_exists}
232+
case Workspace.ensure_user(token, provider_login) do
233+
{:ok, user} ->
234+
activity_attrs = %{type: :claim_submitted, notify_users: [user.id]}
235+
236+
claim_attrs = %{
237+
target_id: target.id,
238+
source_id: source.id,
239+
user_id: user.id,
240+
type: type,
241+
status: status,
242+
url: source.url,
243+
group_id: group_id,
244+
group_share: group_share
245+
}
246+
247+
# Try to find existing claim
248+
existing_claim =
249+
Repo.get_by(Claim,
250+
target_id: target.id,
251+
source_id: source.id,
252+
user_id: user.id
253+
)
254+
255+
case existing_claim do
256+
nil ->
257+
# Create new claim
258+
%Claim{}
259+
|> Claim.changeset(claim_attrs)
260+
|> Repo.insert_with_activity(activity_attrs)
261+
262+
claim ->
263+
claim
264+
|> Claim.changeset(claim_attrs)
265+
|> Repo.update_with_activity(activity_attrs)
266+
end
252267

253268
{:error, _reason} = error ->
254269
error
@@ -306,7 +321,7 @@ defmodule Algora.Bounties do
306321
},
307322
opts :: [installation_id: integer()]
308323
) ::
309-
{:ok, Bounty.t()} | {:error, atom()}
324+
{:ok, [Claim.t()]} | {:error, atom()}
310325
def claim_bounty(
311326
%{
312327
user: user,
@@ -328,25 +343,89 @@ defmodule Algora.Bounties do
328343
Repo.transact(fn ->
329344
with {:ok, token} <- token_res,
330345
{:ok, target} <- Workspace.ensure_ticket(token, target_repo_owner, target_repo_name, target_number),
331-
{:ok, source} <- Workspace.ensure_ticket(token, source_repo_owner, source_repo_name, source_number),
332-
{:ok, [claim | _]} <-
333-
do_claim_bounties(%{
334-
provider_logins: [user.provider_login | coauthor_provider_logins],
335-
token: token,
336-
target: target,
337-
source: source,
338-
status: status,
339-
type: type
340-
}),
341-
{:ok, _job} <- notify_claim(%{claim: claim}, installation_id: installation_id) do
342-
broadcast()
343-
{:ok, claim}
346+
{:ok, source} <- Workspace.ensure_ticket(token, source_repo_owner, source_repo_name, source_number) do
347+
# Get all active claims for this PR
348+
active_claims = get_active_claims(source.id)
349+
requested_participants = [user.provider_login | coauthor_provider_logins]
350+
351+
cond do
352+
# Case 1: Target changed - cancel all claims and create new ones
353+
target_changed?(active_claims, target.id) ->
354+
with :ok <- cancel_all_claims(active_claims) do
355+
create_new_claims(token, source, target, requested_participants, status, type, installation_id)
356+
end
357+
358+
# Case 3: Participants changed - cancel old claims and create new ones
359+
participants_changed?(active_claims, requested_participants) ->
360+
with :ok <- cancel_all_claims(active_claims) do
361+
create_new_claims(token, source, target, requested_participants, status, type, installation_id)
362+
end
363+
364+
# Case 4: No existing claims - create new ones
365+
Enum.empty?(active_claims) ->
366+
create_new_claims(token, source, target, requested_participants, status, type, installation_id)
367+
368+
# Case 5: No changes needed
369+
true ->
370+
{:ok, active_claims}
371+
end
344372
else
345373
{:error, _reason} = error -> error
346374
end
347375
end)
348376
end
349377

378+
def get_active_claims(source_id) do
379+
Repo.all(
380+
from c in Claim,
381+
where: c.source_id == ^source_id,
382+
where: c.status != :cancelled,
383+
preload: [:user]
384+
)
385+
end
386+
387+
defp target_changed?(claims, target_id) do
388+
Enum.any?(claims, fn claim -> claim.target_id != target_id end)
389+
end
390+
391+
defp participants_changed?(claims, requested_participants) do
392+
current_participants =
393+
claims
394+
|> Enum.map(& &1.user.provider_login)
395+
|> Enum.sort()
396+
397+
requested_participants
398+
|> Enum.sort()
399+
|> Kernel.!=(current_participants)
400+
end
401+
402+
def cancel_all_claims(claims) do
403+
Enum.reduce_while(claims, :ok, fn claim, :ok ->
404+
case claim
405+
|> Claim.changeset(%{status: :cancelled})
406+
|> Repo.update() do
407+
{:ok, _} -> {:cont, :ok}
408+
error -> error
409+
end
410+
end)
411+
end
412+
413+
defp create_new_claims(token, source, target, participants, status, type, installation_id) do
414+
with {:ok, [claim | _] = claims} <-
415+
do_claim_bounties(%{
416+
provider_logins: participants,
417+
token: token,
418+
target: target,
419+
source: source,
420+
status: status,
421+
type: type
422+
}),
423+
{:ok, _job} <- notify_claim(%{claim: claim}, installation_id: installation_id) do
424+
broadcast()
425+
{:ok, claims}
426+
end
427+
end
428+
350429
@spec notify_claim(
351430
%{claim: Claim.t()},
352431
opts :: [installation_id: integer()]

lib/algora/workspace/workspace.ex

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,14 @@ defmodule Algora.Workspace do
1919
@type command_source :: :ticket | :comment
2020

2121
def ensure_ticket(token, owner, repo, number) do
22-
ticket_query =
22+
case get_ticket(owner, repo, number) do
23+
%Ticket{} = ticket -> {:ok, ticket}
24+
nil -> create_ticket_from_github(token, owner, repo, number)
25+
end
26+
end
27+
28+
def get_ticket(owner, repo, number) do
29+
Repo.one(
2330
from(t in Ticket,
2431
join: r in assoc(t, :repository),
2532
join: u in assoc(r, :user),
@@ -29,11 +36,7 @@ defmodule Algora.Workspace do
2936
where: u.provider_login == ^owner,
3037
preload: [repository: {r, user: u}]
3138
)
32-
33-
case Repo.one(ticket_query) do
34-
%Ticket{} = ticket -> {:ok, ticket}
35-
nil -> create_ticket_from_github(token, owner, repo, number)
36-
end
39+
)
3740
end
3841

3942
def create_ticket_from_github(token, owner, repo, number) do

lib/algora_web/controllers/webhooks/github_controller.ex

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ defmodule AlgoraWeb.Webhooks.GithubController do
4949

5050
def process_delivery(webhook) do
5151
with :ok <- ensure_human_author(webhook),
52-
:ok <- process_commands(webhook),
53-
:ok <- process_event(webhook) do
52+
{:ok, commands} <- process_commands(webhook),
53+
:ok <- process_event(webhook, commands) do
5454
Logger.debug("✅ #{inspect(webhook.event_action)}")
5555
:ok
5656
end
@@ -78,11 +78,14 @@ defmodule AlgoraWeb.Webhooks.GithubController do
7878
end
7979
end
8080

81-
defp process_event(%Webhook{event_action: "pull_request.closed", payload: %{"pull_request" => %{"merged_at" => nil}}}) do
81+
defp process_event(
82+
%Webhook{event_action: "pull_request.closed", payload: %{"pull_request" => %{"merged_at" => nil}}},
83+
_commands
84+
) do
8285
:ok
8386
end
8487

85-
defp process_event(%Webhook{event_action: "pull_request.closed", payload: payload}) do
88+
defp process_event(%Webhook{event_action: "pull_request.closed", payload: payload}, _commands) do
8689
with {:ok, token} <- Github.get_installation_token(payload["installation"]["id"]),
8790
{:ok, source} <-
8891
Workspace.ensure_ticket(
@@ -228,7 +231,31 @@ defmodule AlgoraWeb.Webhooks.GithubController do
228231
end
229232
end
230233

231-
defp process_event(_webhook), do: :ok
234+
defp process_event(%Webhook{event_action: event_action, payload: payload}, commands)
235+
when event_action in ["pull_request.opened", "pull_request.reopened", "pull_request.edited"] do
236+
if Enum.any?(commands, &match?({:claim, _}, &1)) do
237+
:ok
238+
else
239+
source =
240+
Workspace.get_ticket(
241+
payload["repository"]["owner"]["login"],
242+
payload["repository"]["name"],
243+
payload["pull_request"]["number"]
244+
)
245+
246+
case source do
247+
nil ->
248+
:ok
249+
250+
source ->
251+
source.id
252+
|> Bounties.get_active_claims()
253+
|> Bounties.cancel_all_claims()
254+
end
255+
end
256+
end
257+
258+
defp process_event(_webhook, _commands), do: :ok
232259

233260
defp execute_command(%Webhook{event_action: event_action, payload: payload} = webhook, {:bounty, args})
234261
when event_action in ["issues.opened", "issues.edited", "issue_comment.created", "issue_comment.edited"] do
@@ -397,34 +424,38 @@ defmodule AlgoraWeb.Webhooks.GithubController do
397424
{:ok,
398425
commands
399426
|> Enum.map(&build_command(&1, commands))
400-
|> Enum.reject(&is_nil/1)}
427+
|> Enum.reject(&is_nil/1)
428+
# keep only the first claim command if multiple claims are present
429+
|> Enum.reduce([], fn
430+
{:claim, _} = claim, acc -> if Enum.any?(acc, &match?({:claim, _}, &1)), do: acc, else: [claim | acc]
431+
command, acc -> [command | acc]
432+
end)
433+
|> Enum.reverse()}
401434

402435
{:error, reason} = error ->
403436
Logger.error("Error parsing commands: #{inspect(reason)}")
404437
error
405438
end
406439
end
407440

408-
defp process_commands(%Webhook{event_action: event_action, body: body, hook_id: hook_id} = webhook) do
409-
case build_commands(body) do
410-
{:ok, commands} ->
411-
Enum.reduce_while(commands, :ok, fn command, :ok ->
412-
case execute_command(webhook, command) do
413-
{:ok, _result} ->
414-
{:cont, :ok}
415-
416-
error ->
417-
Logger.error(
418-
"Command execution failed for #{event_action}(#{hook_id}): #{inspect(command)}: #{inspect(error)}"
419-
)
441+
defp process_commands(%Webhook{body: body} = webhook) do
442+
with {:ok, commands} <- build_commands(body),
443+
:ok <- execute_commands(webhook, commands) do
444+
{:ok, commands}
445+
end
446+
end
420447

421-
{:halt, error}
422-
end
423-
end)
448+
defp execute_commands(%Webhook{event_action: event_action, hook_id: hook_id} = webhook, commands) do
449+
Enum.reduce_while(commands, :ok, fn command, :ok ->
450+
case execute_command(webhook, command) do
451+
{:ok, _result} ->
452+
Logger.debug("✅ #{inspect(command)}")
453+
{:cont, :ok}
424454

425-
{:error, reason} = error ->
426-
Logger.error("Error parsing commands: #{inspect(reason)}")
427-
error
428-
end
455+
error ->
456+
Logger.error("Command execution failed for #{event_action}(#{hook_id}): #{inspect(command)}: #{inspect(error)}")
457+
{:halt, error}
458+
end
459+
end)
429460
end
430461
end

test/algora/bounties_test.exs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ defmodule Algora.BountiesTest do
77

88
alias Algora.Activities.Notifier
99
alias Algora.Activities.SendEmail
10-
alias Algora.Activities.Views
1110

1211
def setup_github_mocks(_context) do
1312
import Algora.Mocks.GithubMock
@@ -59,7 +58,7 @@ defmodule Algora.BountiesTest do
5958

6059
assert {:ok, bounty} = Algora.Bounties.create_bounty(bounty_params, [])
6160

62-
assert {:ok, claim} =
61+
assert {:ok, claims} =
6362
Algora.Bounties.claim_bounty(
6463
%{
6564
user: recipient,
@@ -72,15 +71,15 @@ defmodule Algora.BountiesTest do
7271
installation_id: installation.id
7372
)
7473

75-
claim = Algora.Repo.one(Algora.Bounties.Claim.preload(claim.id))
74+
claims = Repo.preload(claims, :user)
7675

7776
assert {:ok, _bounty} =
7877
Algora.Bounties.reward_bounty(
7978
%{
8079
owner: owner,
8180
amount: ~M[4000]usd,
8281
bounty_id: bounty.id,
83-
claims: [claim]
82+
claims: claims
8483
},
8584
installation_id: installation.id
8685
)
@@ -94,14 +93,14 @@ defmodule Algora.BountiesTest do
9493
recipient: recipient
9594
},
9695
ticket_ref: ticket_ref,
97-
claims: [claim]
96+
claims: claims
9897
)
9998

10099
assert_activity_names([:bounty_posted, :claim_submitted, :bounty_awarded, :tip_awarded])
101100
assert_activity_names_for_user(creator.id, [:bounty_posted, :bounty_awarded, :tip_awarded])
102101
assert_activity_names_for_user(recipient.id, [:claim_submitted, :tip_awarded])
103102

104-
assert [bounty, claim, awarded, tip] = Enum.reverse(Algora.Activities.all())
103+
assert [bounty, _claim, _awarded, tip] = Enum.reverse(Algora.Activities.all())
105104
assert "tip_activities" == tip.assoc_name
106105
assert tip.notify_users == [recipient.id]
107106
assert activity = Algora.Activities.get_with_preloaded_assoc(tip.assoc_name, tip.id)

0 commit comments

Comments
 (0)