simplify params parsing and update affected controllers/states

apps/api_web/lib/api_web/controllers/prediction_controller.ex

@@ -238,10 +238,8 @@ defmodule ApiWeb.PredictionController do
   defp add_revenue_matchers(matchers, :error),
     do: add_revenue_matchers(matchers, {:ok, :REVENUE})
 
-  defp add_revenue_matchers(matchers, {:ok, :ALL}), do: matchers
-
-  defp add_revenue_matchers(matchers, {:ok, revenue_matcher}) do
-    for matcher <- matchers do
+  defp add_revenue_matchers(matchers, {:ok, revenue_matchers}) do
+    for revenue_matcher <- List.wrap(revenue_matchers), matcher <- matchers do
       Map.put(matcher, :revenue, revenue_matcher)
     end
   end

apps/api_web/lib/api_web/params.ex

@@ -211,22 +211,20 @@ defmodule ApiWeb.Params do
   @doc """
   Parse revenue filter to valid params
   """
-  def revenue(value) when is_binary(value) do
-    value
+  def revenue(values) when is_binary(values) do
+    values
     |> split_on_comma()
-    |> MapSet.new()
-    |> MapSet.intersection(MapSet.new(["REVENUE", "NON_REVENUE"]))
-    |> MapSet.to_list()
-    |> Enum.sort()
+    |> Enum.reduce_while({:ok, []}, fn
+      "REVENUE", {:ok, acc} -> {:cont, {:ok, [:REVENUE | acc]}}
+      "NON_REVENUE", {:ok, acc} -> {:cont, {:ok, [:NON_REVENUE | acc]}}
+      _, _ -> {:halt, :error}
+    end)
     |> revenue()
   end
 
