Skip to content

Commit

Permalink
Merge branch 'feat/DTFS2-7121/create-companies-house-endpoint' of htt…
Browse files Browse the repository at this point in the history
…ps://github.com/UK-Export-Finance/mdm-api into feat/DTFS2-7121/create-companies-house-endpoint
  • Loading branch information
oscar-richardson-softwire committed May 15, 2024
2 parents da0260f + ba0b814 commit 3b194e1
Show file tree
Hide file tree
Showing 23 changed files with 114 additions and 49 deletions.
9 changes: 4 additions & 5 deletions .env.sample
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,16 @@ APIM_INFORMATICA_URL=
APIM_INFORMATICA_USERNAME=
APIM_INFORMATICA_PASSWORD=
APIM_INFORMATICA_MAX_REDIRECTS=
APIM_INFORMATICA_TIMEOUT=
APIM_INFORMATICA_TIMEOUT= # in milliseconds

# ORDNANCE SURVEY
# Use .uk domain, instead of .co.uk
ORDNANCE_SURVEY_URL=
ORDNANCE_SURVEY_URL=https://api.os.uk
ORDNANCE_SURVEY_KEY=
ORDNANCE_SURVEY_MAX_REDIRECTS=
ORDNANCE_SURVEY_TIMEOUT=
ORDNANCE_SURVEY_TIMEOUT= # in milliseconds

# COMPANIES HOUSE
COMPANIES_HOUSE_URL=
COMPANIES_HOUSE_KEY=
COMPANIES_HOUSE_MAX_REDIRECTS=
COMPANIES_HOUSE_TIMEOUT=
COMPANIES_HOUSE_TIMEOUT= # in milliseconds
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,7 @@ The most important prefixes you should have in mind are:
1. `fix:` which represents bug fixes, and correlates to a [SemVer](https://semver.org/) **patch**.
2. `feat:` which represents a new feature, and correlates to a [SemVer](https://semver.org/) **minor**.
3. `feat!:`, `fix!:` or `refactor!:`, etc., which represent a breaking change (indicated by the `!`) and will result in a [SemVer](https://semver.org/) **major**.

### Special notes for .env settings

- ORDNANCE_SURVEY_URL - Domain [https://api.os.co.uk](https://api.os.co.uk) redirects to [https://api.os.uk](https://api.os.uk), so please use [https://api.os.uk](https://api.os.uk).
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"typescript": "^5.4.5"
},
"devDependencies": {
"@commitlint/cli": "^18.6.1",
"@commitlint/cli": "^18.6.0",
"@commitlint/config-conventional": "^18.6.3",
"@nestjs/cli": "^10.3.2",
"@nestjs/schematics": "^10.1.1",
Expand Down
2 changes: 1 addition & 1 deletion src/config/informatica.config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe('informaticaConfig', () => {
{
configPropertyName: 'timeout',
environmentVariableName: 'APIM_INFORMATICA_TIMEOUT',
defaultConfigValue: 30000,
defaultConfigValue: 30000, // in milliseconds
},
];

Expand Down
2 changes: 1 addition & 1 deletion src/config/informatica.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ export default registerAs(
username: process.env.APIM_INFORMATICA_USERNAME,
password: process.env.APIM_INFORMATICA_PASSWORD,
maxRedirects: getIntConfig(process.env.APIM_INFORMATICA_MAX_REDIRECTS, 5),
timeout: getIntConfig(process.env.APIM_INFORMATICA_TIMEOUT, 30000),
timeout: getIntConfig(process.env.APIM_INFORMATICA_TIMEOUT, 30000), // in milliseconds
}),
);
2 changes: 1 addition & 1 deletion src/config/ordnance-survey.config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('ordnanceSurveyConfig', () => {
{
configPropertyName: 'timeout',
environmentVariableName: 'ORDNANCE_SURVEY_TIMEOUT',
defaultConfigValue: 30000,
defaultConfigValue: 30000, // in milliseconds
},
];

Expand Down
2 changes: 1 addition & 1 deletion src/config/ordnance-survey.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export default registerAs(
baseUrl: process.env.ORDNANCE_SURVEY_URL,
key: process.env.ORDNANCE_SURVEY_KEY,
maxRedirects: getIntConfig(process.env.ORDNANCE_SURVEY_MAX_REDIRECTS, 5),
timeout: getIntConfig(process.env.ORDNANCE_SURVEY_TIMEOUT, 30000),
timeout: getIntConfig(process.env.ORDNANCE_SURVEY_TIMEOUT, 30000), // in milliseconds
}),
);
2 changes: 1 addition & 1 deletion src/constants/enums/geospatialCountries.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export enum GeospatialCountriesEnum {
E = 'England',
N = 'Northern Ireland',
S = 'Scotland',
W = 'Wales',
N = 'Northern Ireland',
}
6 changes: 5 additions & 1 deletion src/constants/geospatial.constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ 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',
},
REGEX: {
// UK postcode regex is from DTFS project and slightly optimised by lint.
// https://github.com/UK-Export-Finance/dtfs2/blob/main/portal-api/src/constants/regex.js
UK_POSTCODE: /^[A-Za-z]{1,2}[\dRr][\dA-Za-z]?\s?\d[ABD-HJLNP-UW-Zabd-hjlnp-uw-z]{2}$/,
},
};

