Skip to content

Commit

Permalink
EUI-8737: Fix FirstErrorPipe implementation and Welsh Translation sup…
Browse files Browse the repository at this point in the history
…port (#1541)

Reimplement `FirstErrorPipe` to use the `RpxTranslationService` directly instead of `RpxTranslatePipe`; remove incorrect use of `RpxTranslatePipe` from palette components where `FirstErrorPipe` is used in the component's HTML template.

Co-authored-by: David Rajkumar Jayakumar <[email protected]>
  • Loading branch information
Daniel-Lam and DavidJayakumar authored Dec 8, 2023
1 parent 1ed91b6 commit 7c17bc1
Show file tree
Hide file tree
Showing 22 changed files with 98 additions and 44 deletions.
6 changes: 6 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
## RELEASE NOTES
### Version 6.19.15-welsh-translation-FirstErrorPipe-fix
**EUI-8737** Re-tag for re-release following deployment of Restricted Case Access feature to live

### Version 6.19.15-case-flags-v2-manage-case-flags-flag-update-comments-fix-2
**EUI-9035** Re-tag for re-release following deployment of Restricted Case Access feature to live

Expand All @@ -11,6 +14,9 @@
### Version 6.19.6-restricted-case-access-v4
**EUI-6645** Restricted case access

### Version 6.19.13-welsh-translation-FirstErrorPipe-fix
**EUI-8737** Reimplement `FirstErrorPipe` to use the `RpxTranslationService` directly instead of `RpxTranslatePipe`; remove incorrect use of `RpxTranslatePipe` from palette components where `FirstErrorPipe` is used in the component's HTML template

### Version 6.19.13-case-flags-v2-manage-case-flags-value-caching-fix
**EUI-9020** Fix bug where previous changes to a flag are retained in the UI when the user starts over and selects the same flag to update again

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hmcts/ccd-case-ui-toolkit",
"version": "6.19.15-welsh-yes-no-fixes-v1",
"version": "6.19.15-welsh-translation-FirstErrorPipe-fix",
"engines": {
"yarn": "^3.5.0",
"npm": "^8.10.0"
Expand Down
2 changes: 1 addition & 1 deletion projects/ccd-case-ui-toolkit/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@hmcts/ccd-case-ui-toolkit",
"version": "6.19.15-welsh-yes-no-fixes-v1",
"version": "6.19.15-welsh-translation-FirstErrorPipe-fix",
"engines": {
"yarn": "^3.5.0",
"npm": "^8.10.0"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { DebugElement } from '@angular/core';
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { FormControl, FormGroup, ReactiveFormsModule } from '@angular/forms';
import { RpxTranslatePipe } from 'rpx-xui-translation';
import { RpxTranslatePipe, RpxTranslationService } from 'rpx-xui-translation';
import { of } from 'rxjs';
import { CaseField, FieldType } from '../../../domain/definition';
import { MockRpxTranslatePipe } from '../../../test/mock-rpx-translate.pipe';
import { PaletteUtilsModule } from '../utils';
Expand Down Expand Up @@ -37,6 +38,8 @@ describe('WriteCaseLinkFieldComponent', () => {
let component: WriteCaseLinkFieldComponent;
let fixture: ComponentFixture<WriteCaseLinkFieldComponent>;
let de: DebugElement;
const rpxTranslationServiceSpy = jasmine.createSpyObj('RpxTranslationService', ['getTranslation$', 'translate']);
rpxTranslationServiceSpy.getTranslation$.and.callFake((someString: string) => of(someString));

beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
Expand All @@ -49,7 +52,8 @@ describe('WriteCaseLinkFieldComponent', () => {
MockRpxTranslatePipe
],
providers: [
{ provide: RpxTranslatePipe, useClass: MockRpxTranslatePipe }
{ provide: RpxTranslatePipe, useClass: MockRpxTranslatePipe },
{ provide: RpxTranslationService, useValue: rpxTranslationServiceSpy }
]
})
.compileComponents();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="caseReferenceControl.errors && (caseReferenceControl.dirty || caseReferenceControl.touched)">
{{ caseReferenceControl.errors | ccdFirstError:caseField.label | rpxTranslate}}
{{caseReferenceControl.errors | ccdFirstError:caseField.label}}
</span>
<input class="form-control bottom-30" [id]="id()" type="text" [formControl]="caseReferenceControl">
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ <h2 class="heading-h2 error-spacing">
<h2 class="heading-h2 error-spacing" *ngIf="caseField.hint_text || formArray.errors">
<span *ngIf="caseField.hint_text" class="form-hint">{{caseField.hint_text | rpxTranslate }}</span>
<span *ngIf="formArray.errors" class="error-message">
{{(formArray.errors | ccdFirstError:caseField.label ) | rpxTranslate}}
{{(formArray.errors | ccdFirstError:caseField.label)}}
</span>
</h2>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</legend>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="dateControl && dateControl.errors && (dateControl.dirty || dateControl.touched)">
{{ (dateControl.errors | ccdFirstError:caseField.label ) | rpxTranslate}}
{{(dateControl.errors | ccdFirstError:caseField.label)}}
</span>
<cut-date-input [id]="id()"
[isDateTime]="isDateTime()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<span class="form-label" *ngIf="caseField.label">{{(caseField | ccdFieldLabel) | rpxTranslate }}</span>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message"
*ngIf="dateControl && dateControl.errors && dateControl.dirty && !(minError || maxError)">{{(dateControl.errors | ccdFirstError:caseField.label) | rpxTranslate}}</span>
*ngIf="dateControl && dateControl.errors && dateControl.dirty && !(minError || maxError)">{{(dateControl.errors | ccdFirstError:caseField.label)}}</span>
<span class="error-message"
*ngIf="dateControl && dateControl.dirty && minError">{{'This date is older than the minimum date allowed' | rpxTranslate}}</span>
<span class="error-message"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<span class="form-label" *ngIf="!caseField.label && !caseField.hint_text">{{'Select an option from the dropdown' | rpxTranslate}}</span>
<span class="error-message"
*ngIf="dynamicListFormControl.errors && dynamicListFormControl.dirty">{{(dynamicListFormControl.errors |
ccdFirstError:caseField.label) | rpxTranslate}}</span>
ccdFirstError:caseField.label)}}</span>

<select class="form-control ccd-dropdown bottom-30" [id]="id()"
[formControl]="dynamicListFormControl">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<span class="form-label" *ngIf="!caseField.label && !caseField.hint_text">{{'Select an option below' | rpxTranslate}}</span>
<span class="error-message"
*ngIf="dynamicRadioListControl.errors && dynamicRadioListControl.dirty">{{(dynamicRadioListControl.errors |
ccdFirstError) | rpxTranslate}}</span>
ccdFirstError)}}</span>
</label>

