even more dependency injection removal + tests

lib/dotcom/trip_plan/related_link.ex

@@ -15,8 +15,7 @@ defmodule Dotcom.TripPlan.RelatedLink do
 
   @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
   @default_opts [
-    route_by_id: &Routes.Repo.get/1,
-    stop_by_id: Function.capture(@stops_repo, :get_parent, 1)
+    route_by_id: &Routes.Repo.get/1
   ]
 
   import Phoenix.HTML.Link, only: [link: 2]
@@ -120,16 +119,16 @@ defmodule Dotcom.TripPlan.RelatedLink do
     for leg <- itinerary,
         {:ok, route_id} <- [Leg.route_id(leg)],
         %Route{custom_route?: false} = route <- [route_by_id.(route_id)] do
-      fare_link(route, leg, opts)
+      fare_link(route, leg)
     end
     |> Enum.uniq()
     |> simplify_fare_text
   end
 
-  defp fare_link(route, leg, opts) do
+  defp fare_link(route, leg) do
     type_atom = Route.type_atom(route)
     text = fare_link_text(type_atom)
-    {fare_section, opts} = fare_link_url_opts(type_atom, leg, opts)
+    {fare_section, opts} = fare_link_url_opts(type_atom, leg)
     url = fare_path(DotcomWeb.Endpoint, :show, fare_section, opts)
     new(["View ", text, " fare information"], url)
   end
@@ -138,21 +137,19 @@ defmodule Dotcom.TripPlan.RelatedLink do
     Atom.to_string(type) |> String.replace("_", " ")
   end
 
-  defp fare_link_url_opts(type, leg, opts) when type in [:commuter_rail, :ferry] do
-    stop_by_id = Keyword.get(opts, :stop_by_id)
-
+  defp fare_link_url_opts(type, leg) when type in [:commuter_rail, :ferry] do
     link_opts =
       for {key, stop_id} <- [origin: leg.from.stop_id, destination: leg.to.stop_id],
           is_binary(stop_id) do
         # fetch a parent stop ID
-        stop_id = stop_by_id.(stop_id).id
+        stop_id = @stops_repo.get_parent(stop_id).id
         {key, stop_id}
       end
 
     {type, link_opts}
   end
 
-  defp fare_link_url_opts(type, _leg, _opts) when type in [:bus, :subway] do
+  defp fare_link_url_opts(type, _leg) when type in [:bus, :subway] do
     {"#{type}-fares", []}
   end
 

lib/dotcom_web/controllers/schedule/line.ex

@@ -9,27 +9,10 @@ defmodule DotcomWeb.ScheduleController.Line do
 
   alias Plug.Conn
   alias Routes.Route
-  alias DotcomWeb.ScheduleController.Line.Dependencies, as: Dependencies
   alias DotcomWeb.ScheduleController.Line.Helpers, as: LineHelpers
   alias DotcomWeb.ScheduleController.Line.Maps
   alias Stops.{RouteStops, RouteStop}
 
-  defmodule Dependencies do
-    @moduledoc """
-    Actions pulled in from elsewhere
-    """
-    defstruct [:stops_by_route_fn]
-
-    @type t :: %__MODULE__{
-            stops_by_route_fn:
-              Function.capture(
-                Application.compile_env!(:dotcom, :repo_modules)[:stops],
-                :by_route,
-                3
-              )
-          }
-  end
-
   @type query_param :: String.t() | nil
   @type direction_id :: 0 | 1
 
@@ -48,7 +31,7 @@ defmodule DotcomWeb.ScheduleController.Line do
             direction_id: direction_id
           }
         } = conn,
-        args
+        _
       ) do
     # check if URL has parameters like direction_id, origin or variant
     # If so, they will get changed to:
@@ -72,8 +55,7 @@ defmodule DotcomWeb.ScheduleController.Line do
           direction_id
         end
 
-      deps = Keyword.get(args, :deps, %Dependencies{})
-      update_conn(conn, route, direction_id_value, deps)
+      update_conn(conn, route, direction_id_value)
     else
       # overwrite query_params in `conn` with the correct format:
       conn = %{conn | query_params: new_params}
@@ -108,13 +90,13 @@ defmodule DotcomWeb.ScheduleController.Line do
     end
   end
 
-  @spec update_conn(Conn.t(), Route.t(), direction_id, Dependencies.t()) :: Conn.t()
-  defp update_conn(conn, route, direction_id, deps) do
+  @spec update_conn(Conn.t(), Route.t(), direction_id) :: Conn.t()
+  defp update_conn(conn, route, direction_id) do
     schedule_direction = Map.get(conn.query_params, "schedule_direction", %{})
     variant = schedule_direction["variant"]
     expanded = Map.get(conn.query_params, "expanded")
     reverse_direction_id = reverse_direction(direction_id)
