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.

Sign up for GitHub

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

Jump to bottom

feat(cb2-9882): Add ability to produce a rejection letter for TRLs on provisional tech records #1350

Merged
merged 9 commits into from
Jan 10, 2024
Merged

feat(cb2-9882): Add ability to produce a rejection letter for TRLs on provisional tech records #1350

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cf46f91
feat(cb2-9882): provisional letter work
JunYanBJSS Jan 8, 2024
fdc6abf
feat(cb2-9882): fix the unexpected display buy
JunYanBJSS Jan 8, 2024
5fe6f13
feat(cb2-9882): refactor code to more readable
JunYanBJSS Jan 8, 2024
2521bf2
feat(cb2-9882): add unit tests
JunYanBJSS Jan 9, 2024
ab512e7
feat(cb2-9882): remove unnecessary variable
JunYanBJSS Jan 9, 2024
11730cf
feat(cb2-9882): amend variable name
JunYanBJSS Jan 9, 2024
e786c4c
feat(cb2-9882): amend the logic of ineligibility
JunYanBJSS Jan 10, 2024
5dc7382
feat(cb2-9882): fix memory leak
pbardy2000 Jan 10, 2024
456ca67
feat(cb2-9882): update tests
pbardy2000 Jan 10, 2024
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
7 changes: 6 additions & 1 deletion ...ch-record/components/tech-record-generate-letter/tech-record-generate-letter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
CustomFormControl, FormNodeOption, FormNodeTypes, FormNodeWidth,
} from '@forms/services/dynamic-form.types';
import { LETTER_TYPES } from '@forms/templates/general/letter-types';
import { V3TechRecordModel } from '@models/vehicle-tech-record.model';
import { StatusCodes, V3TechRecordModel } from '@models/vehicle-tech-record.model';
import { Actions, ofType } from '@ngrx/effects';
import { Store } from '@ngrx/store';
import { TechnicalRecordService } from '@services/technical-record/technical-record.service';
Expand Down Expand Up @@ -64,6 +64,11 @@ export class GenerateLetterComponent implements OnInit {
}

get reasons(): Array<FormNodeOption<string>> {
if (this.techRecord?.techRecord_statusCode === StatusCodes.PROVISIONAL) {
return [
{ label: 'Trailer rejected', value: LETTER_TYPES[1].value },
];
}
return [
{ label: 'Trailer accepted', value: LETTER_TYPES[0].value },
{ label: 'Trailer rejected', value: LETTER_TYPES[1].value },
Expand Down
36 changes: 36 additions & 0 deletions src/app/forms/custom-sections/letters/letters.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,22 @@ import { UserService } from '@services/user-service/user-service';
import { SharedModule } from '@shared/shared.module';
import { State, initialAppState } from '@store/index';
import { of } from 'rxjs';
import { StatusCodes } from '@models/vehicle-tech-record.model';
import { TechnicalRecordService } from '@services/technical-record/technical-record.service';
import { TechRecordSearchSchema } from '@dvsa/cvs-type-definitions/types/v3/tech-record/get/search';
import { LettersComponent } from './letters.component';

const mockTechRecordService = {
techRecordHistory$: of([{
vin: 'test',
techRecord_statusCode: 'current',
techRecord_vehicleType: 'trl',
createdTimestamp: '12345',
systemNumber: '123',
techRecord_manufactureYear: 2021,
}] as TechRecordSearchSchema[]),
};

describe('LettersComponent', () => {
let component: LettersComponent;
let fixture: ComponentFixture<LettersComponent>;
Expand All @@ -41,8 +55,13 @@ describe('LettersComponent', () => {
provide: APP_BASE_HREF,
useValue: '/',
},
{
provide: TechnicalRecordService,
shivangidas marked this conversation as resolved.
Show resolved Hide resolved
useValue: mockTechRecordService,
},
],
}).compileComponents();

});

beforeEach(() => {
Expand Down Expand Up @@ -70,6 +89,23 @@ describe('LettersComponent', () => {
(component.techRecord as TechRecordType<'trl'>).techRecord_approvalType = ApprovalType.NTA;
expect(component.eligibleForLetter).toBeFalsy();
});

it('should return false if the statuscode is archived', () => {
(component.techRecord as TechRecordType<'trl'>).techRecord_approvalType = ApprovalType.GB_WVTA;
(component.techRecord as TechRecordType<'trl'>).techRecord_statusCode = StatusCodes.ARCHIVED;
expect(component.eligibleForLetter).toBeFalsy();
});
});