<ng-container>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<span class="form-label" *ngIf="caseField.label">{{(caseField | ccdFieldLabel) | rpxTranslate}}</span>
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="emailControl.errors && (emailControl.dirty || emailControl.touched)">{{(emailControl.errors | ccdFirstError:caseField.label) | rpxTranslate}}</span>
<span class="error-message" *ngIf="emailControl.errors && (emailControl.dirty || emailControl.touched)">{{(emailControl.errors | ccdFirstError:caseField.label)}}</span>

<input class="form-control" [ngClass]="{'govuk-input--error': emailControl.errors && (emailControl.dirty || emailControl.touched)}"
[id]="id()" type="email" [formControl]="emailControl">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="fixedListFormControl.errors && (fixedListFormControl.dirty || fixedListFormControl.touched)">
{{ (fixedListFormControl.errors | ccdFirstError:caseField.label ) | rpxTranslate}}
{{(fixedListFormControl.errors | ccdFirstError:caseField.label)}}
</span>

<select class="form-control ccd-dropdown bottom-30" [id]="id()" [formControl]="fixedListFormControl">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<span class="form-label" *ngIf="caseField.label">{{(caseField | ccdFieldLabel) | rpxTranslate}}</span>
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate }}</span>
<span class="error-message" *ngIf="fixedRadioListControl.errors && (fixedRadioListControl.dirty || fixedRadioListControl.touched)">{{(fixedRadioListControl.errors | ccdFirstError:caseField.label) | rpxTranslate}}</span>
<span class="error-message" *ngIf="fixedRadioListControl.errors && (fixedRadioListControl.dirty || fixedRadioListControl.touched)">{{(fixedRadioListControl.errors | ccdFirstError:caseField.label)}}</span>
</legend>
<ng-container>
<div class="multiple-choice" *ngFor="let radioButton of caseField.field_type.fixed_list_items" [ngClass]="{selected: fixedRadioListControl.value === radioButton.code}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<span class="form-label" *ngIf="caseField.label">{{caseField | ccdFieldLabel | rpxTranslate}}</span>
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="moneyGbpControl.errors && (moneyGbpControl.dirty || moneyGbpControl.touched)">{{moneyGbpControl.errors | ccdFirstError:caseField.label | rpxTranslate}}</span>
<span class="error-message" *ngIf="moneyGbpControl.errors && (moneyGbpControl.dirty || moneyGbpControl.touched)">{{moneyGbpControl.errors | ccdFirstError:caseField.label}}</span>

