From 328096eb8f7128c8ccda31cffc711a21b3bced24 Mon Sep 17 00:00:00 2001 From: SimpleTest Date: Mon, 20 Jul 2026 18:38:29 +0300 Subject: [PATCH] fix: harden realtime forms and blocked activity privacy --- lib/who_need_help/activities.ex | 12 ++ lib/who_need_help/trust.ex | 107 +++++++++-------- lib/who_need_help_web/form_params.ex | 19 +++ .../live/activity_live/new.ex | 5 + .../live/activity_live/show.ex | 20 +++- .../live/category_proposal_live.ex | 10 +- .../live/request_live/new.ex | 5 + .../live/request_live/show.ex | 21 +++- test/who_need_help/activities_test.exs | 12 ++ test/who_need_help/mutual_aid_flow_test.exs | 3 + .../live/mutual_aid_live_test.exs | 111 +++++++++++++++++- 11 files changed, 269 insertions(+), 56 deletions(-) create mode 100644 lib/who_need_help_web/form_params.ex diff --git a/lib/who_need_help/activities.ex b/lib/who_need_help/activities.ex index 027088b..805cde7 100644 --- a/lib/who_need_help/activities.ex +++ b/lib/who_need_help/activities.ex @@ -102,6 +102,18 @@ defmodule WhoNeedHelp.Activities do participant.activity_id == activity.id and participant.user_id == ^user.id and participant.status in [:requested, :approved] ) + |> where( + [activity], + activity.creator_id not in subquery( + from block in Block, where: block.blocker_id == ^user.id, select: block.blocked_id + ) + ) + |> where( + [activity], + activity.creator_id not in subquery( + from block in Block, where: block.blocked_id == ^user.id, select: block.blocker_id + ) + ) |> before_my_activity(cursor) |> order_by([activity], desc: activity.starts_at, desc: activity.id) |> limit(^(limit + 1)) diff --git a/lib/who_need_help/trust.ex b/lib/who_need_help/trust.ex index 795389e..775018a 100644 --- a/lib/who_need_help/trust.ex +++ b/lib/who_need_help/trust.ex @@ -48,61 +48,76 @@ defmodule WhoNeedHelp.Trust do attrs = stringify_keys(attrs) with {:ok, _limit} <- authorize_action(scope, :review) do - Repo.transact(fn -> - current = - Assignment - |> where([current], current.id == ^assignment.id) - |> lock("FOR UPDATE") - |> Repo.one() + result = + Repo.transact(fn -> + current = + Assignment + |> where([current], current.id == ^assignment.id) + |> lock("FOR UPDATE") + |> Repo.one() - if current do - request = Repo.get!(HelpRequest, current.request_id) + if current do + request = Repo.get!(HelpRequest, current.request_id) - cond do - current.status != :completed -> - {:error, :forbidden} + cond do + current.status != :completed -> + {:error, :forbidden} - user.id not in [current.helper_id, request.requester_id] -> - {:error, :forbidden} + user.id not in [current.helper_id, request.requester_id] -> + {:error, :forbidden} - true -> - reviewee_id = - if user.id == current.helper_id, - do: request.requester_id, - else: current.helper_id + true -> + reviewee_id = + if user.id == current.helper_id, + do: request.requester_id, + else: current.helper_id - with {:ok, review} <- - %Review{} - |> Review.changeset( - Map.merge(attrs, %{ - "assignment_id" => current.id, - "reviewer_id" => user.id, - "reviewee_id" => reviewee_id - }) - ) - |> Repo.insert(), - reviews <- Repo.all(from r in Review, where: r.assignment_id == ^current.id), - true <- length(reviews) <= 2, - revealed? <- length(reviews) == 2, - {_count, _rows} <- - maybe_reveal_reviews(current.id, revealed?), - {:ok, _audit} <- - audit(user.id, "review.submitted", "assignment", current.id, %{ - "revealed" => revealed? - }) do - {:ok, review} - else - false -> {:error, :invalid_review_count} - other -> other - end + with {:ok, review} <- + %Review{} + |> Review.changeset( + Map.merge(attrs, %{ + "assignment_id" => current.id, + "reviewer_id" => user.id, + "reviewee_id" => reviewee_id + }) + ) + |> Repo.insert(), + reviews <- Repo.all(from r in Review, where: r.assignment_id == ^current.id), + true <- length(reviews) <= 2, + revealed? <- length(reviews) == 2, + {_count, _rows} <- + maybe_reveal_reviews(current.id, revealed?), + {:ok, _audit} <- + audit(user.id, "review.submitted", "assignment", current.id, %{ + "revealed" => revealed? + }) do + {:ok, review} + else + false -> {:error, :invalid_review_count} + other -> other + end + end + else + {:error, :not_found} end - else - {:error, :not_found} - end - end) + end) + + with {:ok, review} <- result do + :ok = Help.notify_request_updated(assignment.request_id) + {:ok, review} + end end end + def review_submitted?(%Scope{user: %User{id: user_id}}, %Assignment{id: assignment_id}) do + Repo.exists?( + from review in Review, + where: review.assignment_id == ^assignment_id and review.reviewer_id == ^user_id + ) + end + + def review_submitted?(_scope, _assignment), do: false + defp maybe_reveal_reviews(assignment_id, true) do Repo.update_all(from(r in Review, where: r.assignment_id == ^assignment_id), set: [revealed_at: DateTime.utc_now(:second)] diff --git a/lib/who_need_help_web/form_params.ex b/lib/who_need_help_web/form_params.ex new file mode 100644 index 0000000..efeaf47 --- /dev/null +++ b/lib/who_need_help_web/form_params.ex @@ -0,0 +1,19 @@ +defmodule WhoNeedHelpWeb.FormParams do + @moduledoc false + + @live_view_unused_prefix "_unused_" + + def drop_unused_structured_fields(%{"structured_data" => data} = params) + when is_map(data) do + data = + Map.reject(data, fn {key, _value} -> + key + |> to_string() + |> String.starts_with?(@live_view_unused_prefix) + end) + + Map.put(params, "structured_data", data) + end + + def drop_unused_structured_fields(params), do: params +end diff --git a/lib/who_need_help_web/live/activity_live/new.ex b/lib/who_need_help_web/live/activity_live/new.ex index e54c991..0040f6d 100644 --- a/lib/who_need_help_web/live/activity_live/new.ex +++ b/lib/who_need_help_web/live/activity_live/new.ex @@ -3,6 +3,7 @@ defmodule WhoNeedHelpWeb.ActivityLive.New do alias WhoNeedHelp.{Activities, Catalog} alias WhoNeedHelp.Activities.Activity + alias WhoNeedHelpWeb.FormParams @impl true def mount(_params, _session, socket) do @@ -17,6 +18,8 @@ defmodule WhoNeedHelpWeb.ActivityLive.New do @impl true def handle_event("validate", %{"activity" => params}, socket) do + params = FormParams.drop_unused_structured_fields(params) + changeset = %Activity{} |> Activities.change_activity(normalize_datetimes(params)) @@ -29,6 +32,8 @@ defmodule WhoNeedHelpWeb.ActivityLive.New do end def handle_event("save", %{"activity" => params}, socket) do + params = FormParams.drop_unused_structured_fields(params) + case Activities.create_activity(socket.assigns.current_scope, normalize_datetimes(params)) do {:ok, activity} -> {:noreply, diff --git a/lib/who_need_help_web/live/activity_live/show.ex b/lib/who_need_help_web/live/activity_live/show.ex index 20d6275..e1e7182 100644 --- a/lib/who_need_help_web/live/activity_live/show.ex +++ b/lib/who_need_help_web/live/activity_live/show.ex @@ -80,7 +80,10 @@ defmodule WhoNeedHelpWeb.ActivityLive.Show do params ) do {:ok, message} -> - {:noreply, put_realtime_message(socket, message)} + {:noreply, + socket + |> put_realtime_message(message) + |> push_event("reset-message-form", %{id: "activity-message-form"})} {:error, reason} -> {:noreply, put_flash(socket, :error, error_message(reason))} @@ -120,7 +123,8 @@ defmodule WhoNeedHelpWeb.ActivityLive.Show do socket |> assign(:report_form, report_form()) |> assign(:report_message_id, nil) - |> put_flash(:info, gettext("Report sent to moderators."))} + |> put_flash(:info, gettext("Report sent to moderators.")) + |> push_event("reset-message-form", %{id: "activity-report-form"})} {:error, reason} -> {:noreply, put_flash(socket, :error, error_message(reason))} @@ -527,7 +531,10 @@ defmodule WhoNeedHelpWeb.ActivityLive.Show do