-    route_stops = LineHelpers.get_route_stops(route.id, direction_id, deps.stops_by_route_fn)
+    route_stops = LineHelpers.get_route_stops(route.id, direction_id)
     route_patterns = LineHelpers.get_map_route_patterns(route.id, route.type)
     route_patterns_map = map_route_patterns_by_direction(route_patterns)
 
@@ -133,7 +115,7 @@ defmodule DotcomWeb.ScheduleController.Line do
       )
 
     reverse_route_stops =
-      LineHelpers.get_route_stops(route.id, reverse_direction_id, deps.stops_by_route_fn)
+      LineHelpers.get_route_stops(route.id, reverse_direction_id)
 
     branch_route_stops = LineHelpers.get_branch_route_stops(route, direction_id)
 

lib/dotcom_web/controllers/schedule/line/helpers.ex

@@ -165,22 +165,22 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do
     RoutesRepo.get_shapes(route_id, direction_id: direction_id)
   end
 
-  @spec get_route_stops(Route.id_t(), direction_id, Stops.Repo.stop_by_route()) ::
+  @spec get_route_stops(Route.id_t(), direction_id) ::
           stops_by_route()
-  def get_route_stops("Green", direction_id, stops_by_route_fn) do
+  def get_route_stops("Green", direction_id) do
     GreenLine.branch_ids()
-    |> Task.async_stream(&do_get_route_stops(&1, direction_id, stops_by_route_fn))
+    |> Task.async_stream(&do_get_route_stops(&1, direction_id))
     |> Enum.reduce(%{}, fn {:ok, value}, acc -> Map.merge(acc, value) end)
   end
 
-  def get_route_stops(route_id, direction_id, stops_by_route_fn) do
-    do_get_route_stops(route_id, direction_id, stops_by_route_fn)
+  def get_route_stops(route_id, direction_id) do
+    do_get_route_stops(route_id, direction_id)
   end
 
-  @spec do_get_route_stops(Route.id_t(), direction_id, Stops.Repo.stop_by_route()) ::
+  @spec do_get_route_stops(Route.id_t(), direction_id) ::
           stops_by_route()
-  defp do_get_route_stops(route_id, direction_id, stops_by_route_fn) do
-    case stops_by_route_fn.(route_id, direction_id, []) do
+  defp do_get_route_stops(route_id, direction_id) do
+    case @stops_repo.by_route(route_id, direction_id, []) do
       {:error, _} -> %{}
       stops -> %{route_id => stops}
     end

lib/stops/nearby.ex

@@ -12,13 +12,12 @@ defmodule Stops.Nearby do
 
   @type route_with_direction :: %{direction_id: 0 | 1 | nil, route: Route.t()}
 
-  defmodule Options do
-    @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
+  @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
+  defmodule Options do
     @moduledoc "Defines shared options and defaults for this module's functions."
     defstruct api_fn: &Stops.Nearby.api_around/2,
               keys_fn: &Stops.Nearby.keys/1,
-              fetch_fn: Function.capture(@stops_repo, :get_parent, 1),
               routes_fn: &Routes.Repo.by_stop_and_direction/2,
               limit: nil
   end
@@ -52,7 +51,7 @@ defmodule Stops.Nearby do
       subway_stops |> Task.await() |> sort(position) |> no_more_than(1, opts.keys_fn),
       bus_stops |> Task.await() |> sort(position) |> no_more_than(2, opts.keys_fn)
     )
-    |> Task.async_stream(&opts.fetch_fn.(&1.id))
+    |> Task.async_stream(&@stops_repo.get_parent(&1.id))
     |> Enum.map(fn {:ok, result} -> result end)
   end
 

test/dotcom_web/controllers/schedule/line/helpers_test.exs

@@ -10,7 +10,6 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do
   doctest Helpers
 
   @routes_repo_api Application.compile_env!(:dotcom, :routes_repo_api)