<div class="form-money">
<span class="form-currency">&#163;</span>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<span class="form-label" *ngIf="caseField.label">{{caseField | ccdFieldLabel | rpxTranslate}}</span>
</legend>
<span *ngIf="caseField.hint_text" class="form-hint">{{caseField.hint_text | rpxTranslate}}</span>
<span *ngIf="checkboxes.errors && (checkboxes.dirty || checkboxes.touched)" class="error-message">{{checkboxes.errors | ccdFirstError:caseField.label | rpxTranslate}}</span>
<span *ngIf="checkboxes.errors && (checkboxes.dirty || checkboxes.touched)" class="error-message">{{checkboxes.errors | ccdFirstError:caseField.label}}</span>

<ng-container *ngFor="let checkbox of caseField.field_type.fixed_list_items">

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="numberControl.errors && numberControl.dirty">
{{ numberControl.errors | ccdFirstError:caseField.label | rpxTranslate}}
{{numberControl.errors | ccdFirstError:caseField.label}}
</span>

<input class="form-control bottom-30" [ngClass]="{'govuk-input--error': numberControl.errors && numberControl.dirty}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="phoneUkControl.errors && (phoneUkControl.dirty || phoneUkControl.touched)">
{{phoneUkControl.errors | ccdFirstError:caseField.label | rpxTranslate}}
{{phoneUkControl.errors | ccdFirstError:caseField.label}}
</span>
<input class="form-control bottom-30" [ngClass]="{'govuk-input--error': phoneUkControl.errors && phoneUkControl.dirty}"
[id]="id()" type="tel" [formControl]="phoneUkControl">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="textareaControl.errors && (textareaControl.dirty || textareaControl.touched)">
{{textareaControl.errors | ccdFirstError:caseField.label | rpxTranslate}}
{{textareaControl.errors | ccdFirstError:caseField.label}}
</span>

<textarea (input)="autoGrow($event)" class="form-control bottom-30" [ngClass]="{'govuk-textarea--error': textareaControl.errors && (textareaControl.dirty || textareaControl.touched)}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
</label>
<span class="form-hint" *ngIf="caseField.hint_text">{{caseField.hint_text | rpxTranslate}}</span>
<span class="error-message" *ngIf="textControl?.errors && (textControl.dirty || textControl.touched)">
{{textControl.errors | ccdFirstError:caseField.label | rpxTranslate}}
{{textControl.errors | ccdFirstError:caseField.label}}
</span>
<input class="form-control bottom-30" [ngClass]="{'govuk-input--error': textControl?.errors && (textControl.dirty || textControl.touched)}"
[id]="id()" type="text" [formControl]="textControl" (blur)="onBlur($event)">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Pipe, PipeTransform } from '@angular/core';
import { RpxTranslatePipe } from 'rpx-xui-translation';
import { ChangeDetectorRef, Injector, Pipe, PipeTransform } from '@angular/core';
import { Replacements, RpxTranslationService } from 'rpx-xui-translation';
import { of } from 'rxjs';
import { FirstErrorPipe } from './first-error.pipe';

@Pipe({ name: 'rpxTranslate' })
Expand All @@ -8,26 +9,23 @@ export class MockRpxTranslatePipe implements PipeTransform {
return value;
}
}