-  for value <- ~w(NON_REVENUE REVENUE)a do
-    def revenue([unquote(Atom.to_string(value))]), do: {:ok, unquote(value)}
-  end
-
-  def revenue(["NON_REVENUE", "REVENUE"]), do: {:ok, :ALL}
-  def revenue(_value), do: :error
+  def revenue({:ok, []}), do: :error
+  def revenue(nil), do: :error
+  def revenue(val), do: val
 
   @doc """
   Parses and integer value from params.

apps/api_web/test/api_web/params_test.exs

@@ -156,23 +156,24 @@ defmodule ApiWeb.ParamsTest do
       assert :error = Params.revenue("revenue")
       assert :error = Params.revenue("non_revenue")
       assert :error = Params.revenue("revenue,non_revenue")
-      assert {:ok, :REVENUE} = Params.revenue("REVENUE,non_revenue")
-      assert {:ok, :NON_REVENUE} = Params.revenue("revenue,NON_REVENUE")
+      assert :error = Params.revenue("REVENUE,non_revenue")
+      assert :error = Params.revenue("revenue,NON_REVENUE")
     end
 
     test "it parses a list of values" do
-      assert {:ok, :ALL} = Params.revenue("REVENUE,NON_REVENUE")
-      assert {:ok, :ALL} = Params.revenue("NON_REVENUE,REVENUE")
-      assert {:ok, :ALL} = Params.revenue("REVENUE,other,NON_REVENUE")
-      assert {:ok, :ALL} = Params.revenue("NON_REVENUE,,REVENUE")
+      assert {:ok, [:NON_REVENUE, :REVENUE]} = Params.revenue("REVENUE,NON_REVENUE")
+      assert {:ok, [:REVENUE, :NON_REVENUE]} = Params.revenue("NON_REVENUE,REVENUE")
+      assert :error = Params.revenue("REVENUE,other,NON_REVENUE")
+      assert {:ok, [:REVENUE, :NON_REVENUE]} = Params.revenue("NON_REVENUE,,REVENUE")
       assert :error = Params.revenue("NOT,VALID,PARAMS")
     end
 
     test "it parses single values" do
-      assert {:ok, :REVENUE} = Params.revenue("REVENUE")
-      assert {:ok, :NON_REVENUE} = Params.revenue("NON_REVENUE")
+      assert {:ok, [:REVENUE]} = Params.revenue("REVENUE")
+      assert {:ok, [:NON_REVENUE]} = Params.revenue("NON_REVENUE")
       assert :error = Params.revenue("INVALID")
       assert :error = Params.revenue("ALL")
+      assert :error = Params.revenue("")
     end
   end
 end

apps/state/lib/state/trip.ex

@@ -18,7 +18,7 @@ defmodule State.Trip do
           optional(:direction_id) => direction_id,
           optional(:date) => Date.t(),
           optional(:route_patterns) => [String.t()],
-          optional(:revenue_status) => :all | :non_revenue | :revenue
+          optional(:revenue) => [:NON_REVENUE | :REVENUE]
         }
 
   @fetch_trips {:fetch, "trips.txt"}
@@ -171,7 +171,7 @@ defmodule State.Trip do
       |> build_filters(:direction_id, filters[:direction_id])
       |> build_filters(:route_pattern_id, filters[:route_patterns])
       |> build_filters(:id, filters[:ids])
-      |> build_filters(:revenue, filters[:revenue])
+      |> build_filters(:revenue, Map.get(filters, :revenue, :REVENUE))
 
     idx = get_index(filters)
     trips = State.Trip.Added.select(matchers, idx) ++ State.Trip.select(matchers, idx)
@@ -182,13 +182,6 @@ defmodule State.Trip do
     end
   end
 
-  defp build_filters(matchers, :revenue, :ALL), do: matchers
-
-  defp build_filters(matchers, :revenue, nil), do: build_filters(matchers, :revenue, :REVENUE)
-
-  defp build_filters(matchers, :revenue, value) when value in [:NON_REVENUE, :REVENUE],
-    do: build_filters(matchers, :revenue, [value])
-
   defp build_filters(matchers, _key, nil), do: matchers
 
   defp build_filters(matchers, key, values) do

apps/state/lib/state/vehicle.ex

@@ -20,7 +20,7 @@ defmodule State.Vehicle do
           optional(:routes) => [Model.Route.id(), ...],
           optional(:direction_id) => Model.Direction.id() | nil,
           optional(:route_types) => [Model.Route.route_type(), ...],
-          optional(:revenue) => :ALL | :NON_REVENUE | :REVENUE
+          optional(:revenue) => [:NON_REVENUE | :REVENUE]
         }
 
   @impl State.Server
@@ -101,18 +101,10 @@ defmodule State.Vehicle do
     end
   end
 
-  defp build_filters(matchers, :revenue, revenue_param, _filters) do
-    revenue = revenue_service_value(revenue_param)
-
-    for matcher <- matchers do
-      Map.put(matcher, :revenue, revenue)
-    end
+  defp build_filters(matchers, key, values, _filters) do
+    for matcher <- matchers, value <- List.wrap(values), do: Map.put(matcher, key, value)
   end
 
-  defp revenue_service_value(:ALL), do: :_
-  defp revenue_service_value(:NON_REVENUE), do: :NON_REVENUE
-  defp revenue_service_value(_), do: :REVENUE
-
   @spec do_post_search_filter([Vehicle.t()], filter_opts) :: [Vehicle.t()]
   defp do_post_search_filter(vehicles, %{labels: labels}) when is_list(labels) do
     labels = MapSet.new(labels)

apps/state/test/state/trip_test.exs

@@ -507,50 +507,56 @@ defmodule State.TripTest do
                "trip8"
              ]
 
-      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-9"], revenue: :REVENUE}) ==
+      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-9"], revenue: [:REVENUE]}) ==
                [
                  "trip1",
                  "trip2",
                  "trip6"
                ]
 
-      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-10"], revenue: :REVENUE}) ==
+      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-10"], revenue: [:REVENUE]}) ==
                [
                  "trip3",
                  "trip4",
                  "trip8"
                ]
 
-      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: :REVENUE}) ==
+      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: [:REVENUE]}) ==
                ["trip1", "trip2", "trip3", "trip4", "trip6", "trip8"]
 
-      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-9"], revenue: :ALL}) == [
+      assert mapped_and_sorted_filter_by(%{
+               routes: ["rev-route-9"],
+               revenue: [:NON_REVENUE, :REVENUE]
+             }) == [
                "trip1",
                "trip2",
                "trip5",
                "trip6"
              ]
 
-      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-10"], revenue: :ALL}) == [
+      assert mapped_and_sorted_filter_by(%{
+               routes: ["rev-route-10"],
+               revenue: [:NON_REVENUE, :REVENUE]
+             }) == [
                "trip3",
                "trip4",
                "trip7",
                "trip8"
              ]
 
-      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: :ALL}) ==
+      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: [:NON_REVENUE, :REVENUE]}) ==
                ["trip1", "trip2", "trip3", "trip4", "trip5", "trip6", "trip7", "trip8"]
 
-      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-9"], revenue: :NON_REVENUE}) ==
+      assert mapped_and_sorted_filter_by(%{routes: ["rev-route-9"], revenue: [:NON_REVENUE]}) ==
                ["trip5"]
 
       assert mapped_and_sorted_filter_by(%{
                routes: ["rev-route-10"],
-               revenue: :NON_REVENUE
+               revenue: [:NON_REVENUE]
              }) ==
                ["trip7"]
 
-      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: :NON_REVENUE}) ==
+      assert mapped_and_sorted_filter_by(%{route_pattern_id: 1, revenue: [:NON_REVENUE]}) ==
                ["trip5", "trip7"]
     end