Skip to content

Commit

Permalink
Fix account deletion (#398)
Browse files Browse the repository at this point in the history
Why
---
The FxA broker doesn’t wait for a response before re-trying an event.
This means event-handling needs to be idempotent.  Account deletion is
not currently idempotent.  This is leading to needless retries and a
noisy log.

What
----
* Replace `Dash.fxa_uid_to_deleted_list!/1` with the idempotent
  `fxa_uid_to_deleted_list/1`
  • Loading branch information
bryanenders authored Jul 26, 2023
1 parent d7a34f3 commit 9dcb078
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
6 changes: 3 additions & 3 deletions lib/dash.ex
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,9 @@ defmodule Dash do
:ok
end

@spec fxa_uid_to_deleted_list!(String.t()) :: :ok
def fxa_uid_to_deleted_list!(fxa_uid) when is_binary(fxa_uid) do
Dash.Repo.insert!(%Dash.DeletedFxaAccount{fxa_uid: fxa_uid})
@spec fxa_uid_to_deleted_list(String.t()) :: :ok
def fxa_uid_to_deleted_list(fxa_uid) when is_binary(fxa_uid) do
Dash.Repo.insert(%Dash.DeletedFxaAccount{fxa_uid: fxa_uid})
:ok
end

Expand Down
4 changes: 2 additions & 2 deletions lib/dash_web/fxa_events.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ defmodule DashWeb.FxaEvents do
nil ->
Logger.warn("FxA account deletion error: No account for fxa_uid to delete")

Dash.fxa_uid_to_deleted_list!(fxa_uid)
Dash.fxa_uid_to_deleted_list(fxa_uid)
:ok

%Dash.Account{} ->
Dash.Account.delete_account_and_hubs(account)
Dash.fxa_uid_to_deleted_list!(fxa_uid)
Dash.fxa_uid_to_deleted_list(fxa_uid)
end
end

Expand Down
6 changes: 3 additions & 3 deletions test/dash_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ defmodule DashTest do
describe "was_deleted?/1" do
test "if on deleted list, should return true" do
fxa_uid = "fxa_uid_test"
Dash.fxa_uid_to_deleted_list!(fxa_uid)
Dash.fxa_uid_to_deleted_list(fxa_uid)

assert true === Dash.was_deleted?(fxa_uid)
end
Expand All @@ -107,12 +107,12 @@ defmodule DashTest do
end
end

describe "fxa_uid_to_deleted_list!/1" do
describe "fxa_uid_to_deleted_list/1" do
test "adds fxa_uid to the deleted list" do
fxa_uid = "fxa_uid_test"
false = Dash.was_deleted?(fxa_uid)

Dash.fxa_uid_to_deleted_list!(fxa_uid)
Dash.fxa_uid_to_deleted_list(fxa_uid)
assert Dash.was_deleted?(fxa_uid)
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/plugs/auth_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ defmodule DashWeb.Plugs.AuthTest do
describe "account was deleted tests" do
test "returns 401, if account was deleted", %{conn: conn} do
fxa_uid = get_default_test_uid()
Dash.fxa_uid_to_deleted_list!(fxa_uid)
Dash.fxa_uid_to_deleted_list(fxa_uid)

conn =
conn
Expand Down

0 comments on commit 9dcb078

Please sign in to comment.