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

🍎 Ability to filter V3 API route patterns by date #687

Merged
merged 5 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
146 changes: 128 additions & 18 deletions apps/api_web/lib/api_web/controllers/route_pattern_controller.ex
Original file line number Diff line number Diff line change
@@ -1,16 +1,20 @@
defmodule ApiWeb.RoutePatternController do
@moduledoc """
Controller for Routes. Filterable by:
Controller for Route Patterns. Filterable by:

* id
* date
* direction_id
* route_id
* stop
* canonical
"""
use ApiWeb.Web, :api_controller
alias State.RoutePattern
alias State.{RoutePattern, RoutesByService, ServiceByDate, Trip}

plug(:ensure_path_matches_version)

@filters ~w(id canonical route direction_id stop)
@filters ~w(id canonical route direction_id stop date)
@includes ~w(route representative_trip)
@pagination_opts [:offset, :limit, :order_by]
@description """
Expand Down Expand Up @@ -48,6 +52,7 @@ defmodule ApiWeb.RoutePatternController do
filter_param(:direction_id)
filter_param(:stop_id, includes_children: true)
filter_param(:canonical)
filter_param(:date, description: "Filter by date that route pattern is active")

consumes("application/vnd.api+json")
produces("application/vnd.api+json")
Expand Down Expand Up @@ -92,25 +97,130 @@ defmodule ApiWeb.RoutePatternController do
defp reject_invalid_canonical_filter(filters), do: filters

@spec format_filters(%{optional(String.t()) => String.t()}) :: RoutePattern.filters()
defp format_filters(filters) do
Map.new(filters, fn {key, value} ->
case {key, value} do
{"id", ids} ->
{:ids, Params.split_on_comma(ids)}
defp format_filters(filters, acc \\ %{})

{"route", route_ids} ->
{:route_ids, Params.split_on_comma(route_ids)}
defp format_filters(%{"id" => ids} = filters, acc) do
new_acc = Map.put(acc, :ids, Params.split_on_comma(ids))

{"direction_id", direction_id} ->
{:direction_id, Params.direction_id(%{"direction_id" => direction_id})}
filters
paulswartz marked this conversation as resolved.
Show resolved Hide resolved
|> Map.delete("id")
|> format_filters(new_acc)
end

defp format_filters(%{"direction_id" => direction_id} = filters, acc) do
new_acc = Map.put(acc, :direction_id, Params.direction_id(%{"direction_id" => direction_id}))

filters
|> Map.delete("direction_id")
|> format_filters(new_acc)
end

defp format_filters(%{"route" => route_ids} = filters, acc) do
new_acc = Map.put(acc, :route_ids, Params.split_on_comma(route_ids))

filters
|> Map.delete("route")
|> format_filters(new_acc)
end

defp format_filters(%{"date" => date} = filters, acc) do
case process_date(date, acc) do
:error ->
filters
|> Map.delete("date")
|> format_filters(acc)

new_acc ->
filters
|> Map.delete("date")
|> format_filters(new_acc)
end
end

defp format_filters(%{"stop" => stop_ids} = filters, acc) do
new_acc = Map.put(acc, :stop_ids, Params.split_on_comma(stop_ids))

filters
|> Map.delete("stop")
|> format_filters(new_acc)
end

defp format_filters(%{"canonical" => canonical} = filters, acc) do
new_acc = Map.put(acc, :canonical, Params.canonical(canonical))

filters
|> Map.delete("canonical")
|> format_filters(new_acc)
end

defp format_filters(_, acc) do
acc
end

{"stop", stop_ids} ->
{:stop_ids, Params.split_on_comma(stop_ids)}
defp consolidate_route_ids(%{route_ids: [_ | _] = route_ids}, [_ | _] = date_specific_route_ids) do
route_ids
|> MapSet.new()
|> MapSet.intersection(MapSet.new(date_specific_route_ids))
|> MapSet.to_list()
end

defp consolidate_route_ids(_, date_specific_route_ids), do: date_specific_route_ids

defp gather_trip_based_route_pattern_ids(
%{ids: ids, date: _date, route_ids: [_ | _] = route_ids} = acc
) do
acc
|> Map.take([:date, :direction_id, :route_patterns])
|> Map.put(:route_patterns, ids)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|> Map.take([:date, :direction_id, :route_patterns])
|> Map.put(:route_patterns, ids)
|> Map.take([:date, :direction_id])
|> Map.put(:route_patterns, ids)

suggestion: it looks like you're taking route_patterns and them immediately overwriting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops yep, initially had it setup slightly different, setting that key in a different function. removed

|> Map.put(:routes, route_ids)
|> gather_trip_based_route_pattern_ids(acc)
end

defp gather_trip_based_route_pattern_ids(%{date: _date, route_ids: [_ | _] = route_ids} = acc) do
acc
|> Map.take([:date, :direction_id, :route_patterns])
|> Map.put(:routes, route_ids)
|> gather_trip_based_route_pattern_ids(acc)
end

defp gather_trip_based_route_pattern_ids(acc), do: acc

defp gather_trip_based_route_pattern_ids(filters, acc) do
route_pattern_ids =
filters
|> Trip.filter_by()
|> Enum.map(& &1.route_pattern_id)

Map.put(acc, :ids, route_pattern_ids)
end

defp process_date(date_str, acc) do
case process_date(date_str) do
:error ->
:error

%{date: date, route_ids: date_specific_route_ids} ->
filtered_route_ids = consolidate_route_ids(acc, date_specific_route_ids)

{"canonical", canonical} ->
{:canonical, Params.canonical(canonical)}
end
end)
acc
|> Map.merge(%{route_ids: filtered_route_ids, date: date})
|> gather_trip_based_route_pattern_ids()
end
end

defp process_date(date_str) do
case Date.from_iso8601(date_str) do
{:ok, date} ->
ids =
date
|> ServiceByDate.by_date()
|> RoutesByService.for_service_ids()

%{date: date, route_ids: ids}

{:error, _} ->
:error
end
end

defp pagination_opts(params, conn) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,207 @@ defmodule ApiWeb.RoutePatternControllerTest do

assert [%{"sort_order" => 1}, %{"sort_order" => 2}, %{"sort_order" => 3}] = attrs
end

test "can filter by date", %{conn: base_conn} do
today = Parse.Time.service_date()
bad_date = Date.add(today, 2)

service = %Model.Service{
id: "service",
start_date: today,
end_date: today,
added_dates: [today]
}

route = %Model.Route{id: "route"}
route_pattern = %Model.RoutePattern{id: "rp1", route_id: "route"}

trip = %Model.Trip{
id: "trip",
route_id: route.id,
service_id: service.id,
route_pattern_id: route_pattern.id
}

State.Service.new_state([service])
State.Route.new_state([route])
State.RoutePattern.new_state([route_pattern])
State.Trip.new_state([trip])
State.Trip.reset_gather()
State.RoutesByService.update!()

today_iso = Date.to_iso8601(today)
bad_date_iso = Date.to_iso8601(bad_date)

params = %{"filter" => %{"date" => today_iso}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern] == data

params = put_in(params["filter"]["date"], bad_date_iso)
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [] == data

params = %{"filter" => %{"date" => ""}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern] == data

params = %{"filter" => %{"date" => "invalid"}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern] == data
end

test "can filter by date and route(s)", %{conn: base_conn} do
today = Parse.Time.service_date()
bad_date = Date.add(today, 2)

service = %Model.Service{
id: "service",
start_date: today,
end_date: today,
added_dates: [today]
}

future_service = %Model.Service{
id: "future_service",
start_date: bad_date,
end_date: bad_date,
added_dates: [bad_date]
}

route1 = %Model.Route{id: "route1"}
route2 = %Model.Route{id: "route2"}
route3 = %Model.Route{id: "route3"}
route_pattern1 = %Model.RoutePattern{id: "rp1", route_id: route1.id, canonical: false}
route_pattern2 = %Model.RoutePattern{id: "rp2", route_id: route2.id, canonical: false}
route_pattern3 = %Model.RoutePattern{id: "rp3", route_id: route3.id, canonical: true}

route_pattern4 = %Model.RoutePattern{
id: "rp4",
route_id: route3.id,
canonical: false,
direction_id: 0
}

route_pattern5 = %Model.RoutePattern{
id: "rp5",
route_id: route3.id,
canonical: false,
direction_id: 1
}

trip1 = %Model.Trip{
id: "trip1",
route_id: route1.id,
service_id: service.id,
route_pattern_id: route_pattern1.id
}

trip2 = %Model.Trip{
id: "trip2",
route_id: route2.id,
service_id: future_service.id,
route_pattern_id: route_pattern2.id
}

trip3 = %Model.Trip{
id: "trip3",
route_id: route3.id,
service_id: service.id,
route_pattern_id: route_pattern3.id
}

trip4 = %Model.Trip{
id: "trip4",
route_id: route3.id,
direction_id: 0,
service_id: future_service.id,
route_pattern_id: route_pattern4.id
}

trip5 = %Model.Trip{
id: "trip5",
route_id: route3.id,
direction_id: 1,
service_id: future_service.id,
route_pattern_id: route_pattern5.id
}

State.Service.new_state([service, future_service])
State.Route.new_state([route1, route2, route3])

State.RoutePattern.new_state([
route_pattern1,
route_pattern2,
route_pattern3,
route_pattern4,
route_pattern5
])

State.Trip.new_state([trip1, trip2, trip3, trip4, trip5])
State.Trip.reset_gather()
State.RoutesByService.update!()

today_iso = Date.to_iso8601(today)
future_date_iso = Date.to_iso8601(bad_date)

params = %{"filter" => %{"date" => today_iso}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1, route_pattern3] == data

params = %{"filter" => %{"date" => today_iso, "canonical" => "false"}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1] == data

params = %{"filter" => %{"date" => today_iso, "canonical" => "true"}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern3] == data

params = %{"filter" => %{"date" => today_iso, "route" => "route1,route2"}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1] == data

params = put_in(params["filter"]["route"], "route3,route4,Red,Blue")
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern3] == data

params = %{"filter" => %{"date" => future_date_iso, "route" => "route1,route2,route3"}}
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: can you also add a test for "date" + "id" ?

data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern2, route_pattern4, route_pattern5] == data

params = %{
"filter" => %{
"date" => future_date_iso,
"route" => "route1,route2,route3",
"direction_id" => "0"
}
}

data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern4] == data

params = %{
"filter" => %{
"date" => future_date_iso,
"route" => "route1,route2,route3",
"direction_id" => "1"
}
}

data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern5] == data

params = %{"filter" => %{"date" => today_iso, "route" => ""}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1, route_pattern3] == data

params = %{"filter" => %{"date" => today_iso, "route" => nil}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1, route_pattern3] == data

params = %{"filter" => %{"date" => today_iso, "id" => "rp1,rp2"}}
data = ApiWeb.RoutePatternController.index_data(base_conn, params)
assert [route_pattern1] == data
end
end

describe "show" do
Expand Down
Loading