Skip to content

Commit

Permalink
fix(DTFS2-7203): fix bug where /companies returns 500 for companies w…
Browse files Browse the repository at this point in the history
…/o SIC codes (#932)

## Introduction ✏️
As part of https://ukef-dtfs.atlassian.net/browse/DTFS2-7051, we created
a new APIM endpoint for returning details from Companies House. During
local dev, it was noticed that this new endpoint was returning 500
responses for companies without SIC codes, such as
https://find-and-update.company-information.service.gov.uk/company/SL00911A.

## Resolution ✔️
- fixed bug by adding `?` guard when calling `forEach` on property
- added unit test for bug

## Miscellaneous ➕
- added `?` guard for other properties
- added unit tests for these cases
- fixed lint issue
  • Loading branch information
oscar-richardson-softwire authored Jun 11, 2024
1 parent e19b53a commit d397861
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 30 deletions.
141 changes: 124 additions & 17 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
"eslint-plugin-jest": "^28.5.0",
"eslint-plugin-jest-formatting": "^3.1.0",
"eslint-plugin-n": "^17.7.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-optimize-regex": "^1.2.1",
"eslint-plugin-prettier": "^5.1.3",
"eslint-plugin-security": "^3.0.0",
Expand Down
42 changes: 42 additions & 0 deletions src/modules/companies/companies.service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { resetAllWhenMocks, when } from 'jest-when';

import { SectorIndustriesService } from '../sector-industries/sector-industries.service';
import { CompaniesService } from './companies.service';
import { GetCompanyResponse } from './dto/get-company-response.dto';
import { CompaniesOverseasCompanyException } from './exception/companies-overseas-company-exception.exception';

describe('CompaniesService', () => {
Expand Down Expand Up @@ -80,6 +81,47 @@ describe('CompaniesService', () => {
expect(response).toEqual(getCompanyResponse);
});

it('returns a mapped form of the company returned by the CompaniesHouseService when it has no type', async () => {
const { type: _removed, ...getCompanyCompaniesHouseResponseNoType } = getCompanyCompaniesHouseResponse;

when(companiesHouseServiceGetCompanyByRegistrationNumber).calledWith(testRegistrationNumber).mockReturnValueOnce(getCompanyCompaniesHouseResponseNoType);
when(sectorIndustriesServiceFind).calledWith(null, null).mockReturnValueOnce(findSectorIndustriesResponse);

const response = await service.getCompanyByRegistrationNumber(testRegistrationNumber);

expect(response).toEqual(getCompanyResponse);
});

it('returns a mapped form of the company returned by the CompaniesHouseService when it has no SIC codes', async () => {
const { sic_codes: _removed, ...getCompanyCompaniesHouseResponseNoSicCodes } = getCompanyCompaniesHouseResponse;
const { industries: _removed2, ...getCompanyResponseNoIndustries } = getCompanyResponse;
(getCompanyResponseNoIndustries as GetCompanyResponse).industries = [];

when(companiesHouseServiceGetCompanyByRegistrationNumber)
.calledWith(testRegistrationNumber)
.mockReturnValueOnce(getCompanyCompaniesHouseResponseNoSicCodes);
when(sectorIndustriesServiceFind).calledWith(null, null).mockReturnValueOnce(findSectorIndustriesResponse);

const response = await service.getCompanyByRegistrationNumber(testRegistrationNumber);

expect(response).toEqual(getCompanyResponseNoIndustries);
});

it('returns a mapped form of the company returned by the CompaniesHouseService when it has no registered office address', async () => {
const { registered_office_address: _removed, ...getCompanyCompaniesHouseResponseNoRegisteredOfficeAddress } = getCompanyCompaniesHouseResponse;
const { registeredAddress: _removed2, ...getCompanyResponseNoRegisteredAddress } = getCompanyResponse;
(getCompanyResponseNoRegisteredAddress as GetCompanyResponse).registeredAddress = {};

when(companiesHouseServiceGetCompanyByRegistrationNumber)
.calledWith(testRegistrationNumber)
.mockReturnValueOnce(getCompanyCompaniesHouseResponseNoRegisteredOfficeAddress);
when(sectorIndustriesServiceFind).calledWith(null, null).mockReturnValueOnce(findSectorIndustriesResponse);

const response = await service.getCompanyByRegistrationNumber(testRegistrationNumber);

expect(response).toEqual(getCompanyResponseNoRegisteredAddress);
});

it('throws a NotFoundException if the call to getCompanyByRegistrationNumber on the CompaniesHouseService throws a CompaniesHouseNotFoundException', async () => {
const companiesHouseNotFoundException = new CompaniesHouseNotFoundException(`Company with registration number ${testRegistrationNumber} was not found.`);
when(companiesHouseServiceGetCompanyByRegistrationNumber).calledWith(testRegistrationNumber).mockRejectedValueOnce(companiesHouseNotFoundException);
Expand Down
18 changes: 9 additions & 9 deletions src/modules/companies/companies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class CompaniesService {
}

private validateCompanyIsUkCompany(company: GetCompanyCompaniesHouseResponse, registrationNumber: string): never | undefined {
if (company.type.includes('oversea')) {
if (company.type?.includes('oversea')) {
throw new CompaniesOverseasCompanyException(
`Company with registration number ${registrationNumber} is an overseas company. UKEF can only process applications from companies based in the UK.`,
);
Expand All @@ -53,13 +53,13 @@ export class CompaniesService {
companiesHouseRegistrationNumber: company.company_number,
companyName: company.company_name,
registeredAddress: {
organisationName: address.organisation_name,
addressLine1: address.address_line_1,
addressLine2: address.address_line_2,
addressLine3: address.address_line_3,
locality: address.locality,
postalCode: address.postal_code,
country: address.country,
organisationName: address?.organisation_name,
addressLine1: address?.address_line_1,
addressLine2: address?.address_line_2,
addressLine3: address?.address_line_3,
locality: address?.locality,
postalCode: address?.postal_code,
country: address?.country,
},
industries: this.mapSicCodes(company.sic_codes, industryClasses),
};
Expand All @@ -68,7 +68,7 @@ export class CompaniesService {
private mapSicCodes(sicCodes: string[], industryClasses: SectorIndustryEntity[]): Industry[] {
const industries = [];

sicCodes.forEach((sicCode) => {
sicCodes?.forEach((sicCode) => {
industryClasses.forEach((industryClass) => {
if (sicCode === industryClass.ukefIndustryId) {
industries.push({
Expand Down
Loading

0 comments on commit d397861

Please sign in to comment.