diff --git a/lib/mix/tasks/wnh.staging_full_e2e.ex b/lib/mix/tasks/wnh.staging_full_e2e.ex index 6f19f4d..d3dac13 100644 --- a/lib/mix/tasks/wnh.staging_full_e2e.ex +++ b/lib/mix/tasks/wnh.staging_full_e2e.ex @@ -10,6 +10,7 @@ defmodule Mix.Tasks.Wnh.StagingFullE2e do alias WhoNeedHelp.Catalog.{CategoryProposal, CategoryVote} alias WhoNeedHelp.Help.{Assignment, HelpRequest} alias WhoNeedHelp.Messaging.Message + alias WhoNeedHelp.Notifications.Notification alias WhoNeedHelp.Repo alias WhoNeedHelp.Tracking.{Position, TrackingSession} @@ -149,6 +150,7 @@ defmodule Mix.Tasks.Wnh.StagingFullE2e do message_ids = ids(Message, :assignment_id, assignment_ids) tracking_session_ids = ids(TrackingSession, :assignment_id, assignment_ids) + notification_ids = ids(Notification, :user_id, user_ids) activity_ids = ids(Activity, :creator_id, user_ids) validate_activity_membership!(activity_ids, user_ids) @@ -192,12 +194,13 @@ defmodule Mix.Tasks.Wnh.StagingFullE2e do signals: abuse_signal_ids }) - job_ids = fixture_push_job_ids(user_ids) + job_ids = fixture_job_ids(user_ids, request_ids, notification_ids) {:ok, deleted} = Repo.transaction(fn -> %{ push_jobs: delete_ids(Job, job_ids), + notifications: delete_ids(Notification, notification_ids), audit_events: AuditEvent |> where([event], event.actor_id in ^user_ids) @@ -384,14 +387,15 @@ defmodule Mix.Tasks.Wnh.StagingFullE2e do end end - defp fixture_push_job_ids(user_ids) do + defp fixture_job_ids(user_ids, request_ids, notification_ids) do recipients = Enum.map(user_ids, &"user:#{&1}") Job |> where( [job], - job.worker == "WhoNeedHelp.Push.DeliveryWorker" and - fragment("?->>'recipient' = ANY(?)", job.args, ^recipients) + fragment("?->>'recipient' = ANY(?)", job.args, ^recipients) or + fragment("?->>'request_id' = ANY(?)", job.args, ^request_ids) or + fragment("?->>'notification_id' = ANY(?)", job.args, ^notification_ids) ) |> select([job], job.id) |> Repo.all() diff --git a/lib/who_need_help/help.ex b/lib/who_need_help/help.ex index 63caa73..c8c5643 100644 --- a/lib/who_need_help/help.ex +++ b/lib/who_need_help/help.ex @@ -289,7 +289,13 @@ defmodule WhoNeedHelp.Help do end with {:ok, request} <- result do - _ = ProductAnalytics.increment("request.created", to_string(request.urgency)) + _ = + ProductAnalytics.increment_for_user( + user, + "request.created", + to_string(request.urgency) + ) + broadcast({:request_created, get_request!(request.id)}) {:ok, request} end @@ -364,7 +370,7 @@ defmodule WhoNeedHelp.Help do case result do {:ok, assignment} -> - _ = ProductAnalytics.increment("request.accepted") + _ = ProductAnalytics.increment_for_user(helper, "request.accepted") request = get_request!(assignment.request_id) broadcast({:request_updated, request}) @@ -482,7 +488,11 @@ defmodule WhoNeedHelp.Help do |> case do {:ok, request} -> _ = - ProductAnalytics.increment("request.cancelled", to_string(request.cancellation_reason)) + ProductAnalytics.increment_for_user( + user, + "request.cancelled", + to_string(request.cancellation_reason) + ) WhoNeedHelp.Tracking.cleanup_finished_sessions() request = get_request!(request.id) @@ -541,7 +551,7 @@ defmodule WhoNeedHelp.Help do |> after_transition() |> case do {:ok, _assignment} = result -> - _ = ProductAnalytics.increment("assignment.withdrawn") + _ = ProductAnalytics.increment_for_user(user, "assignment.withdrawn") WhoNeedHelp.Tracking.cleanup_finished_sessions() result @@ -749,7 +759,7 @@ defmodule WhoNeedHelp.Help do :error -> {:error, :not_found} end |> after_transition() - |> record_assignment_metric(action) + |> record_assignment_metric(user, action) end defp maybe_complete(changeset, request) do @@ -952,22 +962,26 @@ defmodule WhoNeedHelp.Help do defp maybe_notify_completion(_assignment, _request_id), do: {:ok, :not_completed} - defp record_assignment_metric({:ok, _assignment} = result, :start) do - _ = ProductAnalytics.increment("assignment.started") + defp record_assignment_metric({:ok, _assignment} = result, user, :start) do + _ = ProductAnalytics.increment_for_user(user, "assignment.started") result end - defp record_assignment_metric({:ok, _assignment} = result, :arrive) do - _ = ProductAnalytics.increment("assignment.arrived") + defp record_assignment_metric({:ok, _assignment} = result, user, :arrive) do + _ = ProductAnalytics.increment_for_user(user, "assignment.arrived") result end - defp record_assignment_metric({:ok, %Assignment{status: :completed}} = result, :confirm) do - _ = ProductAnalytics.increment("request.completed") + defp record_assignment_metric( + {:ok, %Assignment{status: :completed}} = result, + user, + :confirm + ) do + _ = ProductAnalytics.increment_for_user(user, "request.completed") result end - defp record_assignment_metric(result, _action), do: result + defp record_assignment_metric(result, _user, _action), do: result defp broadcast(event) do Phoenix.PubSub.broadcast(WhoNeedHelp.PubSub, @topic, event) diff --git a/lib/who_need_help/notifications.ex b/lib/who_need_help/notifications.ex index 6ee9f16..af5ea21 100644 --- a/lib/who_need_help/notifications.ex +++ b/lib/who_need_help/notifications.ex @@ -105,11 +105,12 @@ defmodule WhoNeedHelp.Notifications do :ok end - def notification_opened(%Scope{} = scope, notification_id) do + def notification_opened(%Scope{user: user} = scope, notification_id) do case mark_read(scope, notification_id) do {:ok, notification} = result -> _ = - WhoNeedHelp.ProductAnalytics.increment( + WhoNeedHelp.ProductAnalytics.increment_for_user( + user, "notification.opened", to_string(notification.kind) ) diff --git a/lib/who_need_help/product_analytics.ex b/lib/who_need_help/product_analytics.ex index e9abd8f..c03fc08 100644 --- a/lib/who_need_help/product_analytics.ex +++ b/lib/who_need_help/product_analytics.ex @@ -31,6 +31,7 @@ defmodule WhoNeedHelp.ProductAnalytics do } @allowed_metrics Map.keys(@allowed_dimensions) + @synthetic_email_suffix "@example.invalid" def increment(metric, dimension \\ "all") @@ -57,6 +58,18 @@ defmodule WhoNeedHelp.ProductAnalytics do def increment(_metric, _dimension), do: {:error, :invalid_metric} + def increment_for_user(user, metric, dimension \\ "all") + + def increment_for_user(%{email: email}, metric, dimension) when is_binary(email) do + if synthetic_email?(email) do + {:ok, :synthetic_account_skipped} + else + increment(metric, dimension) + end + end + + def increment_for_user(_user, _metric, _dimension), do: {:error, :invalid_user} + def paginate(%Scope{user: user}, options \\ []) do if Accounts.moderator_authorized?(user) do limit = Pagination.limit(options) @@ -75,6 +88,13 @@ defmodule WhoNeedHelp.ProductAnalytics do def allowed_metrics, do: @allowed_metrics + defp synthetic_email?(email) do + email + |> String.trim() + |> String.downcase() + |> String.ends_with?(@synthetic_email_suffix) + end + defp before(query, nil), do: query defp before(query, {inserted_at, id}) do diff --git a/lib/who_need_help_web/controllers/google_auth_controller.ex b/lib/who_need_help_web/controllers/google_auth_controller.ex index 479a4df..dcc8d0e 100644 --- a/lib/who_need_help_web/controllers/google_auth_controller.ex +++ b/lib/who_need_help_web/controllers/google_auth_controller.ex @@ -98,7 +98,7 @@ defmodule WhoNeedHelpWeb.GoogleAuthController do "locale" => normalize_locale(params["locale"] || pending.locale), "terms_accepted" => true }) do - _ = ProductAnalytics.increment("account.registered", "google") + _ = ProductAnalytics.increment_for_user(user, "account.registered", "google") conn |> GoogleAuthPending.delete() diff --git a/lib/who_need_help_web/controllers/user_registration_controller.ex b/lib/who_need_help_web/controllers/user_registration_controller.ex index d3c22e6..f9f16dd 100644 --- a/lib/who_need_help_web/controllers/user_registration_controller.ex +++ b/lib/who_need_help_web/controllers/user_registration_controller.ex @@ -27,7 +27,7 @@ defmodule WhoNeedHelpWeb.UserRegistrationController do result <- Accounts.register_user(user_params) do case result do {:ok, user} -> - _ = ProductAnalytics.increment("account.registered", "email") + _ = ProductAnalytics.increment_for_user(user, "account.registered", "email") deliver_registration_instructions(conn, user) registration_response(conn) diff --git a/test/who_need_help/product_analytics_test.exs b/test/who_need_help/product_analytics_test.exs index 652fc41..d3d2246 100644 --- a/test/who_need_help/product_analytics_test.exs +++ b/test/who_need_help/product_analytics_test.exs @@ -31,4 +31,17 @@ defmodule WhoNeedHelp.ProductAnalyticsTest do assert [%DailyMetric{metric: "request.created", count: 2}] = ProductAnalytics.paginate(moderator).entries end + + test "does not count actions from reserved synthetic fixture accounts" do + synthetic_user = %{email: "browser-run@example.invalid"} + + assert {:ok, :synthetic_account_skipped} = + ProductAnalytics.increment_for_user( + synthetic_user, + "request.created", + "now" + ) + + refute Repo.get_by(DailyMetric, metric: "request.created", dimension: "now") + end end