Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(TripPlan.ItineraryGroups): improve handling for "arrive by" selection #2329

Merged
merged 13 commits into from
Jan 22, 2025
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat(TripPlan.ItineraryGroup): use struct to describe group
thecristen committed Jan 14, 2025
commit f445fd3c8b591865b42e77da41fa732b9105fd17
74 changes: 74 additions & 0 deletions lib/dotcom/trip_plan/itinerary_group.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
defmodule Dotcom.TripPlan.ItineraryGroup do
@moduledoc """
A single group of related itineraries
"""

alias Dotcom.TripPlan.Itinerary

defstruct [:itineraries, :representative_index, :representative_time, :summary]

@type summarized_leg :: %{
routes: [Routes.Route.t()],
walk_minutes: non_neg_integer()
}

@type summary :: %{
accessible?: boolean() | nil,
duration: non_neg_integer(),
start: DateTime.t(),
stop: DateTime.t(),
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking): The names start and stop don't convey that these are times. And stop more frequently refers to a location where a bus allows passengers to embark and disembark. I would advocate for start_time and end_time.

(Note: This is only non-blocking because I know that the names aren't being introduced in this PR, and were like this already, and that fixing them could lead to PR bloat.)

summarized_legs: [summarized_leg()],
tag: String.t(),
total_cost: non_neg_integer(),
walk_distance: float()
}

@type t :: %__MODULE__{
itineraries: [Itinerary.t()],
representative_index: non_neg_integer(),
representative_time: :start | :stop,
thecristen marked this conversation as resolved.
Show resolved Hide resolved
summary: summary()
}

@doc """
List of either start times or stop times for this group
"""
@spec options_text(t()) :: String.t() | nil
def options_text(%__MODULE__{itineraries: []}), do: nil
def options_text(%__MODULE__{itineraries: [_single]}), do: nil

def options_text(
%__MODULE__{
representative_index: representative_index,
representative_time: start_or_stop
} =
group
) do
{_, other_times} =
group
|> all_times()
|> List.pop_at(representative_index)
thecristen marked this conversation as resolved.
Show resolved Hide resolved

phrase = options_phrase(start_or_stop == :stop, Enum.count(other_times))

formatted_times =
other_times
|> Enum.map(&Timex.format!(&1, "%-I:%M", :strftime))
|> Enum.join(", ")

"Similar #{phrase} #{formatted_times}"
end

defp options_phrase(true, 1), do: "trip arrives by"
defp options_phrase(true, _), do: "trips arrive by"
defp options_phrase(false, 1), do: "trip departs at"
defp options_phrase(_, _), do: "trips depart at"
thecristen marked this conversation as resolved.
Show resolved Hide resolved

@doc """
List of either start times or stop times for this group
"""
@spec all_times(t()) :: [DateTime.t()]
def all_times(%__MODULE__{itineraries: itineraries, representative_time: start_or_stop}) do
Enum.map(itineraries, &Map.get(&1, start_or_stop))
end
end
88 changes: 88 additions & 0 deletions test/dotcom/trip_plan/itinerary_group_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
defmodule Dotcom.TripPlan.ItineraryGroupTest do
@moduledoc false

use ExUnit.Case, async: true

import Mox

alias Dotcom.TripPlan.ItineraryGroup
alias Test.Support.Factories.{Stops.Stop, TripPlanner.TripPlanner}

setup :verify_on_exit!

setup do
stub(Stops.Repo.Mock, :get, &Stop.build(:stop, id: &1))
:ok
end

describe "options_text/1" do
test "does nothing if no alternate times" do
refute ItineraryGroup.options_text(%ItineraryGroup{itineraries: []})

refute ItineraryGroup.options_text(%ItineraryGroup{
itineraries: [TripPlanner.build(:itinerary)]
})
end

test "describes alternate times for the group" do
# SETUP
itineraries = TripPlanner.build_list(Faker.Util.pick(2..4), :itinerary)
representative_index = Faker.Util.pick(0..(length(itineraries) - 1))
start_or_stop = Faker.Util.pick([:start, :stop])

group = %ItineraryGroup{
itineraries: itineraries,
representative_index: representative_index,
representative_time: start_or_stop
}

# EXERCISE
text = ItineraryGroup.options_text(group)

# VERIFY
if Enum.count(itineraries) == 2 do
thecristen marked this conversation as resolved.
Show resolved Hide resolved
# only one alternate trip
assert text =~ "trip "

if group.representative_time == :start do
assert text =~ "departs at"
else
assert text =~ "arrives by"
end
else
assert text =~ "trips "

if group.representative_time == :start do
assert text =~ "depart at"
else
assert text =~ "arrive by"
end
end
end
end

describe "all_times/1" do
test "uses representative_time to map to chosen times" do
# SETUP
itineraries = TripPlanner.build_list(4, :itinerary)

# EXERCISE
start_times =
%ItineraryGroup{
itineraries: itineraries,
representative_time: :start
}
|> ItineraryGroup.all_times()

stop_times =
%ItineraryGroup{
itineraries: itineraries,
representative_time: :stop
}
|> ItineraryGroup.all_times()

# VERIFY
assert start_times != stop_times
thecristen marked this conversation as resolved.
Show resolved Hide resolved
end
end
end