Skip to content

Commit

Permalink
fix(APIM-611): notify, companies house bug fixes (#1113)
Browse files Browse the repository at this point in the history
## Introduction ✏️
This PR aims to mitigate following bugs:

* Large payload entity too large exception (413) thrown when sending
`file` with Base64 buffer when invoking GovNotify endpoint.
* Company house response, where `due_on` property doest not exist for
certain companies.

## Resolution ✔️
* Increased `json` and `urlEncoded` payload size to `2MB`.
* Added optional chain to few `nextAccounts` and `accounts` object
properties.

## Miscellaneous ➕
* Dependencies updates.
* Minor documentation improvement.
* Minor lint fixes.

---------

Co-authored-by: Abhi Markan <amarkan>
  • Loading branch information
abhi-markan authored Dec 18, 2024
1 parent 8e79a01 commit 2c5b84a
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 73 deletions.
120 changes: 69 additions & 51 deletions package-lock.json

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

11 changes: 11 additions & 0 deletions src/app.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { INestApplication, ValidationPipe, VersioningType } from '@nestjs/common';
import { ConfigService } from '@nestjs/config';
import { ApiKeyAuthGuard } from '@ukef/auth/guard/api-key.guard';
import { GOVUK_NOTIFY } from '@ukef/constants';
import { TransformInterceptor } from '@ukef/helpers';
import { SwaggerDocs } from '@ukef/swagger';
import compression from 'compression';
import express from 'express';
import { Logger } from 'nestjs-pino';

export class App {
Expand All @@ -20,6 +22,7 @@ export class App {
const globalPrefix: string = this.getConfig<string>('app.globalPrefix');
const version: string = this.getConfig<string>('app.versioning.version');
const versioningPrefix: string = this.getConfig<string>('app.versioning.prefix');

app.enableVersioning({
type: VersioningType.URI,
defaultVersion: version,
Expand Down Expand Up @@ -54,6 +57,14 @@ export class App {
},
}),
);

/**
* Payload request size
* Default is 100KB this has been increased to 2MB
* to allow GovNotify file inclusion.
*/
app.use(express.json({ limit: GOVUK_NOTIFY.FILE.SIZE.MAX }));
app.use(express.urlencoded({ limit: GOVUK_NOTIFY.FILE.SIZE.MAX, extended: true }));
}

private getConfig<T = any>(key: string): T {
Expand Down
5 changes: 5 additions & 0 deletions src/constants/govuk-notify.constant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,9 @@ export const GOVUK_NOTIFY = {
TRANSACTION_ID: 36,
SERVICE_ID: 36,
},
FILE: {
SIZE: {
MAX: '2mb',
},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ export type GetCompanyCompaniesHouseResponse = {
month: string;
day: string;
};
overdue: boolean;
next_made_up_to: string;
next_due: string;
overdue?: boolean;
next_made_up_to?: string;
next_due?: string;
next_accounts: {
period_start_on: string;
due_on: string;
period_end_on: string;
overdue: boolean;
period_start_on?: string;
due_on?: string;
period_end_on?: string;
overdue?: boolean;
};
};
company_name: string;
Expand Down
1 change: 1 addition & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const main = async () => {
logger.log(`==========================================================`);
logger.log(`Main app will serve on PORT ${app.port}`, 'MainApplication');
logger.log(`==========================================================`);

return app.listen();
};

Expand Down
14 changes: 7 additions & 7 deletions src/modules/companies/companies.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ export class CompaniesService {
type: company.accounts.last_accounts?.period_start_on,
},
nextAccounts: {
dueOn: company.accounts.next_accounts.due_on,
overdue: company.accounts.next_accounts.overdue,
periodEndOn: company.accounts.next_accounts.period_end_on,
periodStartOn: company.accounts.next_accounts.period_start_on,
dueOn: company.accounts.next_accounts?.due_on,
overdue: company.accounts.next_accounts?.overdue,
periodEndOn: company.accounts.next_accounts?.period_end_on,
periodStartOn: company.accounts.next_accounts?.period_start_on,
},
nextDue: company.accounts.next_due,
nextMadeUpTo: company.accounts.next_made_up_to,
overdue: company.accounts.overdue,
nextDue: company.accounts?.next_due,
nextMadeUpTo: company.accounts?.next_made_up_to,
overdue: company.accounts?.overdue,
},
companiesHouseRegistrationNumber: company.company_number,
companyName: company.company_name,
Expand Down
8 changes: 4 additions & 4 deletions src/modules/companies/dto/get-company-response.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ class AccountsLastAccounts {
}

class AccountsNextAccounts {
dueOn: string;
overdue: boolean;
periodEndOn: string;
periodStartOn: string;
dueOn?: string;
overdue?: boolean;
periodEndOn?: string;
periodStartOn?: string;
}

class Accounts {
Expand Down
7 changes: 3 additions & 4 deletions src/modules/emails/dto/post-emails-request.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,14 @@ export class PostEmailsRequestDto {
@IsString()
@IsOptional()
@MinLength(1)
// 100 characters is an arbitrary max limit. GOV.UK Notify can accept references at least 400 characters long.
@MaxLength(100)
@MaxLength(400)
@ApiProperty({
example: GOVUK_NOTIFY.EXAMPLES.REFERENCE,
description: 'Reference for the email sent',
required: false,
nullable: true,
minLength: 1,
maxLength: 100,
maxLength: 400,
})
readonly reference?: string | null;

Expand All @@ -58,7 +57,7 @@ export class PostEmailsRequestDto {
@MaxLength(400)
@ApiProperty({
example: GOVUK_NOTIFY.EXAMPLES.FILE,
description: 'File for GovNotify to consume and generate a link to download',
description: `File for GovNotify to consume and generate a link to download with supported file types. The file size must be smaller than ${GOVUK_NOTIFY.FILE.SIZE.MAX}`,
required: false,
nullable: true,
minLength: 1,
Expand Down

0 comments on commit 2c5b84a

Please sign in to comment.