From bf09ccc21127e410dd09c462fd5ecb2a750de2c1 Mon Sep 17 00:00:00 2001 From: Eddie Maldonado Date: Thu, 10 Aug 2023 16:28:52 -0400 Subject: [PATCH] Provide autocomplete-related data for location search (#2177) * refactor: rename SearchResult to Place * feat: return both suggestion text and place id if available * feat: get location by ID * refactor: get rid of single-call pipelines * feat: useLocationSearchResultById hook * feat: functionality for manipulating location search suggestions * feat: fetchLocationSearchSuggestions function * feat: useLocationSearchSuggestions hook * chore: fix typos --- assets/src/api.ts | 29 +++++++ .../src/hooks/useLocationSearchResultById.ts | 31 +++++++ .../src/hooks/useLocationSearchSuggestions.ts | 31 +++++++ assets/src/models/locationSearchSuggestion.ts | 4 + .../models/locationSearchSuggestionData.ts | 23 ++++++ assets/tests/api.test.ts | 54 ++++++++++++ .../factories/locationSearchSuggestion.ts | 10 +++ .../factories/locationSearchSuggestionData.ts | 10 +++ .../hooks/useLocationSearchResultById.test.ts | 47 +++++++++++ .../useLocationSearchSuggestions.test.ts | 51 ++++++++++++ .../locationSearchSuggestionData.test.ts | 40 +++++++++ .../location_search/aws_location_request.ex | 61 +++++++++++--- .../{search_result.ex => place.ex} | 6 +- lib/skate/location_search/suggestion.ex | 15 ++++ .../controllers/location_search_controller.ex | 13 ++- lib/skate_web/router.ex | 1 + .../aws_location_request_test.exs | 82 ++++++++++++++++--- .../location_search_controller_test.exs | 47 ++++++++++- test/support/factory.ex | 24 +++--- 19 files changed, 540 insertions(+), 39 deletions(-) create mode 100644 assets/src/hooks/useLocationSearchResultById.ts create mode 100644 assets/src/hooks/useLocationSearchSuggestions.ts create mode 100644 assets/src/models/locationSearchSuggestion.ts create mode 100644 assets/src/models/locationSearchSuggestionData.ts create mode 100644 assets/tests/factories/locationSearchSuggestion.ts create mode 100644 assets/tests/factories/locationSearchSuggestionData.ts create mode 100644 assets/tests/hooks/useLocationSearchResultById.test.ts create mode 100644 assets/tests/hooks/useLocationSearchSuggestions.test.ts create mode 100644 assets/tests/models/locationSearchSuggestionData.test.ts rename lib/skate/location_search/{search_result.ex => place.ex} (82%) create mode 100644 lib/skate/location_search/suggestion.ex diff --git a/assets/src/api.ts b/assets/src/api.ts index fb07efa5d..a4fdbb26c 100644 --- a/assets/src/api.ts +++ b/assets/src/api.ts @@ -29,8 +29,14 @@ import * as Sentry from "@sentry/react" import { LocationSearchResult } from "./models/locationSearchResult" import { LocationSearchResultData, + locationSearchResultFromData, locationSearchResultsFromData, } from "./models/locationSearchResultData" +import { + LocationSearchSuggestionData, + locationSearchSuggestionsFromData, +} from "./models/locationSearchSuggestionData" +import { LocationSearchSuggestion } from "./models/locationSearchSuggestion" export interface RouteData { id: string @@ -227,6 +233,29 @@ export const fetchLocationSearchResults = ( defaultResult: [], }) +export const fetchLocationSearchResultById = ( + placeId: string +): Promise => + checkedApiCall({ + url: `api/location_search/place/${placeId}`, + dataStruct: LocationSearchResultData, + parser: nullableParser(locationSearchResultFromData), + defaultResult: null, + }) + +export const fetchLocationSearchSuggestions = ( + searchText: string +): Promise => + checkedApiCall< + LocationSearchSuggestionData[], + LocationSearchSuggestion[] | null + >({ + url: `api/location_search/suggest?query=${searchText}`, + dataStruct: array(LocationSearchSuggestionData), + parser: nullableParser(locationSearchSuggestionsFromData), + defaultResult: null, + }) + export const putNotificationReadState = ( newReadState: NotificationState, notificationIds: NotificationId[] diff --git a/assets/src/hooks/useLocationSearchResultById.ts b/assets/src/hooks/useLocationSearchResultById.ts new file mode 100644 index 000000000..a33c4c12b --- /dev/null +++ b/assets/src/hooks/useLocationSearchResultById.ts @@ -0,0 +1,31 @@ +import { useEffect, useState } from "react" +import { fetchLocationSearchResultById } from "../api" +import { LocationSearchResult } from "../models/locationSearchResult" + +export const useLocationSearchResultById = ( + placeId: string | null +): LocationSearchResult | null => { + const [searchResult, setSearchResult] = useState( + null + ) + + useEffect(() => { + let shouldUpdate = true + + if (placeId) { + fetchLocationSearchResultById(placeId).then((results) => { + if (shouldUpdate) { + setSearchResult(results) + } + }) + } else { + setSearchResult(null) + } + + return () => { + shouldUpdate = false + } + }, [placeId]) + + return searchResult +} diff --git a/assets/src/hooks/useLocationSearchSuggestions.ts b/assets/src/hooks/useLocationSearchSuggestions.ts new file mode 100644 index 000000000..6b3a8d33b --- /dev/null +++ b/assets/src/hooks/useLocationSearchSuggestions.ts @@ -0,0 +1,31 @@ +import { useEffect, useState } from "react" +import { fetchLocationSearchSuggestions } from "../api" +import { LocationSearchSuggestion } from "../models/locationSearchSuggestion" + +export const useLocationSearchSuggestions = ( + text: string | null +): LocationSearchSuggestion[] | null => { + const [searchSuggestions, setSearchSuggestions] = useState< + LocationSearchSuggestion[] | null + >(null) + + useEffect(() => { + let shouldUpdate = true + + if (text) { + fetchLocationSearchSuggestions(text).then((results) => { + if (shouldUpdate) { + setSearchSuggestions(results) + } + }) + } else { + setSearchSuggestions(null) + } + + return () => { + shouldUpdate = false + } + }, [text]) + + return searchSuggestions +} diff --git a/assets/src/models/locationSearchSuggestion.ts b/assets/src/models/locationSearchSuggestion.ts new file mode 100644 index 000000000..82cdc9181 --- /dev/null +++ b/assets/src/models/locationSearchSuggestion.ts @@ -0,0 +1,4 @@ +export interface LocationSearchSuggestion { + text: string + placeId: string | null +} diff --git a/assets/src/models/locationSearchSuggestionData.ts b/assets/src/models/locationSearchSuggestionData.ts new file mode 100644 index 000000000..be472f4d6 --- /dev/null +++ b/assets/src/models/locationSearchSuggestionData.ts @@ -0,0 +1,23 @@ +import { Infer, nullable, string, type } from "superstruct" +import { LocationSearchSuggestion } from "./locationSearchSuggestion" + +export const LocationSearchSuggestionData = type({ + text: string(), + place_id: nullable(string()), +}) +export type LocationSearchSuggestionData = Infer< + typeof LocationSearchSuggestionData +> + +export const locationSearchSuggestionFromData = ({ + text, + place_id, +}: LocationSearchSuggestionData): LocationSearchSuggestion => ({ + text, + placeId: place_id, +}) + +export const locationSearchSuggestionsFromData = ( + locationSearchSuggestionsData: LocationSearchSuggestionData[] +): LocationSearchSuggestion[] => + locationSearchSuggestionsData.map(locationSearchSuggestionFromData) diff --git a/assets/tests/api.test.ts b/assets/tests/api.test.ts index 1720f9487..2f4b4b410 100644 --- a/assets/tests/api.test.ts +++ b/assets/tests/api.test.ts @@ -15,6 +15,8 @@ import { fetchStations, fetchRoutePatterns, fetchLocationSearchResults, + fetchLocationSearchResultById, + fetchLocationSearchSuggestions, } from "../src/api" import routeFactory from "./factories/route" import routeTabFactory from "./factories/routeTab" @@ -25,6 +27,8 @@ import { LocationType } from "../src/models/stopData" import * as Sentry from "@sentry/react" import locationSearchResultDataFactory from "./factories/locationSearchResultData" import locationSearchResultFactory from "./factories/locationSearchResult" +import locationSearchSuggestionDataFactory from "./factories/locationSearchSuggestionData" +import locationSearchSuggestionFactory from "./factories/locationSearchSuggestion" jest.mock("@sentry/react", () => ({ __esModule: true, @@ -764,6 +768,56 @@ describe("fetchLocationSearchResults", () => { }) }) +describe("fetchLocationSearchResultById", () => { + test("parses location returned", (done) => { + const result = locationSearchResultDataFactory.build({ + name: "Some Landmark", + address: "123 Test St", + latitude: 1, + longitude: 2, + }) + + mockFetch(200, { + data: result, + }) + + fetchLocationSearchResultById("query").then((results) => { + expect(results).toEqual( + locationSearchResultFactory.build({ + name: "Some Landmark", + address: "123 Test St", + latitude: 1, + longitude: 2, + }) + ) + done() + }) + }) +}) + +describe("fetchLocationSearchSuggestions", () => { + test("parses location search suggestions", (done) => { + const result = locationSearchSuggestionDataFactory.build({ + text: "Some Landmark", + place_id: "test-place", + }) + + mockFetch(200, { + data: [result], + }) + + fetchLocationSearchSuggestions("query").then((result) => { + expect(result).toEqual([ + locationSearchSuggestionFactory.build({ + text: "Some Landmark", + placeId: "test-place", + }), + ]) + done() + }) + }) +}) + describe("putUserSetting", () => { test("uses PUT and CSRF token", () => { mockFetch(200, "") diff --git a/assets/tests/factories/locationSearchSuggestion.ts b/assets/tests/factories/locationSearchSuggestion.ts new file mode 100644 index 000000000..fb290b561 --- /dev/null +++ b/assets/tests/factories/locationSearchSuggestion.ts @@ -0,0 +1,10 @@ +import { Factory } from "fishery" +import { LocationSearchSuggestion } from "../../src/models/locationSearchSuggestion" + +const locationSearchSuggestionFactory = + Factory.define(({ sequence }) => ({ + text: "Some Search Term", + placeId: `${sequence}`, + })) + +export default locationSearchSuggestionFactory diff --git a/assets/tests/factories/locationSearchSuggestionData.ts b/assets/tests/factories/locationSearchSuggestionData.ts new file mode 100644 index 000000000..34146db48 --- /dev/null +++ b/assets/tests/factories/locationSearchSuggestionData.ts @@ -0,0 +1,10 @@ +import { Factory } from "fishery" +import { LocationSearchSuggestionData } from "../../src/models/locationSearchSuggestionData" + +const locationSearchSuggestionDataFactory = + Factory.define(({ sequence }) => ({ + text: "Some Search Term", + place_id: `${sequence}`, + })) + +export default locationSearchSuggestionDataFactory diff --git a/assets/tests/hooks/useLocationSearchResultById.test.ts b/assets/tests/hooks/useLocationSearchResultById.test.ts new file mode 100644 index 000000000..dede3d270 --- /dev/null +++ b/assets/tests/hooks/useLocationSearchResultById.test.ts @@ -0,0 +1,47 @@ +import { useLocationSearchResultById } from "../../src/hooks/useLocationSearchResultById" +import { renderHook } from "@testing-library/react" +import * as Api from "../../src/api" +import { instantPromise } from "../testHelpers/mockHelpers" +import locationSearchResultFactory from "../factories/locationSearchResult" + +jest.mock("../../src/api", () => ({ + __esModule: true, + + fetchLocationSearchResultById: jest.fn(() => new Promise(() => {})), +})) + +describe("useLocationSearchResultById", () => { + test("returns null if no search query is given", () => { + const mockFetchLocationSearchResultById: jest.Mock = + Api.fetchLocationSearchResultById as jest.Mock + + const { result } = renderHook(() => useLocationSearchResultById(null)) + + expect(result.current).toBeNull() + expect(mockFetchLocationSearchResultById).not.toHaveBeenCalled() + }) + + test("returns null while loading", () => { + const mockFetchLocationSearchResultById: jest.Mock = + Api.fetchLocationSearchResultById as jest.Mock + + const { result } = renderHook(() => useLocationSearchResultById("place_id")) + + expect(result.current).toBeNull() + expect(mockFetchLocationSearchResultById).toHaveBeenCalled() + }) + + test("returns results", () => { + const results = [locationSearchResultFactory.build()] + const mockFetchLocationSearchResultById: jest.Mock = + Api.fetchLocationSearchResultById as jest.Mock + mockFetchLocationSearchResultById.mockImplementationOnce(() => + instantPromise(results) + ) + + const { result } = renderHook(() => useLocationSearchResultById("place_id")) + + expect(result.current).toEqual(results) + expect(mockFetchLocationSearchResultById).toHaveBeenCalled() + }) +}) diff --git a/assets/tests/hooks/useLocationSearchSuggestions.test.ts b/assets/tests/hooks/useLocationSearchSuggestions.test.ts new file mode 100644 index 000000000..c57f24955 --- /dev/null +++ b/assets/tests/hooks/useLocationSearchSuggestions.test.ts @@ -0,0 +1,51 @@ +import { useLocationSearchSuggestions } from "../../src/hooks/useLocationSearchSuggestions" +import { renderHook } from "@testing-library/react" +import * as Api from "../../src/api" +import { instantPromise } from "../testHelpers/mockHelpers" +import locationSearchSuggestionFactory from "../factories/locationSearchSuggestion" + +jest.mock("../../src/api", () => ({ + __esModule: true, + + fetchLocationSearchSuggestions: jest.fn(() => new Promise(() => {})), +})) + +describe("useLocationSearchSuggestions", () => { + test("returns null if no search query is given", () => { + const mockFetchLocationSearchSuggestions: jest.Mock = + Api.fetchLocationSearchSuggestions as jest.Mock + + const { result } = renderHook(() => useLocationSearchSuggestions(null)) + + expect(result.current).toBeNull() + expect(mockFetchLocationSearchSuggestions).not.toHaveBeenCalled() + }) + + test("returns null while loading", () => { + const mockFetchLocationSearchSuggestions: jest.Mock = + Api.fetchLocationSearchSuggestions as jest.Mock + + const { result } = renderHook(() => + useLocationSearchSuggestions("search string") + ) + + expect(result.current).toBeNull() + expect(mockFetchLocationSearchSuggestions).toHaveBeenCalled() + }) + + test("returns results", () => { + const results = [locationSearchSuggestionFactory.build()] + const mockFetchLocationSearchSuggestions: jest.Mock = + Api.fetchLocationSearchSuggestions as jest.Mock + mockFetchLocationSearchSuggestions.mockImplementationOnce(() => + instantPromise(results) + ) + + const { result } = renderHook(() => + useLocationSearchSuggestions("search string") + ) + + expect(result.current).toEqual(results) + expect(mockFetchLocationSearchSuggestions).toHaveBeenCalled() + }) +}) diff --git a/assets/tests/models/locationSearchSuggestionData.test.ts b/assets/tests/models/locationSearchSuggestionData.test.ts new file mode 100644 index 000000000..be3189582 --- /dev/null +++ b/assets/tests/models/locationSearchSuggestionData.test.ts @@ -0,0 +1,40 @@ +import { + locationSearchSuggestionFromData, + locationSearchSuggestionsFromData, +} from "../../src/models/locationSearchSuggestionData" +import locationSearchSuggestionFactory from "../factories/locationSearchSuggestion" +import locationSearchSuggestionDataFactory from "../factories/locationSearchSuggestionData" + +describe("locationSearchSuggestionFromData", () => { + test("passes supplied data through", () => { + const data = locationSearchSuggestionDataFactory.build({ + text: "Some Landmark", + place_id: "test-id", + }) + + expect(locationSearchSuggestionFromData(data)).toEqual( + locationSearchSuggestionFactory.build({ + text: "Some Landmark", + placeId: "test-id", + }) + ) + }) +}) + +describe("locationSearchSuggestionsFromData", () => { + test("passes supplied data through", () => { + const data = [ + locationSearchSuggestionDataFactory.build({ + text: "Some Landmark", + place_id: "test-id", + }), + ] + + expect(locationSearchSuggestionsFromData(data)).toEqual([ + locationSearchSuggestionFactory.build({ + text: "Some Landmark", + placeId: "test-id", + }), + ]) + }) +}) diff --git a/lib/skate/location_search/aws_location_request.ex b/lib/skate/location_search/aws_location_request.ex index d1f87ea97..eb578c986 100644 --- a/lib/skate/location_search/aws_location_request.ex +++ b/lib/skate/location_search/aws_location_request.ex @@ -1,26 +1,44 @@ defmodule Skate.LocationSearch.AwsLocationRequest do - alias Skate.LocationSearch.SearchResult + alias Skate.LocationSearch.Place + alias Skate.LocationSearch.Suggestion - @spec search(String.t()) :: {:ok, map()} | {:error, term()} + @spec get(Place.id()) :: {:ok, Place.t()} | {:error, term()} + def get(place_id) do + request_fn = Application.get_env(:skate, :aws_request_fn, &ExAws.request/1) + + path = + "/places/v0/indexes/" <> + Application.get_env(:skate, :aws_place_index) <> "/places/" <> place_id + + case request_fn.(%ExAws.Operation.RestQuery{ + http_method: :get, + path: path, + service: :places + }) do + {:ok, response} -> {:ok, parse_get_response(response, place_id)} + {:error, error} -> {:error, error} + end + end + + @spec search(String.t()) :: {:ok, [Place.t()]} | {:error, term()} def search(text) do request_fn = Application.get_env(:skate, :aws_request_fn, &ExAws.request/1) path = "/places/v0/indexes/" <> Application.get_env(:skate, :aws_place_index) <> "/search/text" - case %ExAws.Operation.RestQuery{ + case request_fn.(%ExAws.Operation.RestQuery{ http_method: :post, path: path, body: Map.merge(base_arguments(), %{Text: text}), service: :places - } - |> request_fn.() do + }) do {:ok, response} -> {:ok, parse_search_response(response)} {:error, error} -> {:error, error} end end - @spec suggest(String.t()) :: {:ok, map()} | {:error, term()} + @spec suggest(String.t()) :: {:ok, [Suggestion.t()]} | {:error, term()} def suggest(text) do request_fn = Application.get_env(:skate, :aws_request_fn, &ExAws.request/1) @@ -28,18 +46,34 @@ defmodule Skate.LocationSearch.AwsLocationRequest do "/places/v0/indexes/" <> Application.get_env(:skate, :aws_place_index) <> "/search/suggestions" - case %ExAws.Operation.RestQuery{ + case request_fn.(%ExAws.Operation.RestQuery{ http_method: :post, path: path, body: Map.merge(base_arguments(), %{Text: text}), service: :places - } - |> request_fn.() do + }) do {:ok, response} -> {:ok, parse_suggest_response(response)} {:error, error} -> {:error, error} end end + defp parse_get_response(%{status_code: 200, body: body}, place_id) do + %{"Place" => place} = Jason.decode!(body) + + %{"Label" => label, "Geometry" => %{"Point" => [longitude, latitude]}} = place + + {name, address} = + separate_label_text(label, Map.get(place, "AddressNumber"), Map.get(place, "Street")) + + %Place{ + id: place_id, + name: name, + address: address, + latitude: latitude, + longitude: longitude + } + end + defp parse_search_response(%{status_code: 200, body: body}) do %{"Results" => results} = Jason.decode!(body) @@ -51,7 +85,7 @@ defmodule Skate.LocationSearch.AwsLocationRequest do {name, address} = separate_label_text(label, Map.get(place, "AddressNumber"), Map.get(place, "Street")) - %SearchResult{ + %Place{ id: id, name: name, address: address, @@ -64,7 +98,12 @@ defmodule Skate.LocationSearch.AwsLocationRequest do defp parse_suggest_response(%{status_code: 200, body: body}) do %{"Results" => results} = Jason.decode!(body) - Enum.map(results, fn result -> Map.get(result, "Text") end) + Enum.map(results, fn result -> + text = Map.get(result, "Text") + place_id = Map.get(result, "PlaceId") + + %Suggestion{text: text, place_id: place_id} + end) end @spec separate_label_text(String.t(), String.t() | nil, String.t() | nil) :: diff --git a/lib/skate/location_search/search_result.ex b/lib/skate/location_search/place.ex similarity index 82% rename from lib/skate/location_search/search_result.ex rename to lib/skate/location_search/place.ex index 8e3755349..82e86bb21 100644 --- a/lib/skate/location_search/search_result.ex +++ b/lib/skate/location_search/place.ex @@ -1,6 +1,8 @@ -defmodule Skate.LocationSearch.SearchResult do +defmodule Skate.LocationSearch.Place do + @type id :: String.t() + @type t :: %__MODULE__{ - id: String.t(), + id: id(), name: String.t() | nil, address: String.t(), latitude: float(), diff --git a/lib/skate/location_search/suggestion.ex b/lib/skate/location_search/suggestion.ex new file mode 100644 index 000000000..8c6ecc96a --- /dev/null +++ b/lib/skate/location_search/suggestion.ex @@ -0,0 +1,15 @@ +defmodule Skate.LocationSearch.Suggestion do + @type t :: %__MODULE__{ + text: String.t(), + place_id: String.t() | nil + } + + @enforce_keys [:text, :place_id] + + @derive {Jason.Encoder, only: [:text, :place_id]} + + defstruct [ + :text, + :place_id + ] +end diff --git a/lib/skate_web/controllers/location_search_controller.ex b/lib/skate_web/controllers/location_search_controller.ex index 6bf69e2f7..40f84b95f 100644 --- a/lib/skate_web/controllers/location_search_controller.ex +++ b/lib/skate_web/controllers/location_search_controller.ex @@ -3,11 +3,20 @@ defmodule SkateWeb.LocationSearchController do alias Skate.LocationSearch.AwsLocationRequest + @spec get(Plug.Conn.t(), map()) :: Plug.Conn.t() + def get(conn, %{"id" => id}) do + get_fn = Application.get_env(:skate, :location_get_fn, &AwsLocationRequest.get/1) + + {:ok, result} = get_fn.(id) + + json(conn, %{data: result}) + end + @spec search(Plug.Conn.t(), map()) :: Plug.Conn.t() def search(conn, %{"query" => query}) do - seach_fn = Application.get_env(:skate, :location_search_fn, &AwsLocationRequest.search/1) + search_fn = Application.get_env(:skate, :location_search_fn, &AwsLocationRequest.search/1) - {:ok, result} = seach_fn.(query) + {:ok, result} = search_fn.(query) json(conn, %{data: result}) end diff --git a/lib/skate_web/router.ex b/lib/skate_web/router.ex index 197ad1b6d..a000db2f2 100644 --- a/lib/skate_web/router.ex +++ b/lib/skate_web/router.ex @@ -127,6 +127,7 @@ defmodule SkateWeb.Router do put "/route_tabs", RouteTabsController, :update put "/notification_read_state", NotificationReadStatesController, :update get "/swings", SwingsController, :index + get "/location_search/place/:id", LocationSearchController, :get get "/location_search/search", LocationSearchController, :search get "/location_search/suggest", LocationSearchController, :suggest end diff --git a/test/skate/location_search/aws_location_request_test.exs b/test/skate/location_search/aws_location_request_test.exs index 303aff2fe..8d30fe597 100644 --- a/test/skate/location_search/aws_location_request_test.exs +++ b/test/skate/location_search/aws_location_request_test.exs @@ -5,14 +5,48 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do import Test.Support.Helpers alias Skate.LocationSearch.AwsLocationRequest - alias Skate.LocationSearch.SearchResult + alias Skate.LocationSearch.Place + alias Skate.LocationSearch.Suggestion setup do reassign_env(:skate, :aws_place_index, "test-index") end + describe "get/1" do + test "returns a place when found" do + place = build(:amazon_location_place) + + response = %{ + status_code: 200, + body: Jason.encode!(%{"Place" => place}) + } + + reassign_env(:skate, :aws_request_fn, fn %{ + path: + "/places/v0/indexes/test-index/places/" <> + _place_id + } -> + {:ok, response} + end) + + assert {:ok, %Place{}} = AwsLocationRequest.get("place_id") + end + + test "returns errors" do + reassign_env(:skate, :aws_request_fn, fn %{ + path: + "/places/v0/indexes/test-index/places/" <> + _place_id + } -> + {:error, "error"} + end) + + assert {:error, "error"} = AwsLocationRequest.get("place_id") + end + end + describe "search/1" do - test "transforms result with name into SearchResult structs" do + test "transforms result with name into Place structs" do name = "Some Landmark" address_number = "123" street = "Test St" @@ -41,11 +75,11 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do expected_address = "#{address_number} #{street}, #{address_suffix}" - assert {:ok, [%SearchResult{name: ^name, address: ^expected_address}]} = + assert {:ok, [%Place{name: ^name, address: ^expected_address}]} = AwsLocationRequest.search("search text") end - test "transforms result without name into SearchResult structs" do + test "transforms result without name into Place structs" do address_number = "123" street = "Test St" address_suffix = "MA 02201, United States" @@ -73,11 +107,11 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do expected_address = "#{address_number} #{street}, #{address_suffix}" - assert {:ok, [%SearchResult{name: nil, address: ^expected_address}]} = + assert {:ok, [%Place{name: nil, address: ^expected_address}]} = AwsLocationRequest.search("search text") end - test "transforms result without address prefix information to go on into SearchResult structs" do + test "transforms result without address prefix information to go on into Place structs" do address_suffix = "Some Neighborhood, Boston, MA" response = %{ @@ -101,7 +135,7 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do {:ok, response} end) - assert {:ok, [%SearchResult{name: nil, address: ^address_suffix}]} = + assert {:ok, [%Place{name: nil, address: ^address_suffix}]} = AwsLocationRequest.search("search text") end @@ -117,12 +151,39 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do end describe "suggest/1" do - test "pulls out suggested search text" do + test "pulls out suggested search text and place ID" do + suggestion = build(:amazon_location_suggest_result, %{"Text" => "some place"}) + + response = %{ + status_code: 200, + body: + Jason.encode!(%{ + "Results" => [suggestion] + }) + } + + reassign_env(:skate, :aws_request_fn, fn %{ + path: + "/places/v0/indexes/test-index/search/suggestions" + } -> + {:ok, response} + end) + + place_id = Map.get(suggestion, "PlaceId") + + assert {:ok, [%Suggestion{text: "some place", place_id: ^place_id}]} = + AwsLocationRequest.suggest("text") + end + + test "pulls out suggested search text when no place ID present" do + suggestion = + build(:amazon_location_suggest_result, %{"Text" => "some place", "PlaceId" => nil}) + response = %{ status_code: 200, body: Jason.encode!(%{ - "Results" => [build(:amazon_location_suggest_result, %{"Text" => "some place"})] + "Results" => [suggestion] }) } @@ -133,7 +194,8 @@ defmodule Skate.LocationSearch.AwsLocationRequestTest do {:ok, response} end) - assert {:ok, ["some place"]} = AwsLocationRequest.suggest("text") + assert {:ok, [%Suggestion{text: "some place", place_id: nil}]} = + AwsLocationRequest.suggest("text") end test "returns errors" do diff --git a/test/skate_web/controllers/location_search_controller_test.exs b/test/skate_web/controllers/location_search_controller_test.exs index 049d3e05b..d622eb0c1 100644 --- a/test/skate_web/controllers/location_search_controller_test.exs +++ b/test/skate_web/controllers/location_search_controller_test.exs @@ -2,7 +2,44 @@ defmodule SkateWeb.LocationSearchControllerTest do use SkateWeb.ConnCase import Test.Support.Helpers - alias Skate.LocationSearch.SearchResult + alias Skate.LocationSearch.Place + alias Skate.LocationSearch.Suggestion + + describe "GET /api/location_search/place" do + test "when logged out, redirects you to cognito auth", %{conn: conn} do + conn = + conn + |> api_headers() + |> get("/api/location_search/place/place_id") + + assert redirected_to(conn) == "/auth/cognito" + end + + @tag :authenticated + test "returns data", %{conn: conn} do + place = %Place{ + id: "test_id", + name: "Landmark", + address: "123 Fake St", + latitude: 0, + longitude: 0 + } + + reassign_env(:skate, :location_get_fn, fn _query -> {:ok, place} end) + + conn = + conn + |> api_headers() + |> get("/api/location_search/place/#{place.id}") + + assert json_response(conn, 200) == %{ + "data" => + place + |> Map.from_struct() + |> Map.new(fn {key, value} -> {Atom.to_string(key), value} end) + } + end + end describe "GET /api/location_search/search" do test "when logged out, redirects you to cognito auth", %{conn: conn} do @@ -16,7 +53,7 @@ defmodule SkateWeb.LocationSearchControllerTest do @tag :authenticated test "returns data", %{conn: conn} do - result = %SearchResult{ + result = %Place{ id: "test_id", name: "Landmark", address: "123 Fake St", @@ -53,7 +90,7 @@ defmodule SkateWeb.LocationSearchControllerTest do @tag :authenticated test "returns data", %{conn: conn} do - result = "suggested search" + result = %Suggestion{text: "suggested search", place_id: nil} reassign_env(:skate, :location_suggest_fn, fn _query -> {:ok, [result]} end) @@ -62,7 +99,9 @@ defmodule SkateWeb.LocationSearchControllerTest do |> api_headers() |> get("/api/location_search/suggest?query=test") - assert json_response(conn, 200) == %{"data" => [result]} + assert json_response(conn, 200) == %{ + "data" => [%{"text" => "suggested search", "place_id" => nil}] + } end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 2798afb2a..b2bc56ebc 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -199,22 +199,26 @@ defmodule Skate.Factory do } end - def amazon_location_search_result_factory(attrs) do + def amazon_location_place_factory(attrs) do address_number = Map.get(attrs, :address_number, sequence(:address_number, &to_string/1)) street = Map.get(attrs, :street, "Test St") name = Map.get(attrs, :name, "Landmark") address_suffix = Map.get(attrs, :address_suffix, "MA 02201, United States") - result = %{ - "Place" => %{ - "AddressNumber" => address_number, - "Geometry" => %{ - "Point" => [0, 0] - }, - "Label" => - "#{name && name <> ", "}#{address_number && address_number <> " "}#{street && street <> ", "}#{address_suffix}", - "Street" => street + %{ + "AddressNumber" => address_number, + "Geometry" => %{ + "Point" => [0, 0] }, + "Label" => + "#{name && name <> ", "}#{address_number && address_number <> " "}#{street && street <> ", "}#{address_suffix}", + "Street" => street + } + end + + def amazon_location_search_result_factory(attrs) do + result = %{ + "Place" => fn -> build(:amazon_location_place, attrs) end, "PlaceId" => "test_id_#{sequence(:place_id, &to_string/1)}" }