From 8acf7072c0392401f45e97dfc4e47d1e83b134a8 Mon Sep 17 00:00:00 2001 From: Hannah Purcell Date: Thu, 2 Jan 2025 13:02:28 -0500 Subject: [PATCH 1/4] fix: Channel name in broadcast for detours by route --- lib/skate/detours/detours.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/skate/detours/detours.ex b/lib/skate/detours/detours.ex index 7c097f05f..9582c97a1 100644 --- a/lib/skate/detours/detours.ex +++ b/lib/skate/detours/detours.ex @@ -274,7 +274,7 @@ defmodule Skate.Detours.Detours do Phoenix.PubSub.broadcast( Skate.PubSub, - "detours:active" <> route_id, + "detours:active:" <> route_id, {:detour_activated, db_detour_to_detour(detour)} ) @@ -290,7 +290,7 @@ defmodule Skate.Detours.Detours do Phoenix.PubSub.broadcast( Skate.PubSub, - "detours:active" <> route_id, + "detours:active:" <> route_id, {:detour_deactivated, db_detour_to_detour(detour)} ) From e269c7dc28037f4ea6d2adf92046ef8f4dba0096 Mon Sep 17 00:00:00 2001 From: Hannah Purcell Date: Mon, 6 Jan 2025 09:12:14 -0500 Subject: [PATCH 2/4] fix: need to parse backend data to frontend struct --- assets/src/hooks/useDetours.ts | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/assets/src/hooks/useDetours.ts b/assets/src/hooks/useDetours.ts index 5812803d4..bfc151850 100644 --- a/assets/src/hooks/useDetours.ts +++ b/assets/src/hooks/useDetours.ts @@ -1,5 +1,9 @@ import { Channel, Socket } from "phoenix" -import { SimpleDetour } from "../models/detoursList" +import { + SimpleDetour, + SimpleDetourData, + simpleDetourFromData, +} from "../models/detoursList" import { useEffect, useState } from "react" import { reload } from "../models/browser" import { userUuid } from "../util/userUuid" @@ -21,17 +25,25 @@ const subscribe = ( const channel = socket.channel(topic) handleDrafted && - channel.on("drafted", ({ data: data }) => handleDrafted(data)) + channel.on("drafted", ({ data: data }) => + handleDrafted(simpleDetourFromData(data)) + ) handleActivated && - channel.on("activated", ({ data: data }) => handleActivated(data)) + channel.on("activated", ({ data: data }) => + handleActivated(simpleDetourFromData(data)) + ) handleDeactivated && - channel.on("deactivated", ({ data: data }) => handleDeactivated(data)) + channel.on("deactivated", ({ data: data }) => + handleDeactivated(simpleDetourFromData(data)) + ) channel.on("auth_expired", reload) channel .join() - .receive("ok", ({ data: data }: { data: SimpleDetour[] }) => { - const detoursMap = Object.fromEntries(data.map((v) => [v.id, v])) + .receive("ok", ({ data: data }: { data: SimpleDetourData[] }) => { + const detoursMap = Object.fromEntries( + data.map((v) => [v.id, simpleDetourFromData(v)]) + ) initializeChannel(detoursMap) }) @@ -182,8 +194,10 @@ const subscribeByRoute = ( channel .join() - .receive("ok", ({ data: data }: { data: SimpleDetour[] }) => { - const detoursMap = Object.fromEntries(data.map((v) => [v.id, v])) + .receive("ok", ({ data: data }: { data: SimpleDetourData[] }) => { + const detoursMap = Object.fromEntries( + data.map((v) => [v.id, simpleDetourFromData(v)]) + ) setDetours((detoursByRouteId) => ({ ...detoursByRouteId, [routeId]: detoursMap, From 18605d4603eaeaff3547942a12ca98a6d813470b Mon Sep 17 00:00:00 2001 From: Hannah Purcell Date: Mon, 6 Jan 2025 11:45:48 -0500 Subject: [PATCH 3/4] fix: don't mix-match route name and id! --- lib/skate/detours/detours.ex | 12 +++++------ .../channels/detours_channel_test.exs | 4 ++-- .../controllers/detours_controller_test.exs | 6 ++++++ test/support/factories/detour_factory.ex | 20 +++++++++++++++---- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/skate/detours/detours.ex b/lib/skate/detours/detours.ex index 9582c97a1..f95c22290 100644 --- a/lib/skate/detours/detours.ex +++ b/lib/skate/detours/detours.ex @@ -40,7 +40,7 @@ defmodule Skate.Detours.Detours do def active_detours_by_route(route_id) do list_detours() |> Enum.filter(fn detour -> - categorize_detour(detour) == :active and get_detour_route(detour) == route_id + categorize_detour(detour) == :active and get_detour_route_id(detour) == route_id end) |> Enum.map(fn detour -> db_detour_to_detour(detour) end) end @@ -131,9 +131,9 @@ defmodule Skate.Detours.Detours do def categorize_detour(_detour_context), do: :draft - @spec get_detour_route(detour :: map()) :: String.t() - defp get_detour_route(%{state: %{"context" => %{"route" => %{"name" => route_name}}}}), - do: route_name + @spec get_detour_route_id(detour :: map()) :: String.t() + defp get_detour_route_id(%{state: %{"context" => %{"route" => %{"id" => route_id}}}}), + do: route_id @doc """ Gets a single detour. @@ -264,7 +264,7 @@ defmodule Skate.Detours.Detours do |> User.get_by_id!() |> Map.get(:uuid) - route_id = get_detour_route(detour) + route_id = get_detour_route_id(detour) Phoenix.PubSub.broadcast( Skate.PubSub, @@ -286,7 +286,7 @@ defmodule Skate.Detours.Detours do end defp broadcast_detour(:past, detour, _author_id) do - route_id = get_detour_route(detour) + route_id = get_detour_route_id(detour) Phoenix.PubSub.broadcast( Skate.PubSub, diff --git a/test/skate_web/channels/detours_channel_test.exs b/test/skate_web/channels/detours_channel_test.exs index d0086002f..7f523de76 100644 --- a/test/skate_web/channels/detours_channel_test.exs +++ b/test/skate_web/channels/detours_channel_test.exs @@ -30,10 +30,10 @@ defmodule SkateWeb.DetoursChannelTest do draft_detour = :detour_snapshot |> build() |> with_id(1) active_detour_one = - :detour_snapshot |> build() |> activated |> with_id(2) |> with_route_name("57") + :detour_snapshot |> build() |> activated |> with_id(2) |> with_route("57") active_detour_two = - :detour_snapshot |> build() |> activated |> with_id(3) |> with_route_name("66") + :detour_snapshot |> build() |> activated |> with_id(3) |> with_route("66") past_detour = :detour_snapshot |> build() |> deactivated |> with_id(4) diff --git a/test/skate_web/controllers/detours_controller_test.exs b/test/skate_web/controllers/detours_controller_test.exs index 9a8ba03ca..996ce393f 100644 --- a/test/skate_web/controllers/detours_controller_test.exs +++ b/test/skate_web/controllers/detours_controller_test.exs @@ -132,6 +132,7 @@ defmodule SkateWeb.DetoursControllerTest do "snapshot" => %{ "context" => %{ "route" => %{ + "id" => "23", "name" => "23", "directionNames" => %{ "0" => "Outbound", @@ -154,6 +155,7 @@ defmodule SkateWeb.DetoursControllerTest do "snapshot" => %{ "context" => %{ "route" => %{ + "id" => "47", "name" => "47", "directionNames" => %{ "0" => "Outbound", @@ -176,6 +178,7 @@ defmodule SkateWeb.DetoursControllerTest do "snapshot" => %{ "context" => %{ "route" => %{ + "id" => "75", "name" => "75", "directionNames" => %{ "0" => "Outbound", @@ -358,6 +361,7 @@ defmodule SkateWeb.DetoursControllerTest do Detours.upsert_from_snapshot(other_user.id, %{ "context" => %{ "route" => %{ + # "id" => "23", "name" => "23", "directionNames" => %{ "0" => "Outbound", @@ -423,6 +427,7 @@ defmodule SkateWeb.DetoursControllerTest do "snapshot" => %{ "context" => %{ "route" => %{ + "id" => "23", "name" => "23", "directionNames" => %{ "0" => "Outbound", @@ -444,6 +449,7 @@ defmodule SkateWeb.DetoursControllerTest do "snapshot" => %{ "context" => %{ "route" => %{ + "id" => "23", "name" => "23" }, "routePattern" => %{ diff --git a/test/support/factories/detour_factory.ex b/test/support/factories/detour_factory.ex index 132563a47..ff1e06911 100644 --- a/test/support/factories/detour_factory.ex +++ b/test/support/factories/detour_factory.ex @@ -26,6 +26,7 @@ defmodule Skate.DetourFactory do "context" => %{ "uuid" => nil, "route" => %{ + "id" => sequence("detour_route_id:"), "name" => sequence("detour_route_name:"), "directionNames" => %{ "0" => "Outbound", @@ -74,12 +75,23 @@ defmodule Skate.DetourFactory do put_in(state["value"], %{"Detour Drawing" => "Past"}) end - def with_route_name(%Skate.Detours.Db.Detour{} = detour, route_name) do - %{detour | state: with_route_name(detour.state, route_name)} + def with_route(%Skate.Detours.Db.Detour{} = detour, route) do + %{detour | state: with_route(detour.state, route)} end - def with_route_name(%{"context" => %{"route" => %{"name" => _}}} = state, route_name) do - put_in(state["context"]["route"]["name"], route_name) + def with_route( + %{ + "context" => %{ + "route" => %{"name" => _, "id" => _, "directionNames" => direction_names} + } + } = state, + route + ) do + put_in(state["context"]["route"], %{ + "name" => route, + "id" => route, + "directionNames" => direction_names + }) end def with_direction(%Skate.Detours.Db.Detour{} = detour, direction) do From 071dcb26374bb598e3aa0d7d121faed56d5d59d7 Mon Sep 17 00:00:00 2001 From: Hannah Purcell Date: Mon, 6 Jan 2025 12:55:23 -0500 Subject: [PATCH 4/4] fix: Tests needed updated with detour parsing, but also caught a bug --- assets/src/hooks/useDetours.ts | 15 ++-- assets/tests/factories/detourListFactory.ts | 19 +++-- assets/tests/hooks/useDetours.test.ts | 88 +++++++++++---------- 3 files changed, 70 insertions(+), 52 deletions(-) diff --git a/assets/src/hooks/useDetours.ts b/assets/src/hooks/useDetours.ts index bfc151850..e618c1e40 100644 --- a/assets/src/hooks/useDetours.ts +++ b/assets/src/hooks/useDetours.ts @@ -25,15 +25,15 @@ const subscribe = ( const channel = socket.channel(topic) handleDrafted && - channel.on("drafted", ({ data: data }) => + channel.on("drafted", ({ data: data }: { data: SimpleDetourData }) => handleDrafted(simpleDetourFromData(data)) ) handleActivated && - channel.on("activated", ({ data: data }) => + channel.on("activated", ({ data: data }: { data: SimpleDetourData }) => handleActivated(simpleDetourFromData(data)) ) handleDeactivated && - channel.on("deactivated", ({ data: data }) => + channel.on("deactivated", ({ data: data }: { data: SimpleDetourData }) => handleDeactivated(simpleDetourFromData(data)) ) channel.on("auth_expired", reload) @@ -178,13 +178,16 @@ const subscribeByRoute = ( ): Channel => { const channel = socket.channel(topic + routeId) - channel.on("activated", ({ data: data }) => { + channel.on("activated", ({ data: data }: { data: SimpleDetourData }) => { setDetours((activeDetours) => ({ ...activeDetours, - [routeId]: { ...activeDetours[routeId], [data.id]: data }, + [routeId]: { + ...activeDetours[routeId], + [data.id]: simpleDetourFromData(data), + }, })) }) - channel.on("deactivated", ({ data: data }) => { + channel.on("deactivated", ({ data: data }: { data: SimpleDetourData }) => { setDetours((activeDetours) => { delete activeDetours[routeId][data.id] return activeDetours diff --git a/assets/tests/factories/detourListFactory.ts b/assets/tests/factories/detourListFactory.ts index 8520c1eaf..4884046be 100644 --- a/assets/tests/factories/detourListFactory.ts +++ b/assets/tests/factories/detourListFactory.ts @@ -1,27 +1,34 @@ import { Factory } from "fishery" import { GroupedSimpleDetours, - SimpleDetour, + SimpleDetourData, + simpleDetourFromData, } from "../../src/models/detoursList" export const detourListFactory = Factory.define(() => { return { active: [ - simpleDetourFactory.build(), - simpleDetourFactory.build({ direction: "Outbound" }), + simpleDetourFromData(simpleDetourDataFactory.build()), + simpleDetourFromData( + simpleDetourDataFactory.build({ direction: "Outbound" }) + ), ], draft: undefined, - past: [simpleDetourFactory.build({ name: "Headsign Z" })], + past: [ + simpleDetourFromData( + simpleDetourDataFactory.build({ name: "Headsign Z" }) + ), + ], } }) -export const simpleDetourFactory = Factory.define( +export const simpleDetourDataFactory = Factory.define( ({ sequence }) => ({ id: sequence, route: `${sequence}`, direction: "Inbound", name: `Headsign ${sequence}`, intersection: `Street A${sequence} & Avenue B${sequence}`, - updatedAt: 1724866392, + updated_at: 1724866392, }) ) diff --git a/assets/tests/hooks/useDetours.test.ts b/assets/tests/hooks/useDetours.test.ts index 6620ba425..17c04b258 100644 --- a/assets/tests/hooks/useDetours.test.ts +++ b/assets/tests/hooks/useDetours.test.ts @@ -1,20 +1,28 @@ import { describe, expect, test } from "@jest/globals" import { makeMockChannel, makeMockSocket } from "../testHelpers/socketHelpers" import { act, renderHook } from "@testing-library/react" -import { simpleDetourFactory } from "../factories/detourListFactory" +import { simpleDetourDataFactory } from "../factories/detourListFactory" import { useActiveDetours, useActiveDetoursByRoute, useDraftDetours, usePastDetours, } from "../../src/hooks/useDetours" -import { SimpleDetour } from "../../src/models/detoursList" +import { + SimpleDetourData, + simpleDetourFromData, +} from "../../src/models/detoursList" import { RouteId } from "../../src/schedule" -const detourA = simpleDetourFactory.build() -const detourB = simpleDetourFactory.build() -const detourC = simpleDetourFactory.build() -const detourD = simpleDetourFactory.build() +const detourA = simpleDetourDataFactory.build() +const detourB = simpleDetourDataFactory.build() +const detourC = simpleDetourDataFactory.build() +const detourD = simpleDetourDataFactory.build() + +const parsedDetourA = simpleDetourFromData(detourA) +const parsedDetourB = simpleDetourFromData(detourB) +const parsedDetourC = simpleDetourFromData(detourC) +const parsedDetourD = simpleDetourFromData(detourD) const detours = [detourA, detourB, detourC] @@ -26,9 +34,9 @@ describe("useActiveDetours", () => { const { result } = renderHook(() => useActiveDetours(mockSocket)) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }) }) @@ -38,7 +46,7 @@ describe("useActiveDetours", () => { const mockEvents: Record< string, - undefined | ((data: { data: SimpleDetour }) => void) + undefined | ((data: { data: SimpleDetourData }) => void) > = { activated: undefined, } @@ -54,10 +62,10 @@ describe("useActiveDetours", () => { act(() => mockEvents["activated"]?.({ data: detourD })) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, - [detourD.id]: detourD, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, + [detourD.id]: parsedDetourD, }) }) @@ -67,7 +75,7 @@ describe("useActiveDetours", () => { const mockEvents: Record< string, - undefined | ((data: { data: SimpleDetour }) => void) + undefined | ((data: { data: SimpleDetourData }) => void) > = { deactivated: undefined, } @@ -83,8 +91,8 @@ describe("useActiveDetours", () => { act(() => mockEvents["deactivated"]?.({ data: detourA })) expect(result.current).toStrictEqual({ - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }) }) }) @@ -97,9 +105,9 @@ describe("usePastDetours", () => { const { result } = renderHook(() => usePastDetours(mockSocket)) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }) }) @@ -109,7 +117,7 @@ describe("usePastDetours", () => { const mockEvents: Record< string, - undefined | ((data: { data: SimpleDetour }) => void) + undefined | ((data: { data: SimpleDetourData }) => void) > = { deactivated: undefined, } @@ -125,10 +133,10 @@ describe("usePastDetours", () => { act(() => mockEvents["deactivated"]?.({ data: detourD })) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, - [detourD.id]: detourD, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, + [detourD.id]: parsedDetourD, }) }) }) @@ -141,9 +149,9 @@ describe("useDraftDetours", () => { const { result } = renderHook(() => useDraftDetours(mockSocket)) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }) }) @@ -153,7 +161,7 @@ describe("useDraftDetours", () => { const mockEvents: Record< string, - undefined | ((data: { data: SimpleDetour }) => void) + undefined | ((data: { data: SimpleDetourData }) => void) > = { drafted: undefined, } @@ -169,10 +177,10 @@ describe("useDraftDetours", () => { act(() => mockEvents["drafted"]?.({ data: detourD })) expect(result.current).toStrictEqual({ - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, - [detourD.id]: detourD, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, + [detourD.id]: parsedDetourD, }) }) @@ -182,7 +190,7 @@ describe("useDraftDetours", () => { const mockEvents: Record< string, - undefined | ((data: { data: SimpleDetour }) => void) + undefined | ((data: { data: SimpleDetourData }) => void) > = { activated: undefined, } @@ -198,8 +206,8 @@ describe("useDraftDetours", () => { act(() => mockEvents["activated"]?.({ data: detourA })) expect(result.current).toStrictEqual({ - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }) }) }) @@ -257,9 +265,9 @@ describe("useActiveDetoursByRoute", () => { expect(result.current).toEqual({ "1": { - [detourA.id]: detourA, - [detourB.id]: detourB, - [detourC.id]: detourC, + [detourA.id]: parsedDetourA, + [detourB.id]: parsedDetourB, + [detourC.id]: parsedDetourC, }, }) }) @@ -281,7 +289,7 @@ describe("useActiveDetoursByRoute", () => { expect(result.current).toEqual({ "1": { - [detourD.id]: detourD, + [detourD.id]: parsedDetourD, }, }) })