describe('FirstErrorPipe', () => {
const ERROR_MESSAGE = 'This is wrong';
const FIELD_LABEL = 'Field1';

let translationServiceMock: jasmine.SpyObj<RpxTranslationService>;
let changeDetectorRefMock: jasmine.SpyObj<ChangeDetectorRef>;
let injectorMock: jasmine.SpyObj<Injector>;
let firstError: FirstErrorPipe;

beforeEach(() => {
firstError = new FirstErrorPipe({
transform: (value: string, replacements: { [key: string]: string } = {}): string => {
Object.keys(replacements).forEach(key => {
// Ideally use replaceAll here, but that isn't fully compatible with targeted browsers and packaging yet
const search = `%${key}%`;
while (value.indexOf(search) !== -1) {
value = value.replace(search, replacements[key]);
}
});

return value;
}
} as RpxTranslatePipe);
translationServiceMock = jasmine.createSpyObj('RpxTranslationService', ['getTranslation$', 'getTranslationWithReplacements$']);
changeDetectorRefMock = jasmine.createSpyObj('ChangeDetectorRef', ['markForCheck']);
injectorMock = jasmine.createSpyObj('Injector', ['get']);
injectorMock.get.and.returnValue(changeDetectorRefMock);
firstError = new FirstErrorPipe(translationServiceMock, injectorMock);
translationServiceMock.getTranslation$.and.callFake((someString: string) => of(someString));
});

it('should return empty string when null errors', () => {
Expand All @@ -43,59 +41,92 @@ describe('FirstErrorPipe', () => {
});

it('should return only error when 1 error', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
errorkey: ERROR_MESSAGE
});

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith('Field');
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(ERROR_MESSAGE, { FIELDLABEL: 'Field' });
expect(message).toBe(ERROR_MESSAGE);
});

it('should return only first error when multiple errors', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
errorkey: ERROR_MESSAGE,
error2: 'some other error'
});

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith('Field');
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(ERROR_MESSAGE, { FIELDLABEL: 'Field' });
expect(message).toBe(ERROR_MESSAGE);
});

it('should return exact error along with label name when field value is MANDATORY', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
required: true
}, FIELD_LABEL);

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith(FIELD_LABEL);
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(
'%FIELDLABEL% is required', { FIELDLABEL: FIELD_LABEL });
expect(message).toBe(`${FIELD_LABEL} is required`);
});

it('should return exact error along with label name when pattern does not match', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
pattern: {actualValue: 'test ', requiredPattern: '^[0-9 +().-]{9,}$'}
}, FIELD_LABEL);

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith(FIELD_LABEL);
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(
'The data entered is not valid for %FIELDLABEL%', { FIELDLABEL: FIELD_LABEL });
expect(message).toBe(`The data entered is not valid for ${FIELD_LABEL}`);
});

it('should return exact error along with label name when field value is below minimum length', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
minlength: {actualValue: 'test', requiredLength: 5}
}, FIELD_LABEL);

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith(FIELD_LABEL);
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(
'%FIELDLABEL% is below the minimum length', { FIELDLABEL: FIELD_LABEL });
expect(message).toBe(`${FIELD_LABEL} is below the minimum length`);
});

it('should return exact error along with label name when field value exceeds maximum length', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
maxlength: {actualValue: 'test is done', requiredLength: 10}
}, FIELD_LABEL);

expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith(FIELD_LABEL);
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(
'%FIELDLABEL% exceeds the maximum length', { FIELDLABEL: FIELD_LABEL });
expect(message).toBe(`${FIELD_LABEL} exceeds the maximum length`);
});

it('should return exact error along with label name when field value is not valid date', () => {
translationServiceMock.getTranslationWithReplacements$.and.callFake(
(someString: string, someReplacements: Replacements) => of(someString.replace('%FIELDLABEL%', someReplacements['FIELDLABEL'])));
const message = firstError.transform({
matDatetimePickerParse: true
}, FIELD_LABEL);
const errorMessage = 'The date entered is not valid. Please provide a valid date';

expect(message).toBe(`The date entered is not valid. Please provide a valid date`);
expect(translationServiceMock.getTranslation$).toHaveBeenCalledWith(FIELD_LABEL);
expect(translationServiceMock.getTranslationWithReplacements$).toHaveBeenCalledWith(errorMessage, { FIELDLABEL: FIELD_LABEL });
expect(message).toBe(errorMessage);
});
});
Loading

0 comments on commit 7c17bc1

Please sign in to comment.