- <.form for={@report_form} phx-submit="report" class="mt-4 space-y-2"> + <.form + for={@report_form} + id="activity-report-form" + phx-submit="report" + class="mt-4 space-y-2" + >
put_flash(:info, gettext("Proposal published for community voting.")) - |> load()} + |> load() + |> push_event("reset-message-form", %{id: "category-proposal-form"})} {:error, %Ecto.Changeset{} = changeset} -> {:noreply, assign(socket, :form, to_form(changeset))} @@ -85,7 +86,12 @@ defmodule WhoNeedHelpWeb.CategoryProposalLive do

{gettext("Proposals and votes guide moderators. Approval remains a human decision.")}

- <.form for={@form} phx-submit="propose" class="mt-7 space-y-4 rounded-3xl bg-base-200 p-6"> + <.form + for={@form} + id="category-proposal-form" + phx-submit="propose" + class="mt-7 space-y-4 rounded-3xl bg-base-200 p-6" + > <.input field={@form[:proposed_name]} label={gettext("Proposed category")} diff --git a/lib/who_need_help_web/live/request_live/new.ex b/lib/who_need_help_web/live/request_live/new.ex index 97c2729..9b45498 100644 --- a/lib/who_need_help_web/live/request_live/new.ex +++ b/lib/who_need_help_web/live/request_live/new.ex @@ -4,6 +4,7 @@ defmodule WhoNeedHelpWeb.RequestLive.New do alias WhoNeedHelp.Catalog alias WhoNeedHelp.Help alias WhoNeedHelp.Help.HelpRequest + alias WhoNeedHelpWeb.FormParams @impl true def mount(_params, _session, socket) do @@ -28,6 +29,8 @@ defmodule WhoNeedHelpWeb.RequestLive.New do @impl true def handle_event("validate", %{"help_request" => params}, socket) do + params = FormParams.drop_unused_structured_fields(params) + changeset = %HelpRequest{} |> Help.change_request(normalize_expiry(params)) @@ -40,6 +43,8 @@ defmodule WhoNeedHelpWeb.RequestLive.New do end def handle_event("save", %{"help_request" => params}, socket) do + params = FormParams.drop_unused_structured_fields(params) + case Help.create_request(socket.assigns.current_scope, normalize_expiry(params)) do {:ok, request} -> {:noreply, diff --git a/lib/who_need_help_web/live/request_live/show.ex b/lib/who_need_help_web/live/request_live/show.ex index 3120a01..5577e72 100644 --- a/lib/who_need_help_web/live/request_live/show.ex +++ b/lib/who_need_help_web/live/request_live/show.ex @@ -258,7 +258,8 @@ defmodule WhoNeedHelpWeb.RequestLive.Show do socket |> assign(:report_form, report_form()) |> assign(:report_message_id, nil) - |> put_flash(:info, gettext("Report sent to moderators."))} + |> put_flash(:info, gettext("Report sent to moderators.")) + |> push_event("reset-message-form", %{id: "request-report-form"})} {:error, reason} -> {:noreply, put_flash(socket, :error, message(reason))} @@ -422,6 +423,7 @@ defmodule WhoNeedHelpWeb.RequestLive.Show do ) |> assign(:assignment, assignment) |> assign(:participant, participant) + |> assign(:review_submitted, Trust.review_submitted?(socket.assigns.current_scope, assignment)) |> assign(:other_user_id, other_user_id) |> assign( :blocked_by_current, @@ -816,7 +818,15 @@ defmodule WhoNeedHelpWeb.RequestLive.Show do

{gettext("Your review is revealed only after both participants submit.")}

- <.form for={@review_form} phx-submit="review" class="mt-4 space-y-3"> +

+ {gettext("Review saved. It appears after both participants review.")} +

+ <.form + :if={!@review_submitted} + for={@review_form} + phx-submit="review" + class="mt-4 space-y-3" + > <.input field={@review_form[:rating]} type="select" @@ -1068,7 +1078,12 @@ defmodule WhoNeedHelpWeb.RequestLive.Show do > {gettext("Unblock this user")} - <.form for={@report_form} phx-submit="report" class="mt-4 space-y-2"> + <.form + for={@report_form} + id="request-report-form" + phx-submit="report" + class="mt-4 space-y-2" + >
"Kind" }) + assert Trust.review_submitted?(context.requester_scope, assignment) + refute Trust.review_submitted?(context.helper_scope, assignment) assert Trust.visible_reviews(context.helper.id) == [] {:ok, _} = Trust.submit_review(context.helper_scope, assignment, %{"rating" => 5, "comment" => "Clear"}) + assert Trust.review_submitted?(context.helper_scope, assignment) assert [%{rating: 5}] = Trust.visible_reviews(context.helper.id) assert [%{rating: 5}] = Trust.visible_reviews(context.requester.id) end diff --git a/test/who_need_help_web/live/mutual_aid_live_test.exs b/test/who_need_help_web/live/mutual_aid_live_test.exs index 5264700..25b2fe3 100644 --- a/test/who_need_help_web/live/mutual_aid_live_test.exs +++ b/test/who_need_help_web/live/mutual_aid_live_test.exs @@ -199,6 +199,20 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do assert html =~ "Medicine pickup status" assert html =~ "Pickup and deliver a legal medicine" assert html =~ "help_request[structured_data][pickup_status]" + + html = + render_change(view, "validate", %{ + "help_request" => %{ + "category_id" => category.id, + "structured_data" => %{ + "_unused_pickup_status" => "", + "pickup_status" => "" + } + } + }) + + assert html =~ "pickup_status is required" + refute html =~ "_unused_pickup_status is not allowed" end test "category descriptions use the current Ukrainian locale", %{conn: conn} do @@ -273,6 +287,20 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do assert html =~ "activity[structured_data][route_plan]" assert html =~ "activity[structured_data][safety_plan_confirmed]" assert html =~ "approve participants individually" + + html = + render_change(view, "validate", %{ + "activity" => %{ + "category_id" => hiking.id, + "structured_data" => %{ + "_unused_route_plan" => "", + "route_plan" => "" + } + } + }) + + assert html =~ "route_plan is required" + refute html =~ "_unused_route_plan is not allowed" end test "organizer approves an activity participant and group chat updates live", %{ @@ -354,8 +382,17 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do |> render_submit() assert html =~ "I will be there." + assert_push_event(participant_view, "reset-message-form", %{id: "activity-message-form"}) assert render(organizer_view) =~ "I will be there." + participant_view + |> form("#activity-report-form", + report: %{reason: "harassment", details: "Review this activity context."} + ) + |> render_submit() + + assert_push_event(participant_view, "reset-message-form", %{id: "activity-report-form"}) + now = DateTime.utc_now(:second) for index <- 1..51 do @@ -384,6 +421,13 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do refute bounded_activity_html =~ "oldest-realtime-activity-message" assert bounded_activity_html =~ "newest-realtime-activity-message" + organizer_view + |> element("button[phx-click='complete']") + |> render_click() + + assert render(participant_view) =~ "Completed" + refute has_element?(participant_view, "button[phx-click='leave']") + stop_live_view(organizer_view) stop_live_view(participant_view) end @@ -561,12 +605,77 @@ defmodule WhoNeedHelpWeb.MutualAidLiveTest do requester = user_fixture(display_name: "Requester") {:ok, request} = Help.create_request(user_scope_fixture(requester), request_attrs(category)) - {:ok, _view, html} = live(conn, ~p"/requests/#{request.id}") + {:ok, view, html} = live(conn, ~p"/requests/#{request.id}") assert html =~ "Safety controls" assert html =~ "Block this user" assert html =~ "Send report" assert html =~ "This is not an emergency service" + + view + |> form("#request-report-form", + report: %{reason: "dangerous_request", details: "Review the request context."} + ) + |> render_submit() + + assert_push_event(view, "reset-message-form", %{id: "request-report-form"}) + end + + test "successful category proposal resets the browser form", %{conn: conn} do + Catalog.seed_defaults() + {:ok, view, _html} = live(conn, ~p"/categories/proposals") + + view + |> form("#category-proposal-form", + category_proposal: %{ + proposed_name: "Chain repair", + mode: "help", + reason: "A reusable category for urgent roadside bicycle chain repair." + } + ) + |> render_submit() + + assert_push_event(view, "reset-message-form", %{id: "category-proposal-form"}) + assert render(view) =~ "Chain repair" + end + + test "review submission updates both participant pages and hides the submitted form" do + category = Catalog.seed_defaults() + requester = user_fixture(display_name: "Review requester") + helper = user_fixture(display_name: "Review helper") + requester_scope = user_scope_fixture(requester) + helper_scope = user_scope_fixture(helper) + + {:ok, request} = Help.create_request(requester_scope, request_attrs(category)) + {:ok, assignment} = Help.accept_request(helper_scope, request.id) + {:ok, _} = Help.confirm_completion(requester_scope, assignment.id) + {:ok, _} = Help.confirm_completion(helper_scope, assignment.id) + + {:ok, _assignment} = + Help.verify_handover(helper_scope, assignment.id, Help.handover_code(request.id)) + + {:ok, requester_view, _html} = + build_conn() |> log_in_user(requester) |> live(~p"/requests/#{request.id}") + + {:ok, helper_view, _html} = + build_conn() |> log_in_user(helper) |> live(~p"/requests/#{request.id}") + + requester_view + |> form("form[phx-submit='review']", review: %{rating: "5", comment: "Kind"}) + |> render_submit() + + refute has_element?(requester_view, "form[phx-submit='review']") + assert has_element?(helper_view, "form[phx-submit='review']") + + helper_view + |> form("form[phx-submit='review']", review: %{rating: "4", comment: "Clear"}) + |> render_submit() + + refute has_element?(helper_view, "form[phx-submit='review']") + assert render(requester_view) =~ "rating 4.0" + assert render(helper_view) =~ "rating 4.0" + assert render(requester_view) =~ ">5.0<" + assert render(helper_view) =~ ">5.0<" end defp request_attrs(category) do