describe('checkRecordHistoryHasCurrent', () => {
it('should return false if the current technical record history has current status', () => {
expect(component.checkForCurrentRecordInHistory).toBeFalsy();
});

it('should return true if the provisional technical record history has current status', () => {
(component.techRecord as TechRecordType<'trl'>).techRecord_statusCode = 'provisional';
expect(component.checkForCurrentRecordInHistory).toBeTruthy();
});
});

describe('letter', () => {
Expand Down
28 changes: 23 additions & 5 deletions src/app/forms/custom-sections/letters/letters.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output,
} from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { TechRecordSearchSchema } from '@dvsa/cvs-type-definitions/types/v3/tech-record/get/search';
import { ParagraphIds } from '@dvsa/cvs-type-definitions/types/v3/tech-record/get/trl/complete';
import { TechRecordType } from '@dvsa/cvs-type-definitions/types/v3/tech-record/tech-record-vehicle-type';
import { TechRecordType as TechRecordTypeVehicleVerb } from '@dvsa/cvs-type-definitions/types/v3/tech-record/tech-record-verb-vehicle-type';
Expand All @@ -13,6 +14,7 @@ import { LettersTemplate } from '@forms/templates/general/letters.template';
import { Roles } from '@models/roles.enum';
import { LettersIntoAuthApprovalType, LettersOfAuth, StatusCodes } from '@models/vehicle-tech-record.model';
import { Store } from '@ngrx/store';
import { TechnicalRecordService } from '@services/technical-record/technical-record.service';
import { updateScrollPosition } from '@store/technical-records';
import { Subscription, debounceTime } from 'rxjs';

Expand All @@ -33,6 +35,7 @@ export class LettersComponent implements OnInit, OnDestroy, OnChanges {

constructor(
private dynamicFormService: DynamicFormService,
private techRecordService: TechnicalRecordService,
private viewportScroller: ViewportScroller,
private router: Router,
private route: ActivatedRoute,
Expand Down Expand Up @@ -73,20 +76,35 @@ export class LettersComponent implements OnInit, OnDestroy, OnChanges {
}
: undefined;
}
// checking if the Technical Record History has current status code.
get checkForCurrentRecordInHistory(): boolean {
let hasCurrent = false;
this.techRecordService.techRecordHistory$.subscribe((historyArray: TechRecordSearchSchema[] | undefined) => {
historyArray?.forEach((history: TechRecordSearchSchema) => {
if (history.techRecord_statusCode === StatusCodes.CURRENT
&& this.techRecord?.techRecord_statusCode === StatusCodes.PROVISIONAL) hasCurrent = true;
});
});
return hasCurrent;
}
Copy link
Contributor

@pbardy2000 pbardy2000 Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

All the code in this PR works functionally, and I'm happy with it, but this bit causes a memory leak (can show you how if needed). Not sure what standard practice in VTM is @shivangidas but this looks like it could be its own selector on the tech record history service that you:
a. Subscribe to on ngOnInit
b. Have a takeUntil destroyed pipe, so it stops listening after this component is destroyed (stopping the memory leak)
c. assign as a member of the class so it doesn't create a new subscription on every component re-render, as it currently does using the getter approach.

Copy link
Contributor

@shivangidas shivangidas Jan 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Currently, there's a bit of everything, but would certainly like the solution recommended, think there already is a tech record history selector present selectTechRecordHistory here


get eligibleForLetter(): boolean {
const currentTechRecord = this.techRecord?.techRecord_statusCode === StatusCodes.CURRENT;

return this.correctApprovalType && currentTechRecord && !this.isEditing;
const isArchivedTechRecord = this.techRecord?.techRecord_statusCode === StatusCodes.ARCHIVED;
return this.correctApprovalType && !isArchivedTechRecord && !this.isEditing && !this.checkForCurrentRecordInHistory;
}

get reasonForIneligibility(): string {
if (this.isEditing) {
return 'This section is not available when amending or creating a technical record.';
}

if (this.techRecord?.techRecord_statusCode !== StatusCodes.CURRENT) {
return 'Generating letters is only applicable to current technical records.';
if (this.techRecord?.techRecord_statusCode === StatusCodes.PROVISIONAL) {
if (this.checkForCurrentRecordInHistory) {
// eslint-disable-next-line max-len
return 'Generating letters is not applicable to provisional records, where a current record also exists for a vehicle. Open the current record to generate letters.';
}
} else if (this.techRecord?.techRecord_statusCode === StatusCodes.ARCHIVED) {
return 'Generating letters is not applicable to archived technical records.';
}

if (!this.correctApprovalType) {
Expand Down