Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/CB2-12313: Refactor certificate generation into commands #193

Merged
merged 7 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions package-lock.json

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

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
"aws-lambda": "^1.0.5",
"aws-xray-sdk": "^3.6.0",
"js-yaml": "^3.14.1",
"lodash.merge": "^4.6.2",
"moment": "^2.24.0",
"node-yaml": "^4.0.1",
"reflect-metadata": "^0.1.13",
Expand All @@ -80,6 +81,7 @@
"@types/jest": "^28.1.8",
"@types/jest-plugin-context": "^2.9.2",
"@types/lambda-tester": "^4.0.3",
"@types/lodash.merge": "^4.6.9",
"@types/node": "^12.12.5",
"@types/sinon": "^7.5.0",
"@types/uuid": "^10.0.0",
Expand Down
94 changes: 94 additions & 0 deletions src/certificate/CertificatePayloadGenerator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import merge from 'lodash.merge';
import { Service } from 'typedi';
import { ITestResult } from '../models';
import { ICertificatePayload } from '../models';
import { CERTIFICATE_DATA } from '../models/Enums';
import { CertificatePayloadStateBag } from './CertificatePayloadStateBag';
import { ICertificatePayloadCommand } from './ICertificatePayloadCommand';
import { AdrCertificateCommand } from './commands/AdrCertificateCommand';
import { DefectsCommand } from './commands/DefectsCommand';
import { IvaCertificateCommand } from './commands/IvaCertificateCommand';
import { MakeAndModelCommand } from './commands/MakeAndModelCommand';
import { MsvaCertificateCommand } from './commands/MsvaCertificateCommand';
import { OdometerHistoryCommand } from './commands/OdometerHistoryCommand';
import { PassOrFailCertificateCommand } from './commands/PassOrFailCertificateCommand';
import { RoadworthinessCertificateCommand } from './commands/RoadworthinessCertificateCommand';
import { SignatureCommand } from './commands/SignatureCommand';
import { TestHistoryCommand } from './commands/TestHistoryCommand';
import { WatermarkCommand } from './commands/WatermarkCommand';

@Service()
export class CertificatePayloadGenerator implements ICertificatePayloadCommand {
private commands: ICertificatePayloadCommand[] = [
this.passOrFailGenerator,
this.rwtGenerator,
this.adrGenerator,
this.ivaGenerator,
this.msvaGenerator,
this.signatureCommand,
this.watermarkCommand,
this.testHistoryCommand,
this.defectsCommand,
this.makeAndModelCommand,
this.odometerHistoryCommand,
];

/**
* Creates a new instance of the certificate payload generator. Generates a payload
* that can be used for generating a certificate.
*/
constructor(
private passOrFailGenerator: PassOrFailCertificateCommand,
private rwtGenerator: RoadworthinessCertificateCommand,
private adrGenerator: AdrCertificateCommand,
private ivaGenerator: IvaCertificateCommand,
private msvaGenerator: MsvaCertificateCommand,
private signatureCommand: SignatureCommand,
private watermarkCommand: WatermarkCommand,
private testHistoryCommand: TestHistoryCommand,
private defectsCommand: DefectsCommand,
private makeAndModelCommand: MakeAndModelCommand,
private odometerHistoryCommand: OdometerHistoryCommand
) {}

/**
* Generates certificate data for a given test result and certificate type
* @param testResult - the source test result for certificate generation
* @param type - the certificate type
* @param isWelsh - the boolean value whether the atf where test was conducted resides in Wales
*/
public async generateCertificateData(
testResult: ITestResult,
type: CERTIFICATE_DATA,
isWelsh = false
): Promise<ICertificatePayload> {
this.initialise({
type,
isWelsh,
testResult,
});
return this.generate();
}

/**
* Initialises the certificate generation process.
* @param type The type of certificate to generate
* @param isWelsh True if a Welsh certificate should also be generated.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the end of the world but it looks like this was the doc block prior to the state bag implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove this in another PR so we don't have to re-request reviews, if that's ok? 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes no problem, thanks.

public initialise(state: CertificatePayloadStateBag) {
this.commands.forEach((cmd) => cmd.initialise(state));
}

/**
* Generates certificate data for a given test result and certificate type
* @param testResult the source test result for certificate generation
* @returns A generated certificate payload
*/
public async generate(): Promise<ICertificatePayload> {
// Map over all the commands and get their certificate data.
const results = await Promise.all(this.commands.map((cmd) => cmd.generate()));

// Flatten all the certificate data into our result payload.
return Promise.resolve(merge({} as ICertificatePayload, ...results));
}
}
8 changes: 8 additions & 0 deletions src/certificate/CertificatePayloadStateBag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ITestResult } from '../models';
import { CERTIFICATE_DATA } from '../models/Enums';

export type CertificatePayloadStateBag = {
type: CERTIFICATE_DATA;
isWelsh: boolean;
testResult: ITestResult;
};
40 changes: 40 additions & 0 deletions src/certificate/CertificateTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { Service } from 'typedi';
import { IMOTConfig } from '../models';
import { Configuration } from '../utils/Configuration';

