From 739ba8c17ed5cde62e9e26212e4f74047c6ac591 Mon Sep 17 00:00:00 2001 From: KaylaBrady <31781298+KaylaBrady@users.noreply.github.com> Date: Fri, 28 Jul 2023 17:04:17 -0400 Subject: [PATCH] refactor(useSearchResultsByProperty): Return location results too --- .../mapPage/searchResultsByProperty.tsx | 80 +++++++++---------- assets/src/hooks/useSearchResults.ts | 18 ++--- .../src/hooks/useSearchResultsByProperty.ts | 31 +++++-- .../mapPage/searchResultsByProperty.test.tsx | 79 +++++++----------- 4 files changed, 104 insertions(+), 104 deletions(-) diff --git a/assets/src/components/mapPage/searchResultsByProperty.tsx b/assets/src/components/mapPage/searchResultsByProperty.tsx index 016c5fc927..b99ca4eafd 100644 --- a/assets/src/components/mapPage/searchResultsByProperty.tsx +++ b/assets/src/components/mapPage/searchResultsByProperty.tsx @@ -10,7 +10,6 @@ import { Vehicle, Ghost } from "../../realtime" import { setPropertyMatchLimit } from "../../state/searchPageState" import SearchResults, { NoResults } from "../searchResults" import React from "react" -import { useLocationSearchResults } from "../../hooks/useLocationSearchResults" import { Card, CardBody } from "../card" import { LocationSearchResult } from "../../models/locationSearchResult" import { @@ -29,14 +28,11 @@ const VehicleSearchResultSection = ({ onShowMore, }: { property: SearchProperty - results: LoadingResult | Ok | null + results: LoadingResult | Ok> | null onSelectVehicle: (vehicle: Vehicle | Ghost) => void onShowMore: () => void }) => { - if ( - results === null || - (isOk(results) && results.ok.matchingVehicles.length === 0) - ) { + if (results === null || (isOk(results) && results.ok.matches.length === 0)) { return <> } @@ -53,10 +49,10 @@ const VehicleSearchResultSection = ({ {isLoading(results) ? ( - ) : results.ok.matchingVehicles.length > 0 ? ( + ) : results.ok.matches.length > 0 ? ( <> @@ -79,18 +75,17 @@ const VehicleSearchResultSection = ({ } const LocationSearchResultSection = ({ - text, - limit, + results, onSelectLocation, onShowMore, }: { - text: string - limit: number + results: LoadingResult | Ok> | null onSelectLocation: (location: LocationSearchResult) => void onShowMore: () => void }) => { - const locationSearchResults = useLocationSearchResults(text) - + if (results === null || (isOk(results) && results.ok.matches.length === 0)) { + return <> + } return (
{searchPropertyDisplayConfig.location.name} - {locationSearchResults === null ? ( + {isLoading(results) ? ( - ) : locationSearchResults.length > 0 ? ( + ) : ( <>
    - {locationSearchResults - .slice(0, limit) - .map((locationSearchResult) => ( -
  • - onSelectLocation(locationSearchResult)} - > - {locationSearchResult.name && - locationSearchResult.address && ( - {locationSearchResult.address} - )} - -
    • - ))} + {results.ok.matches.map((locationSearchResult) => ( +
  • + onSelectLocation(locationSearchResult)} + > + {locationSearchResult.name && + locationSearchResult.address && ( + {locationSearchResult.address} + )} + +
    • + ))}
- {locationSearchResults.length > limit && ( + {results.ok.hasMoreMatches && (
) @@ -167,10 +158,16 @@ const SearchResultsByProperty = ({ const searchHasNoResults = Object.values(resultsByProperty) .filter( - (result): result is LoadingResult | Ok => - result !== null + ( + result + ): result is + | LoadingResult + | Ok> + | Ok> => result !== null + ) + .every( + (result) => !("is_loading" in result) && result.ok.matches.length === 0 ) - .every((result) => isOk(result) && result.ok.matchingVehicles.length === 0) return (
@@ -192,8 +189,7 @@ const SearchResultsByProperty = ({ property === "location" ? ( onShowMore(property, limit)} /> diff --git a/assets/src/hooks/useSearchResults.ts b/assets/src/hooks/useSearchResults.ts index e320ea542a..eae4f49c7d 100644 --- a/assets/src/hooks/useSearchResults.ts +++ b/assets/src/hooks/useSearchResults.ts @@ -35,22 +35,22 @@ const useSearchResults = ( }) } -const limitedSearchResultsData = type({ +const limitedVehicleSearchResultsData = type({ matching_vehicles: dataStruct, has_more_matches: boolean(), }) -type LimitedSearchResultsData = Infer -export type LimitedSearchResults = { - matchingVehicles: (Vehicle | Ghost)[] +type LimitedSearchResultsData = Infer +export type LimitedSearchResults = { + matches: T[] hasMoreMatches: boolean } const parseLimitedSearchResults = ( data: LimitedSearchResultsData -): Ok => ({ +): Ok> => ({ ok: { - matchingVehicles: parser(data.matching_vehicles), + matches: parser(data.matching_vehicles), hasMoreMatches: data.has_more_matches, }, }) @@ -60,19 +60,19 @@ const loadingState: Loading = { is_loading: true } export const useLimitedSearchResults = ( socket: Socket | undefined, query: { property: SearchProperty; text: string; limit: number } | null -): Ok | Loading | null => { +): Ok> | Loading | null => { const topic: string | null = query && `vehicles_search:limited:${query.property}:${query.text}` const [state, pushUpdate] = useCheckedTwoWayChannel< LimitedSearchResultsData, - Ok | Loading, + Ok> | Loading, { limit: number } >({ socket, topic: topic, event: "search", - dataStruct: limitedSearchResultsData, + dataStruct: limitedVehicleSearchResultsData, parser: parseLimitedSearchResults, loadingState: loadingState, }) diff --git a/assets/src/hooks/useSearchResultsByProperty.ts b/assets/src/hooks/useSearchResultsByProperty.ts index d3695c07d2..7f8b795c85 100644 --- a/assets/src/hooks/useSearchResultsByProperty.ts +++ b/assets/src/hooks/useSearchResultsByProperty.ts @@ -5,16 +5,24 @@ import { LimitedSearchResults, useLimitedSearchResults, } from "./useSearchResults" +import { useLocationSearchResults } from "./useLocationSearchResults" +import { LocationSearchResult } from "../models/locationSearchResult" +import { Vehicle, Ghost } from "../realtime" + +type vehicleResultType = + | Ok> + | Loading + | null const useSearchResultsByProperty = ( socket: Socket | undefined, text: string, properties: PropertyLimits ): { - vehicle: Ok | Loading | null - operator: Ok | Loading | null - run: Ok | Loading | null - location: Ok | Loading | null + vehicle: vehicleResultType + operator: vehicleResultType + run: vehicleResultType + location: Ok> | Loading | null } => { // To avoid conditionally calling hooks, use search result hooks for each property. // Conditionally use the results only if the limit is present @@ -37,11 +45,24 @@ const useSearchResultsByProperty = ( ? null : { property: "run", text: text, limit: properties.run } ) + + const locationResults = useLocationSearchResults(text) + return { vehicle: properties.vehicle === null ? null : vehicleResults, operator: properties.operator === null ? null : operatorResults, run: properties.run === null ? null : runResults, - location: null, + location: + properties.location === null + ? null + : locationResults === null + ? { is_loading: true } + : { + ok: { + matches: locationResults.slice(0, properties.location), + hasMoreMatches: locationResults.length > properties.location, + }, + }, } } diff --git a/assets/tests/components/mapPage/searchResultsByProperty.test.tsx b/assets/tests/components/mapPage/searchResultsByProperty.test.tsx index 478cf6406c..45bb7ff45f 100644 --- a/assets/tests/components/mapPage/searchResultsByProperty.test.tsx +++ b/assets/tests/components/mapPage/searchResultsByProperty.test.tsx @@ -2,7 +2,6 @@ import React from "react" import { render, screen, within } from "@testing-library/react" import "@testing-library/jest-dom" import SearchResultsByProperty from "../../../src/components/mapPage/searchResultsByProperty" -import { useLocationSearchResults } from "../../../src/hooks/useLocationSearchResults" import vehicleFactory from "../../factories/vehicle" import { StateDispatchProvider } from "../../../src/contexts/stateDispatchContext" import stateFactory from "../../factories/applicationState" @@ -18,38 +17,34 @@ jest.mock("../../../src/hooks/useSearchResultsByProperty", () => ({ default: jest.fn(() => null), })) -jest.mock("../../../src/hooks/useLocationSearchResults", () => ({ - __esModule: true, - useLocationSearchResults: jest.fn(() => null), -})) - const runMatch = vehicleFactory.build() const operatorMatch = vehicleFactory.build() const vehicleMatch = vehicleFactory.build() +const locationMatch = locationSearchResultFactory.build() beforeEach(() => { ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ run: { ok: { - matchingVehicles: [runMatch], + matches: [runMatch], hasMoreMatches: false, }, }, vehicle: { ok: { - matchingVehicles: [vehicleMatch], + matches: [vehicleMatch], hasMoreMatches: true, }, }, operator: { ok: { - matchingVehicles: [operatorMatch], + matches: [operatorMatch], hasMoreMatches: false, }, }, location: { ok: { - matchingVehicles: [], + matches: [locationMatch], hasMoreMatches: false, }, }, @@ -63,27 +58,23 @@ afterEach(() => { describe("searchResultsByProperty", () => { test("Includes only sections that have results", () => { ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ - run: { ok: { matchingVehicles: [], hasMoreMatches: false } }, + run: { ok: { matches: [], hasMoreMatches: false } }, vehicle: { ok: { - matchingVehicles: [], + matches: [], hasMoreMatches: false, }, }, operator: { ok: { - matchingVehicles: [operatorMatch], + matches: [operatorMatch], hasMoreMatches: false, }, }, location: { - ok: { - matchingVehicles: [], - hasMoreMatches: false, - }, + ok: { matches: [], hasMoreMatches: false }, }, }) - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => []) render( { }) test("only includes results for the properties included in the query", () => { - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => []) - render( { expect( screen.queryByRole("heading", { name: "Operators" }) ).not.toBeInTheDocument() + expect( + screen.queryByRole("heading", { name: "location" }) + ).not.toBeInTheDocument() }) test("Includes the sections in the expected order", () => { - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => []) - render( { }) test("Lists the matching vehicles under the appropriate header", () => { - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => []) - render( { }) test("Lists the matching locations under the locations header", () => { - const locationMatch = locationSearchResultFactory.build() - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => [ - locationMatch, - ]) - render( { }) test("Shows loading indication for locations", () => { - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => null) - + ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ + run: null, + vehicle: null, + operator: null, + location: { is_loading: true }, + }) render( { ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ run: { ok: { - matchingVehicles: [runMatch], + matches: [runMatch], hasMoreMatches: false, }, }, vehicle: { ok: { - matchingVehicles: [vehicleMatch], + matches: [vehicleMatch], hasMoreMatches: true, }, }, operator: { ok: { - matchingVehicles: [operatorMatch], + matches: [operatorMatch], hasMoreMatches: false, }, }, location: { ok: { - matchingVehicles: [], + matches: [], hasMoreMatches: false, }, }, }) - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => []) render( { locations.push(locationSearchResultFactory.build()) } - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => locations) - + ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ + run: null, + vehicle: null, + operator: null, + location: { ok: { matches: locations, hasMoreMatches: true } }, + }) render( { }) test("When locations section does not have more matches, doesn't include a 'Show more' button", () => { - ;(useLocationSearchResults as jest.Mock).mockImplementation(() => [ - locationSearchResultFactory.build(), - ]) - render( { ;(useSearchResultsByProperty as jest.Mock).mockReturnValue({ run: { ok: { - matchingVehicles: [], + matches: [], hasMoreMatches: false, }, }, vehicle: { ok: { - matchingVehicles: [], + matches: [], hasMoreMatches: true, }, }, operator: null, location: { - ok: { - matchingVehicles: [], - hasMoreMatches: false, - }, + ok: { matches: [], hasMoreMatches: false }, }, })