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.

Sign up for GitHub

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(RoutePatterns.Repo): use factories and mock for tests #2027

Merged
merged 12 commits into from
May 6, 2024
Merged

refactor(RoutePatterns.Repo): use factories and mock for tests #2027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
878c99b
chore: rename existing behaviour
thecristen May 1, 2024
fafd376
chore: rename existing mock
thecristen May 1, 2024
1120e4a
chore: fix dialyzer, race conditions was removed
thecristen May 1, 2024
2a5ffc1
fix(RoutePattern): amend struct values
thecristen May 1, 2024
1967571
refactor(RoutePatterns.Repo): use the mock for tests
thecristen May 1, 2024
353eb7d
more docs
thecristen May 2, 2024
5f34cd8
feedback
thecristen May 2, 2024
5e692fb
tests(StopController): add mock for test
thecristen May 2, 2024
3a1f56a
refactor factories: just do the thing
thecristen May 6, 2024
5457fd7
simple item
thecristen May 6, 2024
72bab36
fixup
thecristen May 6, 2024
7e204ca
remove magic number in test!
thecristen May 6, 2024
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
19 changes: 9 additions & 10 deletions assets/ts/__v3api.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,20 +349,19 @@ export interface Shape {
}

export interface RoutePattern {
typicality: 1 | 2 | 3 | 4;
time_desc: string | null;
route_id: string;
canonical: boolean;
thecristen marked this conversation as resolved.
Show resolved Hide resolved
direction_id: DirectionId;
headsign: string;
id: string;
name: string;
representative_trip_id: string;
representative_trip_polyline: string;
route_id: string;
shape_id: string;
shape_priority: number;
stop_ids: string[];
headsign: string;
name: string;
id: string;
direction_id: DirectionId;
sort_order: number;
canonical: boolean;
stop_ids: string[];
time_desc: string | null;
typicality: 0 | 1 | 2 | 3 | 4 | 5;
}

