Repository navigation
RU-T58 Fixing store and api issues #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| jest.mock('../../common/client', () => ({ | ||
| createApiEndpoint: jest.fn(() => ({ | ||
| get: jest.fn(), | ||
| post: jest.fn(), | ||
| put: jest.fn(), | ||
| delete: jest.fn(), | ||
| })), | ||
| })); | ||
|
|
||
| import { createApiEndpoint } from '../../common/client'; | ||
| import { toggleCallTimers } from '../check-in-timers'; | ||
|
|
||
| const mockCreateApiEndpoint = createApiEndpoint as jest.MockedFunction<typeof createApiEndpoint>; | ||
|
|
||
| describe('toggleCallTimers', () => { | ||
| it('sends callId and enabled in the query string with an empty body', async () => { | ||
| const endpoint = { get: jest.fn(), post: jest.fn(), put: jest.fn().mockResolvedValue({ data: { Id: '' } }), delete: jest.fn() }; | ||
| mockCreateApiEndpoint.mockReturnValueOnce(endpoint as unknown as ReturnType<typeof createApiEndpoint>); | ||
|
|
||
| await toggleCallTimers(101, true); | ||
|
|
||
| expect(mockCreateApiEndpoint).toHaveBeenLastCalledWith('/CheckInTimers/ToggleCallTimers?callId=101&enabled=true'); | ||
| expect(endpoint.put).toHaveBeenCalledWith({}); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| // Id-scoped endpoints are created per call, so responses are looked up by path when each endpoint is made. | ||
| jest.mock('../../common/client', () => { | ||
| const endpoints: Record<string, { get: jest.Mock }> = {}; | ||
| const responses: Record<string, unknown> = {}; | ||
| return { | ||
| __endpoints: endpoints, | ||
| __responses: responses, | ||
| createApiEndpoint: (path: string) => { | ||
| const api = { get: jest.fn().mockResolvedValue({ data: path in responses ? responses[path] : { Data: {} } }) }; | ||
| endpoints[path] = api; | ||
| return api; | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| jest.mock('../../common/cached-client', () => { | ||
| const endpoints: Record<string, { get: jest.Mock }> = {}; | ||
| return { | ||
| __endpoints: endpoints, | ||
| createCachedApiEndpoint: (path: string) => { | ||
| const api = { get: jest.fn().mockResolvedValue({ data: { Data: {} } }) }; | ||
| endpoints[path] = api; | ||
| return api; | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| jest.mock('@/lib/storage/app', () => ({ | ||
| getBaseApiUrl: () => 'https://api.example.test/api/v4', | ||
| })); | ||
|
|
||
| import { getCustomMap, getCustomMapLayer, getCustomMapRegionsGeoJSON, getIndoorMap, getIndoorMapFloor, getIndoorMapZonesGeoJSON, getMapLayerGeoJSON, getPoi } from '../mapping'; | ||
|
|
||
| const mockClient = jest.requireMock('../../common/client') as { __endpoints: Record<string, { get: jest.Mock }>; __responses: Record<string, unknown> }; | ||
| const mockEndpoints = mockClient.__endpoints; | ||
| const mockResponses = mockClient.__responses; | ||
| const mockCachedEndpoints: Record<string, { get: jest.Mock }> = jest.requireMock('../../common/cached-client').__endpoints; | ||
|
|
||
| const featureCollection = { type: 'FeatureCollection', features: [{ type: 'Feature', geometry: null, properties: { name: 'Zone A' } }] }; | ||
|
|
||
| describe('mapping id-scoped routes', () => { | ||
| it('never creates the id-less query-string routes', () => { | ||
| const created = [...Object.keys(mockEndpoints), ...Object.keys(mockCachedEndpoints)]; | ||
| ['GetPoi', 'GetIndoorMap', 'GetIndoorMapFloor', 'GetCustomMap', 'GetCustomMapLayer', 'GetMapLayerGeoJSON', 'GetIndoorMapZonesGeoJSON', 'GetCustomMapRegionsGeoJSON'].forEach((action) => { | ||
| expect(created).not.toContain(`/Mapping/${action}`); | ||
| }); | ||
| }); | ||
|
|
||
| it('puts the POI id in the GetPoi path', async () => { | ||
| await getPoi(15); | ||
|
|
||
| expect(mockCachedEndpoints['/Mapping/GetPoi/15'].get).toHaveBeenCalledWith(); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ['GetIndoorMap', getIndoorMap], | ||
| ['GetIndoorMapFloor', getIndoorMapFloor], | ||
| ['GetCustomMap', getCustomMap], | ||
| ['GetCustomMapLayer', getCustomMapLayer], | ||
| ])('puts the id in the %s path', async (action, call) => { | ||
| await call('a/b c'); | ||
|
|
||
| expect(mockEndpoints[`/Mapping/${action}/a%2Fb%20c`].get).toHaveBeenCalledWith(); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ['GetMapLayerGeoJSON', getMapLayerGeoJSON], | ||
| ['GetIndoorMapZonesGeoJSON', getIndoorMapZonesGeoJSON], | ||
| ['GetCustomMapRegionsGeoJSON', getCustomMapRegionsGeoJSON], | ||
| ])('puts the id in the %s path and wraps the raw FeatureCollection in Data', async (action, call) => { | ||
| const path = `/Mapping/${action}/layer-1`; | ||
| mockResponses[path] = featureCollection; | ||
|
|
||
| const result = await call('layer-1'); | ||
|
|
||
| expect(mockEndpoints[path].get).toHaveBeenCalledWith(); | ||
| expect(result.Data).toEqual(featureCollection); | ||
| }); | ||
|
|
||
| it('falls back to an empty FeatureCollection when the body has no features', async () => { | ||
| mockResponses['/Mapping/GetMapLayerGeoJSON/empty-layer'] = ''; | ||
|
|
||
| const result = await getMapLayerGeoJSON('empty-layer'); | ||
|
|
||
| expect(result.Data).toEqual({ type: 'FeatureCollection', features: [] }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,10 +26,6 @@ const getMayLayersApi = createApiEndpoint('/Mapping/GetMayLayers'); | |
| const getMapDataAndMarkersApi = createApiEndpoint('/Mapping/GetMapDataAndMarkers'); | ||
| // Not cached: the legend follows the department's types, statuses, priorities and uploads, so every open asks again. | ||
| const getMapIconLegendApi = createApiEndpoint('/Mapping/GetMapIconLegend'); | ||
| const getPoiApi = createCachedApiEndpoint('/Mapping/GetPoi', { | ||
| ttl: 5 * 60 * 1000, | ||
| enabled: true, | ||
| }); | ||
| const getPoisApi = createCachedApiEndpoint('/Mapping/GetPois', { | ||
| ttl: 5 * 60 * 1000, | ||
| enabled: true, | ||
|
|
@@ -44,9 +40,6 @@ const getIndoorMapsApi = createCachedApiEndpoint('/Mapping/GetIndoorMaps', { | |
| ttl: 5 * 60 * 1000, | ||
| enabled: true, | ||
| }); | ||
| const getIndoorMapApi = createApiEndpoint('/Mapping/GetIndoorMap'); | ||
| const getIndoorMapFloorApi = createApiEndpoint('/Mapping/GetIndoorMapFloor'); | ||
| const getIndoorMapZonesGeoJSONApi = createApiEndpoint('/Mapping/GetIndoorMapZonesGeoJSON'); | ||
| const searchIndoorLocationsApi = createApiEndpoint('/Mapping/SearchIndoorLocations'); | ||
| const getNearbyIndoorMapsApi = createApiEndpoint('/Mapping/GetNearbyIndoorMaps'); | ||
|
|
||
|
|
@@ -55,12 +48,20 @@ const getCustomMapsApi = createCachedApiEndpoint('/Mapping/GetCustomMaps', { | |
| ttl: 5 * 60 * 1000, | ||
| enabled: true, | ||
| }); | ||
| const getCustomMapApi = createApiEndpoint('/Mapping/GetCustomMap'); | ||
| const getCustomMapLayerApi = createApiEndpoint('/Mapping/GetCustomMapLayer'); | ||
| const getMapLayerGeoJSONApi = createApiEndpoint('/Mapping/GetMapLayerGeoJSON'); | ||
| const getCustomMapRegionsGeoJSONApi = createApiEndpoint('/Mapping/GetCustomMapRegionsGeoJSON'); | ||
| const searchCustomMapRegionsApi = createApiEndpoint('/Mapping/SearchCustomMapRegions'); | ||
|
|
||
| /** | ||
| * The GeoJSON endpoints return a raw `application/geo+json` FeatureCollection body | ||
| * (not the standard v4 `{ Data: ... }` envelope). Accept either shape. | ||
| */ | ||
| const unwrapGeoJSON = (body: unknown): FeatureCollection => { | ||
| const asAny = body as { type?: string; Data?: FeatureCollection }; | ||
| if (asAny?.type === 'FeatureCollection') { | ||
| return body as FeatureCollection; | ||
| } | ||
| return asAny?.Data ?? { type: 'FeatureCollection', features: [] }; | ||
| }; | ||
|
|
||
| // Discovery endpoints | ||
| const getAllActiveLayersApi = createCachedApiEndpoint('/Mapping/GetAllActiveLayers', { | ||
| ttl: 5 * 60 * 1000, | ||
|
|
@@ -99,10 +100,9 @@ export const getPois = async (poiTypeId?: number, destinationOnly?: boolean) => | |
| return response.data; | ||
| }; | ||
|
|
||
| // The id-scoped endpoints below are path routes on the server (`/{id}`), so each call builds its own endpoint. | ||
| export const getPoi = async (poiId: number | string) => { | ||
| const response = await getPoiApi.get<PoiResult>({ | ||
| poiId: encodeURIComponent(String(poiId)), | ||
| }); | ||
| const response = await createCachedApiEndpoint(`/Mapping/GetPoi/${encodeURIComponent(String(poiId))}`, { ttl: 5 * 60 * 1000, enabled: true }).get<PoiResult>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unhandled failures from createCachedApiEndpoint(...).get() lose operation and poiId context and are not mapped or rethrown consistently. Wrap the external API call in try/catch, log the getPoi operation with poiId and error, then map or rethrow the failure appropriately. Kody rule violation: Add try-catch blocks for external calls let response;
try {
response = await getPoiEndpoint(poiId).get<PoiResult>();
} catch (error) {
logger.error('getPoi failed', { operation: 'getPoi', poiId, error });
throw error;
}Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| return response.data; | ||
| }; | ||
|
|
||
|
|
@@ -124,24 +124,18 @@ export const getIndoorMaps = async () => { | |
| }; | ||
|
|
||
| export const getIndoorMap = async (mapId: string) => { | ||
| const response = await getIndoorMapApi.get<GetIndoorMapResult>({ | ||
| id: encodeURIComponent(mapId), | ||
| }); | ||
| const response = await createApiEndpoint(`/Mapping/GetIndoorMap/${encodeURIComponent(mapId)}`).get<GetIndoorMapResult>(); | ||
| return response.data; | ||
| }; | ||
|
|
||
| export const getIndoorMapFloor = async (floorId: string) => { | ||
| const response = await getIndoorMapFloorApi.get<GetIndoorMapFloorResult>({ | ||
| floorId: encodeURIComponent(floorId), | ||
| }); | ||
| const response = await createApiEndpoint(`/Mapping/GetIndoorMapFloor/${encodeURIComponent(floorId)}`).get<GetIndoorMapFloorResult>(); | ||
| return response.data; | ||
| }; | ||
|
|
||
| export const getIndoorMapZonesGeoJSON = async (floorId: string) => { | ||
| const response = await getIndoorMapZonesGeoJSONApi.get<GetGeoJSONResult>({ | ||
| floorId: encodeURIComponent(floorId), | ||
| }); | ||
| return response.data; | ||
| export const getIndoorMapZonesGeoJSON = async (floorId: string): Promise<GetGeoJSONResult> => { | ||
| const response = await createApiEndpoint(`/Mapping/GetIndoorMapZonesGeoJSON/${encodeURIComponent(floorId)}`).get<unknown>(); | ||
| return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; | ||
| }; | ||
|
|
||
| export const searchIndoorLocations = async (term: string, mapId?: string) => { | ||
|
|
@@ -174,31 +168,23 @@ export const getCustomMaps = async (type?: number) => { | |
| }; | ||
|
|
||
| export const getCustomMap = async (mapId: string) => { | ||
| const response = await getCustomMapApi.get<GetCustomMapResult>({ | ||
| id: encodeURIComponent(mapId), | ||
| }); | ||
| const response = await createApiEndpoint(`/Mapping/GetCustomMap/${encodeURIComponent(mapId)}`).get<GetCustomMapResult>(); | ||
| return response.data; | ||
| }; | ||
|
|
||
| export const getCustomMapLayer = async (layerId: string) => { | ||
| const response = await getCustomMapLayerApi.get<GetCustomMapLayerResult>({ | ||
| layerId: encodeURIComponent(layerId), | ||
| }); | ||
| const response = await createApiEndpoint(`/Mapping/GetCustomMapLayer/${encodeURIComponent(layerId)}`).get<GetCustomMapLayerResult>(); | ||
| return response.data; | ||
| }; | ||
|
|
||
| export const getMapLayerGeoJSON = async (layerId: string) => { | ||
| const response = await getMapLayerGeoJSONApi.get<GetGeoJSONResult>({ | ||
| layerId: encodeURIComponent(layerId), | ||
| }); | ||
| return response.data; | ||
| export const getMapLayerGeoJSON = async (layerId: string): Promise<GetGeoJSONResult> => { | ||
| const response = await createApiEndpoint(`/Mapping/GetMapLayerGeoJSON/${encodeURIComponent(layerId)}`).get<unknown>(); | ||
| return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; | ||
| }; | ||
|
|
||
| export const getCustomMapRegionsGeoJSON = async (layerId: string) => { | ||
| const response = await getCustomMapRegionsGeoJSONApi.get<GetGeoJSONResult>({ | ||
| layerId: encodeURIComponent(layerId), | ||
| }); | ||
| return response.data; | ||
| export const getCustomMapRegionsGeoJSON = async (layerId: string): Promise<GetGeoJSONResult> => { | ||
| const response = await createApiEndpoint(`/Mapping/GetCustomMapRegionsGeoJSON/${encodeURIComponent(layerId)}`).get<unknown>(); | ||
| return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; | ||
| }; | ||
|
|
||
| export const searchCustomMapRegions = async (term: string, layerId?: string) => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Script executed:
Repository: Resgrid/Unit
Length of output: 8413
Script executed:
Repository: Resgrid/Unit
Length of output: 2437
Validate the GeoJSON shape in
unwrapGeoJSON.If a successful response body is
{ Data: {} },unwrapGeoJSONreturns{}as aFeatureCollection. If a raw body hastype: 'FeatureCollection'but nofeatures, it is returned unchanged. The map store then caches or stores that value without a shape check. Code that readsfeaturesfrom the stored value can receiveundefined.Check both
typeandfeaturesfor the raw body and forData. Fall back to an empty collection when either check fails.Suggested fix
Source: Learnings