From ccd0f27f8d363bb8d8699641a198add512821134 Mon Sep 17 00:00:00 2001 From: Kayla Firestack Date: Wed, 6 Mar 2024 13:15:09 -0500 Subject: [PATCH] feat: add connection points to finished detour panel (#2473) * refactor(ex/skate/missed_stops): convert return type for missed stops to struct * feat(ex/skate/missed_stops): return connection segments from `missed_segments/2` * feat(ex/skate/missed_stops): return connection stops from API * feat(ts/models/finishedDetour): get connection points from API response * feat(ts/components/diversionPage): add connection points to text area and `useDetour` hook --- .../src/components/detours/diversionPage.tsx | 13 +- assets/src/hooks/useDetour.ts | 4 + assets/src/models/detour.ts | 4 + assets/src/models/finishedDetour.ts | 14 ++- assets/src/models/stopData.ts | 26 ++-- assets/tests/api.test.ts | 59 +++++++++- .../components/detours/diversionPage.test.tsx | 10 ++ .../tests/factories/finishedDetourFactory.ts | 4 + assets/tests/hooks/useDetour.test.ts | 37 ++++++ lib/skate/detours/missed_stops.ex | 52 ++++++-- .../controllers/detours_controller.ex | 15 ++- test/skate/detours/missed_stops_test.exs | 111 ++++++++++++++++-- .../controllers/detours_controller_test.exs | 57 ++++++++- 13 files changed, 372 insertions(+), 34 deletions(-) diff --git a/assets/src/components/detours/diversionPage.tsx b/assets/src/components/detours/diversionPage.tsx index 8f6bc8bae..9847b8be9 100644 --- a/assets/src/components/detours/diversionPage.tsx +++ b/assets/src/components/detours/diversionPage.tsx @@ -39,6 +39,7 @@ export const DiversionPage = ({ missedStops, routeSegments, + connectionPoints, canUndo, undo, @@ -60,11 +61,21 @@ export const DiversionPage = ({ "Turn-by-Turn Directions:", ...(directions?.map((v) => v.instruction) ?? []), , + "Connection Points:", + connectionPoints?.start?.name ?? "N/A", + connectionPoints?.end?.name ?? "N/A", + , `Missed Stops (${missedStops?.length}):`, ...(missedStops?.map(({ name }) => name) ?? ["no stops"]), ].join("\n") ) - }, [originalRoute, directions, missedStops]) + }, [ + originalRoute, + directions, + missedStops, + connectionPoints?.start?.name, + connectionPoints?.end?.name, + ]) const [showConfirmCloseModal, setShowConfirmCloseModal] = useState(false) diff --git a/assets/src/hooks/useDetour.ts b/assets/src/hooks/useDetour.ts index 307253505..a2c7a7442 100644 --- a/assets/src/hooks/useDetour.ts +++ b/assets/src/hooks/useDetour.ts @@ -169,6 +169,10 @@ export const useDetour = (routePatternId: RoutePatternId) => { * Three partial route-shape segments: before, during, and after the detour */ routeSegments: finishedDetour?.routeSegments, + /** + * Connection Points + */ + connectionPoints: finishedDetour?.connectionPoint, /** * Reports if {@link undo} will do anything. diff --git a/assets/src/models/detour.ts b/assets/src/models/detour.ts index 1aaa0271e..8ee428ac7 100644 --- a/assets/src/models/detour.ts +++ b/assets/src/models/detour.ts @@ -27,5 +27,9 @@ export interface RouteSegments { export interface FinishedDetour { missedStops: Stop[] + connectionPoint: { + start?: Stop + end?: Stop + } routeSegments: RouteSegments } diff --git a/assets/src/models/finishedDetour.ts b/assets/src/models/finishedDetour.ts index 26c6a664e..36e86af7e 100644 --- a/assets/src/models/finishedDetour.ts +++ b/assets/src/models/finishedDetour.ts @@ -1,6 +1,6 @@ -import { array, Infer, number, type } from "superstruct" +import { array, Infer, nullable, number, type } from "superstruct" import { FinishedDetour } from "./detour" -import { StopData, stopsFromData } from "./stopData" +import { StopData, stopFromData, stopsFromData } from "./stopData" const Coordinate = type({ lat: number(), @@ -9,6 +9,8 @@ const Coordinate = type({ export const FinishedDetourData = type({ missed_stops: array(StopData), + connection_stop_start: nullable(StopData), + connection_stop_end: nullable(StopData), route_segments: type({ before_detour: array(Coordinate), detour: array(Coordinate), @@ -22,6 +24,14 @@ export const finishedDetourFromData = ( ): FinishedDetour => { return { missedStops: stopsFromData(finishedDetour.missed_stops), + connectionPoint: { + start: finishedDetour.connection_stop_start + ? stopFromData(finishedDetour.connection_stop_start) + : undefined, + end: finishedDetour.connection_stop_end + ? stopFromData(finishedDetour.connection_stop_end) + : undefined, + }, routeSegments: { beforeDetour: finishedDetour.route_segments.before_detour, detour: finishedDetour.route_segments.detour, diff --git a/assets/src/models/stopData.ts b/assets/src/models/stopData.ts index 1dc1e5760..9806db168 100644 --- a/assets/src/models/stopData.ts +++ b/assets/src/models/stopData.ts @@ -48,16 +48,18 @@ export const StopData = type({ }) export type StopData = Infer +export const stopFromData = (stopData: StopData): Stop => ({ + id: stopData.id, + name: stopData.name, + lat: stopData.lat, + lon: stopData.lon, + locationType: + stopData.location_type === "station" + ? LocationType.Station + : LocationType.Stop, + vehicleType: stopData.vehicle_type || null, + routes: stopData.routes, +}) + export const stopsFromData = (stopsData: StopData[]): Stop[] => - stopsData.map((stopData) => ({ - id: stopData.id, - name: stopData.name, - lat: stopData.lat, - lon: stopData.lon, - locationType: - stopData.location_type === "station" - ? LocationType.Station - : LocationType.Stop, - vehicleType: stopData.vehicle_type || null, - routes: stopData.routes, - })) + stopsData.map(stopFromData) diff --git a/assets/tests/api.test.ts b/assets/tests/api.test.ts index 17b283b0c..7e81823b0 100644 --- a/assets/tests/api.test.ts +++ b/assets/tests/api.test.ts @@ -34,7 +34,12 @@ import routeTabFactory from "./factories/routeTab" import stopFactory from "./factories/stop" import * as browser from "../src/models/browser" import { string, unknown } from "superstruct" -import { LocationType, RouteType, stopsFromData } from "../src/models/stopData" +import { + LocationType, + RouteType, + stopFromData, + stopsFromData, +} from "../src/models/stopData" import * as Sentry from "@sentry/react" import locationSearchResultDataFactory from "./factories/locationSearchResultData" import locationSearchResultFactory from "./factories/locationSearchResult" @@ -395,7 +400,51 @@ describe("fetchShapeForRoute", () => { }) describe("fetchFinishedDetour", () => { - test("fetches missed stops in finished detour", () => { + test("fetches a finished detour given a Route Pattern and connection points", () => { + const stopData = stopDataFactory.buildList(3) + const [connection_stop_start, connection_stop_end] = + stopDataFactory.buildList(2) + + const stops = stopsFromData(stopData) + + const beforeDetour = shapePointFactory.buildList(3) + const detour = shapePointFactory.buildList(3) + const afterDetour = shapePointFactory.buildList(3) + + mockFetch(200, { + data: { + missed_stops: stopData, + connection_stop_start, + connection_stop_end, + route_segments: { + before_detour: beforeDetour, + detour: detour, + after_detour: afterDetour, + }, + }, + }) + + return fetchFinishedDetour( + "route_pattern_id", + shapePointFactory.build(), + shapePointFactory.build() + ).then((result) => { + expect(result).toEqual({ + missedStops: stops, + connectionPoint: { + start: stopFromData(connection_stop_start), + end: stopFromData(connection_stop_end), + }, + routeSegments: { + beforeDetour, + detour, + afterDetour, + }, + }) + }) + }) + + test("returns `undefined` for connection points if API result is `null`", () => { const stopData = stopDataFactory.buildList(3) const stops = stopsFromData(stopData) @@ -407,6 +456,8 @@ describe("fetchFinishedDetour", () => { mockFetch(200, { data: { missed_stops: stopData, + connection_stop_start: null, + connection_stop_end: null, route_segments: { before_detour: beforeDetour, detour: detour, @@ -422,6 +473,10 @@ describe("fetchFinishedDetour", () => { ).then((result) => { expect(result).toEqual({ missedStops: stops, + connectionPoint: { + start: undefined, + end: undefined, + }, routeSegments: { beforeDetour, detour, diff --git a/assets/tests/components/detours/diversionPage.test.tsx b/assets/tests/components/detours/diversionPage.test.tsx index 7426427ab..25db98bc8 100644 --- a/assets/tests/components/detours/diversionPage.test.tsx +++ b/assets/tests/components/detours/diversionPage.test.tsx @@ -292,6 +292,8 @@ describe("DiversionPage", () => { test("'Share Detour Details' screen copies text content to clipboard when clicked copy details button", async () => { const stops = stopFactory.buildList(4) + const [start, end] = stopFactory.buildList(2) + jest.mocked(fetchDetourDirections).mockResolvedValue( detourShapeFactory.build({ directions: [ @@ -303,6 +305,10 @@ describe("DiversionPage", () => { ) jest.mocked(fetchFinishedDetour).mockResolvedValue({ missedStops: stops, + connectionPoint: { + start, + end, + }, routeSegments: routeSegmentsFactory.build(), }) @@ -348,6 +354,10 @@ describe("DiversionPage", () => { "Turn right on High Street", "Turn sharp right on Broadway", , + "Connection Points:", + start.name, + end.name, + , `Missed Stops (${stops.length}):`, ...stops.map(({ name }) => name), ].join("\n") diff --git a/assets/tests/factories/finishedDetourFactory.ts b/assets/tests/factories/finishedDetourFactory.ts index 196d0a9ce..b542ee823 100644 --- a/assets/tests/factories/finishedDetourFactory.ts +++ b/assets/tests/factories/finishedDetourFactory.ts @@ -12,4 +12,8 @@ export const routeSegmentsFactory = Factory.define(() => ({ export const finishedDetourFactory = Factory.define(() => ({ missedStops: stopFactory.buildList(3), routeSegments: routeSegmentsFactory.build(), + connectionPoint: { + start: stopFactory.build(), + end: stopFactory.build(), + }, })) diff --git a/assets/tests/hooks/useDetour.test.ts b/assets/tests/hooks/useDetour.test.ts index 60973baec..3381559fe 100644 --- a/assets/tests/hooks/useDetour.test.ts +++ b/assets/tests/hooks/useDetour.test.ts @@ -318,6 +318,26 @@ describe("useDetour", () => { expect(result.current.routeSegments).toEqual(routeSegments) }) + test("when `endPoint` is set, `connectionPoints` is filled in", async () => { + const { result } = renderHook(useDetourWithFakeRoutePattern) + + const connectionPoint = { + start: stopFactory.build(), + end: stopFactory.build(), + } + + jest + .mocked(fetchFinishedDetour) + .mockResolvedValue(finishedDetourFactory.build({ connectionPoint })) + + act(() => result.current.addConnectionPoint?.(shapePointFactory.build())) + act(() => result.current.addConnectionPoint?.(shapePointFactory.build())) + + await waitFor(() => { + expect(result.current.connectionPoints).toEqual(connectionPoint) + }) + }) + test("when `endPoint` is undone, `missedStops` is cleared", async () => { const { result } = renderHook(useDetourWithFakeRoutePattern) @@ -358,6 +378,23 @@ describe("useDetour", () => { }) }) + test("when `endPoint` is undone, `connectionPoints` is cleared", async () => { + const { result } = renderHook(useDetourWithFakeRoutePattern) + + act(() => result.current.addConnectionPoint?.(shapePointFactory.build())) + act(() => result.current.addConnectionPoint?.(shapePointFactory.build())) + + await waitFor(() => { + expect(result.current.connectionPoints).not.toBeUndefined() + }) + + act(() => result.current.undo?.()) + + await waitFor(() => { + expect(result.current.connectionPoints).toBeUndefined() + }) + }) + test("initially, `finishDetour` is `undefined`", () => { const { result } = renderHook(useDetourWithFakeRoutePattern) diff --git a/lib/skate/detours/missed_stops.ex b/lib/skate/detours/missed_stops.ex index b52d69a63..2572f0b4c 100644 --- a/lib/skate/detours/missed_stops.ex +++ b/lib/skate/detours/missed_stops.ex @@ -16,10 +16,21 @@ defmodule Skate.Detours.MissedStops do @enforce_keys [:connection_start, :connection_end, :stops, :shape] defstruct [:connection_start, :connection_end, :stops, :shape] + defmodule Result do + @moduledoc false + @type t :: %__MODULE__{ + missed_stops: [Util.Location.From.t()], + connection_stop_start: Util.Location.From.t() | nil, + connection_stop_end: Util.Location.From.t() | nil + } + @enforce_keys [:missed_stops, :connection_stop_start, :connection_stop_end] + defstruct [:missed_stops, :connection_stop_start, :connection_stop_end] + end + @doc """ Returns the contiguous list of stops, from the input parameter `cfg.stops`. """ - @spec missed_stops(cfg :: __MODULE__.t()) :: [Util.Location.From.t()] + @spec missed_stops(cfg :: __MODULE__.t()) :: __MODULE__.Result.t() def( missed_stops(%__MODULE__{ stops: stops, @@ -30,24 +41,51 @@ defmodule Skate.Detours.MissedStops do ) do segmented_shape = segment_shape_by_stops(shape, stops) - {connection_start, connection_end} - |> missed_segments(segmented_shape) - |> Enum.map(& &1.stop) + %{ + missed_stops: missed_stops, + connection_start_segment: connection_start_segment, + connection_end_segment: connection_end_segment + } = + missed_segments({connection_start, connection_end}, segmented_shape) + + %__MODULE__.Result{ + missed_stops: Enum.map(missed_stops, & &1.stop), + connection_stop_start: + case connection_start_segment do + %Skate.Detours.ShapeSegment{stop: stop} when stop != :none -> stop + _ -> nil + end, + connection_stop_end: + case connection_end_segment do + %Skate.Detours.ShapeSegment{stop: stop} when stop != :none -> stop + _ -> nil + end + } end @spec missed_segments( {connection_start :: Util.Location.From.t(), connection_end :: Util.Location.From.t()}, segmented_shape :: [Skate.Detours.ShapeSegment.t()] - ) :: [Skate.Detours.ShapeSegment.t()] + ) :: %{ + missed_stops: [Skate.Detours.ShapeSegment.t()], + connection_start_segment: Skate.Detours.ShapeSegment.t() | nil, + connection_end_segment: Skate.Detours.ShapeSegment.t() | nil + } defp missed_segments({connection_start, connection_end}, segmented_shape) do %{index: start_index} = get_index_by_min_dist(segmented_shape, connection_start) - remaining_segments = Enum.drop(segmented_shape, start_index) + {first_segments, remaining_segments} = Enum.split(segmented_shape, start_index) %{index: end_count} = get_index_by_min_dist(remaining_segments, connection_end) - Enum.take(remaining_segments, end_count) + {missed_stop_segments, end_segments} = Enum.split(remaining_segments, end_count) + + %{ + missed_stops: missed_stop_segments, + connection_start_segment: List.last(first_segments), + connection_end_segment: List.first(end_segments) + } end @spec segment_shape_by_stops( diff --git a/lib/skate_web/controllers/detours_controller.ex b/lib/skate_web/controllers/detours_controller.ex index d623acd04..41bc498ce 100644 --- a/lib/skate_web/controllers/detours_controller.ex +++ b/lib/skate_web/controllers/detours_controller.ex @@ -19,7 +19,11 @@ defmodule SkateWeb.DetoursController do connection_start_location = Location.new(connection_start["lat"], connection_start["lon"]) connection_end_location = Location.new(connection_end["lat"], connection_end["lon"]) - missed_stops = + %MissedStops.Result{ + missed_stops: missed_stops, + connection_stop_start: connection_stop_start, + connection_stop_end: connection_stop_end + } = missed_stops(%MissedStops{ connection_start: connection_start_location, connection_end: connection_end_location, @@ -34,7 +38,14 @@ defmodule SkateWeb.DetoursController do connection_end_location ) - json(conn, %{data: %{missed_stops: missed_stops, route_segments: route_segments}}) + json(conn, %{ + data: %{ + missed_stops: missed_stops, + route_segments: route_segments, + connection_stop_start: connection_stop_start, + connection_stop_end: connection_stop_end + } + }) else _ -> send_resp(conn, :bad_request, "bad request") end diff --git a/test/skate/detours/missed_stops_test.exs b/test/skate/detours/missed_stops_test.exs index de99a4ffb..e6b7840d0 100644 --- a/test/skate/detours/missed_stops_test.exs +++ b/test/skate/detours/missed_stops_test.exs @@ -1,6 +1,7 @@ defmodule Skate.Detours.MissedStopsTest do use ExUnit.Case + alias Skate.Detours.MissedStops alias Util.Location describe "missed_stops" do @@ -17,12 +18,14 @@ defmodule Skate.Detours.MissedStopsTest do ## (connection_start) (connection_end) # # https://excalidraw.com/#json=OrMM928mw4CR3Qy8sc6oL,sXiFCU1s-K1ugEQvgSvh3g + missed_stop = Location.new(0.001, 5) + param = %Skate.Detours.MissedStops{ connection_start: Location.new(-0.001, 1), connection_end: Location.new(-0.001, 6), stops: [ Location.new(0.001, 0), - Location.new(0.001, 5), + missed_stop, Location.new(0.001, 7) ], shape: [ @@ -37,9 +40,96 @@ defmodule Skate.Detours.MissedStopsTest do ] } - assert [ - Location.new(0.001, 5) - ] == Skate.Detours.MissedStops.missed_stops(param) + assert %MissedStops.Result{missed_stops: [^missed_stop]} = + Skate.Detours.MissedStops.missed_stops(param) + end + + test "returns connection points" do + connection_stop_start = Location.new(0.001, 2) + connection_stop_end = Location.new(0.001, 6) + + param = %Skate.Detours.MissedStops{ + connection_start: Location.new(-0.001, 3), + connection_end: Location.new(-0.001, 5), + stops: [ + Location.new(0.001, 1), + connection_stop_start, + Location.new(0.001, 4), + connection_stop_end, + Location.new(0.001, 7) + ], + shape: [ + Location.new(0, 0), + Location.new(0, 1), + Location.new(0, 2), + Location.new(0, 3), + Location.new(0, 4), + Location.new(0, 5), + Location.new(0, 6), + Location.new(0, 7) + ] + } + + assert %MissedStops.Result{ + connection_stop_start: ^connection_stop_start, + connection_stop_end: ^connection_stop_end + } = Skate.Detours.MissedStops.missed_stops(param) + end + + test "returns nil for connection_start if the first stop is missed" do + connection_stop_end = Location.new(0.001, 7) + + param = %Skate.Detours.MissedStops{ + connection_start: Location.new(-0.001, 1), + connection_end: Location.new(-0.001, 6), + stops: [ + Location.new(0.001, 5), + connection_stop_end + ], + shape: [ + Location.new(0, 0), + Location.new(0, 1), + Location.new(0, 2), + Location.new(0, 3), + Location.new(0, 4), + Location.new(0, 5), + Location.new(0, 6), + Location.new(0, 7) + ] + } + + assert %MissedStops.Result{ + connection_stop_start: nil, + connection_stop_end: ^connection_stop_end + } = Skate.Detours.MissedStops.missed_stops(param) + end + + test "returns nil for connection_end if the last stop is missed" do + connection_stop_start = Location.new(0.001, 0) + + param = %Skate.Detours.MissedStops{ + connection_start: Location.new(-0.001, 1), + connection_end: Location.new(-0.001, 6), + stops: [ + connection_stop_start, + Location.new(0.001, 5) + ], + shape: [ + Location.new(0, 0), + Location.new(0, 1), + Location.new(0, 2), + Location.new(0, 3), + Location.new(0, 4), + Location.new(0, 5), + Location.new(0, 6), + Location.new(0, 7) + ] + } + + assert %MissedStops.Result{ + connection_stop_start: ^connection_stop_start, + connection_stop_end: nil + } = Skate.Detours.MissedStops.missed_stops(param) end test "given a start and end connection points within the same segment, should return empty list" do @@ -63,7 +153,8 @@ defmodule Skate.Detours.MissedStopsTest do ] } - assert [] == Skate.Detours.MissedStops.missed_stops(param) + assert %{missed_stops: []} = + Skate.Detours.MissedStops.missed_stops(param) end test "given a stop that is visited twice, should return missed stops" do @@ -90,12 +181,15 @@ defmodule Skate.Detours.MissedStopsTest do ] } + assert %MissedStops.Result{missed_stops: missed_stops} = + Skate.Detours.MissedStops.missed_stops(param) + assert [ duplicate_stop, Location.new(1, 1), duplicate_stop ] == - Skate.Detours.MissedStops.missed_stops(param) + missed_stops end test "can handle real shapes and stops" do @@ -113,8 +207,11 @@ defmodule Skate.Detours.MissedStopsTest do connection_end: Enum.at(stops, connection_end_index) } - assert Enum.slice(stops, connection_start_index..(connection_end_index - 1)) == + assert %MissedStops.Result{missed_stops: missed_stops} = Skate.Detours.MissedStops.missed_stops(param) + + assert Enum.slice(stops, connection_start_index..(connection_end_index - 1)) == + missed_stops end end end diff --git a/test/skate_web/controllers/detours_controller_test.exs b/test/skate_web/controllers/detours_controller_test.exs index 244e08193..8a0f078f7 100644 --- a/test/skate_web/controllers/detours_controller_test.exs +++ b/test/skate_web/controllers/detours_controller_test.exs @@ -27,7 +27,11 @@ defmodule SkateWeb.DetoursControllerTest do } when stops == shape_with_stops.stops and shape == shape_with_stops.points -> - [missed_stop] + %MissedStops.Result{ + missed_stops: [missed_stop], + connection_stop_start: nil, + connection_stop_end: nil + } end) conn = @@ -45,6 +49,57 @@ defmodule SkateWeb.DetoursControllerTest do assert %{"data" => %{"missed_stops" => [^expected_missed_stop]}} = json_response(conn, 200) end + @tag :authenticated + test "returns connection points", %{conn: conn} do + route_pattern = build(:gtfs_route_pattern) + shape_with_stops = build(:shape_with_stops) + + connection_start = shape_with_stops.points |> Enum.at(1) |> Util.Location.as_location!() + connection_end = shape_with_stops.points |> Enum.at(2) |> Util.Location.as_location!() + missed_stop = Enum.at(shape_with_stops.stops, 1) + + connection_stop_start = Enum.at(shape_with_stops.stops, 0) + connection_stop_end = Enum.at(shape_with_stops.stops, 2) + + reassign_env(:skate_web, :route_pattern_fn, fn _ -> route_pattern end) + reassign_env(:skate_web, :shape_with_stops_fn, fn _ -> shape_with_stops end) + + reassign_env(:skate_web, :missed_stops_fn, fn %MissedStops{ + connection_start: ^connection_start, + connection_end: ^connection_end, + stops: stops, + shape: shape + } + when stops == shape_with_stops.stops and + shape == shape_with_stops.points -> + %MissedStops.Result{ + missed_stops: [missed_stop], + connection_stop_start: connection_stop_start, + connection_stop_end: connection_stop_end + } + end) + + conn = + post(conn, ~p"/api/detours/finished_detour", + route_pattern_id: route_pattern.id, + connection_start: %{ + "lat" => connection_start.latitude, + "lon" => connection_start.longitude + }, + connection_end: %{"lat" => connection_end.latitude, "lon" => connection_end.longitude} + ) + + connection_stop_start = Jason.decode!(Jason.encode!(Enum.at(shape_with_stops.stops, 0))) + connection_stop_end = Jason.decode!(Jason.encode!(Enum.at(shape_with_stops.stops, 2))) + + assert %{ + "data" => %{ + "connection_stop_start" => ^connection_stop_start, + "connection_stop_end" => ^connection_stop_end + } + } = json_response(conn, 200) + end + @tag :authenticated test "returns route segments", %{conn: conn} do points = [