@Service()
export class CertificateTypes {
private readonly config: Configuration = Configuration.getInstance();

public getCertificateType(type: string): string {
const config: IMOTConfig = this.config.getMOTConfig();

const certTypes = {
psv_pass: config.documentNames.vtp20,
psv_pass_bilingual: config.documentNames.vtp20_bilingual,
psv_fail: config.documentNames.vtp30,
psv_fail_bilingual: config.documentNames.vtp30_bilingual,
psv_prs: config.documentNames.psv_prs,
psv_prs_bilingual: config.documentNames.psv_prs_bilingual,
hgv_pass: config.documentNames.vtg5,
hgv_pass_bilingual: config.documentNames.vtg5_bilingual,
hgv_fail: config.documentNames.vtg30,
hgv_fail_bilingual: config.documentNames.vtg30_bilingual,
hgv_prs: config.documentNames.hgv_prs,
hgv_prs_bilingual: config.documentNames.hgv_prs_bilingual,
trl_pass: config.documentNames.vtg5a,
trl_pass_bilingual: config.documentNames.vtg5a_bilingual,
trl_fail: config.documentNames.vtg30,
trl_fail_bilingual: config.documentNames.vtg30_bilingual,
trl_prs: config.documentNames.trl_prs,
trl_prs_bilingual: config.documentNames.trl_prs_bilingual,
rwt: config.documentNames.rwt,
adr_pass: config.documentNames.adr_pass,
iva_fail: config.documentNames.iva_fail,
msva_fail: config.documentNames.msva_fail,
};

const keyTyped = type as keyof typeof certTypes;
return certTypes[keyTyped];
}
}
17 changes: 17 additions & 0 deletions src/certificate/ICertificatePayloadCommand.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { ICertificatePayload } from '../models';
import { CertificatePayloadStateBag } from './CertificatePayloadStateBag';

export interface ICertificatePayloadCommand {
initialise(state: CertificatePayloadStateBag): void;
generate(): Promise<ICertificatePayload>;
}

export abstract class BasePayloadCommand implements ICertificatePayloadCommand {
protected state: CertificatePayloadStateBag = {} as CertificatePayloadStateBag;

initialise(state: CertificatePayloadStateBag) {
this.state = state;
}

abstract generate(): Promise<ICertificatePayload>;
}
65 changes: 65 additions & 0 deletions src/certificate/commands/AdrCertificateCommand.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am i right in thinking, that these commands are a way of busting out what was in the certificate service?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, basically. We want to adhere to the SOLID princples. So each command becomes it's own isolated unit of code to maintain / extend.

Each command becomes easier to parse and extend, in addition the certificate service becomes easier to parse.

In the future we might want to apply a decorator to some of these commands, but again I'm just trying to limit the structural changes here.

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { Service } from 'typedi';
import { ICertificatePayload } from '../../models';
import { CERTIFICATE_DATA } from '../../models/Enums';
import { TechRecordType } from '../../models/Types';
import { TechRecordService } from '../../tech-record/TechRecordService';
import { BasePayloadCommand } from '../ICertificatePayloadCommand';

@Service()
export class AdrCertificateCommand extends BasePayloadCommand {
constructor(private techRecordService: TechRecordService) {
super();
}

private certificateIsAnAdr = (): boolean => this.state.type === CERTIFICATE_DATA.ADR_DATA;

public async generate(): Promise<ICertificatePayload> {
if (!this.certificateIsAnAdr()) {
return {} as ICertificatePayload;
}

const { testResult } = this.state;

const adrDetails: TechRecordType<any> = await this.techRecordService.getAdrDetails(testResult);
const makeAndModel = await this.techRecordService.getVehicleMakeAndModel(testResult);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As these appear non dependent, we could optimise this by utilising Promise.all to make these requests in parallel e.g.

const [adrDetails, makeAndModel] = await Promise.all([
  this.techRecordService.getAdrDetails(testResult),
  this.techRecordService.getVehicleMakeAndModel(testResult)
]);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot! Will update 👍


const docGenPayloadAdr = {
ChasisNumber: testResult.vin,
RegistrationNumber: testResult.vrm,
ApplicantDetails: {
name: adrDetails?.techRecord_applicantDetails_name,
address1: adrDetails?.techRecord_applicantDetails_address1,
address2: adrDetails?.techRecord_applicantDetails_address2,
address3: adrDetails?.techRecord_applicantDetails_address1,
postTown: adrDetails?.techRecord_applicantDetails_postTown,
postCode: adrDetails?.techRecord_applicantDetails_postCode,
telephoneNumber: adrDetails?.techRecord_applicantDetails_telephoneNumber,
emailAddress: adrDetails?.techRecord_applicantDetails_emailAddress,
},
VehicleType: adrDetails?.techRecord_adrDetails_vehicleDetails_type,
PermittedDangerousGoods: adrDetails?.techRecord_adrDetails_permittedDangerousGoods,
BrakeEndurance: adrDetails?.techRecord_adrDetails_brakeEndurance,
Weight: adrDetails?.techRecord_adrDetails_weight,
TankManufacturer: adrDetails?.techRecord_adrDetails_tank_tankDetails_tankStatement_statement
? adrDetails.techRecord_adrDetails_tank_tankDetails_tankManufacturer
: undefined,
Tc2InitApprovalNo: adrDetails?.techRecord_adrDetails_tank_tankDetails_tc2Details_tc2IntermediateApprovalNo,
TankManufactureSerialNo: adrDetails?.techRecord_adrDetails_tank_tankDetails_tankManufacturerSerialNo,
YearOfManufacture: adrDetails?.techRecord_adrDetails_tank_tankDetails_yearOfManufacture,
TankCode: adrDetails?.techRecord_adrDetails_tank_tankDetails_tankCode,
SpecialProvisions: adrDetails?.techRecord_adrDetails_tank_tankDetails_specialProvisions,
TankStatement: adrDetails?.techRecord_adrDetails_tank_tankDetails_tankStatement_statement,
ExpiryDate: testResult.testTypes.testExpiryDate,
AtfNameAtfPNumber: testResult.testStationName + ' ' + testResult.testStationPNumber,
Notes: testResult.testTypes.additionalNotesRecorded,
TestTypeDate: testResult.testTypes.testTypeStartTimestamp,
};

return {
ADR_DATA: {
...docGenPayloadAdr,
...makeAndModel,
},
} as ICertificatePayload;
}
}
Loading
Loading