-  @stops_repo Application.compile_env!(:dotcom, :repo_modules)[:stops]
 
   @stop %Stop{
     id: "110",
@@ -22,16 +21,12 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do
 
   setup :verify_on_exit!
 
-  setup do
-    stub(MBTA.Api.Mock, :get_json, fn "/routes/" <> id, _ ->
-      %JsonApi{data: [build(:route_item, %{id: id})]}
-    end)
-
-    :ok
-  end
-
   describe "get_route/1" do
     test "gets a route given its ID" do
+      stub(MBTA.Api.Mock, :get_json, fn "/routes/" <> id, _ ->
+        %JsonApi{data: [build(:route_item, %{id: id})]}
+      end)
+
       route_id = Faker.Internet.slug()
       assert {:ok, %Route{id: ^route_id}} = Helpers.get_route(route_id)
     end
@@ -47,9 +42,6 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do
   end
 
   describe "get_branch_route_stops/3" do
-    @tag skip: "We'll mock route patterns soon"
-    test "does not return branches for route patterns from multi trip routes"
-
     @tag :external
     test "returns a list of RouteStops, one for each branch of the line" do
       assert [
@@ -768,23 +760,24 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do
 
   describe "get_route_stops" do
     test "gets stops by route for a given route" do
-      stops_by_route_fn = fn route_id, direction_id, _opts ->
+      stub(Stops.Repo.Mock, :by_route, fn route_id, direction_id, _opts ->
         if route_id == "1" and direction_id == 0, do: [@stop], else: []
-      end
+      end)
 
-      assert Helpers.get_route_stops("1", 0, stops_by_route_fn) == @route_stops
+      assert Helpers.get_route_stops("1", 0) == @route_stops
     end
 
-    test "handles an error response from the stops_by_route_fn" do
-      stops_by_route_fn = fn _, _, _ -> {:error, "Error"} end
-
-      assert Helpers.get_route_stops("1", 0, stops_by_route_fn) == %{}
+    test "handles an error response from the stops function" do
+      stub(Stops.Repo.Mock, :by_route, fn _, _, _ -> {:error, "Error"} end)
+      assert Helpers.get_route_stops("1", 0) == %{}
     end
 
     test "gets stops for all Green lines" do
-      stops_by_route_fn = fn _, _, _ -> [@stop] end
+      stub(Stops.Repo.Mock, :by_route, fn _, _, _ ->
+        [@stop]
+      end)
 
-      assert Helpers.get_route_stops("Green", 0, stops_by_route_fn) == %{
+      assert Helpers.get_route_stops("Green", 0) == %{
                "Green-B" => [@stop],
                "Green-C" => [@stop],
                "Green-D" => [@stop],
@@ -847,25 +840,33 @@ defmodule DotcomWeb.ScheduleController.Line.HelpersTest do
   end
 
   describe "get_branches/4" do
-    @tag :external
     test "returns a list of RouteStops, one for each branch of the line" do
-      stops = Helpers.get_route_stops("Red", 0, &@stops_repo.by_route/3)
-      shapes = @routes_repo_api.get_shapes("Red", direction_id: 0)
+      stub(Stops.Repo.Mock, :get, fn id -> %Stop{id: id} end)
+      stub(Stops.Repo.Mock, :get_parent, fn id -> %Stop{id: id} end)
+      stub(Stops.Repo.Mock, :stop_features, fn _, _ -> [] end)
 
-      assert [%RouteStops{}, %RouteStops{}, %RouteStops{}] =
-               Helpers.get_branches(shapes, stops, %Route{id: "Red"}, 0)
-    end
+      stub(MBTA.Api.Mock, :get_json, fn "/routes/" <> _, _ ->
+        %JsonApi{
+          data: [
+            build(:route_item, %{
+              relationships: %{
+                "stops" => build_list(5, :stop_item)
+              }
+            })
+          ]
+        }
+      end)
 
-    @tag :external
-    test "returns RouteStops for all Green line branches" do
-      stops = Helpers.get_route_stops("Green", 0, &@stops_repo.by_route/3)
-      shapes = Helpers.get_shapes_by_direction("Green", 0, 0)
+      shapes = @routes_repo_api.get_shapes("Red", direction_id: 0)
+
+      stops =
+        Enum.flat_map(shapes, & &1.stop_ids)
+        |> Enum.map(&%Stop{id: &1})
 
-      assert [%RouteStops{}, %RouteStops{}, %RouteStops{}, %RouteStops{}] =
-               Helpers.get_branches(shapes, stops, %Route{id: "Green"}, 0)
+      assert [%RouteStops{}, %RouteStops{}, %RouteStops{}] =
+               Helpers.get_branches(shapes, %{"Red" => stops}, %Route{id: "Red"}, 0)
     end
 
-    @tag :external
     test "returns an empty list when given no stops" do
       stops = %{}
       shapes = @routes_repo_api.get_shapes("Red", direction_id: 0)

test/green_line_test.exs

@@ -94,8 +94,6 @@ defmodule GreenLineTest do
     test "each line returns a set of the ids of associated stops" do
       {_, stop_map} = calculate_stops_on_routes(0, Timex.today())
 
-      IO.inspect(stop_map)
-
       assert stop_map["Green-C"] ==
                MapSet.new(["place-clmnl", "place-coecl", "place-gover", "place-kencl"])