From c6ef033f4f2bf1ca13b9520b39143b8b7d02c8b5 Mon Sep 17 00:00:00 2001 From: Tim van Oostrom Date: Wed, 18 Dec 2024 10:34:11 +0100 Subject: [PATCH] Mijn-9880-Bug - Add features check + test, (#1667) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MIJN-9880-Bug - Hot fix (#1666) * Fix pageChangeHook * Don’t show tips when there are no filteredfeatures * Add test for adoptable trash container * Change name * Add mocked responses to var, fix expectation value * Update src/server/services/home.ts Lijkt me een verbetering. Co-authored-by: Terry van Walen * Rename file, add tests * Clarify testing data * Change number in fake address * Change error messages --------- Co-authored-by: Terry van Walen --- src/client/hooks/usePageChange.test.tsx | 8 +- src/client/hooks/usePageChange.ts | 4 +- .../adoptable-trash-containers.test.ts | 151 +++++++++ .../services/adoptable-trash-containers.ts | 4 +- src/server/services/afval/afval.ts | 2 +- src/server/services/controller.ts | 2 +- src/server/services/my-locations.test.ts | 295 ++++++++++++++++++ .../services/{home.ts => my-locations.ts} | 77 ++--- src/server/services/wior.ts | 2 +- src/testing/utils.ts | 48 +++ 10 files changed, 546 insertions(+), 47 deletions(-) create mode 100644 src/server/services/my-locations.test.ts rename src/server/services/{home.ts => my-locations.ts} (66%) diff --git a/src/client/hooks/usePageChange.test.tsx b/src/client/hooks/usePageChange.test.tsx index c46724df3b..657a482f08 100644 --- a/src/client/hooks/usePageChange.test.tsx +++ b/src/client/hooks/usePageChange.test.tsx @@ -1,3 +1,5 @@ +import { ReactNode } from 'react'; + import { renderHook } from '@testing-library/react'; import * as rrd from 'react-router-dom'; import { afterAll, afterEach, describe, expect, it, test, vi } from 'vitest'; @@ -6,7 +8,6 @@ import { trackPageViewWithCustomDimension } from './analytics.hook'; import { usePageChange } from './usePageChange'; import type { TrackingConfig } from '../config/routes'; import { NOT_FOUND_TITLE } from '../config/thema'; -import { ReactNode } from 'react'; const mocks = vi.hoisted(() => { return { @@ -24,8 +25,11 @@ vi.mock('react-router-dom', async (requireActual) => { __setPathname: (name: string) => { mocks.pathname = name; }, + useLocation: () => { + return { pathname: mocks.pathname }; + }, useHistory: () => { - return { action: 'PUSH', location: { pathname: mocks.pathname } }; + return { action: 'PUSH' }; }, }; }); diff --git a/src/client/hooks/usePageChange.ts b/src/client/hooks/usePageChange.ts index 4b9ad27325..44ad57fd9e 100644 --- a/src/client/hooks/usePageChange.ts +++ b/src/client/hooks/usePageChange.ts @@ -1,6 +1,6 @@ import { useEffect, useRef } from 'react'; -import { matchPath, useHistory } from 'react-router-dom'; +import { matchPath, useHistory, useLocation } from 'react-router-dom'; import { trackPageViewWithCustomDimension } from './analytics.hook'; import { useProfileTypeValue } from './useProfileType'; @@ -25,7 +25,7 @@ const sortedPageTitleRoutes = Object.keys(DocumentTitles).sort((a, b) => { export function usePageChange(isAuthenticated: boolean) { const history = useHistory(); - const location = history.location; + const location = useLocation(); const termReplace = useTermReplacement(); const profileType = useProfileTypeValue(); const userCity = useUserCity(); diff --git a/src/server/services/adoptable-trash-containers.test.ts b/src/server/services/adoptable-trash-containers.test.ts index f87b3c928f..4674dc1f2f 100644 --- a/src/server/services/adoptable-trash-containers.test.ts +++ b/src/server/services/adoptable-trash-containers.test.ts @@ -1,4 +1,155 @@ +import { describe, it, expect, vi, Mock } from 'vitest'; + import { forTesting } from './adoptable-trash-containers'; +import { fetchAdoptableTrashContainers } from './adoptable-trash-containers'; +import { fetchBRP } from './brp'; +import { fetchDataset } from './buurt/buurt'; +import { fetchMyLocation } from './my-locations'; +import { + generateRandomPoints, + getAuthProfileAndToken, +} from '../../testing/utils'; +import { + DEFAULT_LAT, + DEFAULT_LNG, +} from '../../universal/config/myarea-datasets'; +import { apiSuccessResult, apiErrorResult } from '../../universal/helpers/api'; + +vi.mock('./brp', () => ({ + fetchBRP: vi.fn(), +})); + +vi.mock('./my-locations', () => ({ + fetchMyLocation: vi.fn(), +})); + +vi.mock('./buurt/buurt', () => ({ + fetchDataset: vi.fn(), +})); + +describe('fetchAdoptableTrashContainers', () => { + const requestID = 'test-request-id'; + const authProfileAndToken = getAuthProfileAndToken(); + const defaultLatLng = { lat: DEFAULT_LAT, lng: DEFAULT_LNG }; + const locationApiResponse = apiSuccessResult([{ latlng: defaultLatLng }]); + const brpApiResponse = apiSuccessResult({ + persoon: { geboortedatum: '2000-01-01' }, + }); + + it('should return tips if all fetches are successful and age is above LATE_TEEN_AGE', async () => { + (fetchBRP as Mock).mockResolvedValue(brpApiResponse); + (fetchMyLocation as Mock).mockResolvedValue(locationApiResponse); + + const [coord] = generateRandomPoints( + { lat: DEFAULT_LAT, lng: DEFAULT_LNG }, + 90, // 90 meters + 1 + ); + + (fetchDataset as Mock).mockResolvedValue( + apiSuccessResult({ + features: [ + { + geometry: { coordinates: [coord.lng, coord.lat] }, + properties: { + datasetId: 'afvalcontainers', + geadopteerd_ind: 'Nee', + }, + }, + ], + }) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('OK'); + expect(result.content?.tips).toHaveLength(1); + }); + + it('should return an error if fetching BRP data fails', async () => { + (fetchBRP as Mock).mockResolvedValue( + apiErrorResult('Error fetching BRP data', null) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('DEPENDENCY_ERROR'); + }); + + it('should return an error if fetching location fails', async () => { + (fetchBRP as Mock).mockResolvedValue(brpApiResponse); + (fetchMyLocation as Mock).mockResolvedValue( + apiErrorResult('Error fetching BAG location', null) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('DEPENDENCY_ERROR'); + }); + + it('should return an error if fetching dataset fails', async () => { + (fetchBRP as Mock).mockResolvedValue(brpApiResponse); + (fetchMyLocation as Mock).mockResolvedValue(locationApiResponse); + (fetchDataset as Mock).mockResolvedValue( + apiErrorResult('Error fetching Map locations dataset', null) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('DEPENDENCY_ERROR'); + }); + + it('should return no tips if age is less than LATE_TEEN_AGE', async () => { + (fetchBRP as Mock).mockResolvedValue( + apiSuccessResult({ persoon: { geboortedatum: '2010-01-01' } }) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('OK'); + expect(result.content?.tips).toHaveLength(0); + }); + + it('should not return tips if there are no adoptable trashcontainers found within the given radius', async () => { + (fetchBRP as Mock).mockResolvedValue(brpApiResponse); + (fetchMyLocation as Mock).mockResolvedValue(locationApiResponse); + + // A coord outside of the radius from the Home location returned by fetchMyLocation + const COORD_LAT_OFFSET = 5; + const coordinates = [DEFAULT_LNG, DEFAULT_LAT + COORD_LAT_OFFSET]; + + (fetchDataset as Mock).mockResolvedValue( + apiSuccessResult({ + features: [ + { + geometry: { coordinates }, + properties: { + datasetId: 'afvalcontainers', + geadopteerd_ind: 'Nee', + }, + }, + ], + }) + ); + + const result = await fetchAdoptableTrashContainers( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('OK'); + expect(result.content?.tips).toHaveLength(0); + }); +}); describe('determineDescriptionText Tests', () => { test('Returns adults text when adult', () => { diff --git a/src/server/services/adoptable-trash-containers.ts b/src/server/services/adoptable-trash-containers.ts index ae9220e285..62c29af8eb 100644 --- a/src/server/services/adoptable-trash-containers.ts +++ b/src/server/services/adoptable-trash-containers.ts @@ -13,7 +13,7 @@ import { filterFeaturesinRadius, getBboxFromFeatures, } from './buurt/helpers'; -import { fetchMyLocation } from './home'; +import { fetchMyLocation } from './my-locations'; import { AppRoutes } from '../../universal/config/routes'; import { Themas } from '../../universal/config/thema'; import { @@ -107,7 +107,7 @@ export async function fetchAdoptableTrashContainers( ); return apiSuccessResult({ - tips: [buildNotification(age, bbox)], + tips: filteredFeatures.length ? [buildNotification(age, bbox)] : [], }); } diff --git a/src/server/services/afval/afval.ts b/src/server/services/afval/afval.ts index e0d2ef9172..3c23e52d15 100644 --- a/src/server/services/afval/afval.ts +++ b/src/server/services/afval/afval.ts @@ -3,7 +3,7 @@ import { apiSuccessResult, } from '../../../universal/helpers/api'; import { AuthProfileAndToken } from '../../auth/auth-types'; -import { fetchMyLocation } from '../home'; +import { fetchMyLocation } from '../my-locations'; import { fetchAfvalpuntenByLatLng } from './afvalpunten'; import { fetchAfvalwijzer } from './afvalwijzer'; diff --git a/src/server/services/controller.ts b/src/server/services/controller.ts index a5c0c1c9f0..8bc047d20d 100644 --- a/src/server/services/controller.ts +++ b/src/server/services/controller.ts @@ -22,7 +22,7 @@ import { fetchBRP } from './brp'; import { fetchCMSCONTENT } from './cms-content'; import { fetchMaintenanceNotificationsActual } from './cms-maintenance-notifications'; import { fetchHLI } from './hli/hli'; -import { fetchMyLocation } from './home'; +import { fetchMyLocation } from './my-locations'; import { fetchHorecaVergunningen } from './horeca'; import { fetchAllKlachten } from './klachten/klachten'; import { fetchKrefia } from './krefia'; diff --git a/src/server/services/my-locations.test.ts b/src/server/services/my-locations.test.ts new file mode 100644 index 0000000000..a149d7b533 --- /dev/null +++ b/src/server/services/my-locations.test.ts @@ -0,0 +1,295 @@ +import { describe, it, expect, vi, Mock } from 'vitest'; + +import { fetchBAG } from './bag'; +import { fetchBRP } from './brp'; +import { fetchKVK, getKvkAddresses } from './kvk'; +import { fetchMyLocation, forTesting } from './my-locations'; +import { getAuthProfileAndToken } from '../../testing/utils'; +import { apiSuccessResult, apiErrorResult } from '../../universal/helpers/api'; +import { Adres } from '../../universal/types'; + +vi.mock('./brp', () => ({ + fetchBRP: vi.fn(), +})); + +vi.mock('./bag', () => ({ + fetchBAG: vi.fn(), +})); + +vi.mock('./kvk', async (importOriginal) => ({ + ...(await importOriginal()), + getKvkAddresses: vi.fn(), + fetchKVK: vi.fn(), +})); + +const adres = { straatnaam: 'address1' } as Adres; +const adres2 = { straatnaam: 'address2' } as Adres; + +describe('fetchPrivate', () => { + const requestID = 'test-request-id'; + const authProfileAndToken = getAuthProfileAndToken(); + + it('should return private addresses if fetching BRP data is successful', async () => { + (fetchBRP as Mock).mockResolvedValueOnce( + apiSuccessResult({ + mokum: true, + adres, + }) + ); + + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 1, lng: 1 }, address: 'Een adres' }) + ); + + const result = await forTesting.fetchPrivate( + requestID, + authProfileAndToken + ); + + expect(fetchBAG).toHaveBeenCalledWith(requestID, adres); + + expect(result.status).toBe('OK'); + expect(result.content).toHaveLength(1); + expect(result.content).toEqual([ + { + address: 'Een adres', + latlng: { + lat: 1, + lng: 1, + }, + profileType: 'private', + }, + ]); + }); + + it('should return default location if no BAG location is found', async () => { + (fetchBRP as Mock).mockResolvedValueOnce( + apiSuccessResult({ + mokum: true, + adres, + }) + ); + + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: null, address: null }) + ); + + const result = await forTesting.fetchPrivate( + requestID, + authProfileAndToken + ); + + expect(result).toStrictEqual({ + content: [ + { + address: null, + latlng: { + lat: 52.3676842478192, + lng: 4.90022569871861, + }, + profileType: 'private', + }, + ], + status: 'OK', + }); + }); + + it('should return a bare response if BRP data is not a Mokum address', async () => { + (fetchBRP as Mock).mockResolvedValueOnce( + apiSuccessResult({ mokum: false, adres: null }) + ); + + const result = await forTesting.fetchPrivate( + requestID, + authProfileAndToken + ); + + expect(result.status).toBe('OK'); + expect(result.content).toHaveLength(1); + expect(result.content).toEqual([ + { + address: null, + latlng: null, + profileType: 'private', + }, + ]); + }); + + it('should return an error if fetching BRP data fails', async () => { + (fetchBRP as Mock).mockResolvedValueOnce( + apiErrorResult('Error fetching BRP data', null) + ); + + const result = await forTesting.fetchPrivate( + requestID, + authProfileAndToken + ); + expect(result.status === 'DEPENDENCY_ERROR' && result.message).toBe( + '[BRP] Error fetching BRP data' + ); + }); +}); + +describe('fetchCommercial', () => { + const requestID = 'test-request-id'; + const authProfileAndToken = getAuthProfileAndToken('commercial'); + + it('should return commercial addresses if fetching KVK data is successful', async () => { + (fetchKVK as Mock).mockResolvedValueOnce( + apiSuccessResult({ vestigingen: [adres, adres2] }) + ); + + (getKvkAddresses as Mock).mockReturnValueOnce([adres, adres2]); + + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 1, lng: 1 }, address: 'Een adres' }) + ); + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 1, lng: 1 }, address: 'Een 2e adres' }) + ); + + const result = await forTesting.fetchCommercial( + requestID, + authProfileAndToken + ); + + expect(fetchBAG).toHaveBeenCalledWith(requestID, adres); + expect(fetchBAG).toHaveBeenCalledWith(requestID, adres2); + + expect(result).toMatchInlineSnapshot(` + { + "content": [ + { + "address": "Een adres", + "latlng": { + "lat": 1, + "lng": 1, + }, + "profileType": "commercial", + }, + { + "address": "Een 2e adres", + "latlng": { + "lat": 1, + "lng": 1, + }, + "profileType": "commercial", + }, + ], + "status": "OK", + } + `); + }); + + it('should return an error if fetching KVK data fails', async () => { + (fetchKVK as Mock).mockResolvedValueOnce( + apiErrorResult('Error fetching KVK data', null) + ); + + const result = await forTesting.fetchCommercial( + requestID, + authProfileAndToken + ); + expect(result.status).toBe('DEPENDENCY_ERROR'); + }); +}); + +describe('fetchMyLocation', () => { + const requestID = 'test-request-id'; + const authProfileAndTokenPrivate = getAuthProfileAndToken(); + const authProfileAndTokenCommercial = getAuthProfileAndToken('commercial'); + + it('should return locations if fetching commercial and private data is successful', async () => { + (fetchBRP as Mock).mockResolvedValueOnce( + apiSuccessResult({ + mokum: true, + adres, + }) + ); + + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 1, lng: 1 }, address: 'Een adres' }) + ); + + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 2, lng: 2 }, address: 'Een 2e adres' }) + ); + + (fetchKVK as Mock).mockResolvedValueOnce( + apiSuccessResult({ vestigingen: [adres2] }) + ); + + (getKvkAddresses as Mock).mockReturnValueOnce([adres2]); + + const result = await fetchMyLocation(requestID, authProfileAndTokenPrivate); + expect(result).toEqual({ + content: [ + { + address: 'Een 2e adres', + latlng: { + lat: 2, + lng: 2, + }, + profileType: 'private', + }, + { + address: 'Een adres', + latlng: { + lat: 1, + lng: 1, + }, + profileType: 'commercial', + }, + ], + status: 'OK', + }); + }); + + it('should return commercial locations if profile type is commercial', async () => { + (fetchBAG as Mock).mockResolvedValueOnce( + apiSuccessResult({ latlng: { lat: 2, lng: 2 }, address: 'Een 2e adres' }) + ); + + (fetchKVK as Mock).mockResolvedValueOnce( + apiSuccessResult({ vestigingen: [adres2] }) + ); + + (getKvkAddresses as Mock).mockReturnValueOnce([adres2]); + + const result = await fetchMyLocation( + requestID, + authProfileAndTokenCommercial + ); + expect(result.status).toBe('OK'); + expect(result.content).toHaveLength(1); + }); + + it('should return an error if fetching commercial data fails', async () => { + (fetchKVK as Mock).mockResolvedValueOnce( + apiErrorResult('Oh Oh Server down', null) + ); + + const result = await fetchMyLocation( + requestID, + authProfileAndTokenCommercial + ); + expect(result.status).toBe('DEPENDENCY_ERROR'); + }); + + it('should return an error if no locations found', async () => { + (fetchKVK as Mock).mockResolvedValueOnce( + apiSuccessResult({ vestigingen: [] }) + ); + (getKvkAddresses as Mock).mockReturnValueOnce([]); + (fetchBRP as Mock).mockResolvedValueOnce( + apiErrorResult('Server down!', null) + ); + + const result = await fetchMyLocation(requestID, authProfileAndTokenPrivate); + expect(result).toEqual({ + content: null, + message: 'Could not fetch locations.', + status: 'ERROR', + }); + }); +}); diff --git a/src/server/services/home.ts b/src/server/services/my-locations.ts similarity index 66% rename from src/server/services/home.ts rename to src/server/services/my-locations.ts index 19cef843d1..5072d60582 100644 --- a/src/server/services/home.ts +++ b/src/server/services/my-locations.ts @@ -61,10 +61,10 @@ async function fetchPrivate( async function fetchCommercial( requestID: RequestID, authProfileAndToken: AuthProfileAndToken -) { +): Promise> { const KVK = await fetchKVK(requestID, authProfileAndToken); - let MY_LOCATION: ApiResponse<(BAGData | null)[] | null>; + let MY_LOCATION: ApiResponse; if (KVK.status === 'OK') { const addresses: Adres[] = getKvkAddresses(KVK.content); @@ -73,13 +73,15 @@ async function fetchCommercial( const locations = await Promise.all( addresses.map((address) => fetchBAG(requestID, address)) ).then((results) => { - return results.map((result) => - result.content !== null - ? Object.assign(result.content, { - profileType: 'commercial', - }) - : null - ); + return results + .map((result) => + result.content !== null + ? Object.assign(result.content, { + profileType: 'commercial', + }) + : null + ) + .filter((location) => location !== null); }); MY_LOCATION = apiSuccessResult(locations); @@ -99,35 +101,34 @@ async function fetchCommercial( export async function fetchMyLocation( requestID: RequestID, authProfileAndToken: AuthProfileAndToken -) { - switch (authProfileAndToken.profile.profileType) { - case 'commercial': - return fetchCommercial(requestID, authProfileAndToken); - - case 'private': - default: { - const privateAddresses = await fetchPrivate( - requestID, - authProfileAndToken - ); - const commercialAddresses = await fetchCommercial( - requestID, - authProfileAndToken - ); +): Promise> { + const commercialResponse = await fetchCommercial( + requestID, + authProfileAndToken + ); - switch (true) { - case privateAddresses.content !== null && - commercialAddresses.content !== null: { - const locations: BAGData[] = [ - ...privateAddresses.content!.filter((a) => a !== null), - ...commercialAddresses.content!.filter((a) => a !== null), - ]; - return apiSuccessResult(locations); - } - case privateAddresses.content !== null: - return privateAddresses; - } - return apiErrorResult('Could not fetch locations.', null); - } + if (authProfileAndToken.profile.profileType === 'commercial') { + return commercialResponse; } + + const { content: privateAddresses } = await fetchPrivate( + requestID, + authProfileAndToken + ); + + const locations: BAGData[] = [ + ...(privateAddresses || []), + ...(commercialResponse.content || []), + ].filter((location) => location !== null); + + if (locations.length === 0) { + return apiErrorResult('Could not fetch locations.', null); + } + + return apiSuccessResult(locations); } + +export const forTesting = { + fetchPrivate, + fetchCommercial, +}; diff --git a/src/server/services/wior.ts b/src/server/services/wior.ts index 142855be67..18ea6ebe98 100644 --- a/src/server/services/wior.ts +++ b/src/server/services/wior.ts @@ -14,7 +14,7 @@ import { filterFeaturesinRadius, getBboxFromFeatures, } from './buurt/helpers'; -import { fetchMyLocation } from './home'; +import { fetchMyLocation } from './my-locations'; import { AppRoutes } from '../../universal/config/routes'; const WITHIN_RADIUS_KM = 1; diff --git a/src/testing/utils.ts b/src/testing/utils.ts index 7445f99b12..a0804f6430 100644 --- a/src/testing/utils.ts +++ b/src/testing/utils.ts @@ -1,5 +1,6 @@ import { millisecondsToSeconds } from 'date-fns'; import { Request, Response } from 'express'; +import { LatLngLiteral } from 'leaflet'; import nock from 'nock'; import UID from 'uid-safe'; @@ -143,3 +144,50 @@ export async function getReqMockWithOidc( export const DEV_JWT = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c'; + +/** + * Generates number of random geolocation points given a center and a radius. + * @param {Object} center A JS object with lat and lng attributes. + * @param {number} radius Radius in meters. + * @param {number} count Number of points to generate. + * @return {array} Array of Objects with lat and lng attributes. + */ +export function generateRandomPoints( + center: LatLngLiteral, + radius: number, + count: number +) { + const points = []; + for (let i = 0; i < count; i++) { + points.push(generateRandomPoint(center, radius)); + } + return points; +} + +/** + * Generates number of random geolocation points given a center and a radius. + * Reference URL: http://goo.gl/KWcPE. + * @param {Object} center A JS object with lat and lng attributes. + * @param {number} radius Radius in meters. + * @return {Object} The generated random points as JS object with lat and lng attributes. + */ +export function generateRandomPoint(center: LatLngLiteral, radius: number) { + const x0 = center.lng; + const y0 = center.lat; + // Convert Radius from meters to degrees. + // eslint-disable-next-line no-magic-numbers + const rd = radius / 111300; + + const u = Math.random(); + const v = Math.random(); + + const w = rd * Math.sqrt(u); + const t = 2 * Math.PI * v; + const x = w * Math.cos(t); + const y = w * Math.sin(t); + + const xp = x / Math.cos(y0); + + // Resulting point. + return { lat: y + y0, lng: xp + x0 }; +}