From 746c9b4099dad8f547ad649871fefdf28edf2256 Mon Sep 17 00:00:00 2001 From: Cristen Jones Date: Tue, 8 Oct 2024 15:33:21 -0400 Subject: [PATCH] chore(TripPlan.InputForm): change modes type from enum to map --- lib/dotcom/trip_plan/input_form.ex | 135 ++++++++++++++++-- .../live_components/trip_planner_form.ex | 91 ++---------- lib/dotcom_web/live/trip_planner.ex | 21 ++- test/dotcom/trip_plan/input_form_test.exs | 110 +++++++++++++- test/dotcom_web/live/trip_planner_test.exs | 9 +- 5 files changed, 268 insertions(+), 98 deletions(-) diff --git a/lib/dotcom/trip_plan/input_form.ex b/lib/dotcom/trip_plan/input_form.ex index e9f96401b4..4b1bc79103 100644 --- a/lib/dotcom/trip_plan/input_form.ex +++ b/lib/dotcom/trip_plan/input_form.ex @@ -10,8 +10,7 @@ defmodule Dotcom.TripPlan.InputForm do alias OpenTripPlannerClient.PlanParams - @valid_modes [:RAIL, :SUBWAY, :BUS, :FERRY] - @time_types [:now, :leave_at, :arrive_by] + @time_types ~W(now leave_at arrive_by)a @error_messages %{ from: "Please specify an origin location.", @@ -25,14 +24,19 @@ defmodule Dotcom.TripPlan.InputForm do typed_embedded_schema do embeds_one(:from, __MODULE__.Location) embeds_one(:to, __MODULE__.Location) + embeds_one(:modes, __MODULE__.Modes) field(:datetime_type, Ecto.Enum, values: @time_types) field(:datetime, :naive_datetime) - field(:modes, {:array, Ecto.Enum}, values: @valid_modes) field(:wheelchair, :boolean, default: true) end def time_types, do: @time_types - def valid_modes, do: @valid_modes + + def initial_modes do + __MODULE__.Modes.fields() + |> Enum.map(&{Atom.to_string(&1), "true"}) + |> Map.new() + end def to_params(%__MODULE__{ from: from, @@ -48,7 +52,7 @@ defmodule Dotcom.TripPlan.InputForm do arriveBy: datetime_type == :arrive_by, date: PlanParams.to_date_param(datetime), time: PlanParams.to_time_param(datetime), - transportModes: PlanParams.to_modes_param(modes), + transportModes: __MODULE__.Modes.selected_mode_keys(modes) |> PlanParams.to_modes_param(), wheelchair: wheelchair } |> PlanParams.new() @@ -60,9 +64,10 @@ defmodule Dotcom.TripPlan.InputForm do def changeset(form, params) do form - |> cast(params, [:datetime_type, :datetime, :modes, :wheelchair]) + |> cast(params, [:datetime_type, :datetime, :wheelchair]) |> cast_embed(:from, required: true) |> cast_embed(:to, required: true) + |> cast_embed(:modes, required: true) end def validate_params(params) do @@ -72,10 +77,11 @@ defmodule Dotcom.TripPlan.InputForm do |> update_change(:to, &update_location_change/1) |> validate_required(:from, message: error_message(:from)) |> validate_required(:to, message: error_message(:to)) - |> validate_required([:datetime_type, :modes, :wheelchair]) + |> validate_required(:modes, message: error_message(:modes)) + |> validate_required([:datetime_type, :wheelchair]) |> validate_same_locations() - |> validate_length(:modes, min: 1, message: error_message(:modes)) |> validate_chosen_datetime() + |> validate_modes() end # make the parent field blank if the location isn't valid @@ -96,6 +102,16 @@ defmodule Dotcom.TripPlan.InputForm do end end + defp validate_modes(changeset) do + case get_change(changeset, :modes) do + %Ecto.Changeset{valid?: false} -> + add_error(changeset, :modes, error_message(:modes)) + + _ -> + changeset + end + end + defp validate_chosen_datetime(changeset) do case get_field(changeset, :datetime_type) do :now -> @@ -164,4 +180,107 @@ defmodule Dotcom.TripPlan.InputForm do end end end + + defmodule Modes do + @moduledoc """ + Represents the set of modes to be selected for a trip plan, additionally + validating that at least one mode is selected. Also provides helper + functions for rendering in forms. + """ + + use TypedEctoSchema + + alias Ecto.Changeset + alias OpenTripPlannerClient.PlanParams + + @primary_key false + typed_embedded_schema do + field(:RAIL, :boolean, default: true) + field(:SUBWAY, :boolean, default: true) + field(:BUS, :boolean, default: true) + field(:FERRY, :boolean, default: true) + end + + def fields, do: __MODULE__.__schema__(:fields) + + def changeset(modes, params) do + modes + |> cast(params, fields()) + |> validate_at_least_one() + end + + defp validate_at_least_one(changeset) do + if Enum.all?(fields(), &(get_change(changeset, &1) == false)) do + add_error(changeset, :FERRY, "") + else + changeset + end + end + + @doc """ + Translates a mode atom into a short string. + """ + @spec mode_label(PlanParams.mode_t()) :: String.t() + def mode_label(:RAIL), do: "Commuter rail" + def mode_label(mode), do: Phoenix.Naming.humanize(mode) + + @spec selected_mode_keys(__MODULE__.t()) :: [PlanParams.mode_t()] + def selected_mode_keys(%__MODULE__{} = modes) do + modes + |> Map.from_struct() + |> Enum.reject(&(elem(&1, 1) == false)) + |> Enum.map(&elem(&1, 0)) + end + + @doc """ + Summarizes the selected mode values into a single short string. + """ + @spec selected_modes(Changeset.t() | __MODULE__.t() | [PlanParams.mode_t()]) :: String.t() + def selected_modes(%Changeset{} = modes_changeset) do + modes_changeset + |> Changeset.apply_changes() + |> selected_modes() + end + + def selected_modes(%__MODULE__{} = modes) do + modes + |> selected_mode_keys() + |> selected_modes() + end + + def selected_modes([]), do: "No transit modes selected" + def selected_modes([mode]), do: mode_name(mode) <> " Only" + + def selected_modes(modes) do + if fields() -- modes == [] do + "All modes" + else + fields() + |> Enum.filter(&(&1 in modes)) + |> summarized_modes() + end + end + + defp summarized_modes([mode1, mode2]) do + mode_name(mode1) <> " and " <> mode_name(mode2) + end + + defp summarized_modes(modes) do + modes + |> Enum.map(&mode_name/1) + |> Enum.intersperse(", ") + |> List.insert_at(-2, "and ") + |> Enum.join("") + end + + defp mode_name(mode) do + case mode do + :RAIL -> :commuter_rail + :SUBWAY -> :subway + :BUS -> :bus + :FERRY -> :ferry + end + |> DotcomWeb.ViewHelpers.mode_name() + end + end end diff --git a/lib/dotcom_web/components/live_components/trip_planner_form.ex b/lib/dotcom_web/components/live_components/trip_planner_form.ex index 213502aa40..5d74a7492e 100644 --- a/lib/dotcom_web/components/live_components/trip_planner_form.ex +++ b/lib/dotcom_web/components/live_components/trip_planner_form.ex @@ -5,18 +5,16 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do use DotcomWeb, :live_component import DotcomWeb.ViewHelpers, only: [svg: 1] - import Phoenix.HTML.Form, only: [input_name: 2, input_value: 2, input_id: 2] - import MbtaMetro.Components.Feedback import MbtaMetro.Components.InputGroup + import Phoenix.HTML.Form, only: [input_name: 2, input_value: 2, input_id: 2] - alias Dotcom.TripPlan.{InputForm, OpenTripPlanner} + alias Dotcom.TripPlan.{InputForm, InputForm.Modes, OpenTripPlanner} - @all_modes [:RAIL, :SUBWAY, :BUS, :FERRY] @form_defaults %{ "datetime_type" => :now, "datetime" => NaiveDateTime.local_now(), - "modes" => @all_modes, + "modes" => InputForm.initial_modes(), "wheelchair" => true } @@ -130,45 +128,18 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do <.fieldset legend="Modes"> <.accordion> <:heading> - <%= selected_modes(input_value(@form, :modes)) %> + <%= Modes.selected_modes(input_value(f, :modes)) %> <:content> -
- "[]"} - value="" - checked="true" - /> - +
<:extra :if={used_input?(f[:modes])}> @@ -231,44 +202,4 @@ defmodule DotcomWeb.Components.LiveComponents.TripPlannerForm do _ = on_submit.(result) result end - - defp mode_atom(mode) do - case mode do - :RAIL -> :commuter_rail - :SUBWAY -> :subway - :BUS -> :bus - :FERRY -> :ferry - other when is_binary(other) and other != "" -> String.to_atom(other) - _ -> :unknown - end - end - - defp mode_name(mode) do - case mode_atom(mode) do - :unknown -> - "" - - other -> - DotcomWeb.ViewHelpers.mode_name(other) - end - end - - defp selected_modes(modes) when modes == @all_modes do - "All modes" - end - - defp selected_modes([]), do: "No transit modes selected" - defp selected_modes(nil), do: "No transit modes selected" - - defp selected_modes([mode]), do: mode_name(mode) <> " Only" - defp selected_modes([mode1, mode2]), do: mode_name(mode1) <> " and " <> mode_name(mode2) - - defp selected_modes(modes) do - modes - |> Enum.map(&mode_name/1) - |> Enum.reject(&(&1 == "")) - |> Enum.intersperse(", ") - |> List.insert_at(-2, "and ") - |> Enum.join("") - end end diff --git a/lib/dotcom_web/live/trip_planner.ex b/lib/dotcom_web/live/trip_planner.ex index 3c3e12bc5f..5ce4f3ca36 100644 --- a/lib/dotcom_web/live/trip_planner.ex +++ b/lib/dotcom_web/live/trip_planner.ex @@ -8,7 +8,7 @@ defmodule DotcomWeb.Live.TripPlanner do use DotcomWeb, :live_view alias DotcomWeb.Components.LiveComponents.TripPlannerForm - alias Dotcom.TripPlan.ItineraryGroups + alias Dotcom.TripPlan.{InputForm.Modes, ItineraryGroups} import DotcomWeb.Components.TripPlanner.ItineraryGroup, only: [itinerary_group: 1] @@ -38,10 +38,21 @@ defmodule DotcomWeb.Live.TripPlanner do on_submit={fn data -> send(self(), {:updated_form, data}) end} />
-

- <%= Enum.count(@groups) %> ways to get from <%= @submitted_values.from.name %> to <%= @submitted_values.to.name %>, using <%= inspect( - @submitted_values.modes - ) %> +

+ Planning trips from <%= @submitted_values.from.name %> + to <%= @submitted_values.to.name %> +
using <%= Modes.selected_modes(@submitted_values.modes) %>, + + <%= if @submitted_values.datetime_type == :arrive_by, do: "Arriving by", else: "Leaving" %> <%= @submitted_values.datetime + |> Timex.format!("{Mfull} {D}, {h12}:{m} {AM}") %> + +

+

+ Found + + <%= Enum.count(@groups) %> <%= Inflex.inflect("way", Enum.count(@groups)) %> + + to go.

diff --git a/test/dotcom/trip_plan/input_form_test.exs b/test/dotcom/trip_plan/input_form_test.exs index b7a334b0b3..7475fbfd9e 100644 --- a/test/dotcom/trip_plan/input_form_test.exs +++ b/test/dotcom/trip_plan/input_form_test.exs @@ -11,13 +11,13 @@ defmodule Dotcom.TripPlan.InputFormTest do "latitude" => "#{Faker.Address.latitude()}", "longitude" => "#{Faker.Address.longitude()}" } - @mode_params InputForm.valid_modes() + @mode_params InputForm.initial_modes() @params %{ "from" => @from_params, "to" => @to_params, "datetime_type" => Faker.Util.pick(InputForm.time_types()) |> to_string(), "datetime" => Faker.DateTime.forward(4) |> to_string(), - "modes" => @mode_params |> Enum.map(&to_string/1), + "modes" => @mode_params, "wheelchair" => Faker.Util.pick(["true", "false"]) } @@ -27,6 +27,11 @@ defmodule Dotcom.TripPlan.InputFormTest do assert {_, [validation: :required]} = changeset.errors[:to] end + test "mode required" do + changeset = InputForm.changeset(%{modes: nil}) + assert {_, [validation: :required]} = changeset.errors[:modes] + end + describe "validate_params/1" do test "validates to & from" do changeset = InputForm.validate_params(@params) @@ -67,6 +72,78 @@ defmodule Dotcom.TripPlan.InputFormTest do expected_error = InputForm.error_message(:from_to_same) assert {^expected_error, _} = changeset.errors[:to] end + + test "at least one mode required" do + changeset = + InputForm.validate_params(%{ + @params + | "modes" => %{RAIL: false, BUS: false, FERRY: false, SUBWAY: false} + }) + + refute changeset.valid? + + expected_error = InputForm.error_message(:modes) + assert {^expected_error, _} = changeset.errors[:modes] + end + + test "adds datetime if using datetime_type == now" do + changeset = + InputForm.validate_params(%{ + @params + | "datetime_type" => "now", + "datetime" => nil + }) + + assert changeset.valid? + assert %DateTime{} = changeset.changes[:datetime] + end + + test "datetime required if using datetime_type != now" do + expected_error = InputForm.error_message(:datetime) + + changeset = + InputForm.validate_params(%{ + @params + | "datetime_type" => "arrive_by", + "datetime" => nil + }) + + refute changeset.valid? + assert {^expected_error, _} = changeset.errors[:datetime] + + changeset = + InputForm.validate_params(%{ + @params + | "datetime_type" => "leave_at", + "datetime" => nil + }) + + refute changeset.valid? + assert {^expected_error, _} = changeset.errors[:datetime] + end + + test "requires date to be in the future" do + changeset = + InputForm.validate_params(%{ + @params + | "datetime_type" => "arrive_by", + "datetime" => Faker.DateTime.forward(1) + }) + + assert changeset.valid? + + expected_error = InputForm.error_message(:datetime) + + changeset = + InputForm.validate_params(%{ + @params + | "datetime_type" => "arrive_by", + "datetime" => Faker.DateTime.backward(1) + }) + + refute changeset.valid? + assert {^expected_error, _} = changeset.errors[:datetime] + end end describe "Location" do @@ -89,4 +166,33 @@ defmodule Dotcom.TripPlan.InputFormTest do assert changeset.changes.name == "#{lat}, #{lon}" end end + + describe "Modes" do + test "selected_modes/1 summarizes two values" do + changeset = + InputForm.Modes.changeset(%InputForm.Modes{}, %{ + "RAIL" => "true", + "SUBWAY" => "true", + "BUS" => "false", + "FERRY" => "false" + }) + + assert "Commuter Rail and Subway" = InputForm.Modes.selected_modes(changeset) + end + + test "selected_modes/1 summarizes one value" do + data = %InputForm.Modes{RAIL: false, SUBWAY: false, BUS: true, FERRY: false} + assert "Bus Only" = InputForm.Modes.selected_modes(data) + end + + test "selected_modes/1 summarizes many values" do + data = %InputForm.Modes{RAIL: true, SUBWAY: false, BUS: true, FERRY: true} + assert "Commuter Rail, Bus, and Ferry" = InputForm.Modes.selected_modes(data) + end + + test "selected_modes/1 summarizes all values" do + data = %InputForm.Modes{RAIL: true, SUBWAY: true, BUS: true, FERRY: true} + assert "All modes" = InputForm.Modes.selected_modes(data) + end + end end diff --git a/test/dotcom_web/live/trip_planner_test.exs b/test/dotcom_web/live/trip_planner_test.exs index c4749e96b5..deb6bd39af 100644 --- a/test/dotcom_web/live/trip_planner_test.exs +++ b/test/dotcom_web/live/trip_planner_test.exs @@ -51,7 +51,7 @@ defmodule DotcomWeb.Live.TripPlannerTest do |> element("form") |> render_change(%{ _target: ["input_form", "modes"], - input_form: %{modes: [:commuter_rail, :subway, :ferry]} + input_form: %{modes: %{RAIL: true, SUBWAY: true, FERRY: true, BUS: false}} }) assert html =~ "Commuter Rail, Subway, and Ferry" @@ -59,7 +59,10 @@ defmodule DotcomWeb.Live.TripPlannerTest do html = view |> element("form") - |> render_change(%{_target: ["input_form", "modes"], input_form: %{modes: [:subway]}}) + |> render_change(%{ + _target: ["input_form", "modes"], + input_form: %{modes: %{SUBWAY: true, BUS: false, RAIL: false, FERRY: false}} + }) assert html =~ "Subway Only" @@ -68,7 +71,7 @@ defmodule DotcomWeb.Live.TripPlannerTest do |> element("form") |> render_change(%{ _target: ["input_form", "modes"], - input_form: %{modes: [:subway, :bus]} + input_form: %{modes: %{SUBWAY: true, BUS: true, RAIL: false, FERRY: false}} }) assert html =~ "Subway and Bus"