From 8293bf0152e02d50356c514eb5f153cfbd4ce750 Mon Sep 17 00:00:00 2001 From: danielhep Date: Sat, 5 Jul 2025 11:51:59 +0800 Subject: [PATCH 1/8] fix: Improve types to remove unsafe casts --- packages/core-utils/src/itinerary.ts | 45 ++++++++++++------- .../trip-details/src/TripDetails.story.tsx | 4 +- .../src/components/fares-v2-table.tsx | 21 +++++---- packages/types/src/index.ts | 12 +++++ 4 files changed, 52 insertions(+), 30 deletions(-) diff --git a/packages/core-utils/src/itinerary.ts b/packages/core-utils/src/itinerary.ts index 414faa6b1..58adf674f 100644 --- a/packages/core-utils/src/itinerary.ts +++ b/packages/core-utils/src/itinerary.ts @@ -1,10 +1,11 @@ import polyline from "@mapbox/polyline"; import { + AppliedFareProduct, Company, Config, + Currency, ElevationProfile, ElevationProfileComponent, - FareProduct, FlexBookingInfo, ItineraryOnlyLegsRequired, LatLngArray, @@ -617,6 +618,11 @@ export const descope = (item: string): string => item?.split(":")?.[1]; export type ExtendedMoney = Money & { originalAmount?: number }; +export const zeroDollars = (currency: Currency): Money => ({ + amount: 0, + currency +}); + /** * Extracts useful data from the fare products on a leg, such as the leg cost and transfer info. * @param leg Leg with fare products (must have used getLegsWithFares) @@ -630,13 +636,12 @@ export function getLegCost( riderCategoryId: string | null, seenFareIds?: string[] ): { - price?: ExtendedMoney; + appliedFareProduct?: AppliedFareProduct; productUseId?: string; - alternateFareProducts?: FareProduct[]; + alternateFareProducts?: AppliedFareProduct[]; + price?: Money; isDependent?: boolean; } { - if (!leg.fareProducts) return { price: undefined }; - const relevantFareProducts = leg.fareProducts .filter(({ product }) => { // riderCategory and medium can be specifically defined as null to handle @@ -656,16 +661,17 @@ export function getLegCost( ); }) .map(fare => { - const clonedFare = structuredClone(fare); - // If we've seen the fare ID already, it's a free transfer - if (seenFareIds?.indexOf(fare.id) > -1) { - (clonedFare.product.price as ExtendedMoney).originalAmount = - fare.product.price.amount; - clonedFare.product.price.amount = 0; - } - return clonedFare; + const alreadySeen = seenFareIds?.indexOf(fare.id) > -1; + const currency = fare.product.price.currency; + return { + id: fare.id, + product: { + ...fare.product, + legPrice: alreadySeen ? zeroDollars(currency) : fare.product.price + } as AppliedFareProduct + }; }) - .sort((a, b) => a.product?.price?.amount - b.product?.price?.amount); + .sort((a, b) => a.product?.legPrice?.amount - b.product?.legPrice?.amount); // Return the cheapest, but include other matches as well const cheapestRelevantFareProduct = relevantFareProducts[0]; @@ -673,7 +679,8 @@ export function getLegCost( // TODO: return one object here instead of dumbing it down? return { alternateFareProducts: relevantFareProducts.splice(1).map(fp => fp.product), - price: cheapestRelevantFareProduct?.product?.price, + price: cheapestRelevantFareProduct?.product.legPrice, + appliedFareProduct: relevantFareProducts[0]?.product, productUseId: cheapestRelevantFareProduct?.id, isDependent: // eslint-disable-next-line no-underscore-dangle @@ -686,6 +693,7 @@ export function getLegCost( * @param legs Itinerary legs with fare products (must have used getLegsWithFares) * @param category Rider category (youth, regular, senior) * @param container Fare container (cash, electronic) + * @param seenFareIds List of fare product IDs that have already been seen on prev legs. * @returns Money object for the total itinerary cost. */ export function getItineraryCost( @@ -698,13 +706,16 @@ export function getItineraryCost( .filter(leg => leg.fareProducts?.length > 0) // Get the leg cost object of each leg .map(leg => getLegCost(leg, mediumId, riderCategoryId)) - .filter(cost => cost.price !== undefined) + .filter(cost => cost.appliedFareProduct?.legPrice !== undefined) // Filter out duplicate use IDs // One fare product can be used on multiple legs, // and we don't want to count it more than once. .reduce<{ productUseId: string; price: Money }[]>((prev, cur) => { if (!prev.some(p => p.productUseId === cur.productUseId)) { - prev.push({ productUseId: cur.productUseId, price: cur.price }); + prev.push({ + productUseId: cur.productUseId, + price: cur.appliedFareProduct?.legPrice + }); } return prev; }, []) diff --git a/packages/trip-details/src/TripDetails.story.tsx b/packages/trip-details/src/TripDetails.story.tsx index 7b5b32290..4f17b195c 100644 --- a/packages/trip-details/src/TripDetails.story.tsx +++ b/packages/trip-details/src/TripDetails.story.tsx @@ -22,7 +22,7 @@ const walkTransitWalkTransitWalkItinerary = require("@opentripplanner/itinerary- const flexItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/otp2.4-transit-itinerary.json"); const otp2ScooterItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/otp2-scooter.json"); const otp2FareProducts = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/leg-fare-products.json"); -const multipleFareOptionsItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-multiple-options.json"); +// const multipleFareOptionsItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-multiple-options.json"); const faresv2ItineraryWithTransfer = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-with-transfer.json"); const faresv2Itinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-fare-components.json"); @@ -201,7 +201,7 @@ export const OTP2FlexItinerary = makeStory({ itinerary: flexItinerary }); export const FaresV2TableStory = (): ReactElement => { return ( <> - + {/* */} diff --git a/packages/trip-details/src/components/fares-v2-table.tsx b/packages/trip-details/src/components/fares-v2-table.tsx index e8333e351..adc88bbac 100644 --- a/packages/trip-details/src/components/fares-v2-table.tsx +++ b/packages/trip-details/src/components/fares-v2-table.tsx @@ -6,7 +6,6 @@ import { FormattedMessage, useIntl } from "react-intl"; import styled from "styled-components"; import { Transfer } from "@styled-icons/boxicons-regular/Transfer"; -import { ExtendedMoney } from "@opentripplanner/core-utils/lib/itinerary"; import { renderFare } from "../utils"; import { InvisibleA11yLabel } from "../styled"; @@ -144,7 +143,7 @@ const FaresV2Table = ({ const { alternateFareProducts, isDependent, - price, + appliedFareProduct, productUseId } = getLegCost( leg, @@ -152,6 +151,8 @@ const FaresV2Table = ({ descope(rider.id) || null, Array.from(productUseIds) ); + const legPrice = appliedFareProduct?.legPrice; + productUseIds.add(productUseId); // Only consider alternateFareProducts if current product is dependent @@ -160,11 +161,9 @@ const FaresV2Table = ({ // Calculate pre-tranfer amount either via alternate fare or fare-id matching (price.originalAmount) const originalAmount = - price?.originalAmount || - (dependentAlternateFareProducts?.[0]?.price?.amount || - (dependentAlternateFareProducts?.[0]?.price as ExtendedMoney) - ?.originalAmount) - price?.amount || - null; + dependentAlternateFareProducts?.[0]?.price.amount - + legPrice?.amount || + appliedFareProduct?.price.amount - legPrice?.amount; const newCell = ( <> @@ -182,7 +181,7 @@ const FaresV2Table = ({ )} 0 && @@ -195,7 +194,7 @@ const FaresV2Table = ({ }, { transferAmount: intl.formatNumber(originalAmount, { - currency: price?.currency?.code, + currency: legPrice?.currency?.code, currencyDisplay: "narrowSymbol", style: "currency" }) @@ -206,8 +205,8 @@ const FaresV2Table = ({ {!Number.isNaN(originalAmount) && originalAmount > 0 && index > 0 && } - {price - ? renderFare(price?.currency?.code, price?.amount) + {legPrice + ? renderFare(legPrice?.currency?.code, legPrice?.amount) : FailDash} diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 0eb6c26c8..87da99194 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -400,6 +400,11 @@ type TemporaryTNCPriceType = { amount: number; }; +export type Currency = { + code: string; + digits: number; +}; + /** * Describes the cost of an itinerary leg. */ @@ -817,6 +822,13 @@ export type FareProduct = { }; }; +/** + * This fare product is designed to represent the fare product applied to a leg. + */ +export type AppliedFareProduct = FareProduct & { + legPrice: Money; +}; + export type FareProductSelector = { mediumId?: string; riderCategoryId?: string; From 554c7c0a5ac522856f9fc7abc6fb7fedfe1d5576 Mon Sep 17 00:00:00 2001 From: danielhep Date: Thu, 10 Jul 2025 16:35:42 +0530 Subject: [PATCH 2/8] fix: sort properties --- packages/core-utils/src/itinerary.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/core-utils/src/itinerary.ts b/packages/core-utils/src/itinerary.ts index 58adf674f..ee2f7a791 100644 --- a/packages/core-utils/src/itinerary.ts +++ b/packages/core-utils/src/itinerary.ts @@ -636,11 +636,11 @@ export function getLegCost( riderCategoryId: string | null, seenFareIds?: string[] ): { - appliedFareProduct?: AppliedFareProduct; - productUseId?: string; alternateFareProducts?: AppliedFareProduct[]; - price?: Money; + appliedFareProduct?: AppliedFareProduct; isDependent?: boolean; + price?: Money; + productUseId?: string; } { const relevantFareProducts = leg.fareProducts .filter(({ product }) => { @@ -679,12 +679,13 @@ export function getLegCost( // TODO: return one object here instead of dumbing it down? return { alternateFareProducts: relevantFareProducts.splice(1).map(fp => fp.product), - price: cheapestRelevantFareProduct?.product.legPrice, appliedFareProduct: relevantFareProducts[0]?.product, - productUseId: cheapestRelevantFareProduct?.id, isDependent: // eslint-disable-next-line no-underscore-dangle - cheapestRelevantFareProduct?.product.__typename === "DependentFareProduct" + cheapestRelevantFareProduct?.product.__typename === + "DependentFareProduct", + price: cheapestRelevantFareProduct?.product.legPrice, + productUseId: cheapestRelevantFareProduct?.id }; } From 9a5badb05c011fd011c1efeaea1c86233b2521d5 Mon Sep 17 00:00:00 2001 From: danielhep Date: Thu, 10 Jul 2025 16:36:06 +0530 Subject: [PATCH 3/8] reinstate null check --- packages/core-utils/src/itinerary.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core-utils/src/itinerary.ts b/packages/core-utils/src/itinerary.ts index ee2f7a791..2e666561f 100644 --- a/packages/core-utils/src/itinerary.ts +++ b/packages/core-utils/src/itinerary.ts @@ -642,6 +642,7 @@ export function getLegCost( price?: Money; productUseId?: string; } { + if (!leg.fareProducts) return { price: undefined }; const relevantFareProducts = leg.fareProducts .filter(({ product }) => { // riderCategory and medium can be specifically defined as null to handle From 17c1e7991b00dc1568441d0932b3a3219bb4ee63 Mon Sep 17 00:00:00 2001 From: danielhep Date: Thu, 10 Jul 2025 16:42:44 +0530 Subject: [PATCH 4/8] reinstate valid story --- packages/trip-details/src/TripDetails.story.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/trip-details/src/TripDetails.story.tsx b/packages/trip-details/src/TripDetails.story.tsx index 4f17b195c..7b5b32290 100644 --- a/packages/trip-details/src/TripDetails.story.tsx +++ b/packages/trip-details/src/TripDetails.story.tsx @@ -22,7 +22,7 @@ const walkTransitWalkTransitWalkItinerary = require("@opentripplanner/itinerary- const flexItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/otp2.4-transit-itinerary.json"); const otp2ScooterItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/otp2-scooter.json"); const otp2FareProducts = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/leg-fare-products.json"); -// const multipleFareOptionsItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-multiple-options.json"); +const multipleFareOptionsItinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-multiple-options.json"); const faresv2ItineraryWithTransfer = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-with-transfer.json"); const faresv2Itinerary = require("@opentripplanner/itinerary-body/src/__mocks__/itineraries/fares-v2-fare-components.json"); @@ -201,7 +201,7 @@ export const OTP2FlexItinerary = makeStory({ itinerary: flexItinerary }); export const FaresV2TableStory = (): ReactElement => { return ( <> - {/* */} + From 0fbb861ec690c563159f5a395562083e799d21b2 Mon Sep 17 00:00:00 2001 From: danielhep Date: Fri, 11 Jul 2025 17:58:29 +0530 Subject: [PATCH 5/8] make sure the fare product has a price at all --- packages/core-utils/src/itinerary.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/core-utils/src/itinerary.ts b/packages/core-utils/src/itinerary.ts index 2e666561f..bfa07e12d 100644 --- a/packages/core-utils/src/itinerary.ts +++ b/packages/core-utils/src/itinerary.ts @@ -658,7 +658,10 @@ export function getLegCost( descope(product?.medium?.id) || product?.medium?.id || null; return ( productRiderCategoryId === riderCategoryId && - productMediaId === mediumId + productMediaId === mediumId && + // Make sure there's a price + // Some fare products don't have a price at all. + product?.price ); }) .map(fare => { From b9abbe5da18a724550b60903384003c17b07ecd5 Mon Sep 17 00:00:00 2001 From: danielhep Date: Fri, 11 Jul 2025 17:59:35 +0530 Subject: [PATCH 6/8] feat: make price optional on FareProduct --- packages/types/src/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 87da99194..4dafbe45c 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -815,7 +815,8 @@ export type FareProduct = { name: string; }; name: string; - price: Money; + // Fare products may not have a price if they don't implement a FareProduct subclass. + price?: Money; riderCategory?: { id: string; name: string; From 3bd7ce68f23689abe7a1cd17835b2d88291feeee Mon Sep 17 00:00:00 2001 From: danielhep Date: Fri, 11 Jul 2025 18:13:49 +0530 Subject: [PATCH 7/8] test: update snapshots --- .../__snapshots__/TripDetails.story.tsx.snap | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/trip-details/src/__snapshots__/TripDetails.story.tsx.snap b/packages/trip-details/src/__snapshots__/TripDetails.story.tsx.snap index 48dd60bc9..7de7051ad 100644 --- a/packages/trip-details/src/__snapshots__/TripDetails.story.tsx.snap +++ b/packages/trip-details/src/__snapshots__/TripDetails.story.tsx.snap @@ -543,11 +543,8 @@ exports[`TripDetails FaresV2TableStory smoke-test 1`] = ` $2.75 - - - - - No fare information for this leg - + + $2.75 - @@ -560,7 +557,7 @@ exports[`TripDetails FaresV2TableStory smoke-test 1`] = ` - $5.50 + $8.25 @@ -621,11 +618,8 @@ exports[`TripDetails FaresV2TableStory smoke-test 1`] = ` $2.75 - - - - - No fare information for this leg - + + $2.75 - @@ -638,7 +632,7 @@ exports[`TripDetails FaresV2TableStory smoke-test 1`] = ` - $5.50 + $8.25 From c7a3f2fd184724e7c4309395d36a124586ea2b6b Mon Sep 17 00:00:00 2001 From: danielhep Date: Wed, 16 Jul 2025 12:06:14 +0530 Subject: [PATCH 8/8] PR cleanup --- packages/core-utils/src/itinerary.ts | 22 +++++++++---------- .../src/components/fares-v2-table.tsx | 1 + 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/packages/core-utils/src/itinerary.ts b/packages/core-utils/src/itinerary.ts index bfa07e12d..ed60c3ca5 100644 --- a/packages/core-utils/src/itinerary.ts +++ b/packages/core-utils/src/itinerary.ts @@ -666,7 +666,7 @@ export function getLegCost( }) .map(fare => { const alreadySeen = seenFareIds?.indexOf(fare.id) > -1; - const currency = fare.product.price.currency; + const { currency } = fare.product.price; return { id: fare.id, product: { @@ -683,7 +683,7 @@ export function getLegCost( // TODO: return one object here instead of dumbing it down? return { alternateFareProducts: relevantFareProducts.splice(1).map(fp => fp.product), - appliedFareProduct: relevantFareProducts[0]?.product, + appliedFareProduct: cheapestRelevantFareProduct?.product, isDependent: // eslint-disable-next-line no-underscore-dangle cheapestRelevantFareProduct?.product.__typename === @@ -706,7 +706,7 @@ export function getItineraryCost( mediumId: string | null, riderCategoryId: string | null ): Money | undefined { - const legCosts = legs + const legCostsObj = legs // Only legs with fares (no walking legs) .filter(leg => leg.fareProducts?.length > 0) // Get the leg cost object of each leg @@ -715,16 +715,14 @@ export function getItineraryCost( // Filter out duplicate use IDs // One fare product can be used on multiple legs, // and we don't want to count it more than once. - .reduce<{ productUseId: string; price: Money }[]>((prev, cur) => { - if (!prev.some(p => p.productUseId === cur.productUseId)) { - prev.push({ - productUseId: cur.productUseId, - price: cur.appliedFareProduct?.legPrice - }); + // Use an object keyed by productUseId to deduplicate, then extract prices + .reduce<{ [productUseId: string]: Money }>((acc, cur) => { + if (cur.productUseId && acc[cur.productUseId] === undefined) { + acc[cur.productUseId] = cur.appliedFareProduct?.legPrice; } - return prev; - }, []) - .map(productUse => productUse.price); + return acc; + }, {}); + const legCosts = Object.values(legCostsObj); if (legCosts.length === 0) return undefined; // Calculate the total diff --git a/packages/trip-details/src/components/fares-v2-table.tsx b/packages/trip-details/src/components/fares-v2-table.tsx index adc88bbac..0bca3c04a 100644 --- a/packages/trip-details/src/components/fares-v2-table.tsx +++ b/packages/trip-details/src/components/fares-v2-table.tsx @@ -205,6 +205,7 @@ const FaresV2Table = ({ {!Number.isNaN(originalAmount) && originalAmount > 0 && index > 0 && } + {/* Leg Price will always be defined if the fare product has a price */} {legPrice ? renderFare(legPrice?.currency?.code, legPrice?.amount) : FailDash}