From 25a70e818da150f114351eaea6013334b45b3aab Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Tue, 2 Jan 2024 14:43:24 -0500 Subject: [PATCH 01/12] refactor: first pass at removing front-end references to old code --- assets/src/components/app.tsx | 8 +- assets/src/components/map.tsx | 5 +- assets/src/components/mapPage.tsx | 8 +- assets/src/components/nav/topNavMobile.tsx | 3 +- .../components/propertiesPanel/miniMap.tsx | 14 +- assets/src/components/searchResults.tsx | 18 +- assets/src/navLinkData.ts | 20 +-- assets/src/userInTestGroup.ts | 2 - assets/src/util/mapMode.ts | 30 ---- assets/tests/components/app.test.tsx | 18 +- assets/tests/components/map.test.tsx | 5 +- assets/tests/components/mapPage.test.tsx | 5 - assets/tests/components/nav.test.tsx | 6 +- .../components/nav/bottomNavMobile.test.tsx | 5 +- assets/tests/components/nav/leftNav.test.tsx | 20 +-- .../propertiesPanel/miniMap.test.tsx | 155 ++++++------------ assets/tests/util/mapMode.test.ts | 33 ---- 17 files changed, 77 insertions(+), 278 deletions(-) delete mode 100644 assets/src/util/mapMode.ts delete mode 100644 assets/tests/util/mapMode.test.ts diff --git a/assets/src/components/app.tsx b/assets/src/components/app.tsx index 8a8fcf7b5..f4bcfc303 100644 --- a/assets/src/components/app.tsx +++ b/assets/src/components/app.tsx @@ -21,10 +21,8 @@ import ShuttleMapPage from "./shuttleMapPage" import { allOpenRouteIds } from "../models/routeTab" import Nav from "./nav" import RightPanel from "./rightPanel" -import { mapModeForUser } from "../util/mapMode" import { Ghost, Vehicle, VehicleInScheduledService } from "../realtime" import MapPage from "./mapPage" -import SearchPage from "./searchPage" import { OpenView, isPagePath } from "../state/pagePanelState" import { usePanelStateFromStateDispatchContext } from "../hooks/usePanelState" import PropertiesPanel from "./propertiesPanel" @@ -61,10 +59,6 @@ export const AppRoutes = () => { vehiclesByRouteIdNeeded ? allOpenRouteIds(routeTabs) : [] ) - const mapMode = mapModeForUser() - - const mapElement = mapMode.path === "/map" ? : - return (
@@ -106,7 +100,7 @@ export const AppRoutes = () => { /> } > - + } /> diff --git a/assets/src/components/map.tsx b/assets/src/components/map.tsx index efc5d928e..c18c9caaa 100644 --- a/assets/src/components/map.tsx +++ b/assets/src/components/map.tsx @@ -22,7 +22,6 @@ import { createControlComponent } from "@react-leaflet/core" import { joinClasses } from "../helpers/dom" import { TrainVehicle, Vehicle, VehicleId } from "../realtime.d" import { Shape, Stop } from "../schedule" -import inTestGroup, { TestGroups } from "../userInTestGroup" import { GarageMarkers, RouteShape, @@ -198,9 +197,7 @@ const Map = (props: Props): ReactElement => { )} diff --git a/assets/src/components/mapPage.tsx b/assets/src/components/mapPage.tsx index 8d36561e8..259da95f3 100644 --- a/assets/src/components/mapPage.tsx +++ b/assets/src/components/mapPage.tsx @@ -31,7 +31,6 @@ import { LocationSearchResult } from "../models/locationSearchResult" import LocationCard from "./mapPage/locationCard" import { useLocationSearchResultById } from "../hooks/useLocationSearchResultById" import { fullStoryEvent } from "../helpers/fullStory" -import inTestGroup, { TestGroups } from "../userInTestGroup" import { usePanelStateFromStateDispatchContext } from "../hooks/usePanelState" const SearchMode = ({ @@ -314,11 +313,8 @@ const MapPage = (): ReactElement => { setVehicleSelection(...args) }} fetchedSelectedLocation={fetchedSelectedLocation} - onVehicleRunClicked={ - inTestGroup(TestGroups.SearchMapsOnMobile) - ? (vehicleOrGhost: Vehicle | Ghost) => - openVehiclePropertiesPanel(vehicleOrGhost, "run") - : undefined + onVehicleRunClicked={(vehicleOrGhost: Vehicle | Ghost) => + openVehiclePropertiesPanel(vehicleOrGhost, "run") } /> ) : ( diff --git a/assets/src/components/nav/topNavMobile.tsx b/assets/src/components/nav/topNavMobile.tsx index f590000f7..7bcaaace6 100644 --- a/assets/src/components/nav/topNavMobile.tsx +++ b/assets/src/components/nav/topNavMobile.tsx @@ -5,7 +5,6 @@ import NotificationBellIcon from "../notificationBellIcon" import { currentTabName, RouteTab } from "../../models/routeTab" import NavMenu from "./navMenu" import { tagManagerEvent } from "../../helpers/googleTagManager" -import { searchMapConfig } from "../../util/mapMode" export const toTitleCase = (str: string): string => { return str.replace( @@ -20,7 +19,7 @@ export const pageOrTabName = ( routeTabs: RouteTab[] ): string => { if (pathname === "/") return currentTabName(routeTabs) - if (pathname === searchMapConfig.path) return searchMapConfig.title + if (pathname === "/map") return "Search Map" else return toTitleCase(pathname.replace("/", "").replace("-", " ")) } diff --git a/assets/src/components/propertiesPanel/miniMap.tsx b/assets/src/components/propertiesPanel/miniMap.tsx index df6e4eb79..7bae852d1 100644 --- a/assets/src/components/propertiesPanel/miniMap.tsx +++ b/assets/src/components/propertiesPanel/miniMap.tsx @@ -9,8 +9,6 @@ import { SelectedEntityType, newSearchSession, } from "../../state/searchPageState" -import inTestGroup, { TestGroups } from "../../userInTestGroup" -import { mapModeForUser } from "../../util/mapMode" import { MapFollowingPrimaryVehicles } from "../map" import { fullStoryEvent } from "../../helpers/fullStory" @@ -27,7 +25,7 @@ const SearchMapLink = ({ vehicleId }: { vehicleId: VehicleId }) => { { fullStoryEvent("Map opened from VPP mini map", {}) dispatch( @@ -56,8 +54,6 @@ const MiniMap = ({ }) => { const stations: Stop[] | null = useStations() - const inMapBetaGroup = inTestGroup(TestGroups.MapBeta) - return ( - <> - {inMapBetaGroup && openMapEnabled && ( - - )} - + <>{openMapEnabled && } ) } diff --git a/assets/src/components/searchResults.tsx b/assets/src/components/searchResults.tsx index 50d144de2..06714809a 100644 --- a/assets/src/components/searchResults.tsx +++ b/assets/src/components/searchResults.tsx @@ -3,7 +3,6 @@ import { StateDispatchContext } from "../contexts/stateDispatchContext" import { isLoggedOut, isVehicle } from "../models/vehicle" import { Ghost, Vehicle } from "../realtime" import { setSearchText } from "../state/searchPageState" -import inTestGroup, { TestGroups } from "../userInTestGroup" import { Card, CardProperties } from "./card" import { vehicleOrGhostProperties } from "./propertiesList" import { RouteVariantName } from "./routeVariantName" @@ -133,8 +132,6 @@ export const NoResults = () => { dispatch, ] = useContext(StateDispatchContext) - const inMapBetaTestGroup = inTestGroup(TestGroups.MapBeta) - return (
No Search Results
@@ -144,17 +141,10 @@ export const NoResults = () => { again using numbers or last names only.

- {inMapBetaTestGroup ? ( -

- Please note that at this time run and operator search is limited to - logged-in personnel. -

- ) : ( -

- Please note that at this time search is limited to active vehicles and - logged-in personnel. -

- )} +

+ Please note that at this time run and operator search is limited to + logged-in personnel. +

diff --git a/assets/tests/components/mapPage.test.tsx b/assets/tests/components/mapPage.test.tsx index 562680aa2..b08e99c04 100644 --- a/assets/tests/components/mapPage.test.tsx +++ b/assets/tests/components/mapPage.test.tsx @@ -951,7 +951,11 @@ describe("", () => { "c-map-page__input-and-results--hidden" ) - await userEvent.click(screen.getByRole("button", { name: vehicle.runId! })) + await userEvent.click( + within(document.getElementById("id-vehicle-map")!).getByRole("button", { + name: vehicle.runId!, + }) + ) expect(screen.getByRole("generic", { name: /search panel/i })).toHaveClass( "c-map-page__input-and-results--visible" @@ -993,7 +997,11 @@ describe("", () => { "c-map-page__input-and-results--hidden" ) - await userEvent.click(screen.getByRole("button", { name: vehicle.runId! })) + await userEvent.click( + within(document.getElementById("id-vehicle-map")!).getByRole("button", { + name: vehicle.runId!, + }) + ) expect(screen.getByRole("generic", { name: /search panel/i })).toHaveClass( "c-map-page__input-and-results--visible" @@ -1402,9 +1410,10 @@ describe("", () => { expect( mapContainer.querySelector(".c-vehicle-map__route-shape") ).toBeInTheDocument() + // there is one additional button to select the run ID from the VPP expect( screen.getAllByRole("button", { name: runIdToLabel(vehicle.runId!) }) - ).toHaveLength(vehicles.length) + ).toHaveLength(vehicles.length + 1) expect(vehiclePropertiesCard.get()).toBeVisible() expect( @@ -1455,9 +1464,10 @@ describe("", () => { ) + // there is one additional button to select the run ID from the VPP expect( screen.getAllByRole("button", { name: selectedVehicle.runId! }) - ).toHaveLength(selectedRouteVehicles.length) + ).toHaveLength(selectedRouteVehicles.length + 1) }) describe("and vehicle is a regular bus", () => { @@ -1490,9 +1500,10 @@ describe("", () => { ) + // there is one additional button to select the run ID from the VPP expect( screen.getAllByRole("button", { name: selectedVehicle.runId! }) - ).toHaveLength(selectedRouteVehicles.length) + ).toHaveLength(selectedRouteVehicles.length + 1) expect( container.querySelectorAll(".c-vehicle-map__stop") From 0c74c32a2209a05622240f58697405a2505b3419 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Wed, 3 Jan 2024 13:31:53 -0500 Subject: [PATCH 03/12] fix: get App tests working --- assets/tests/components/__snapshots__/app.test.tsx.snap | 6 +++--- assets/tests/components/app.test.tsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assets/tests/components/__snapshots__/app.test.tsx.snap b/assets/tests/components/__snapshots__/app.test.tsx.snap index 5194aa9ea..572e6e461 100644 --- a/assets/tests/components/__snapshots__/app.test.tsx.snap +++ b/assets/tests/components/__snapshots__/app.test.tsx.snap @@ -208,15 +208,15 @@ exports[`App renders 1`] = `
  • - Search + Search Map
  • diff --git a/assets/tests/components/app.test.tsx b/assets/tests/components/app.test.tsx index 99a97660d..13ff993e7 100644 --- a/assets/tests/components/app.test.tsx +++ b/assets/tests/components/app.test.tsx @@ -106,7 +106,7 @@ describe("App", () => { const vehicle = vehicleFactory.build() const mockDispatch = jest.fn() - const pagesWithRightPanel = ["/", "/search", "/shuttle-map", "/settings"] + const pagesWithRightPanel = ["/", "/map", "/shuttle-map", "/settings"] describe.each(pagesWithRightPanel)("All views render on %s", (path) => { beforeAll(() => { From 4159cfa5ec0f56cdcb465b3d08a7e18f6d37eaa5 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Wed, 3 Jan 2024 14:17:34 -0500 Subject: [PATCH 04/12] fix: make VehiclePropertiesPanel tests work --- .../vehiclePropertiesPanel.test.tsx | 147 ++++++++++-------- 1 file changed, 83 insertions(+), 64 deletions(-) diff --git a/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx b/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx index 5c6514834..3d1746184 100644 --- a/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx +++ b/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx @@ -29,6 +29,7 @@ import { import { useTripShape } from "../../../src/hooks/useShapes" import { fullStoryEvent } from "../../../src/helpers/fullStory" import { closeButton } from "../../testHelpers/selectors/components/closeButton" +import { MemoryRouter } from "react-router-dom" jest .spyOn(dateTime, "now") @@ -186,13 +187,15 @@ describe("VehiclePropertiesPanel", () => { test("Includes invalid bus banner when vehicle is off course", () => { render( - + + + ) expect(screen.getByRole("heading", { name: "Invalid Bus" })).toBeVisible() }) @@ -294,13 +297,15 @@ describe("VehiclePropertiesPanel", () => { ok: "Atlantic Ave & Summer St", }) const result = render( - + + + ) expect(result.getByText("Atlantic Ave & Summer St")).toBeInTheDocument() }) @@ -311,13 +316,15 @@ describe("VehiclePropertiesPanel", () => { .mockImplementation((_key) => "1") const result = render( - + + + ) expect(result.queryAllByTestId("data-discrepancy")).toHaveLength(2) @@ -329,13 +336,15 @@ describe("VehiclePropertiesPanel", () => { .mockImplementation((_key) => null) const result = render( - + + + ) expect(result.queryAllByTestId("data-discrepancy")).toHaveLength(0) @@ -397,7 +406,7 @@ describe("VehiclePropertiesPanel", () => { expect(mapArgs.secondaryVehicles).toEqual([otherVehicle]) }) - test("map includes station icons when in map beta test group", () => { + test("map includes station icons", () => { ;(useStations as jest.Mock).mockReturnValue([ { id: "station-id", @@ -409,13 +418,15 @@ describe("VehiclePropertiesPanel", () => { ]) const { container } = render( - + + + ) expect(container.innerHTML).toContain("c-station-icon") @@ -425,13 +436,15 @@ describe("VehiclePropertiesPanel", () => { const mockClosePanel = jest.fn() render( - + + + ) await userEvent.click(closeButton.get()) @@ -453,13 +466,15 @@ describe("VehiclePropertiesPanel", () => { const mockSetTabMode = jest.fn() render( - + + + ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) @@ -482,13 +497,15 @@ describe("VehiclePropertiesPanel", () => { const mockedFSEvent = jest.mocked(fullStoryEvent) render( - + + + ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) @@ -514,13 +531,15 @@ describe("VehiclePropertiesPanel", () => { const mockedFSEvent = jest.mocked(fullStoryEvent) render( - + + + ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) From e27bed86afaee237515bb0b286bbd5ca1c5ee400 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Wed, 3 Jan 2024 14:22:10 -0500 Subject: [PATCH 05/12] fix: fix remaining tests --- .../appStateWrapper.test.tsx.snap | 6 +++--- .../tests/components/propertiesPanel.test.tsx | 17 ++++++++++++----- assets/tests/components/searchResults.test.tsx | 18 ------------------ 3 files changed, 15 insertions(+), 26 deletions(-) diff --git a/assets/tests/components/__snapshots__/appStateWrapper.test.tsx.snap b/assets/tests/components/__snapshots__/appStateWrapper.test.tsx.snap index ad86f7910..75725a9ff 100644 --- a/assets/tests/components/__snapshots__/appStateWrapper.test.tsx.snap +++ b/assets/tests/components/__snapshots__/appStateWrapper.test.tsx.snap @@ -240,15 +240,15 @@ exports[`renders 1`] = `
  • - Search + Search Map
  • diff --git a/assets/tests/components/propertiesPanel.test.tsx b/assets/tests/components/propertiesPanel.test.tsx index e60d31509..019f3aaa0 100644 --- a/assets/tests/components/propertiesPanel.test.tsx +++ b/assets/tests/components/propertiesPanel.test.tsx @@ -17,6 +17,7 @@ import useVehicleForId from "../../src/hooks/useVehicleForId" import { TabMode } from "../../src/components/propertiesPanel/tabPanels" import userEvent from "@testing-library/user-event" import { closeButton } from "../testHelpers/selectors/components/closeButton" +import { MemoryRouter } from "react-router-dom" jest .spyOn(dateTime, "now") @@ -160,7 +161,11 @@ describe("PropertiesPanel", () => { test("renders a vehicle with updated live information", () => { ;(useVehicleForId as jest.Mock).mockImplementationOnce(() => vehicle) - const result = render() + const result = render( + + + + ) expect(result.queryByText(/PATTI/)).not.toBeNull() }) @@ -216,10 +221,12 @@ describe("PropertiesPanel", () => { jest.mocked(useVehicleForId).mockReturnValue(vehicle) render( - + + + ) await userEvent.click(closeButton.get()) diff --git a/assets/tests/components/searchResults.test.tsx b/assets/tests/components/searchResults.test.tsx index 5df847a2b..878bd9d0b 100644 --- a/assets/tests/components/searchResults.test.tsx +++ b/assets/tests/components/searchResults.test.tsx @@ -51,24 +51,6 @@ describe("SearchResults", () => { ) - expect( - screen.queryByText(/at this time search is limited/) - ).toBeInTheDocument() - }) - - test("renders no results with user in map beta test group", () => { - ;(getTestGroups as jest.Mock).mockReturnValue(["map-beta"]) - - render( - - - - ) - expect( screen.queryByText(/at this time run and operator search is limited/) ).toBeInTheDocument() From 846f01a1ef37482fe1621cb4ff9f6b1c604aff1e Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Wed, 3 Jan 2024 14:25:13 -0500 Subject: [PATCH 06/12] refactor: remove some now-unused files --- assets/src/components/oldSearchForm.tsx | 133 -- assets/src/components/searchPage.tsx | 143 --- .../__snapshots__/oldSearchForm.test.tsx.snap | 139 -- .../__snapshots__/searchPage.test.tsx.snap | 1129 ----------------- .../tests/components/oldSearchForm.test.tsx | 226 ---- assets/tests/components/searchPage.test.tsx | 231 ---- 6 files changed, 2001 deletions(-) delete mode 100644 assets/src/components/oldSearchForm.tsx delete mode 100644 assets/src/components/searchPage.tsx delete mode 100644 assets/tests/components/__snapshots__/oldSearchForm.test.tsx.snap delete mode 100644 assets/tests/components/__snapshots__/searchPage.test.tsx.snap delete mode 100644 assets/tests/components/oldSearchForm.test.tsx delete mode 100644 assets/tests/components/searchPage.test.tsx diff --git a/assets/src/components/oldSearchForm.tsx b/assets/src/components/oldSearchForm.tsx deleted file mode 100644 index d139dc2b1..000000000 --- a/assets/src/components/oldSearchForm.tsx +++ /dev/null @@ -1,133 +0,0 @@ -import React, { useContext } from "react" -import { StateDispatchContext } from "../contexts/stateDispatchContext" -import { CircleXIcon, SearchIcon } from "../helpers/icon" -import { SearchPropertyQuery, isValidSearchQuery } from "../models/searchQuery" -import { - setOldSearchProperty, - setSearchText, - submitSearch, -} from "../state/searchPageState" -import { fullStoryEvent } from "../helpers/fullStory" - -const SEARCH_PROPERTIES = ["all", "run", "vehicle", "operator"] - -const OldSearchForm = ({ - onSubmit, - onClear, - inputTitle, - formTitle, - submitEvent, -}: { - onSubmit?: () => void - onClear?: () => void - inputTitle?: string - formTitle?: string - submitEvent?: string -}) => { - const [ - { - searchPageState: { query }, - }, - dispatch, - ] = useContext(StateDispatchContext) - const handleTextInput = (event: React.FormEvent): void => { - const value = event.currentTarget.value - dispatch(setSearchText(value)) - } - - const clearTextInput = (event: React.FormEvent): void => { - event.preventDefault() - dispatch(setSearchText("")) - if (onClear) { - onClear() - } - } - - const handlePropertyChange = ( - event: React.FormEvent - ): void => { - dispatch( - setOldSearchProperty(event.currentTarget.value as SearchPropertyQuery) - ) - dispatch(submitSearch()) - } - - const subscribeToSearch = (event: React.FormEvent) => { - event.preventDefault() - - submitEvent && fullStoryEvent(submitEvent, {}) - - dispatch(submitSearch()) - if (onSubmit) { - onSubmit() - } - } - - return ( -
    -
    -
    - - -
    - - -
    - -
    -
      - {SEARCH_PROPERTIES.map((property) => ( -
    • - - -
      • - ))} -
    -
    -
    - ) -} - -export default OldSearchForm diff --git a/assets/src/components/searchPage.tsx b/assets/src/components/searchPage.tsx deleted file mode 100644 index 6755199e5..000000000 --- a/assets/src/components/searchPage.tsx +++ /dev/null @@ -1,143 +0,0 @@ -import { Socket } from "phoenix" -import React, { ReactElement, useContext, useState } from "react" -import { SocketContext } from "../contexts/socketContext" -import { StateDispatchContext } from "../contexts/stateDispatchContext" -import useSearchResults from "../hooks/useSearchResults" -import { useStations } from "../hooks/useStations" -import { filterVehicles } from "../models/vehicle" -import { Ghost, Vehicle } from "../realtime" -import { Stop } from "../schedule" -import { SearchPageState } from "../state/searchPageState" -import { MapFollowingPrimaryVehicles } from "./map" -import RecentSearches from "./recentSearches" -import OldSearchForm from "./oldSearchForm" -import SearchResults from "./searchResults" -import { LayersControl, LayersControlState } from "./map/controls/layersControl" -import { TileType } from "../tilesetUrls" -import { setTileType } from "../state/mapLayersState" -import { usePanelStateFromStateDispatchContext } from "../hooks/usePanelState" - -enum MobileDisplay { - List = 1, - Map, -} - -const thereIsAnActiveSearch = ( - vehicles: (Vehicle | Ghost)[] | null, - searchPageState: SearchPageState -): boolean => vehicles !== null && searchPageState.isActive - -const ToggleMobileDisplayButton = ({ - mobileDisplay, - onToggleMobileDisplay, -}: { - mobileDisplay: MobileDisplay - onToggleMobileDisplay: () => void -}) => { - const otherDisplayName = mobileDisplay === MobileDisplay.List ? "map" : "list" - - return ( - - ) -} - -const SearchPage = (): ReactElement => { - const [ - { - searchPageState, - mobileMenuIsOpen, - mapLayers: { - legacySearchMap: { tileType: tileType }, - }, - }, - dispatch, - ] = useContext(StateDispatchContext) - - const { - currentView: { selectedVehicleOrGhost }, - openVehiclePropertiesPanel, - } = usePanelStateFromStateDispatchContext() - - const { socket }: { socket: Socket | undefined } = useContext(SocketContext) - const stations: Stop[] | null = useStations() - - const vehicles: (Vehicle | Ghost)[] | null = useSearchResults( - socket, - searchPageState.isActive ? searchPageState.query : null - ) - const onlyVehicles: Vehicle[] = filterVehicles(vehicles) - const [mobileDisplay, setMobileDisplay] = useState(MobileDisplay.List) - const toggleMobileDisplay = () => { - setMobileDisplay( - mobileDisplay === MobileDisplay.List - ? MobileDisplay.Map - : MobileDisplay.List - ) - } - - const mobileDisplayClass = - mobileDisplay === MobileDisplay.List - ? "c-search-page--show-list" - : "c-search-page--show-map" - - const mobileMenuClass = mobileMenuIsOpen ? "blurred-mobile" : "" - - return ( -
    -
    -
    - - - -
    - -
    - {vehicles != null && - thereIsAnActiveSearch(vehicles, searchPageState) ? ( - - ) : ( - - )} -
    -
    - -
    - - - {(open, setOpen) => ( - - dispatch(setTileType("legacySearchMap", tileType)) - } - /> - )} - - -
    -
    - ) -} - -export default SearchPage diff --git a/assets/tests/components/__snapshots__/oldSearchForm.test.tsx.snap b/assets/tests/components/__snapshots__/oldSearchForm.test.tsx.snap deleted file mode 100644 index ee2e81e3e..000000000 --- a/assets/tests/components/__snapshots__/oldSearchForm.test.tsx.snap +++ /dev/null @@ -1,139 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SearchForm renders 1`] = ` -
    -
    -
    - - -
    - -
    -
    -
      -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    -
    -
    -`; diff --git a/assets/tests/components/__snapshots__/searchPage.test.tsx.snap b/assets/tests/components/__snapshots__/searchPage.test.tsx.snap deleted file mode 100644 index d492805ac..000000000 --- a/assets/tests/components/__snapshots__/searchPage.test.tsx.snap +++ /dev/null @@ -1,1129 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`SearchPage renders the empty state 1`] = ` - -
    -
    -
    -
    -
    -
    - - -
    - -
    -
    -
      -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    -
    -
    - -
    -
    -
    -
    - Recent Searches -
    -
      -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    - -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    - - - - - - -
    -
    -
    - - - -
    -
    -
    - -
    -
    - - - - - -
    -
    -
    -
    -
    - © - - OpenStreetMap - - contributors -
    -
    -
    -
    -
    -
    - -`; - -exports[`SearchPage renders vehicle data 1`] = ` - -
    -
    -
    -
    -
    -
    - - -
    - -
    -
    -
      -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    • - - -
      • -
    -
    -
    - -
    -
    -
    -
    - Recent Searches -
    -
      -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    - -
    -
    -
    -
    -
    -
    -
    -
    - - - - - - - -
    -
    - - - - - - - - - 1 - - - - - -
    -
    -
    -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    - -
    -
    -
    - - - Albany - - -
    -
    -
    -
    - - - Arborway - - -
    -
    -
    -
    - - - Cabot - - -
    -
    -
    -
    - - - Charlestown and Somerville - - -
    -
    -
    -
    - - - Fellsway - - -
    -
    -
    -
    - - - Lynn - - -
    -
    -
    -
    - - - North Cambridge - - -
    -
    -
    -
    - - - Quincy - - -
    -
    -
    -
    - - - Southampton - - -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    -
    - - - - - - -
    -
    -
    - - - -
    -
    -
    - -
    -
    - - - - - -
    -
    -
    -
    -
    - © - - OpenStreetMap - - contributors -
    -
    -
    -
    -
    -
    - -`; diff --git a/assets/tests/components/oldSearchForm.test.tsx b/assets/tests/components/oldSearchForm.test.tsx deleted file mode 100644 index 6bac7196d..000000000 --- a/assets/tests/components/oldSearchForm.test.tsx +++ /dev/null @@ -1,226 +0,0 @@ -import { jest, describe, test, expect } from "@jest/globals" -import { render, screen } from "@testing-library/react" -import userEvent from "@testing-library/user-event" -import React from "react" -import renderer from "react-test-renderer" -import "@testing-library/jest-dom/jest-globals" -import OldSearchForm from "../../src/components/oldSearchForm" -import { StateDispatchProvider } from "../../src/contexts/stateDispatchContext" -import { initialState } from "../../src/state" -import { - SearchPageState, - setOldSearchProperty, - setSearchText, - submitSearch, -} from "../../src/state/searchPageState" -import { searchPageStateFactory } from "../factories/searchPageState" -import { fullStoryEvent } from "../../src/helpers/fullStory" - -jest.mock("../../src/helpers/fullStory") - -const mockDispatch = jest.fn() - -describe("SearchForm", () => { - test("renders", () => { - const tree = renderer - .create( - - - - ) - .toJSON() - - expect(tree).toMatchSnapshot() - }) - - test("submit button is disabled if there are fewer than 2 characters in the text field", () => { - const invalidSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "1", property: "run" }, - }) - const invalidSearchState = { - ...initialState, - searchPageState: invalidSearch, - } - const result = render( - - - - ) - - expect(result.getByPlaceholderText("Search")).toHaveValue("1") - - expect(result.getByTitle("Submit")).toBeDisabled() - }) - - test("submit button is enabled if there are at least 2 characters in the text field", () => { - const validSearch = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - const result = render( - - - - ) - - expect(result.getByPlaceholderText("Search")).toHaveValue("12") - - expect(result.getByTitle("Submit")).not.toBeDisabled() - }) - - test("clicking the submit button submits the query", async () => { - const testDispatch = jest.fn() - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - const result = render( - - - - ) - - await userEvent.click(result.getByTitle("Submit")) - expect(testDispatch).toHaveBeenCalledWith(submitSearch()) - }) - - test("clicking the submit button also calls onSubmit when set", async () => { - const testDispatch = jest.fn() - const onSubmit = jest.fn() - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - render( - - - - ) - - await userEvent.click(screen.getByTitle("Submit")) - expect(testDispatch).toHaveBeenCalledWith(submitSearch()) - expect(onSubmit).toHaveBeenCalledTimes(1) - }) - - test("clicking the submit button logs a FullStory event when provided", async () => { - const testDispatch = jest.fn() - const onSubmit = jest.fn() - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - const mockedFSEvent = jest.mocked(fullStoryEvent) - - render( - - - - ) - - await userEvent.click(screen.getByTitle("Submit")) - - expect(mockedFSEvent).toHaveBeenCalledWith("Test event", {}) - }) - - test("entering text sets it as the search text", async () => { - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - - const testDispatch = jest.fn() - - const result = render( - - - - ) - - await userEvent.type(result.getByPlaceholderText("Search"), "3") - - expect(testDispatch).toHaveBeenCalledWith(setSearchText("123")) - }) - - test("clicking the clear button empties the search text", async () => { - const testDispatch = jest.fn() - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - render( - - - - ) - - await userEvent.click(screen.getByRole("button", { name: /clear search/i })) - - expect(testDispatch).toHaveBeenCalledWith(setSearchText("")) - }) - - test("clicking the clear button also calls onClear when set", async () => { - const testDispatch = jest.fn() - const onClear = jest.fn() - const validSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "12", property: "run" }, - }) - const validSearchState = { - ...initialState, - searchPageState: validSearch, - } - render( - - - - ) - - await userEvent.click(screen.getByRole("button", { name: /clear search/i })) - - expect(testDispatch).toHaveBeenCalledWith(setSearchText("")) - expect(onClear).toHaveBeenCalledTimes(1) - }) - - test("clicking a search property selects it", async () => { - const testDispatch = jest.fn() - const result = render( - - - - ) - - await userEvent.click(result.getByRole("radio", { name: "run" })) - - expect(testDispatch).toHaveBeenCalledWith(setOldSearchProperty("run")) - }) - - test("clicking a search property submits the search", async () => { - const testDispatch = jest.fn() - const result = render( - - - - ) - - await userEvent.click(result.getByRole("radio", { name: "run" })) - - expect(testDispatch).toHaveBeenCalledWith(submitSearch()) - }) -}) diff --git a/assets/tests/components/searchPage.test.tsx b/assets/tests/components/searchPage.test.tsx deleted file mode 100644 index c9e222438..000000000 --- a/assets/tests/components/searchPage.test.tsx +++ /dev/null @@ -1,231 +0,0 @@ -import { jest, describe, test, expect, beforeAll } from "@jest/globals" -import React from "react" -import { render, screen } from "@testing-library/react" -import { BrowserRouter } from "react-router-dom" -import "@testing-library/jest-dom/jest-globals" -import SearchPage from "../../src/components/searchPage" -import { StateDispatchProvider } from "../../src/contexts/stateDispatchContext" -import useSearchResults from "../../src/hooks/useSearchResults" -import { Ghost, VehicleInScheduledService } from "../../src/realtime" -import { initialState } from "../../src/state" -import * as dateTime from "../../src/util/dateTime" - -import vehicleFactory from "../factories/vehicle" -import ghostFactory from "../factories/ghost" -import stopFactory from "../factories/stop" -import userEvent from "@testing-library/user-event" -import { SearchPageState } from "../../src/state/searchPageState" -import { useStations } from "../../src/hooks/useStations" -import { LocationType } from "../../src/models/stopData" -import { - layersControlButton, - zoomInButton, -} from "../testHelpers/selectors/components/map" -import { searchPageStateFactory } from "../factories/searchPageState" -import { mockTileUrls } from "../testHelpers/mockHelpers" -import { RealDispatchWrapper } from "../testHelpers/wrappers" -jest - .spyOn(dateTime, "now") - .mockImplementation(() => new Date("2018-08-15T17:41:21.000Z")) - -jest.spyOn(Date, "now").mockImplementation(() => 234000) - -const vehicle: VehicleInScheduledService = vehicleFactory.build() - -const ghost: Ghost = ghostFactory.build({ - id: "ghost-trip", - directionId: 0, - routeId: "39", - tripId: "trip", - headsign: "headsign", - blockId: "block", - runId: "123-0123", - viaVariant: "X", - layoverDepartureTime: null, - scheduledTimepointStatus: { - timepointId: "t0", - fractionUntilTimepoint: 0.0, - }, - scheduledLogonTime: null, - routeStatus: "on_route", - blockWaivers: [], -}) -jest.mock("../../src/hooks/useSearchResults", () => ({ - __esModule: true, - default: jest.fn(), -})) - -jest.mock("../../src/hooks/useStations", () => ({ - __esModule: true, - useStations: jest.fn(() => []), -})) - -jest.mock("../../src/tilesetUrls", () => ({ - __esModule: true, - tilesetUrlForType: jest.fn(() => null), -})) - -beforeAll(() => { - mockTileUrls() -}) - -describe("SearchPage", () => { - test("renders the empty state", () => { - ;(useSearchResults as jest.Mock).mockImplementationOnce(() => null) - const result = render( - - - - - - ) - - expect(result.asFragment()).toMatchSnapshot() - }) - - test("Has the layers control", () => { - ;(useSearchResults as jest.Mock).mockReturnValue([]) - render( - - - - - - ) - expect(layersControlButton.get()).toBeInTheDocument() - }) - - test("renders vehicle data", () => { - const searchResults: (VehicleInScheduledService | Ghost)[] = [ - vehicle, - ghost, - ] - ;(useSearchResults as jest.Mock).mockImplementation(() => searchResults) - const result = render( - - - - - - ) - - expect(result.asFragment()).toMatchSnapshot() - }) - - test("on mobile, shows the results list initially", () => { - const result = render( - - - - ) - - expect(result.container.firstChild).toHaveClass("c-search-page--show-list") - }) - - test("clicking a vehicle on the map selects it", async () => { - jest.spyOn(global, "scrollTo").mockImplementationOnce(jest.fn()) - const runId = "clickMe" - const searchResults: (VehicleInScheduledService | Ghost)[] = [ - { ...vehicle, runId: runId }, - ] - ;(useSearchResults as jest.Mock).mockImplementation(() => searchResults) - const mockDispatch = jest.fn() - const result = render( - - - - - - ) - - await userEvent.click(result.getByText(runId)) - expect(mockDispatch).toHaveBeenCalledWith( - expect.objectContaining({ type: "SELECT_VEHICLE" }) - ) - }) - - test("clicking a search result selects that vehicle", async () => { - jest.spyOn(global, "scrollTo").mockImplementationOnce(jest.fn()) - const runId = "clickMe" - const searchResults: (VehicleInScheduledService | Ghost)[] = [ - { ...vehicle, runId: runId }, - ] - ;(useSearchResults as jest.Mock).mockImplementation(() => searchResults) - const activeSearch: SearchPageState = searchPageStateFactory.build({ - query: { text: "clickMe", property: "run" }, - isActive: true, - savedQueries: [], - }) - const mockDispatch = jest.fn() - render( - - - - - - ) - - await userEvent.click(screen.getByRole("button", { name: /run/i })) - expect(mockDispatch).toHaveBeenCalledWith( - expect.objectContaining({ type: "SELECT_VEHICLE" }) - ) - }) - - test("on mobile, allows you to toggle to the map view and back again", async () => { - const result = render( - - - - ) - - await userEvent.click( - result.getByRole("button", { name: "Show map instead" }) - ) - - expect(result.container.firstChild).toHaveClass("c-search-page--show-map") - - await userEvent.click( - result.getByRole("button", { name: "Show list instead" }) - ) - - expect(result.container.firstChild).toHaveClass("c-search-page--show-list") - }) - - test("renders stations on zoom", async () => { - ;(useStations as jest.Mock).mockImplementationOnce(() => [ - stopFactory.build({ locationType: LocationType.Station }), - ]) - - const { container } = render( - - - - ) - - await userEvent.click(zoomInButton.get()) - await userEvent.click(zoomInButton.get()) - - expect(container.querySelector(".c-station-icon")).toBeVisible() - }) - - describe("Map controls", () => { - test("Can change tile layer to satellite", async () => { - const { container } = render( - - - - ) - - await userEvent.click(layersControlButton.get()) - - await userEvent.click(screen.getByLabelText("Satellite")) - - expect( - container.querySelector("img[src^=test_satellite_url") - ).not.toBeNull() - }) - }) -}) From e6c4f8b45eb20c85a0af117350c467086336ebff Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Thu, 4 Jan 2024 10:51:15 -0500 Subject: [PATCH 07/12] refactor: remove old useSearchResults hook --- assets/src/hooks/useSearchResults.ts | 25 +- assets/tests/hooks/useSearchResults.test.ts | 281 +------------------- 2 files changed, 6 insertions(+), 300 deletions(-) diff --git a/assets/src/hooks/useSearchResults.ts b/assets/src/hooks/useSearchResults.ts index df34617ff..9b491625b 100644 --- a/assets/src/hooks/useSearchResults.ts +++ b/assets/src/hooks/useSearchResults.ts @@ -1,13 +1,13 @@ import { Socket } from "phoenix" import { Infer, array, boolean, type, union } from "superstruct" -import { SearchQuery, VehiclePropertyQuery } from "../models/searchQuery" +import { VehiclePropertyQuery } from "../models/searchQuery" import { GhostData, VehicleData, vehicleOrGhostFromData, } from "../models/vehicleData" import { Ghost, Vehicle } from "../realtime" -import { useCheckedChannel, useCheckedTwoWayChannel } from "./useChannel" +import { useCheckedTwoWayChannel } from "./useChannel" import { useEffect } from "react" import { Loading, Ok } from "../util/fetchResult" @@ -16,25 +16,6 @@ const parser = (data: (VehicleData | GhostData)[]): (Vehicle | Ghost)[] => const dataStruct = array(union([VehicleData, GhostData])) -const useSearchResults = ( - socket: Socket | undefined, - searchQuery: SearchQuery | null -): (Vehicle | Ghost)[] | null => { - const topic: string | null = - searchQuery && `vehicles:search:${searchQuery.property}:${searchQuery.text}` - return useCheckedChannel< - (VehicleData | GhostData)[], - (Vehicle | Ghost)[] | null - >({ - socket, - topic, - event: "search", - dataStruct, - parser, - loadingState: null, - }) -} - const limitedVehicleSearchResultsData = type({ matching_vehicles: dataStruct, has_more_matches: boolean(), @@ -89,5 +70,3 @@ export const useLimitedSearchResults = ( return topic ? state : null } - -export default useSearchResults diff --git a/assets/tests/hooks/useSearchResults.test.ts b/assets/tests/hooks/useSearchResults.test.ts index a286260f6..dca8777fa 100644 --- a/assets/tests/hooks/useSearchResults.test.ts +++ b/assets/tests/hooks/useSearchResults.test.ts @@ -1,284 +1,11 @@ import { jest, describe, test, expect } from "@jest/globals" import { act, renderHook } from "@testing-library/react" -import useSearchResults, { - useLimitedSearchResults, -} from "../../src/hooks/useSearchResults" -import { - emptySearchQuery, - SearchQuery, - VehiclePropertyQuery, -} from "../../src/models/searchQuery" -import { - GhostData, - VehicleData, - vehicleFromData, -} from "../../src/models/vehicleData" -import { VehicleInScheduledService, Ghost } from "../../src/realtime" -import { mockUseStateOnce } from "../testHelpers/mockHelpers" +import { useLimitedSearchResults } from "../../src/hooks/useSearchResults" +import { VehiclePropertyQuery } from "../../src/models/searchQuery" +import { VehicleData, vehicleFromData } from "../../src/models/vehicleData" import { makeMockChannel, makeMockSocket } from "../testHelpers/socketHelpers" -import vehicleFactory from "../factories/vehicle" -import vehicleDataFactory from "../factories/vehicle_data" -import { searchQueryRunFactory } from "../factories/searchQuery" - -describe("useSearchResults", () => { - test("returns null initially", () => { - const { result } = renderHook(() => - useSearchResults(undefined, emptySearchQuery) - ) - expect(result.current).toEqual(null) - }) - test("initializing the hook subscribes to the search results", () => { - const mockSocket = makeMockSocket() - const mockChannel = makeMockChannel("ok") - mockSocket.channel.mockImplementationOnce(() => mockChannel) - - const searchQuery: SearchQuery = searchQueryRunFactory.build({ - text: "test", - }) - - renderHook(() => useSearchResults(mockSocket, searchQuery)) - - expect(mockSocket.channel).toHaveBeenCalledTimes(1) - expect(mockSocket.channel).toHaveBeenCalledWith("vehicles:search:run:test") - expect(mockChannel.join).toHaveBeenCalledTimes(1) - }) - - test("initializing the hook without a search query does not subscribe to the search results", () => { - const mockSocket = makeMockSocket() - const mockChannel = makeMockChannel("ok") - mockSocket.channel.mockImplementationOnce(() => mockChannel) - - renderHook(() => useSearchResults(mockSocket, null)) - - expect(mockSocket.channel).toHaveBeenCalledTimes(0) - expect(mockChannel.join).toHaveBeenCalledTimes(0) - }) - test("returns results pushed to the channel", () => { - const vehicleData: VehicleData = vehicleDataFactory.build({ - bearing: 33, - block_id: "block-1", - data_discrepancies: [ - { - attribute: "trip_id", - sources: [ - { - id: "swiftly", - value: "swiftly-trip-id", - }, - { - id: "busloc", - value: "busloc-trip-id", - }, - ], - }, - { - attribute: "route_id", - sources: [ - { - id: "swiftly", - value: null, - }, - { - id: "busloc", - value: "busloc-route-id", - }, - ], - }, - ], - direction_id: 0, - headsign: "Forest Hills", - id: "v1", - is_shuttle: false, - is_overload: false, - is_off_course: false, - is_revenue: true, - layover_departure_time: null, - label: "v1-label", - latitude: 0, - longitude: 0, - operator_id: "op1", - operator_first_name: "PATTI", - operator_last_name: "SMITH", - operator_logon_time: 1_534_340_301, - previous_vehicle_id: "v2", - route_id: "39", - run_id: "run-1", - schedule_adherence_secs: 0, - scheduled_location: { - route_id: "39", - direction_id: 0, - trip_id: "scheduled trip", - run_id: "scheduled run", - time_since_trip_start_time: 0, - headsign: "scheduled headsign", - via_variant: "scheduled via variant", - timepoint_status: { - timepoint_id: "tp1", - fraction_until_timepoint: 0.5, - }, - }, - sources: ["swiftly", "busloc"], - stop_status: { - stop_id: "s1", - stop_name: "Stop Name", - }, - timepoint_status: { - timepoint_id: "tp1", - fraction_until_timepoint: 0.5, - }, - timestamp: 123, - trip_id: "t1", - via_variant: "X", - route_status: "on_route", - end_of_trip_type: "another_trip", - block_waivers: [], - crowding: null, - }) - const searchResultsData: (VehicleData | GhostData)[] = [vehicleData] - const vehicle: VehicleInScheduledService = vehicleFactory.build({ - id: "v1", - label: "v1-label", - runId: "run-1", - timestamp: 123, - latitude: 0, - longitude: 0, - directionId: 0, - routeId: "39", - tripId: "t1", - headsign: "Forest Hills", - viaVariant: "X", - operatorId: "op1", - operatorFirstName: "PATTI", - operatorLastName: "SMITH", - operatorLogonTime: new Date("2018-08-15T13:38:21.000Z"), - bearing: 33, - blockId: "block-1", - previousVehicleId: "v2", - scheduleAdherenceSecs: 0, - isShuttle: false, - isOverload: false, - isOffCourse: false, - isRevenue: true, - layoverDepartureTime: null, - dataDiscrepancies: [ - { - attribute: "trip_id", - sources: [ - { - id: "swiftly", - value: "swiftly-trip-id", - }, - { - id: "busloc", - value: "busloc-trip-id", - }, - ], - }, - { - attribute: "route_id", - sources: [ - { - id: "swiftly", - value: null, - }, - { - id: "busloc", - value: "busloc-route-id", - }, - ], - }, - ], - stopStatus: { - stopId: "s1", - stopName: "Stop Name", - }, - timepointStatus: { - timepointId: "tp1", - fractionUntilTimepoint: 0.5, - }, - scheduledLocation: { - routeId: "39", - directionId: 0, - tripId: "scheduled trip", - runId: "scheduled run", - timeSinceTripStartTime: 0, - headsign: "scheduled headsign", - viaVariant: "scheduled via variant", - timepointStatus: { - timepointId: "tp1", - fractionUntilTimepoint: 0.5, - }, - }, - routeStatus: "on_route", - endOfTripType: "another_trip", - blockWaivers: [], - crowding: null, - }) - const vehicles: (VehicleInScheduledService | Ghost)[] = [vehicle] - - const mockSocket = makeMockSocket() - const mockChannel = makeMockChannel("ok", { data: searchResultsData }) - mockSocket.channel.mockImplementationOnce(() => mockChannel) - - const searchQuery: SearchQuery = searchQueryRunFactory.build({ - text: "test", - }) - const { result } = renderHook(() => - useSearchResults(mockSocket, searchQuery) - ) - - expect(result.current).toEqual(vehicles) - }) - - test("leaves the channel and joins a new one when the search changes", () => { - const mockSocket = makeMockSocket() - const vehicles: VehicleInScheduledService[] = [] - const channel1 = makeMockChannel("ok") - const channel2 = makeMockChannel("ok") - mockSocket.channel.mockImplementationOnce(() => channel1) - mockSocket.channel.mockImplementationOnce(() => channel2) - mockUseStateOnce(vehicles) - mockUseStateOnce(channel1) - mockUseStateOnce(vehicles) - mockUseStateOnce(channel2) - - const search1: SearchQuery = searchQueryRunFactory.build({ - text: "one", - }) - const { rerender } = renderHook( - (searchQuery) => useSearchResults(mockSocket, searchQuery), - { initialProps: search1 } - ) - - const search2: SearchQuery = searchQueryRunFactory.build({ - text: "two", - }) - rerender(search2) - - expect(channel1.leave).toHaveBeenCalled() - expect(channel2.join).toHaveBeenCalled() - }) - - test("leaves the channel when typing even if there is no new channel", () => { - const mockSocket = makeMockSocket() - const mockChannel = makeMockChannel("ok") - mockSocket.channel.mockImplementationOnce(() => mockChannel) - - const search1: SearchQuery | null = searchQueryRunFactory.build({ - text: "validSearch", - }) - const { rerender } = renderHook( - (searchQuery) => useSearchResults(mockSocket, searchQuery), - { initialProps: search1 as SearchQuery | null } - ) - - const search2: SearchQuery | null = null - rerender(search2) - - expect(mockChannel.leave).toHaveBeenCalledTimes(1) - }) -}) +import vehicleDataFactory from "../factories/vehicle_data" describe("useLimitedSearchResults", () => { test("when query given and loading, returns loading", () => { From a1cbf544033cacd9213088c6b75dc0ecf28b67ea Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Thu, 4 Jan 2024 10:54:00 -0500 Subject: [PATCH 08/12] refactor: rename useLimitedSearchResults to just useSearchResults --- assets/src/hooks/useAutocompleteResults.ts | 15 ++++---- assets/src/hooks/useSearchResults.ts | 2 +- .../src/hooks/useSearchResultsByCategory.ts | 7 ++-- assets/tests/hooks/useSearchResults.test.ts | 18 +++++----- .../hooks/useSearchResultsByCategory.test.ts | 34 +++++++++---------- 5 files changed, 34 insertions(+), 42 deletions(-) diff --git a/assets/src/hooks/useAutocompleteResults.ts b/assets/src/hooks/useAutocompleteResults.ts index e45b65061..0d1eb7724 100644 --- a/assets/src/hooks/useAutocompleteResults.ts +++ b/assets/src/hooks/useAutocompleteResults.ts @@ -6,7 +6,7 @@ import { VehiclePropertyQuery, } from "../models/searchQuery" import { Ghost, Vehicle } from "../realtime" -import { useLimitedSearchResults } from "./useSearchResults" +import { useSearchResults } from "./useSearchResults" import { isLoading } from "../util/fetchResult" type AutocompleteResults = Record< @@ -28,17 +28,14 @@ export const useAutocompleteResults = ( searchFilters: SearchProperties, maxResults = 5 ): AutocompleteResults => { - const operator = useLimitedSearchResultsForProperty( + const operator = useSearchResultsForProperty( searchFilters.operator, "operator" ) - const vehicle = useLimitedSearchResultsForProperty( - searchFilters.vehicle, - "vehicle" - ) + const vehicle = useSearchResultsForProperty(searchFilters.vehicle, "vehicle") - const run = useLimitedSearchResultsForProperty(searchFilters.run, "run") + const run = useSearchResultsForProperty(searchFilters.run, "run") return { vehicle, @@ -46,13 +43,13 @@ export const useAutocompleteResults = ( operator, } - function useLimitedSearchResultsForProperty( + function useSearchResultsForProperty( enableSearch: boolean, property: VehiclePropertyQuery ): (Vehicle | Ghost)[] { // Search for the property we need to return, but if it's filtered, // set `query` parameter to `null` - const res = useLimitedSearchResults( + const res = useSearchResults( socket, (enableSearch || null) && { property, diff --git a/assets/src/hooks/useSearchResults.ts b/assets/src/hooks/useSearchResults.ts index 9b491625b..2ff96f055 100644 --- a/assets/src/hooks/useSearchResults.ts +++ b/assets/src/hooks/useSearchResults.ts @@ -38,7 +38,7 @@ const parseLimitedSearchResults = ( const loadingState: Loading = { is_loading: true } -export const useLimitedSearchResults = ( +export const useSearchResults = ( socket: Socket | undefined, query: { property: VehiclePropertyQuery; text: string; limit: number } | null ): Ok> | Loading | null => { diff --git a/assets/src/hooks/useSearchResultsByCategory.ts b/assets/src/hooks/useSearchResultsByCategory.ts index d35fd63fd..10fc1a51d 100644 --- a/assets/src/hooks/useSearchResultsByCategory.ts +++ b/assets/src/hooks/useSearchResultsByCategory.ts @@ -4,10 +4,7 @@ import { SearchPropertyQuery, } from "../models/searchQuery" import { Loading, Ok } from "../util/fetchResult" -import { - LimitedSearchResults, - useLimitedSearchResults, -} from "./useSearchResults" +import { LimitedSearchResults, useSearchResults } from "./useSearchResults" import { useLocationSearchResults } from "./useLocationSearchResults" import { LocationSearchResult } from "../models/locationSearchResult" import { Vehicle, Ghost } from "../realtime" @@ -50,7 +47,7 @@ const useSearchResultsByCategory = ( }, }) || { is_loading: true } return { - vehicle: useLimitedSearchResults( + vehicle: useSearchResults( socket, (shouldSearchVehicles && { property: queryType, diff --git a/assets/tests/hooks/useSearchResults.test.ts b/assets/tests/hooks/useSearchResults.test.ts index dca8777fa..8ba7912d3 100644 --- a/assets/tests/hooks/useSearchResults.test.ts +++ b/assets/tests/hooks/useSearchResults.test.ts @@ -1,20 +1,20 @@ import { jest, describe, test, expect } from "@jest/globals" import { act, renderHook } from "@testing-library/react" -import { useLimitedSearchResults } from "../../src/hooks/useSearchResults" +import { useSearchResults } from "../../src/hooks/useSearchResults" import { VehiclePropertyQuery } from "../../src/models/searchQuery" import { VehicleData, vehicleFromData } from "../../src/models/vehicleData" import { makeMockChannel, makeMockSocket } from "../testHelpers/socketHelpers" import vehicleDataFactory from "../factories/vehicle_data" -describe("useLimitedSearchResults", () => { +describe("useSearchResults", () => { test("when query given and loading, returns loading", () => { const mockSocket = makeMockSocket() const mockChannel = makeMockChannel("ok") mockSocket.channel.mockImplementationOnce(() => mockChannel) const { result } = renderHook(() => - useLimitedSearchResults(mockSocket, { + useSearchResults(mockSocket, { property: "vehicle", text: "1234", limit: 5, @@ -25,9 +25,7 @@ describe("useLimitedSearchResults", () => { test("when no query given, returns null", () => { const mockSocket = makeMockSocket() - const { result } = renderHook(() => - useLimitedSearchResults(mockSocket, null) - ) + const { result } = renderHook(() => useSearchResults(mockSocket, null)) expect(result.current).toEqual(null) }) @@ -35,7 +33,7 @@ describe("useLimitedSearchResults", () => { const mockSocket = makeMockSocket() const { result } = renderHook(() => - useLimitedSearchResults(mockSocket, { + useSearchResults(mockSocket, { text: "", property: "run", limit: 5, @@ -50,7 +48,7 @@ describe("useLimitedSearchResults", () => { mockSocket.channel.mockImplementationOnce(() => mockChannel) renderHook(() => - useLimitedSearchResults(mockSocket, { + useSearchResults(mockSocket, { text: "123", property: "run", limit: 5, @@ -74,7 +72,7 @@ describe("useLimitedSearchResults", () => { mockSocket.channel.mockImplementationOnce(() => mockChannel) const { result } = renderHook(() => - useLimitedSearchResults(mockSocket, { + useSearchResults(mockSocket, { property: "run", text: "123", limit: 5, @@ -123,7 +121,7 @@ describe("useLimitedSearchResults", () => { } const { rerender, result } = renderHook( - (query) => useLimitedSearchResults(mockSocket, query), + (query) => useSearchResults(mockSocket, query), { initialProps: initialQuery, } diff --git a/assets/tests/hooks/useSearchResultsByCategory.test.ts b/assets/tests/hooks/useSearchResultsByCategory.test.ts index a4355bc36..1c55e389e 100644 --- a/assets/tests/hooks/useSearchResultsByCategory.test.ts +++ b/assets/tests/hooks/useSearchResultsByCategory.test.ts @@ -1,6 +1,6 @@ import { jest, describe, test, expect, afterEach } from "@jest/globals" import { renderHook } from "@testing-library/react" -import { useLimitedSearchResults } from "../../src/hooks/useSearchResults" +import { useSearchResults } from "../../src/hooks/useSearchResults" import { makeMockSocket } from "../testHelpers/socketHelpers" import vehicleFactory from "../factories/vehicle" import useSearchResultsByCategory, { @@ -12,7 +12,7 @@ import { LocationSearchResult } from "../../src/models/locationSearchResult" jest.mock("../../src/hooks/useSearchResults", () => ({ __esModule: true, - useLimitedSearchResults: jest.fn(() => null), + useSearchResults: jest.fn(() => null), })) jest.mock("../../src/hooks/useLocationSearchResults", () => ({ @@ -32,22 +32,22 @@ const mockSearchResults = (rawResults: { run?: VehicleResultType location?: LocationSearchResult[] | null }) => { - ;( - useLimitedSearchResults as jest.Mock - ).mockImplementation((_socket, query) => { - switch (query?.property) { - case "vehicle": - return rawResults.vehicle || null - case "run": - return rawResults.run || null - case "operator": - return rawResults.operator || null - case "all": - return rawResults.all || null - default: - return null + ;(useSearchResults as jest.Mock).mockImplementation( + (_socket, query) => { + switch (query?.property) { + case "vehicle": + return rawResults.vehicle || null + case "run": + return rawResults.run || null + case "operator": + return rawResults.operator || null + case "all": + return rawResults.all || null + default: + return null + } } - }) + ) ;(useLocationSearchResults as jest.Mock).mockReturnValue(rawResults.location) } From 24b13545dcf6526f06dacc4e2e1e6971a1fe80b5 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Thu, 4 Jan 2024 11:02:42 -0500 Subject: [PATCH 09/12] refactor: remove code for obsolete vehicles:search channel topic --- lib/skate_web/channels/vehicles_channel.ex | 47 ---------- .../channels/vehicles_channel_test.exs | 87 ------------------- 2 files changed, 134 deletions(-) diff --git a/lib/skate_web/channels/vehicles_channel.ex b/lib/skate_web/channels/vehicles_channel.ex index f0e11db5b..35bafc02d 100644 --- a/lib/skate_web/channels/vehicles_channel.ex +++ b/lib/skate_web/channels/vehicles_channel.ex @@ -46,53 +46,6 @@ defmodule SkateWeb.VehiclesChannel do {:ok, %{data: vehicles_and_ghosts}, Phoenix.Socket.assign(socket, lookup_key: lookup_key)} end - def join_authenticated( - "vehicles:search:" <> search_params, - _message, - socket - ) do - username_from_socket! = - Application.get_env( - :skate, - :username_from_socket!, - &SkateWeb.AuthManager.username_from_socket!/1 - ) - - username = username_from_socket!.(socket) - - subscribe_args = - case search_params do - "all:" <> text -> - %{text: text, property: :all} - - "run:" <> text -> - %{text: text, property: :run} - - "vehicle:" <> text -> - %{text: text, property: :vehicle} - - "operator:" <> text -> - %{text: text, property: :operator} - end - - %{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket) - - subscribe_args = - Map.put( - subscribe_args, - :include_logged_out_vehicles, - Skate.Settings.User.is_in_test_group(user_id, "map-beta") - ) - - Logger.info(fn -> - "User=#{username} searched for property=#{subscribe_args.property}, text=#{subscribe_args.text}" - end) - - {lookup_key, vehicles} = Duration.log_duration(Server, :subscribe_to_search, [subscribe_args]) - - {:ok, %{data: vehicles}, Phoenix.Socket.assign(socket, lookup_key: lookup_key)} - end - def join_authenticated(topic, _message, _socket) do {:error, %{message: "no such topic \"#{topic}\""}} end diff --git a/test/skate_web/channels/vehicles_channel_test.exs b/test/skate_web/channels/vehicles_channel_test.exs index 21fc304c6..b13cb054b 100644 --- a/test/skate_web/channels/vehicles_channel_test.exs +++ b/test/skate_web/channels/vehicles_channel_test.exs @@ -2,7 +2,6 @@ defmodule SkateWeb.VehiclesChannelTest do use SkateWeb.ChannelCase import Test.Support.Helpers import Skate.Factory - import ExUnit.CaptureLog, only: [capture_log: 1] alias Phoenix.Socket alias SkateWeb.{UserSocket, VehiclesChannel} @@ -58,34 +57,6 @@ defmodule SkateWeb.VehiclesChannelTest do ) end - test "subscribes to a vehicle search", %{socket: socket} do - assert {:ok, %{data: []}, %Socket{}} = - subscribe_and_join( - socket, - VehiclesChannel, - "vehicles:search:run:" <> String.slice(@vehicle.run_id, 0, 3) - ) - end - - test "logs that a user subscribed to a vehicle search", %{socket: socket} do - old_level = Logger.level() - - on_exit(fn -> - Logger.configure(level: old_level) - end) - - Logger.configure(level: :info) - - run_search_term = String.slice(@vehicle.run_id, 0, 3) - - log = - capture_log(fn -> - subscribe_and_join(socket, VehiclesChannel, "vehicles:search:run:" <> run_search_term) - end) - - assert log =~ "User=test_uid searched for property=run, text=" <> run_search_term - end - test "returns an error when joining a non-existant topic", %{socket: socket} do assert {:error, %{message: "no such topic \"rooms:1\""}} = subscribe_and_join(socket, VehiclesChannel, "rooms:1") @@ -99,7 +70,6 @@ defmodule SkateWeb.VehiclesChannelTest do "vehicles:route:", "vehicles:run_ids:", "vehicles:block_ids:", - "vehicles:search:", "random:topic:" ], do: @@ -172,63 +142,6 @@ defmodule SkateWeb.VehiclesChannelTest do assert_push("pull_backs", %{data: [^pull_back_vehicle]}) end - test "pushes new search results data onto the socket", %{ - socket: socket, - ets: ets - } do - assert Realtime.Server.update_vehicles({%{}, [@vehicle], []}) == :ok - - {:ok, _, socket} = - subscribe_and_join(socket, VehiclesChannel, "vehicles:search:all:" <> @vehicle.label) - - assert {:noreply, _socket} = - VehiclesChannel.handle_info( - {:new_realtime_data, ets}, - socket - ) - - vehicle = @vehicle - assert_push("search", %{data: [^vehicle]}) - end - - test "when user is in test group to enable searching logged out vehicles, then logged out vehicles are included in their search results", - %{ - socket: socket, - user: user - } do - {:ok, test_group} = Skate.Settings.TestGroup.create("map-beta") - Skate.Settings.TestGroup.update(%{test_group | users: [user]}) - - logged_in_vehicle = - build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id") - - logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil) - - {:ok, _reply, _socket} = - subscribe_and_join(socket, VehiclesChannel, "vehicles:search:all:123") - - Realtime.Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}) - - assert_push("search", %{data: [^logged_in_vehicle, ^logged_out_vehicle]}) - end - - test "when user is not in a test group to enable searching logged out vehicles, then logged out vehicles are included in their search results", - %{ - socket: socket - } do - logged_in_vehicle = - build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id") - - logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil) - - {:ok, _reply, _socket} = - subscribe_and_join(socket, VehiclesChannel, "vehicles:search:all:123") - - Realtime.Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}) - - assert_push("search", %{data: [^logged_in_vehicle]}) - end - test "rejects sending vehicle data when socket is not authenticated", %{ socket: socket, ets: ets From de7cb8a5134c0f1768da030207ec072619b9c560 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Thu, 4 Jan 2024 14:43:25 -0500 Subject: [PATCH 10/12] refactor: remove old logged out vehicle functionality --- lib/realtime/server.ex | 46 +------- lib/skate_web/channels/vehicle_channel.ex | 20 +--- .../channels/vehicles_search_channel.ex | 8 +- test/realtime/server_test.exs | 105 +++++------------- .../channels/vehicle_channel_test.exs | 8 +- 5 files changed, 38 insertions(+), 149 deletions(-) diff --git a/lib/realtime/server.ex b/lib/realtime/server.ex index 7178b3ea6..706ac81e3 100644 --- a/lib/realtime/server.ex +++ b/lib/realtime/server.ex @@ -38,7 +38,6 @@ defmodule Realtime.Server do | {:search, search_params()} | {:limited_search, limited_search_params()} | {:vehicle, String.t()} - | {:vehicle_with_logged_out, String.t()} | {:run_ids, [Run.id()]} | {:block_ids, [Block.id()]} | {:alerts, Route.id()} @@ -46,14 +45,12 @@ defmodule Realtime.Server do @type search_params :: %{ :text => String.t(), :property => search_property(), - optional(:include_logged_out_vehicles) => boolean(), optional(:limit) => pos_integer() } @type limited_search_params :: %{ :text => String.t(), :property => search_property(), - optional(:include_logged_out_vehicles) => boolean(), optional(:limit) => pos_integer() } @@ -142,21 +139,6 @@ defmodule Realtime.Server do )} end - @spec subscribe_to_vehicle_with_logged_out(String.t(), GenServer.server()) :: - {subscription_key(), - [ - VehicleOrGhost.t() - ]} - def subscribe_to_vehicle_with_logged_out(vehicle_id, server \\ default_name()) do - subscription_key = {:vehicle_with_logged_out, vehicle_id} - - {subscription_key, - subscribe( - server, - subscription_key - )} - end - @spec subscribe_to_run_ids([Run.id()], GenServer.server()) :: {subscription_key(), [VehicleOrGhost.t()]} def subscribe_to_run_ids(run_ids, server \\ default_name()) do @@ -194,11 +176,6 @@ defmodule Realtime.Server do lookup({ets, {:vehicle, vehicle_or_ghost_id}}) end - def peek_at_vehicle_by_id_with_logged_out(vehicle_or_ghost_id, server \\ default_name()) do - {_registry_key, ets} = GenServer.call(server, :subscription_info) - lookup({ets, {:vehicle_with_logged_out, vehicle_or_ghost_id}}) - end - @spec subscribe(GenServer.server(), {:route_id, Route.id()}) :: [VehicleOrGhost.t()] @spec subscribe(GenServer.server(), :all_shuttles) :: [Vehicle.t()] @spec subscribe(GenServer.server(), :logged_in_vehicles) :: [Vehicle.t()] @@ -206,9 +183,6 @@ defmodule Realtime.Server do @spec subscribe(GenServer.server(), {:limited_search, search_params()}) :: limited_search_result() @spec subscribe(GenServer.server(), {:vehicle, String.t()}) :: [VehicleOrGhost.t()] - @spec subscribe(GenServer.server(), {:vehicle_with_logged_out, String.t()}) :: [ - VehicleOrGhost.t() - ] @spec subscribe(GenServer.server(), :all_pull_backs) :: [VehicleOrGhost.t()] @spec subscribe(GenServer.server(), {:run_ids, [Run.id()]}) :: [VehicleOrGhost.t()] @spec subscribe(GenServer.server(), {:block_ids, [Block.id()]}) :: [VehicleOrGhost.t()] @@ -258,7 +232,6 @@ defmodule Realtime.Server do @spec lookup({:ets.tid(), {:search, search_params()}}) :: [VehicleOrGhost.t()] @spec lookup({:ets.tid(), {:limited_search, search_params()}}) :: limited_search_result() @spec lookup({:ets.tid(), {:vehicle, String.t()}}) :: [VehicleOrGhost.t()] - @spec lookup({:ets.tid(), {:vehicle_with_logged_out, String.t()}}) :: [VehicleOrGhost.t()] @spec lookup({:ets.tid(), :all_pull_backs}) :: [Vehicle.t()] @spec lookup({:ets.tid(), {:run_ids, [Run.id()]}}) :: [VehicleOrGhost.t()] @spec lookup({:ets.tid(), {:block_ids, [Block.id()]}}) :: [VehicleOrGhost.t()] @@ -267,11 +240,7 @@ defmodule Realtime.Server do logged_in_vehicles = lookup({table, :logged_in_vehicles}) vehicles_to_search = - if Map.get(search_params, :include_logged_out_vehicles, false) do - logged_in_vehicles ++ lookup({table, :logged_out_vehicles}) - else - logged_in_vehicles - end + logged_in_vehicles ++ lookup({table, :logged_out_vehicles}) VehicleOrGhost.find_by(vehicles_to_search, search_params) end @@ -280,11 +249,7 @@ defmodule Realtime.Server do logged_in_vehicles = lookup({table, :logged_in_vehicles}) vehicles_to_search = - if Map.get(search_params, :include_logged_out_vehicles, false) do - logged_in_vehicles ++ lookup({table, :logged_out_vehicles}) - else - logged_in_vehicles - end + logged_in_vehicles ++ lookup({table, :logged_out_vehicles}) VehicleOrGhost.take_limited_matches(vehicles_to_search, search_params) end @@ -318,13 +283,6 @@ defmodule Realtime.Server do end def lookup({table, {:vehicle, vehicle_or_ghost_id}}) do - {table, :logged_in_vehicles} - |> lookup() - |> Enum.find(&(&1.id == vehicle_or_ghost_id)) - |> List.wrap() - end - - def lookup({table, {:vehicle_with_logged_out, vehicle_or_ghost_id}}) do logged_in_vehicles = lookup({table, :logged_in_vehicles}) logged_out_vehicles = lookup({table, :logged_out_vehicles}) diff --git a/lib/skate_web/channels/vehicle_channel.ex b/lib/skate_web/channels/vehicle_channel.ex index 1d5be298d..2bccbc187 100644 --- a/lib/skate_web/channels/vehicle_channel.ex +++ b/lib/skate_web/channels/vehicle_channel.ex @@ -36,26 +36,14 @@ defmodule SkateWeb.VehicleChannel do end def join_authenticated("vehicle:id:" <> vehicle_or_ghost_id, _message, socket) do - %{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket) - - user_in_test_group? = Skate.Settings.User.is_in_test_group(user_id, "map-beta") - vehicle_or_ghost = - if user_in_test_group? do - vehicle_or_ghost_id - |> Realtime.Server.peek_at_vehicle_by_id_with_logged_out() - |> List.first() - else - vehicle_or_ghost_id |> Realtime.Server.peek_at_vehicle_by_id() |> List.first() - end + vehicle_or_ghost_id + |> Realtime.Server.peek_at_vehicle_by_id() + |> List.first() {lookup_key, _vehicle_or_ghost} = if vehicle_or_ghost do - if user_in_test_group? do - Server.subscribe_to_vehicle_with_logged_out(vehicle_or_ghost.id) - else - Server.subscribe_to_vehicle(vehicle_or_ghost.id) - end + Server.subscribe_to_vehicle(vehicle_or_ghost.id) else {nil, nil} end diff --git a/lib/skate_web/channels/vehicles_search_channel.ex b/lib/skate_web/channels/vehicles_search_channel.ex index c2b35a205..a5ddc57ff 100644 --- a/lib/skate_web/channels/vehicles_search_channel.ex +++ b/lib/skate_web/channels/vehicles_search_channel.ex @@ -22,7 +22,6 @@ defmodule SkateWeb.VehiclesSearchChannel do ) username = username_from_socket!.(socket) - %{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket) %{property: property, text: text} = search_params_from_subtopic(subtopic) @@ -36,8 +35,7 @@ defmodule SkateWeb.VehiclesSearchChannel do subscribe_args = %{ property: property, text: text, - limit: limit, - include_logged_out_vehicles: Skate.Settings.User.is_in_test_group(user_id, "map-beta") + limit: limit } Logger.info(fn -> @@ -65,7 +63,6 @@ defmodule SkateWeb.VehiclesSearchChannel do &SkateWeb.AuthManager.username_from_socket!/1 ) - %{id: user_id} = Guardian.Phoenix.Socket.current_resource(socket) username = username_from_socket!.(socket) %{property: property, text: text} = search_params_from_subtopic(subtopic) @@ -75,8 +72,7 @@ defmodule SkateWeb.VehiclesSearchChannel do %{ property: property, text: text, - limit: limit, - include_logged_out_vehicles: Skate.Settings.User.is_in_test_group(user_id, "map-beta") + limit: limit } ]) diff --git a/test/realtime/server_test.exs b/test/realtime/server_test.exs index 3ba831e91..20fee6a92 100644 --- a/test/realtime/server_test.exs +++ b/test/realtime/server_test.exs @@ -33,7 +33,7 @@ defmodule Realtime.ServerTest do @logged_out_vehicle build(:vehicle, route_id: "1", - id: "v1", + id: "v3", label: "v1-label", run_id: nil, block_id: nil, @@ -47,7 +47,7 @@ defmodule Realtime.ServerTest do route_id: "1", block_is_active: false, block_id: "inactive_block", - id: "v2", + id: "v4", label: "v2-label", run_id: "456-7890" ) @@ -313,49 +313,39 @@ defmodule Realtime.ServerTest do setup do {:ok, server_pid} = Server.start_link([]) - :ok = Server.update_vehicles({@vehicles_by_route_id, [], []}, server_pid) + :ok = Server.update_vehicles({@vehicles_by_route_id, [], [@logged_out_vehicle]}, server_pid) %{server_pid: server_pid} end - test "clients get vehicle by ID upon subscribing", %{server_pid: pid} do + test "clients get vehicle by ID upon subscribing, logged in vehicle", %{server_pid: pid} do {lookup_key, vehicle} = Server.subscribe_to_vehicle(@vehicle.id, pid) assert vehicle == [@vehicle] assert lookup_key == {:vehicle, @vehicle.id} end - test "clients get updated data pushed to them", %{server_pid: pid} do - {lookup_key, _} = Server.subscribe_to_vehicle(@vehicle.id, pid) - - Server.update_vehicles({@vehicles_by_route_id, [], []}, pid) - - assert_receive {:new_realtime_data, ets} - assert Server.lookup({ets, lookup_key}) == [@vehicle] - end - end - - describe "subscribe_to_vehicle_with_logged_out/2" do - setup do - {:ok, server_pid} = Server.start_link([]) - - :ok = Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, server_pid) - - %{server_pid: server_pid} - end - - test "clients get vehicle by ID upon subscribing", %{server_pid: pid} do + test "clients get vehicle by ID upon subscribing, logged out vehicle", %{server_pid: pid} do {lookup_key, logged_out_vehicles} = - Server.subscribe_to_vehicle_with_logged_out(@logged_out_vehicle.id, pid) + Server.subscribe_to_vehicle(@logged_out_vehicle.id, pid) assert logged_out_vehicles == [ @logged_out_vehicle ] - assert lookup_key == {:vehicle_with_logged_out, @logged_out_vehicle.id} + assert lookup_key == {:vehicle, @logged_out_vehicle.id} end - test "clients get updated data pushed to them", %{server_pid: pid} do - {lookup_key, _} = Server.subscribe_to_vehicle_with_logged_out(@logged_out_vehicle.id, pid) + test "clients get updated data pushed to them, logged in vehicle", %{server_pid: pid} do + {lookup_key, _} = Server.subscribe_to_vehicle(@vehicle.id, pid) + + Server.update_vehicles({@vehicles_by_route_id, [], []}, pid) + + assert_receive {:new_realtime_data, ets} + assert Server.lookup({ets, lookup_key}) == [@vehicle] + end + + test "clients get updated data pushed to them, logged out vehicle", %{server_pid: pid} do + {lookup_key, _} = Server.subscribe_to_vehicle(@logged_out_vehicle.id, pid) Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, pid) @@ -410,11 +400,11 @@ defmodule Realtime.ServerTest do assert Server.lookup({ets, lookup_key}) == [@vehicle_on_inactive_block] end - test "logged out vehicles are returned when include_logged_out_vehicles is true", + test "logged out vehicles are returned", %{server_pid: pid} do {lookup_key, _} = Server.subscribe_to_search( - %{property: :vehicle, text: "123", include_logged_out_vehicles: true}, + %{property: :vehicle, text: "123"}, pid ) @@ -433,22 +423,6 @@ defmodule Realtime.ServerTest do assert_receive {:new_realtime_data, ets} assert Server.lookup({ets, lookup_key}) == [logged_in_vehicle, logged_out_vehicle] end - - test "logged out vehicles are not returned when include_logged_out_vehicles is not set", - %{server_pid: pid} do - {lookup_key, _} = Server.subscribe_to_search(%{property: :vehicle, text: "123"}, pid) - - logged_in_vehicle = - build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id") - - logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil) - - Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}, pid) - - assert_receive {:new_realtime_data, ets} - - assert Server.lookup({ets, lookup_key}) == [logged_in_vehicle] - end end describe "subscribe_to_limited_search/2" do @@ -504,11 +478,11 @@ defmodule Realtime.ServerTest do Server.lookup({ets, lookup_key}) end - test "logged out vehicles are returned when include_logged_out_vehicles is true", + test "logged out vehicles are returned", %{server_pid: pid} do {lookup_key, _} = Server.subscribe_to_limited_search( - %{property: :vehicle, text: "123", include_logged_out_vehicles: true, limit: 4}, + %{property: :vehicle, text: "123", limit: 4}, pid ) @@ -531,24 +505,6 @@ defmodule Realtime.ServerTest do has_more_matches: false } == Server.lookup({ets, lookup_key}) end - - test "logged out vehicles are not returned when include_logged_out_vehicles is not set", - %{server_pid: pid} do - {lookup_key, _} = - Server.subscribe_to_limited_search(%{property: :vehicle, text: "123", limit: 5}, pid) - - logged_in_vehicle = - build(:vehicle, id: "y1235", label: "1235", route_id: "1", run_id: "run_id") - - logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil) - - Server.update_vehicles({%{"1" => [logged_in_vehicle]}, [], [logged_out_vehicle]}, pid) - - assert_receive {:new_realtime_data, ets} - - assert %{matching_vehicles: [logged_in_vehicle], has_more_matches: false} == - Server.lookup({ets, lookup_key}) - end end describe "update_limited_search_subscription/2" do @@ -870,22 +826,17 @@ defmodule Realtime.ServerTest do describe "peek_at_vehicles_by_id/2" do test "looks up the vehicle or ghost with given ID" do {:ok, server_pid} = Server.start_link([]) - Server.update_vehicles({@vehicles_by_route_id, [@shuttle], []}, server_pid) + + Server.update_vehicles( + {@vehicles_by_route_id, [@shuttle], [@logged_out_vehicle]}, + server_pid + ) assert Server.peek_at_vehicle_by_id("no_such_vehicle", server_pid) == [] assert Server.peek_at_vehicle_by_id("v1", server_pid) == [@vehicle] assert Server.peek_at_vehicle_by_id("g1", server_pid) == [@ghost] - end - end - - describe "peek_at_vehicles_by_id_with_logged_out/2" do - test "looks up the vehicle or ghost with given ID" do - {:ok, server_pid} = Server.start_link([]) - Server.update_vehicles({%{}, [], [@logged_out_vehicle]}, server_pid) - - assert Server.peek_at_vehicle_by_id_with_logged_out("no_such_vehicle", server_pid) == [] - assert Server.peek_at_vehicle_by_id_with_logged_out(@logged_out_vehicle.id, server_pid) == [ + assert Server.peek_at_vehicle_by_id(@logged_out_vehicle.id, server_pid) == [ @logged_out_vehicle ] end diff --git a/test/skate_web/channels/vehicle_channel_test.exs b/test/skate_web/channels/vehicle_channel_test.exs index cda980fde..730296eb4 100644 --- a/test/skate_web/channels/vehicle_channel_test.exs +++ b/test/skate_web/channels/vehicle_channel_test.exs @@ -113,13 +113,9 @@ defmodule SkateWeb.VehicleChannelTest do subscribe_and_join(socket, VehicleChannel, "vehicle:id:#{@vehicle.id}") end - test "subscribes to the logged out vehicle for given ID when user is in the test group", %{ - socket: socket, - user: user + test "subscribes to logged out vehicle for given ID", %{ + socket: socket } do - {:ok, test_group} = Skate.Settings.TestGroup.create("map-beta") - Skate.Settings.TestGroup.update(%{test_group | users: [user]}) - logged_out_vehicle = build(:vehicle, id: "y1234", label: "1234", route_id: nil, run_id: nil) Realtime.Server.update_vehicles({%{}, [], [logged_out_vehicle]}) From 20c57b8158d2009a84a31fb8723645c627eb03c2 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Fri, 5 Jan 2024 11:40:48 -0500 Subject: [PATCH 11/12] fixup! fix: make VehiclePropertiesPanel tests work --- .../vehiclePropertiesPanel.test.tsx | 159 +++++++++--------- 1 file changed, 77 insertions(+), 82 deletions(-) diff --git a/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx b/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx index 3d1746184..d728b18a6 100644 --- a/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx +++ b/assets/tests/components/propertiesPanel/vehiclePropertiesPanel.test.tsx @@ -1,5 +1,5 @@ import { jest, describe, test, expect } from "@jest/globals" -import React from "react" +import React, { ReactNode } from "react" import renderer from "react-test-renderer" import routeFactory from "../../factories/route" import * as map from "../../../src/components/map" @@ -126,6 +126,10 @@ const vehicle: VehicleInScheduledService = vehicleFactory.build({ crowding: null, }) +const MemoryRouterWrapper = ({ children }: { children: ReactNode }) => ( + {children} +) + describe("VehiclePropertiesPanel", () => { test("renders a vehicle properties panel", () => { const tree = renderer @@ -187,15 +191,14 @@ describe("VehiclePropertiesPanel", () => { test("Includes invalid bus banner when vehicle is off course", () => { render( - - - + , + { wrapper: MemoryRouterWrapper } ) expect(screen.getByRole("heading", { name: "Invalid Bus" })).toBeVisible() }) @@ -297,15 +300,14 @@ describe("VehiclePropertiesPanel", () => { ok: "Atlantic Ave & Summer St", }) const result = render( - - - + , + { wrapper: MemoryRouterWrapper } ) expect(result.getByText("Atlantic Ave & Summer St")).toBeInTheDocument() }) @@ -316,15 +318,14 @@ describe("VehiclePropertiesPanel", () => { .mockImplementation((_key) => "1") const result = render( - - - + , + { wrapper: MemoryRouterWrapper } ) expect(result.queryAllByTestId("data-discrepancy")).toHaveLength(2) @@ -336,15 +337,14 @@ describe("VehiclePropertiesPanel", () => { .mockImplementation((_key) => null) const result = render( - - - + , + { wrapper: MemoryRouterWrapper } ) expect(result.queryAllByTestId("data-discrepancy")).toHaveLength(0) @@ -418,15 +418,14 @@ describe("VehiclePropertiesPanel", () => { ]) const { container } = render( - - - + , + { wrapper: MemoryRouterWrapper } ) expect(container.innerHTML).toContain("c-station-icon") @@ -436,15 +435,14 @@ describe("VehiclePropertiesPanel", () => { const mockClosePanel = jest.fn() render( - - - + , + { wrapper: MemoryRouterWrapper } ) await userEvent.click(closeButton.get()) @@ -466,15 +464,14 @@ describe("VehiclePropertiesPanel", () => { const mockSetTabMode = jest.fn() render( - - - + , + { wrapper: MemoryRouterWrapper } ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) @@ -497,15 +494,14 @@ describe("VehiclePropertiesPanel", () => { const mockedFSEvent = jest.mocked(fullStoryEvent) render( - - - + , + { wrapper: MemoryRouterWrapper } ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) @@ -531,15 +527,14 @@ describe("VehiclePropertiesPanel", () => { const mockedFSEvent = jest.mocked(fullStoryEvent) render( - - - + , + { wrapper: MemoryRouterWrapper } ) await userEvent.click(screen.getByRole("tab", { name: clickTarget })) From 696c982a25409e41207fea6cfdc56914098df6fb Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Fri, 5 Jan 2024 11:49:08 -0500 Subject: [PATCH 12/12] fixup! fix: get MapPage tests working --- assets/tests/components/mapPage.test.tsx | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/assets/tests/components/mapPage.test.tsx b/assets/tests/components/mapPage.test.tsx index b08e99c04..62f2d1168 100644 --- a/assets/tests/components/mapPage.test.tsx +++ b/assets/tests/components/mapPage.test.tsx @@ -1410,10 +1410,12 @@ describe("", () => { expect( mapContainer.querySelector(".c-vehicle-map__route-shape") ).toBeInTheDocument() - // there is one additional button to select the run ID from the VPP expect( - screen.getAllByRole("button", { name: runIdToLabel(vehicle.runId!) }) - ).toHaveLength(vehicles.length + 1) + within(document.getElementById("id-vehicle-map")!).getAllByRole( + "button", + { name: runIdToLabel(vehicle.runId!) } + ) + ).toHaveLength(vehicles.length) expect(vehiclePropertiesCard.get()).toBeVisible() expect( @@ -1464,10 +1466,12 @@ describe("", () => { ) - // there is one additional button to select the run ID from the VPP expect( - screen.getAllByRole("button", { name: selectedVehicle.runId! }) - ).toHaveLength(selectedRouteVehicles.length + 1) + within(document.getElementById("id-vehicle-map")!).getAllByRole( + "button", + { name: selectedVehicle.runId! } + ) + ).toHaveLength(selectedRouteVehicles.length) }) describe("and vehicle is a regular bus", () => { @@ -1500,10 +1504,12 @@ describe("", () => { ) - // there is one additional button to select the run ID from the VPP expect( - screen.getAllByRole("button", { name: selectedVehicle.runId! }) - ).toHaveLength(selectedRouteVehicles.length + 1) + within(document.getElementById("id-vehicle-map")!).getAllByRole( + "button", + { name: selectedVehicle.runId! } + ) + ).toHaveLength(selectedRouteVehicles.length) expect( container.querySelectorAll(".c-vehicle-map__stop")