export interface StopHours {
Expand Down
4 changes: 0 additions & 4 deletions assets/ts/schedule/components/ScheduleDirection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,6 @@ const ScheduleDirection = ({
if (result.length === 0 || current.typicality < result[0].typicality)
return [current];
if (current.typicality === result[0].typicality) {
if (result[0].shape_priority < 0 && current.shape_priority > 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why this is being removed in the context of this PR. A note in the PR would help.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, totally forgot to mention it. Priority was an attribute of V3 API shapes that's been removed in more recent versions, and as far as I could tell we had already stopped reading/parsing it in our own codebase. Added a note to the main PR text too.

return [current];
if (current.shape_priority < 0 && result[0].shape_priority > 0)
return result;
return result.concat(current);
}
return result;
Expand Down
1 change: 0 additions & 1 deletion assets/ts/schedule/components/__schedule.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {

export interface EnhancedRoutePattern extends RoutePattern {
shape_id: string;
shape_priority: number;
headsign: string;
}

Expand Down
2 changes: 0 additions & 2 deletions assets/ts/schedule/components/__tests__/ScheduleFinderTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ const routePatternsByDirection = {
typicality: 1,
time_desc: "School Trip",
shape_id: "9840004",
shape_priority: 1,
route_id: "CR-Fitchburg",
representative_trip_id: "CR-Weekday-Spring-19-401",
representative_trip_polyline: "qwerty123@777njhgb",
Expand All @@ -159,7 +158,6 @@ const routePatternsByDirection = {
typicality: 1,
time_desc: "School Trip",
shape_id: "9840003",
shape_priority: 1,
route_id: "CR-Fitchburg",
representative_trip_id: "CR-Weekday-Spring-19-400",
representative_trip_polyline: "lkjhg987bvcxz88!",
Expand Down
4 changes: 0 additions & 4 deletions assets/ts/schedule/components/__tests__/test-data/routePatternsByDirectionData.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
"typicality": 1,
"time_desc": null,
"shape_id": "shape-1",
"shape_priority": 1,
"route_id": "route-1",
"representative_trip_id": "trip-1",
"representative_trip_polyline": "trip-1-polyline",
Expand All @@ -25,7 +24,6 @@
"typicality": 1,
"time_desc": null,
"shape_id": "shape-3",
"shape_priority": 1,
"route_id": "route-1",
"representative_trip_id": "trip-3",
"representative_trip_polyline": "trip-3-polyline",
Expand All @@ -46,7 +44,6 @@
"typicality": 3,
"time_desc": null,
"shape_id": "shape-4",
"shape_priority": 1,
"route_id": "route-1",
"representative_trip_id": "trip-4",
"representative_trip_polyline": "trip-4-polyline",
Expand All @@ -69,7 +66,6 @@
"typicality": 2,
"time_desc": null,
"shape_id": "shape-2",
"shape_priority": 1,
"route_id": "route-1",
"representative_trip_id": "trip-1",
"representative_trip_polyline": "trip-2-polyline",
Expand Down
2 changes: 0 additions & 2 deletions assets/ts/schedule/components/direction/__tests__/BusMenuTest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const routePatterns: EnhancedRoutePattern[] = [
stop_ids: ["123", "456", "789"],
route_id: "66",
shape_id: "660140",
shape_priority: 1,
time_desc: null,
typicality: 1,
sort_order: 5,
Expand All @@ -30,7 +29,6 @@ const routePatterns: EnhancedRoutePattern[] = [
{
typicality: 3,
time_desc: "School days only",
shape_priority: 1,
shape_id: "660141-2",
route_id: "66",
representative_trip_id: "43773700_2",
Expand Down
4 changes: 0 additions & 4 deletions assets/ts/schedule/components/direction/__tests__/reducer-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const routePatternsForInbound: EnhancedRoutePattern[] = [
stop_ids: ["123", "456", "789"],
route_id: "111",
shape_id: "1110177",
shape_priority: 1,
time_desc: null,
typicality: 1,
sort_order: 1,
Expand All @@ -29,7 +28,6 @@ const routePatternsForInbound: EnhancedRoutePattern[] = [
stop_ids: ["123", "555", "789"],
route_id: "111",
shape_id: "1110157",
shape_priority: 1,
time_desc: "Weekdays only",
typicality: 2,
sort_order: 2,
Expand All @@ -56,7 +54,6 @@ const initialState: State = {
representative_trip_polyline: "asdf444$hhhhmnb",
stop_ids: ["123", "555", "777"],
route_id: "111",
shape_priority: 1,
shape_id: "1110180",
time_desc: null,
typicality: 1,
Expand Down Expand Up @@ -90,7 +87,6 @@ it("menuReducer handles 'toggleDirection'", () => {
stop_ids: ["123", "456", "789"],
route_id: "111",
shape_id: "1110177",
shape_priority: 1,
time_desc: null,
typicality: 1,
sort_order: 1,
Expand Down
1 change: 1 addition & 0 deletions config/config.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ config :elixir, ansi_enabled: true
config :dotcom, :httpoison, HTTPoison

config :dotcom, :mbta_api_module, MBTA.Api
config :dotcom, :repo_modules, route_patterns: RoutePatterns.Repo
thecristen marked this conversation as resolved.
Show resolved Hide resolved

config :dotcom, :redis, Dotcom.Cache.Multilevel.Redis
config :dotcom, :redix, Redix
Expand Down
4 changes: 0 additions & 4 deletions config/dotcom/dotcom.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ config :dotcom, route_populate_caches?: config_env() == :prod
routes_repo = if config_env() == :test, do: Routes.MockRepoApi, else: Routes.Repo
config :dotcom, :routes_repo_api, routes_repo

repo_module = if config_env() == :test, do: RoutePatterns.MockRepo, else: RoutePatterns.Repo

config :dotcom, :route_patterns_repo_api, repo_module

predictions_broadcast_interval_ms = if config_env() == :test, do: 50, else: 10_000

config :dotcom, predictions_broadcast_interval_ms: predictions_broadcast_interval_ms
1 change: 1 addition & 0 deletions config/test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ config :dotcom, :cache, Dotcom.Cache.TestCache
config :dotcom, :httpoison, HTTPoison.Mock

config :dotcom, :mbta_api_module, MBTA.Api.Mock
config :dotcom, :repo_modules, route_patterns: RoutePatterns.Repo.Mock

config :dotcom, :redis, Dotcom.Redis.Mock
config :dotcom, :redix, Dotcom.Redix.Mock
Expand Down
4 changes: 3 additions & 1 deletion lib/dotcom_web/controllers/schedule/finder_api.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ defmodule DotcomWeb.ScheduleController.FinderApi do

require Logger

@route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns]

@type react_keys :: :date | :direction | :is_current
@type react_strings :: [{react_keys, String.t()}]
@type converted_values :: {Date.t(), integer, boolean}
Expand Down Expand Up @@ -508,7 +510,7 @@ defmodule DotcomWeb.ScheduleController.FinderApi do
with %Schedules.Trip{route_pattern_id: route_pattern_id} when not is_nil(route_pattern_id) <-
Schedules.Repo.trip(trip_id),
%RoutePatterns.RoutePattern{route_id: route_id} <-
RoutePatterns.Repo.get(route_pattern_id) do
@route_patterns_repo.get(route_pattern_id) do
route_id
else
_ ->
Expand Down
18 changes: 6 additions & 12 deletions lib/dotcom_web/controllers/schedule/line/helpers.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do
Helpers for the line page
"""

alias RoutePatterns.Repo, as: RoutePatternsRepo
alias RoutePatterns.RoutePattern
alias Routes.Repo, as: RoutesRepo
alias Routes.{Route, Shape}
alias Stops.Repo, as: StopsRepo
alias Stops.{RouteStop, RouteStops, Stop}

@route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns]

@type query_param :: String.t() | nil
@type direction_id :: 0 | 1
@typep stops_by_route :: %{String.t() => [Stop.t()]}
Expand Down Expand Up @@ -88,7 +89,7 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do

def get_map_route_patterns(route_id, type) do
route_id
|> RoutePatternsRepo.by_route_id(
|> @route_patterns_repo.by_route_id(
include: "representative_trip.shape,representative_trip.stops"
)
|> filter_map_route_patterns(type)
Expand All @@ -110,19 +111,12 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do

@doc """
Filters a list of route patterns down to the route patterns sharing the lowest
number for the "typicality" property, additionally removing route patterns
associated with a negative shape_priority value.
number for the "typicality" property.
"""
@spec filtered_by_typicality([RoutePattern.t()]) :: [RoutePattern.t()]
def filtered_by_typicality(route_patterns) do
route_patterns
|> filter_by_min_typicality()
|> Enum.filter(fn x ->
# TODO: Deprecate our use of shape priority entirely,
# because it's no longer supported in the V3 API
# For now, be less strict if using the most typical route pattern
if x.typicality == 1, do: true, else: x.shape_priority > 0
end)
end

# Filters route patterns by the smallest typicality found in the array
Expand Down Expand Up @@ -274,12 +268,12 @@ defmodule DotcomWeb.ScheduleController.Line.Helpers do
base_opts
end

RoutePatternsRepo.by_route_id(route_id, opts)
@route_patterns_repo.by_route_id(route_id, opts)
|> Enum.filter(&(&1.route_id == route_id))
end

defp get_line_route_patterns(_route, _direction_id, route_pattern_id) do
case RoutePatternsRepo.get(route_pattern_id,
case @route_patterns_repo.get(route_pattern_id,
include: "representative_trip.stops"
) do
%RoutePattern{} = route_pattern ->
Expand Down
4 changes: 3 additions & 1 deletion lib/dotcom_web/controllers/schedule/timetable_controller.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ defmodule DotcomWeb.ScheduleController.TimetableController do

require Logger

@route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns]

plug(DotcomWeb.Plugs.Route)
plug(DotcomWeb.Plugs.DateInRating)
plug(:tab_name)
Expand Down Expand Up @@ -61,7 +63,7 @@ defmodule DotcomWeb.ScheduleController.TimetableController do
} = build_timetable(conn.assigns.all_stops, timetable_schedules, direction_id)

canonical_rps =
RoutePatterns.Repo.by_route_id(route.id,
@route_patterns_repo.by_route_id(route.id,
direction_id: direction_id,
canonical: true,
include: "representative_trip.stops"
Expand Down
4 changes: 3 additions & 1 deletion lib/dotcom_web/controllers/stop_controller.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ defmodule DotcomWeb.StopController do
alias Stops.{Repo, Stop}
alias Util.AndOr

@route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns]

plug(:alerts)
plug(DotcomWeb.Plugs.AlertsByTimeframe)

Expand Down Expand Up @@ -90,7 +92,7 @@ defmodule DotcomWeb.StopController do
@spec route_patterns_by_route_and_headsign(Stop.id_t()) :: by_route_and_headsign()
defp route_patterns_by_route_and_headsign(stop_id) do
stop_id
|> RoutePatterns.Repo.by_stop_id()
|> @route_patterns_repo.by_stop_id()
|> Stream.reject(&ends_at?(&1, stop_id))
|> Stream.reject(&exclusively_drop_offs?(&1, stop_id))
|> Enum.group_by(& &1.route_id)
Expand Down
4 changes: 3 additions & 1 deletion lib/predictions/stream_topic.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ defmodule Predictions.StreamTopic do

defstruct [:topic, :fetch_keys, :streams]

@route_patterns_repo Application.compile_env!(:dotcom, :repo_modules)[:route_patterns]

@type filter_params :: String.t()
@type clear_keys :: Store.fetch_keys()
@type t :: %__MODULE__{
Expand Down Expand Up @@ -56,7 +58,7 @@ defmodule Predictions.StreamTopic do

@spec streams_from_fetch_keys(Store.fetch_keys()) :: [{clear_keys, filter_params}]
defp streams_from_fetch_keys(stop: stop_id) do
RoutePatterns.Repo.by_stop_id(stop_id)
@route_patterns_repo.by_stop_id(stop_id)
|> Enum.map(&{to_keys(&1), to_filter_name(&1)})
end

Expand Down
Loading
Loading