This file was deleted.

4 changes: 2 additions & 2 deletions src/helper-modules/ordnance-survey/ordnance-survey.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { OrdnanceSurveyService } from './ordnance-survey.service';
imports: [ConfigModule],
inject: [ConfigService],
useFactory: (configService: ConfigService) => {
const { baseUrl, maxRedirects, timeout } = configService.get<OrdnanceSurveyConfig>(ORDNANCE_SURVEY_CONFIG_KEY);
const { baseUrl: baseURL, maxRedirects, timeout } = configService.get<OrdnanceSurveyConfig>(ORDNANCE_SURVEY_CONFIG_KEY);
return {
baseURL: baseUrl,
baseURL,
maxRedirects,
timeout,
};
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);
});
});
});
6 changes: 3 additions & 3 deletions src/modules/geospatial/geospatial.controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Controller, Get, Query } from '@nestjs/common';
import { Controller, Get, HttpStatus, Query } from '@nestjs/common';
import { ApiNotFoundResponse, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger';

import { GetAddressesByPostcodeQueryDto } from './dto/get-addresses-by-postcode-query.dto';
Expand All @@ -16,12 +16,12 @@ export class GeospatialController {
"A search based on a property's postcode. Will accept a full valid postcode. Returns addresses from Ordnance Survey Delivery Point Address (DPA) system.",
})
@ApiResponse({
status: 200,
status: HttpStatus.OK,
description: 'Returns simplified addresses that are ready to show to users.',
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
11 changes: 6 additions & 5 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,17 +13,18 @@ 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) => {
// Item can have key DPA or LPI, so we get data dynamically, even if we expect key to always be DPA.
const item_data = item[Object.keys(item)[0]];
// Filter out empty values and join values with single space.
const addressLine1 = [item_data.BUILDING_NAME, item_data.BUILDING_NUMBER, item_data.THOROUGHFARE_NAME].filter(Boolean).join(' ');
addresses.push({
organisationName: item_data.ORGANISATION_NAME || null,
// Filter out empty values and join values with single space.
addressLine1: [item_data.BUILDING_NAME, item_data.BUILDING_NUMBER, item_data.THOROUGHFARE_NAME].filter(Boolean).join(' '),
addressLine1,
addressLine2: item_data.DEPENDENT_LOCALITY || null,
addressLine3: null,
locality: item_data.POST_TOWN || null,
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}`);
});
});
Loading

0 comments on commit 3b194e1

Please sign in to comment.