From f6a807758e67b3e8836c713879fb6aaefbcc3404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joakim=20J=C3=A4derberg?= Date: Mon, 1 Dec 2025 07:49:59 +0000 Subject: [PATCH] Merged in chore/remove-enrichHotel-call (pull request #3244) Chore/remove enrichHotel call * chore: remove enrichHotel api call * include cityId when errors happen * remove unused funciton and cleanup code * tests no longer expect us to have called a function that is not used * remove commented code Approved-by: Linus Flood --- .../destinations/[country]/[city]/route.ts | 1 + .../[country]/createDataResponse.ts | 2 +- .../api/web/destinations/[country]/route.ts | 6 +- .../trpc/lib/routers/autocomplete/schema.ts | 2 +- .../util/filterAutoCompleteLocations.ts | 2 +- packages/trpc/lib/routers/hotels/output.ts | 62 ---------- .../hotels/services/getHotelIdsByCityId.ts | 8 +- .../services/getLocationsByCountries.test.ts | 62 ++-------- .../services/getLocationsByCountries.ts | 106 +++++++++--------- packages/trpc/lib/types/locations.ts | 2 +- 10 files changed, 77 insertions(+), 176 deletions(-) diff --git a/apps/scandic-web/app/api/web/destinations/[country]/[city]/route.ts b/apps/scandic-web/app/api/web/destinations/[country]/[city]/route.ts index f8fd873b2..2e01cd98e 100644 --- a/apps/scandic-web/app/api/web/destinations/[country]/[city]/route.ts +++ b/apps/scandic-web/app/api/web/destinations/[country]/[city]/route.ts @@ -66,6 +66,7 @@ export async function GET( (x) => x.isActive && x.isPublished && + x.relationships.city.name && equalsIgnoreCaseAndAccents(x.relationships.city.name, cityParam) ) diff --git a/apps/scandic-web/app/api/web/destinations/[country]/createDataResponse.ts b/apps/scandic-web/app/api/web/destinations/[country]/createDataResponse.ts index bcf6784a5..b978e944f 100644 --- a/apps/scandic-web/app/api/web/destinations/[country]/createDataResponse.ts +++ b/apps/scandic-web/app/api/web/destinations/[country]/createDataResponse.ts @@ -9,7 +9,7 @@ export function createDataResponse( hotels: Array<{ name: string distanceToCentre?: number | undefined - relationships: { city: { name: string } } + relationships: { city: { name: string | undefined } } images?: { large?: string } | undefined }> }, diff --git a/apps/scandic-web/app/api/web/destinations/[country]/route.ts b/apps/scandic-web/app/api/web/destinations/[country]/route.ts index 66e750236..8a0d49400 100644 --- a/apps/scandic-web/app/api/web/destinations/[country]/route.ts +++ b/apps/scandic-web/app/api/web/destinations/[country]/route.ts @@ -63,8 +63,10 @@ export async function GET( (x) => x.isActive && x.isPublished && - cities.some((c) => - equalsIgnoreCaseAndAccents(x.relationships.city.name, c.name) + cities.some( + (c) => + x.relationships.city.name && + equalsIgnoreCaseAndAccents(x.relationships.city.name, c.name) ) ) diff --git a/packages/trpc/lib/routers/autocomplete/schema.ts b/packages/trpc/lib/routers/autocomplete/schema.ts index b32a62927..7f58887af 100644 --- a/packages/trpc/lib/routers/autocomplete/schema.ts +++ b/packages/trpc/lib/routers/autocomplete/schema.ts @@ -5,7 +5,7 @@ export const autoCompleteLocationSchema = z.object({ name: z.string(), type: z.enum(["cities", "hotels", "countries"]), searchTokens: z.array(z.string()), - destination: z.string(), + destination: z.string().optional(), url: z.string().optional(), cityIdentifier: z.string().optional(), }) diff --git a/packages/trpc/lib/routers/autocomplete/util/filterAutoCompleteLocations.ts b/packages/trpc/lib/routers/autocomplete/util/filterAutoCompleteLocations.ts index 7f5714594..7d07f3258 100644 --- a/packages/trpc/lib/routers/autocomplete/util/filterAutoCompleteLocations.ts +++ b/packages/trpc/lib/routers/autocomplete/util/filterAutoCompleteLocations.ts @@ -38,7 +38,7 @@ export function filterAutoCompleteLocations( const searchable = locations.map((x) => ({ ...x, nameTokens: extractTokens(x.name), - destinationTokens: extractTokens(x.destination), + destinationTokens: extractTokens(x.destination ?? ""), })) fuseConfig.setCollection(searchable) diff --git a/packages/trpc/lib/routers/hotels/output.ts b/packages/trpc/lib/routers/hotels/output.ts index ca9c334a4..ad20e4ed5 100644 --- a/packages/trpc/lib/routers/hotels/output.ts +++ b/packages/trpc/lib/routers/hotels/output.ts @@ -27,8 +27,6 @@ import { addressSchema } from "./schemas/hotel/address" import { detailedFacilitiesSchema } from "./schemas/hotel/detailedFacility" import { locationSchema } from "./schemas/hotel/location" import { imageSchema } from "./schemas/image" -import { locationCitySchema } from "./schemas/location/city" -import { locationHotelSchema } from "./schemas/location/hotel" import { relationshipsSchema } from "./schemas/relationships" import { roomConfigurationSchema } from "./schemas/roomAvailability/configuration" import { rateDefinitionSchema } from "./schemas/roomAvailability/rateDefinition" @@ -429,66 +427,6 @@ export const citiesSchema = z return null }) -export const locationsSchema = z.object({ - data: z - .array( - z - .discriminatedUnion("type", [locationCitySchema, locationHotelSchema]) - .transform((location) => { - if (location.type === "cities") { - return { - ...location.attributes, - country: location.attributes.countryName || "", - id: location.id, - type: location.type, - } - } - return { - ...location.attributes, - id: location.id, - relationships: { - city: { - cityIdentifier: "", - ianaTimeZoneId: "", - id: "", - isPublished: false, - keywords: [], - name: "", - timeZoneId: "", - type: "cities", - url: location?.relationships?.city?.links?.related ?? "", - }, - }, - type: location.type, - operaId: location.attributes.operaId ?? "", - } - }) - ) - .transform((data) => - data - .filter((node) => !!node && node.isPublished) - .filter((node) => { - if (node.type === "hotels") { - if (!node.operaId) { - return false - } - } else { - if (!node.cityIdentifier) { - return false - } - } - return true - }) - .sort((a, b) => { - if (a.type === b.type) { - return a.name.localeCompare(b.name) - } else { - return a.type === "cities" ? -1 : 1 - } - }) - ), -}) - export type BreakfastPackages = z.output export const breakfastPackagesSchema = z .object({ diff --git a/packages/trpc/lib/routers/hotels/services/getHotelIdsByCityId.ts b/packages/trpc/lib/routers/hotels/services/getHotelIdsByCityId.ts index 6e3947e39..c43a658c8 100644 --- a/packages/trpc/lib/routers/hotels/services/getHotelIdsByCityId.ts +++ b/packages/trpc/lib/routers/hotels/services/getHotelIdsByCityId.ts @@ -42,14 +42,18 @@ export async function getHotelIdsByCityId({ if (!apiResponse.ok) { await metricsGetHotelIdsByCityId.httpError(apiResponse) - throw new Error("Unable to fetch hotelIds by cityId") + throw new Error(`Unable to fetch hotelIds by cityId`, { + cause: { cityId }, + }) } const apiJson = await apiResponse.json() const validatedHotelIds = getHotelIdsSchema.safeParse(apiJson) if (!validatedHotelIds.success) { metricsGetHotelIdsByCityId.validationError(validatedHotelIds.error) - throw new Error("Unable to parse data for hotelIds by cityId") + throw new Error(`Unable to parse data for hotelIds by cityId`, { + cause: { cityId, errors: validatedHotelIds.error }, + }) } return validatedHotelIds.data diff --git a/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.test.ts b/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.test.ts index b0b8e7461..f75fcc7dd 100644 --- a/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.test.ts +++ b/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.test.ts @@ -4,7 +4,7 @@ import { Lang } from "@scandic-hotels/common/constants/language" import { getCacheClient } from "@scandic-hotels/common/dataCache" import * as api from "../../../api" -import { getCity } from "./getCity" +// import { getCity } from "./getCity" import { getLocationsByCountries } from "./getLocationsByCountries" import type { CitiesGroupedByCountry } from "../../../types/locations" @@ -35,15 +35,16 @@ vi.mock("./getCity", () => { vi.mock("@scandic-hotels/common/logger/createLogger", () => { return { createLogger: () => ({ - error: vi.fn(), info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), }), } }) const mockedGetCacheClient = getCacheClient as unknown as Mock const mockedApiGet = api.get as unknown as Mock -const mockedGetCity = getCity as unknown as Mock +// const mockedGetCity = getCity as unknown as Mock describe("getLocationsByCountries", () => { const mockedCacheClient = { @@ -125,20 +126,6 @@ describe("getLocationsByCountries", () => { json: async () => apiPayload, }) - // getCity returns enriched city object for hotel relationship - const mockedCity: Awaited> = { - cityIdentifier: "remote-ci-1", - ianaTimeZoneId: "Europe/Stockholm", - id: "remote-city-id", - isPublished: true, - keywords: [], - name: "RemoteCity", - timeZoneId: "Europe/Stockholm", - type: "cities", - } - - mockedGetCity.mockResolvedValueOnce(mockedCity) - const citiesByCountry = { CountryX: [{ name: "CityAA" }], } as unknown as CitiesGroupedByCountry @@ -164,16 +151,13 @@ describe("getLocationsByCountries", () => { expect(cityNode!.country).toBe("CountryX") // country assigned based on citiesByCountry expect(hotelNode).toBeDefined() - expect(mockedGetCity).toHaveBeenCalledWith({ - cityUrl: "https://api/cities/city1", - serviceToken: "token", - }) + // hotel relationships.city should be the object returned by getCity (merged) expect(hotelNode?.relationships).toBeDefined() expect(hotelNode?.relationships.city).toEqual( expect.objectContaining({ - id: mockedCity.id, - name: mockedCity.name, + id: "city1", + name: "City1", }) ) }) @@ -192,19 +176,6 @@ describe("getLocationsByCountries", () => { json: async () => apiPayload, }) - // getCity returns enriched city object for hotel relationship - const mockedCity: Awaited> = { - cityIdentifier: "remote-ci-1", - ianaTimeZoneId: "Europe/Stockholm", - id: "remote-city-id", - isPublished: true, - keywords: [], - name: "RemoteCity", - timeZoneId: "Europe/Stockholm", - type: "cities", - } - mockedGetCity.mockResolvedValue(mockedCity) - const citiesByCountry = { CountryX: [{ name: "CityAA" }], } as unknown as CitiesGroupedByCountry @@ -230,17 +201,13 @@ describe("getLocationsByCountries", () => { .filter((n) => n.type === "hotels") .find((n) => n.name === "Hotel1") expect(hotel1).toBeDefined() - expect(mockedGetCity).toHaveBeenCalledWith({ - cityUrl: "https://api/cities/city1", - serviceToken: "token", - }) // hotel relationships.city should be the object returned by getCity (merged) expect(hotel1?.relationships).toBeDefined() expect(hotel1?.relationships.city).toEqual( expect.objectContaining({ - id: mockedCity.id, - name: mockedCity.name, + id: "city1", + name: "City1", }) ) @@ -248,20 +215,15 @@ describe("getLocationsByCountries", () => { .filter((n) => n.type === "hotels") .find((n) => n.name === "Hotel2") expect(hotel2).toBeDefined() - expect(mockedGetCity).toHaveBeenCalledWith({ - cityUrl: "https://api/cities/city2", - serviceToken: "token", - }) + // hotel relationships.city should be the object returned by getCity (merged) expect(hotel2?.relationships).toBeDefined() expect(hotel2?.relationships.city).toEqual( expect.objectContaining({ - id: mockedCity.id, - name: mockedCity.name, + id: "city2", + name: "City2", }) ) - - expect(mockedGetCity).toHaveBeenCalledTimes(2) }) it("filters out unpublished cities", async () => { diff --git a/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.ts b/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.ts index 07da73f93..4bdd38a7e 100644 --- a/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.ts +++ b/packages/trpc/lib/routers/hotels/services/getLocationsByCountries.ts @@ -1,16 +1,13 @@ -import deepmerge from "deepmerge" import { z } from "zod" import { getCacheClient } from "@scandic-hotels/common/dataCache" import { createLogger } from "@scandic-hotels/common/logger/createLogger" -import { chunk } from "@scandic-hotels/common/utils/chunk" import * as api from "../../../api" import { serverErrorByStatus } from "../../../errors" import { toApiLang } from "../../../utils" import { locationCitySchema } from "../schemas/location/city" import { locationHotelSchema } from "../schemas/location/hotel" -import { getCity } from "./getCity" import type { Country } from "@scandic-hotels/common/constants/country" import type { Lang } from "@scandic-hotels/common/constants/language" @@ -21,6 +18,14 @@ type CitiesNamesByCountry = Record< Country | (string & {}), Array<{ name: string }> > | null +type Hotel = Extract< + z.infer["data"][number], + { type: "hotels" } +> +type City = Extract< + z.infer["data"][number], + { type: "cities" } +> export async function getLocationsByCountries({ lang, @@ -73,21 +78,11 @@ export async function getLocationsByCountries({ const data = cleanData(verifiedLocations.data.data) const cities = data .filter((x) => x.type === "cities") - .map((x) => enrichCity(x, citiesByCountry)) + .map((city) => addCountryDataToCity(city, citiesByCountry)) - const chunkedHotels = chunk( - data.filter((x) => x.type === "hotels"), - 10 - ) - const hotels = ( - await Promise.all( - chunkedHotels.flatMap(async (chunk) => { - return await Promise.all( - chunk.flatMap(async (hotel) => enrichHotel(hotel, serviceToken)) - ) - }) - ) - ).flat() + const hotels = data + .filter((x) => x.type === "hotels") + .map((hotel) => addCityDataToHotel(hotel, cities)) let locations: z.infer["data"] = [ ...cities, @@ -100,48 +95,32 @@ export async function getLocationsByCountries({ ) } -async function enrichHotel( - hotel: Extract< - z.infer["data"][number], - { type: "hotels" } - >, - serviceToken: string -): Promise< - Extract["data"][number], { type: "hotels" }> -> { - if (hotel.type !== "hotels") { - return hotel - } - - if (!hotel.relationships.city?.url) { - return hotel - } - const city = await getCity({ - cityUrl: hotel.relationships.city.url, - serviceToken, - }) - +function addCityDataToHotel(hotel: Hotel, cities: City[]) { + const city = cities.find((c) => c.id === hotel.relationships.city.id) if (!city) { + hotelUtilsLogger.warn( + `City with id ${hotel.relationships.city.id} not found for hotel ${hotel.id}` + ) return hotel } - return deepmerge(hotel, { + return { + ...hotel, relationships: { - city, + city: { + ...city, + cityIdentifier: city.cityIdentifier, + url: hotel.relationships.city.url, + keywords: city.keyWords ?? [], + }, }, - }) + } satisfies Hotel } -function enrichCity( - city: Extract< - z.infer["data"][number], - { type: "cities" } - >, +function addCountryDataToCity( + city: City, citiesByCountry: CitiesNamesByCountry | null -): Extract< - z.infer["data"][number], - { type: "cities" } -> { +): City { if (!citiesByCountry) { return city } @@ -206,13 +185,13 @@ export const locationsSchema = z.object({ id: location.id, relationships: { city: { - cityIdentifier: "", - ianaTimeZoneId: "", - id: "", + cityIdentifier: undefined as string | undefined, + id: extractCityId( + location.relationships?.city?.links?.related ?? "" + ), isPublished: false, - keywords: [], - name: "", - timeZoneId: "", + keywords: [] as string[], + name: undefined as string | undefined, type: "cities", url: location?.relationships?.city?.links?.related ?? "", }, @@ -223,3 +202,18 @@ export const locationsSchema = z.object({ }) ), }) + +function extractCityId(cityUrl: string): string | null { + try { + const url = new URL(cityUrl) + if (!url.pathname.toLowerCase().includes("/cities/")) { + return null + } + + const id = url.pathname.split("/").at(-1) + + return id ?? null + } catch { + return null + } +} diff --git a/packages/trpc/lib/types/locations.ts b/packages/trpc/lib/types/locations.ts index cff831edd..79ad79fad 100644 --- a/packages/trpc/lib/types/locations.ts +++ b/packages/trpc/lib/types/locations.ts @@ -4,8 +4,8 @@ import type { z } from "zod" import type { citiesByCountrySchema, countriesSchema, - locationsSchema, } from "../routers/hotels/output" +import type { locationsSchema } from "../routers/hotels/services/getLocationsByCountries" export interface LocationSchema extends z.output {}