From 9dd6be26dd459931da3ce5505e2f8d0be43b9245 Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Tue, 7 Feb 2023 15:48:10 +0700 Subject: [PATCH] simplify --- CHANGELOG.md | 2 +- lib/plausible/auth/invitation.ex | 2 +- lib/plausible/helpers/changeset.ex | 13 +++++++++++++ .../controllers/api/external_controller.ex | 14 ++------------ .../controllers/site/membership_controller.ex | 13 +++++++------ .../site/membership_controller_test.exs | 6 +++--- 6 files changed, 27 insertions(+), 23 deletions(-) create mode 100644 lib/plausible/helpers/changeset.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ea5e22db071..5c5ce9428160 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ All notable changes to this project will be documented in this file. - Fix breakdown API pagination when using event metrics plausible/analytics#2562 - Automatically update all visible dashboard reports in the realtime view - Connect via TLS when using HTTPS scheme in ClickHouse URL plausible/analytics#2570 -- Add more descriptive error message in case a transfer to an invited (but not joined) user is requested plausible/analytics#2651 +- Add error message in case a transfer to an invited (but not joined) user is requested plausible/analytics#2651 ### Changed - Reject events with long URIs and data URIs plausible/analytics#2536 diff --git a/lib/plausible/auth/invitation.ex b/lib/plausible/auth/invitation.ex index 5a24b11ba2ec..0250f9b4d6e6 100644 --- a/lib/plausible/auth/invitation.ex +++ b/lib/plausible/auth/invitation.ex @@ -22,7 +22,7 @@ defmodule Plausible.Auth.Invitation do |> unique_constraint([:email, :site_id], name: :invitations_site_id_email_index, error_key: :invitation, - message: "already sent" + message: "invitation already exists" ) end end diff --git a/lib/plausible/helpers/changeset.ex b/lib/plausible/helpers/changeset.ex new file mode 100644 index 000000000000..e7b6d88221ba --- /dev/null +++ b/lib/plausible/helpers/changeset.ex @@ -0,0 +1,13 @@ +defmodule Plausible.ChangesetHelpers do + @moduledoc "Helper function for working with Ecto changesets" + + def traverse_errors(changeset) do + Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} -> + Regex.replace(~r"%{(\w+)}", msg, fn _, key -> + opts + |> Keyword.get(String.to_existing_atom(key), key) + |> to_string() + end) + end) + end +end diff --git a/lib/plausible_web/controllers/api/external_controller.ex b/lib/plausible_web/controllers/api/external_controller.ex index 21ccf0a69625..7ba9fbe781e9 100644 --- a/lib/plausible_web/controllers/api/external_controller.ex +++ b/lib/plausible_web/controllers/api/external_controller.ex @@ -26,7 +26,7 @@ defmodule PlausibleWeb.Api.ExternalController do conn |> put_resp_header("x-plausible-dropped", "#{Enum.count(dropped)}") |> put_status(400) - |> json(%{errors: traverse_errors(first_invalid_changeset)}) + |> json(%{errors: Plausible.ChangesetHelpers.traverse_errors(first_invalid_changeset)}) else conn |> put_resp_header("x-plausible-dropped", "#{Enum.count(dropped)}") @@ -38,7 +38,7 @@ defmodule PlausibleWeb.Api.ExternalController do {:error, %Ecto.Changeset{} = changeset} -> conn |> put_status(400) - |> json(%{errors: traverse_errors(changeset)}) + |> json(%{errors: Plausible.ChangesetHelpers.traverse_errors(changeset)}) end end @@ -102,14 +102,4 @@ defmodule PlausibleWeb.Api.ExternalController do end end) end - - defp traverse_errors(changeset) do - Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} -> - Regex.replace(~r"%{(\w+)}", msg, fn _, key -> - opts - |> Keyword.get(String.to_existing_atom(key), key) - |> to_string() - end) - end) - end end diff --git a/lib/plausible_web/controllers/site/membership_controller.ex b/lib/plausible_web/controllers/site/membership_controller.ex index 0b47cdaa9872..0f95815487f1 100644 --- a/lib/plausible_web/controllers/site/membership_controller.ex +++ b/lib/plausible_web/controllers/site/membership_controller.ex @@ -136,16 +136,17 @@ defmodule PlausibleWeb.Site.MembershipController do put_flash(conn, :success, "Site transfer request has been sent to #{email}") {:error, changeset} -> - errors = Ecto.Changeset.traverse_errors(changeset, fn {msg, _} -> msg end) + errors = Plausible.ChangesetHelpers.traverse_errors(changeset) message = - if errors[:invitation] do - "#{email} has already been invited but hasn't yet accepted the join request to #{site_domain}" - else - "Site transfer request to #{email} has failed" + case errors do + %{invitation: [error | _]} -> String.capitalize(error) + _other -> "Site transfer request to #{email} has failed" end - put_flash(conn, :error, message) + conn + |> put_flash(:error_title, "Transfer error") + |> put_flash(:error, message) end redirect(conn, to: Routes.site_path(conn, :settings_people, site.domain)) diff --git a/test/plausible_web/controllers/site/membership_controller_test.exs b/test/plausible_web/controllers/site/membership_controller_test.exs index 34ea502b9f3f..51b64c6b13e1 100644 --- a/test/plausible_web/controllers/site/membership_controller_test.exs +++ b/test/plausible_web/controllers/site/membership_controller_test.exs @@ -230,9 +230,9 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do conn = post(conn, "/sites/#{site.domain}/transfer-ownership", %{email: invited}) conn = get(recycle(conn), redirected_to(conn, 302)) - - assert html_response(conn, 200) =~ - "#{invited} has already been invited but hasn't yet accepted the join request to #{site.domain}" + html = html_response(conn, 200) + assert html =~ "Transfer error" + assert html =~ "Invitation already exists" end end