Skip to content

Commit

Permalink
feat(DTFS2-7052): change GET /geospatial/addresses/postcode?postcode=…
Browse files Browse the repository at this point in the history
… empty response from 200 to 404
  • Loading branch information
avaitonis committed May 13, 2024
1 parent 38939db commit 33c9e65
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 28 deletions.
5 changes: 4 additions & 1 deletion src/constants/geospatial.constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ export const GEOSPATIAL = {
DATASET: 'DPA',
},
EXAMPLES: {
POSTCODE: 'SW1A 2AQ',
ENGLISH_POSTCODE: 'SW1A 2AQ',
NORTHERN_IRELAND_POSTCODE: 'BT7 3GG',
WALES_POSTCODE: 'SY23 3AR',
SCOTLAND_POSTCODE: 'EH1 1JF',
ORGANISATION_NAME: 'CHURCHILL MUSEUM & CABINET WAR ROOMS',
ADDRESS_LINE_1: 'CLIVE STEPS KING CHARLES STREET',
LOCALITY: 'LONDON',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('OrdnanceSurveyService', () => {
let configServiceGet: jest.Mock;
let service: OrdnanceSurveyService;

const testPostcode = GEOSPATIAL.EXAMPLES.POSTCODE;
const testPostcode = GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE;
const testKey = valueGenerator.string({ length: 10 });
const basePath = '/search/places/v1/postcode';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
import { ApiProperty } from '@nestjs/swagger';
import { GEOSPATIAL } from '@ukef/constants';
import { Matches, MaxLength, MinLength } from 'class-validator';
import { IsString, Matches, MaxLength, MinLength } from 'class-validator';

export class GetAddressesByPostcodeQueryDto {
@ApiProperty({
example: GEOSPATIAL.EXAMPLES.POSTCODE,
example: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE,
description: 'Postcode to search for',
minLength: 5,
maxLength: 8,
pattern: GEOSPATIAL.REGEX.UK_POSTCODE.source,
})
@IsString()
@MinLength(5)
@MaxLength(8)
@Matches(GEOSPATIAL.REGEX.UK_POSTCODE)
Expand Down
2 changes: 1 addition & 1 deletion src/modules/geospatial/dto/get-addresses-response.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class GetAddressesResponseItem {

@ApiProperty({
description: 'Postcode',
example: GEOSPATIAL.EXAMPLES.POSTCODE,
example: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE,
nullable: true,
})
readonly postalCode: string | null;
Expand Down
14 changes: 9 additions & 5 deletions src/modules/geospatial/geospatial.controller.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { NotFoundException } from '@nestjs/common';
import { GEOSPATIAL } from '@ukef/constants';
import { GetGeospatialAddressesGenerator } from '@ukef-test/support/generator/get-geospatial-addresses-generator';
import { RandomValueGenerator } from '@ukef-test/support/generator/random-value-generator';
Expand Down Expand Up @@ -30,7 +31,7 @@ describe('GeospatialController', () => {
});

describe('getAddressesByPostcode()', () => {
const postcode = GEOSPATIAL.EXAMPLES.POSTCODE;
const postcode = GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE;

it('returns a single address for the postcode when the service returns a single address', async () => {
when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce(getAddressesByPostcodeResponse[0]);
Expand All @@ -50,13 +51,16 @@ describe('GeospatialController', () => {
expect(response).toEqual(getAddressesByPostcodeMultipleResponse);
});

it('returns an empty response for the postcode when the service returns an empty response', async () => {
when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce([]);
it('passes NotFoundException when it is thrown by geospatialService', async () => {
const errorMessage = valueGenerator.sentence();
when(geospatialServiceGetAddressesByPostcode).calledWith(postcode).mockRejectedValueOnce(new NotFoundException(errorMessage));

const response = await controller.getAddressesByPostcode({ postcode });
const responsePromise = controller.getAddressesByPostcode({ postcode });

expect(geospatialServiceGetAddressesByPostcode).toHaveBeenCalledTimes(1);
expect(response).toEqual([]);

await expect(responsePromise).rejects.toBeInstanceOf(NotFoundException);
await expect(responsePromise).rejects.toThrow(errorMessage);
});
});
});
2 changes: 1 addition & 1 deletion src/modules/geospatial/geospatial.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class GeospatialController {
type: [GetAddressesResponseItem],
})
@ApiNotFoundResponse({
description: 'Postcode not found.',
description: 'No addresses found',
})
getAddressesByPostcode(@Query() query: GetAddressesByPostcodeQueryDto): Promise<GetAddressesResponse> {
return this.geospatialService.getAddressesByPostcode(query.postcode);
Expand Down
8 changes: 5 additions & 3 deletions src/modules/geospatial/geospatial.service.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { NotFoundException } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { OrdnanceSurveyService } from '@ukef/helper-modules/ordnance-survey/ordnance-survey.service';
import { GetGeospatialAddressesGenerator } from '@ukef-test/support/generator/get-geospatial-addresses-generator';
Expand Down Expand Up @@ -56,12 +57,13 @@ describe('GeospatialService', () => {
expect(response).toEqual(getAddressesByPostcodeMultipleResponse);
});

it('can handle empty backend response', async () => {
it('throws NotFoundException in case of empty backend response', async () => {
when(ordnanceSurveyServiceGetAddressesByPostcode).calledWith(postcode).mockResolvedValueOnce(getAddressesOrdnanceSurveyEmptyResponse[0]);

const response = await service.getAddressesByPostcode(postcode);
const responsePromise = service.getAddressesByPostcode(postcode);

expect(response).toEqual([]);
await expect(responsePromise).rejects.toBeInstanceOf(NotFoundException);
await expect(responsePromise).rejects.toThrow('No addresses found');
});

it('returns addressLine1 formatted correctly even if middle value is missing', async () => {
Expand Down
6 changes: 3 additions & 3 deletions src/modules/geospatial/geospatial.service.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Injectable } from '@nestjs/common';
import { Injectable, NotFoundException } from '@nestjs/common';
import { ENUMS } from '@ukef/constants';
import { GetAddressesOrdnanceSurveyResponse } from '@ukef/helper-modules/ordnance-survey/dto/get-addresses-ordnance-survey-response.dto';
import { OrdnanceSurveyService } from '@ukef/helper-modules/ordnance-survey/ordnance-survey.service';
Expand All @@ -13,8 +13,8 @@ export class GeospatialService {
const addresses = [];
const response: GetAddressesOrdnanceSurveyResponse = await this.ordnanceSurveyService.getAddressesByPostcode(postcode);

if (!response?.results) {
return [];
if (!response?.results?.length) {
throw new NotFoundException('No addresses found');
}

response.results.forEach((item) => {
Expand Down
4 changes: 2 additions & 2 deletions test/docs/__snapshots__/get-docs-yaml.api-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ paths:
items:
$ref: '#/components/schemas/GetAddressesResponseItem'
'404':
description: Postcode not found.
description: No addresses found
tags:
- geospatial
info:
Expand Down Expand Up @@ -1174,9 +1174,9 @@ components:
example: England
enum:
- England
- Northern Ireland
- Scotland
- Wales
- Northern Ireland
nullable: true
required:
- organisationName
Expand Down
7 changes: 7 additions & 0 deletions test/exposure-period/exposure-period.api-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,11 @@ describe('Exposure period', () => {
expect(status).toBe(400);
expect(body.message).toContain('productgroup must be one of the following values: EW, BS');
});

it('returns a 404 response if URL is missing /api/v1 at the beginning', async () => {
const url = '/exposure-period?startdate=2017-03-05&enddate=2017-04-05&productgroup=new';
const { status, body } = await api.get(url);
expect(status).toBe(404);
expect(body.message).toContain(`Cannot GET ${url}`);
});
});
60 changes: 52 additions & 8 deletions test/geospatial/get-address-by-postcode.api-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => {

const {
ordnanceSurveyPaths,
ordnanceSurveyKey,
mdmPaths,
getAddressesByPostcodeResponse,
getAddressesByPostcodeMultipleResponse,
Expand All @@ -21,12 +22,14 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => {
getAddressesOrdnanceSurveyMultipleMatchingAddressesResponse,
ordnanceSurveyAuthErrorResponse,
} = new GetGeospatialAddressesGenerator(valueGenerator).generate({
postcode: GEOSPATIAL.EXAMPLES.POSTCODE,
postcode: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE,
key: ENVIRONMENT_VARIABLES.ORDNANCE_SURVEY_KEY,
numberToGenerate: 2,
});

const getMdmUrl = (postcode: string) => `/api/v1/geospatial/addresses/postcode?postcode=${encodeURIComponent(postcode)}`;
const getMdmUrl = (postcode: any) => `/api/v1/geospatial/addresses/postcode?postcode=${encodeURIComponent(postcode)}`;
const getOsUrl = (postcode: any) =>
`/search/places/v1/postcode?postcode=${encodeURIComponent(postcode)}&lr=${GEOSPATIAL.DEFAULT.RESULT_LANGUAGE}&key=${encodeURIComponent(ordnanceSurveyKey)}`;

beforeAll(async () => {
api = await Api.create();
Expand All @@ -49,10 +52,27 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => {
makeRequestWithoutAuth: (incorrectAuth?: IncorrectAuthArg) => api.getWithoutAuth(mdmPaths[0], incorrectAuth?.headerName, incorrectAuth?.headerValue),
});

it('returns a 200 response with the address if it is returned by Ordnance Survey API', async () => {
requestToGetAddressesByPostcode(ordnanceSurveyPaths[0]).reply(200, getAddressesOrdnanceSurveyResponse[0]);
it.each([
{
postcode: GEOSPATIAL.EXAMPLES.ENGLISH_POSTCODE,
description: 'for English postcode',
},
{
postcode: GEOSPATIAL.EXAMPLES.NORTHERN_IRELAND_POSTCODE,
description: 'for Northern Ireland postcode',
},
{
postcode: GEOSPATIAL.EXAMPLES.SCOTLAND_POSTCODE,
description: 'for Scotish postcode',
},
{
postcode: GEOSPATIAL.EXAMPLES.WALES_POSTCODE,
description: 'for Wales postcode',
},
])('returns a 200 response $description', async ({ postcode }) => {
requestToGetAddressesByPostcode(getOsUrl(postcode)).reply(200, getAddressesOrdnanceSurveyResponse[0]);

const { status, body } = await api.get(mdmPaths[0]);
const { status, body } = await api.get(getMdmUrl(postcode));

expect(status).toBe(200);
expect(body).toStrictEqual(getAddressesByPostcodeResponse[0]);
Expand All @@ -67,13 +87,17 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => {
expect(body).toStrictEqual(getAddressesByPostcodeMultipleResponse);
});

it('returns an empty 200 response if Ordnance Survey API returns a 200 without results', async () => {
it('returns a 404 response if Ordnance Survey API returns a 200 without results', async () => {
requestToGetAddressesByPostcode(ordnanceSurveyPaths[0]).reply(200, getAddressesOrdnanceSurveyEmptyResponse[0]);

const { status, body } = await api.get(mdmPaths[0]);

expect(status).toBe(200);
expect(body).toStrictEqual([]);
expect(status).toBe(404);
expect(body).toEqual({
statusCode: 404,
message: 'No addresses found',
error: 'Not Found',
});
});

it('returns a 500 response if Ordnance Survey API returns a status code 401', async () => {
Expand Down Expand Up @@ -125,6 +149,26 @@ describe('GET /geospatial/addresses/postcode?postcode=', () => {
});

it.each([
{
postcode: false,
expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression',
},
{
postcode: 1234567,
expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression',
},
{
postcode: null,
expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression',
},
{
postcode: {},
expectedError: 'postcode must match /^[A-Za-z]{1,2}[\\dRr][\\dA-Za-z]?\\s?\\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/ regular expression',
},
{
postcode: '',
expectedError: 'postcode must be longer than or equal to 5 characters',
},
{
postcode: valueGenerator.string({ length: 4 }),
expectedError: 'postcode must be longer than or equal to 5 characters',
Expand Down
4 changes: 3 additions & 1 deletion test/support/generator/get-geospatial-addresses-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator<AddressVa
}

protected transformRawValuesToGeneratedValues(values: AddressValues[], { postcode, key }: GenerateOptions): GenerateResult {
const useKey = key || 'test';
const useKey = key || this.valueGenerator.string();

const requests: GetAddressesByPostcodeQueryDto[] = values.map((v) => ({ postcode: postcode || v.POSTCODE }) as GetAddressesByPostcodeQueryDto);

Expand Down Expand Up @@ -191,6 +191,7 @@ export class GetGeospatialAddressesGenerator extends AbstractGenerator<AddressVa
requests,
ordnanceSurveyPaths,
mdmPaths,
ordnanceSurveyKey: useKey,
getAddressesByPostcodeResponse,
getAddressesByPostcodeMultipleResponse,
getAddressesOrdnanceSurveyResponse,
Expand Down Expand Up @@ -221,6 +222,7 @@ interface GenerateResult {
requests: GetAddressesByPostcodeQueryDto[];
ordnanceSurveyPaths: string[];
mdmPaths: string[];
ordnanceSurveyKey: string;
getAddressesByPostcodeResponse: GetAddressesResponse[];
getAddressesByPostcodeMultipleResponse: GetAddressesResponse;
getAddressesOrdnanceSurveyResponse: GetAddressesOrdnanceSurveyResponse[];
Expand Down

0 comments on commit 33c9e65

Please sign in to comment.