Bug/1279 metric checkin value is not validated correctly

frontend/cypress/e2e/check-in.cy.ts

@@ -44,6 +44,28 @@ describe('okr check-in', () => {
     cy.contains('- A new action on the action-plan');
   });
 
+  it('should show an error message when new check-in value is not numeric', () => {
+    overviewPage
+      .addKeyResult()
+      .fillKeyResultTitle('This key-result will have errors')
+      .withMetricValues(Unit.NUMBER, '21', '51')
+      .submit();
+
+    const detailPage = keyResultDetailPage
+      .visit('Very important keyresult')
+      .createCheckIn()
+      .fillMetricCheckInValue('asdf');
+    cy.contains('Neuer Wert muss eine Zahl sein.');
+
+    detailPage.fillMetricCheckInValue('21. 2');
+    cy.contains('Neuer Wert muss eine Zahl sein.');
+
+    detailPage.fillMetricCheckInValue('123');
+    cy.contains('Neuer Wert muss eine Zahl sein.')
+      .should('not.exist');
+  });
+
+
   it('should create check-in metric and assert correct owner', () => {
     overviewPage
       .addKeyResult()

frontend/src/app/app.module.ts

@@ -67,7 +67,6 @@ import { CheckInFormComponent } from './components/checkin/check-in-form/check-i
 import { ApplicationTopBarComponent } from './components/application-top-bar/application-top-bar.component';
 import { A11yModule } from '@angular/cdk/a11y';
 import { CustomizationService } from './services/customization.service';
-import { MetricCheckInDirective } from './components/checkin/check-in-form-metric/metric-check-in-directive';
 
 function initOauthFactory(configService: ConfigService, oauthService: OAuthService) {
   return async() => {
@@ -122,8 +121,7 @@ export const MY_FORMATS = {
     CheckInHistoryDialogComponent,
     CheckInFormMetricComponent,
     CheckInFormOrdinalComponent,
-    CheckInFormComponent,
-    MetricCheckInDirective
+    CheckInFormComponent
   ],
   bootstrap: [AppComponent],
   schemas: [CUSTOM_ELEMENTS_SCHEMA],

frontend/src/app/components/checkin/check-in-form-metric/check-in-form-metric.component.html

@@ -8,19 +8,16 @@
             <input
               class="dialog-form-field pe-0"
               [attr.data-testId]="'check-in-metric-value'"
-              [ngClass]="formInputCheck(dialogForm, 'value')"
-              formControlName="value"
+              [ngClass]="formInputCheck(dialogForm, 'metricValue')"
+              formControlName="metricValue"
               id="value"
-              metricCheckIn
             />
           </div>
 
           <span class="okr-form-label col-auto h-fit ps-0">{{ generateUnitLabel() }}</span>
         </form>
       </div>
-      <mat-error *ngIf="hasFormFieldErrors(dialogForm, 'value')">
-        <span>{{ getErrorMessage("MUST_BE_NUMBER", "Neuer Wert") }}</span>
-      </mat-error>
+      <app-error [controlPath]="['metricValue']" [name]="'Neuer Wert'"></app-error>
     </div>
   </div>
 

frontend/src/app/components/checkin/check-in-form-metric/check-in-form-metric.component.spec.ts

@@ -10,6 +10,7 @@ import { MatInputModule } from '@angular/material/input';
 import { MatRadioModule } from '@angular/material/radio';
 import { Unit } from '../../../shared/types/enums/unit';
 import { TranslateTestingModule } from 'ngx-translate-testing';
+// @ts-ignore
 import * as de from '../../../../assets/i18n/de.json';
 
 describe('CheckInFormComponent', () => {
@@ -36,7 +37,7 @@ describe('CheckInFormComponent', () => {
     component.keyResult = keyResultMetric;
     component.checkIn = checkInMetric;
     component.dialogForm = new FormGroup({
-      value: new FormControl<string>('', [Validators.required]),
+      metricValue: new FormControl<string>('', [Validators.required]),
       confidence: new FormControl<number>(5, [Validators.required,
         Validators.min(1),
         Validators.max(10)])
@@ -50,37 +51,87 @@ describe('CheckInFormComponent', () => {
   });
 
   it('should format percent correctly', waitForAsync(async() => {
-    component.keyResult = { ...keyResultMetric,
-      unit: Unit.PERCENT };
+    component.keyResult = {
+      ...keyResultMetric,
+      unit: Unit.PERCENT
+    };
     expect(component.generateUnitLabel())
       .toEqual('%');
   }));
 
   it('should format chf correctly', waitForAsync(async() => {
-    component.keyResult = { ...keyResultMetric,
-      unit: Unit.CHF };
+    component.keyResult = {
+      ...keyResultMetric,
+      unit: Unit.CHF
+    };
     expect(component.generateUnitLabel())
       .toEqual('CHF');
   }));
 
   it('should format eur correctly', waitForAsync(async() => {
-    component.keyResult = { ...keyResultMetric,
-      unit: Unit.EUR };
+    component.keyResult = {
+      ...keyResultMetric,
+      unit: Unit.EUR
+    };
     expect(component.generateUnitLabel())
       .toEqual('EUR');
   }));
 
   it('should format fte correctly', waitForAsync(async() => {
-    component.keyResult = { ...keyResultMetric,
-      unit: Unit.FTE };
+    component.keyResult = {
+      ...keyResultMetric,
+      unit: Unit.FTE
+    };
     expect(component.generateUnitLabel())
       .toEqual('FTE');
   }));
 
   it('should format number correctly', waitForAsync(async() => {
-    component.keyResult = { ...keyResultMetric,
-      unit: Unit.NUMBER };
+    component.keyResult = {
+      ...keyResultMetric,
+      unit: Unit.NUMBER
+    };
     expect(component.generateUnitLabel())
       .toEqual('');
   }));
+
+
+  it.each([
+    [0,
+      true],
+    [150,
+      true],
+    [-23,
+      true],
+    [12.3,
+      true],
+    [-100.3,
+      true],
+    [' 123   ',
+      true],
+    ['    -123.4   ',
+      true],
+    ['12 .3',
+      false],
+    ['1 2',
+      false],
+    ['',
+      false],
+    ['asdf',
+      false],
+    ['a1',
+      false],
+    ['1a',
+      false],
+    [null,
+      false]
+  ])('should correctly validate value input', (value, validity) => {
+    component.dialogForm = new FormGroup({
+      metricValue: new FormControl<number | undefined>(undefined, [Validators.required,
+        Validators.pattern('^\\s*-?\\d+\\.?\\d*\\s*$')])
+    });
+    component.dialogForm.setValue({ metricValue: value });
+    expect(component.dialogForm.valid)
+      .toEqual(validity);
+  });
 });

frontend/src/app/components/checkin/check-in-form-metric/check-in-form-metric.component.ts

@@ -1,8 +1,8 @@
-import { ChangeDetectionStrategy, Component, Input, OnInit } from '@angular/core';
-import { FormGroup, Validators } from '@angular/forms';
+import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
+import { FormGroup } from '@angular/forms';
 import { KeyResultMetric } from '../../../shared/types/model/key-result-metric';
 import { CheckInMin } from '../../../shared/types/model/check-in-min';
-import { formInputCheck, hasFormFieldErrors } from '../../../shared/common';
+import { formInputCheck } from '../../../shared/common';
 import { TranslateService } from '@ngx-translate/core';
 import { CheckInMetricMin } from '../../../shared/types/model/check-in-metric-min';
 
@@ -12,7 +12,7 @@ import { CheckInMetricMin } from '../../../shared/types/model/check-in-metric-mi
   changeDetection: ChangeDetectionStrategy.OnPush,
   standalone: false
 })
-export class CheckInFormMetricComponent implements OnInit {
+export class CheckInFormMetricComponent {
   @Input()
   keyResult!: KeyResultMetric;
 
@@ -24,15 +24,8 @@ export class CheckInFormMetricComponent implements OnInit {
 
   protected readonly formInputCheck = formInputCheck;
 
-  protected readonly hasFormFieldErrors = hasFormFieldErrors;
-
   constructor(private translate: TranslateService) {}
 
-  ngOnInit() {
-    this.dialogForm.controls['value'].setValidators([Validators.required,
-      Validators.pattern('^-?\\d+\\.?\\d*$')]);
-  }
-
   generateUnitLabel(): string {
     switch (this.keyResult.unit) {
       case 'PERCENT':
@@ -48,10 +41,6 @@ export class CheckInFormMetricComponent implements OnInit {
     }
   }
 
-  getErrorMessage(error: string, field: string): string {
-    return field + this.translate.instant('DIALOG_ERRORS.' + error);
-  }
-
   getCheckInMetric(): CheckInMetricMin {
     return this.checkIn as CheckInMetricMin;
   }

frontend/src/app/components/checkin/check-in-form-ordinal/check-in-form-ordinal.component.html

@@ -2,7 +2,7 @@
   <div class="okr-form-row okr-form-label-input-container">
     <label class="okr-form-label okr-form-col" for="ordinal-radio-group">Erreichte Zone wählen:</label>
     <mat-radio-group
-      formControlName="value"
+      formControlName="ordinalZone"
       class="d-flex flex-column gap-2"
       id="ordinal-radio-group"
     >

frontend/src/app/components/checkin/check-in-form-ordinal/check-in-form-ordinal.component.spec.ts

@@ -35,7 +35,7 @@ describe('CheckInFormOrdinalComponent', () => {
     component = fixture.componentInstance;
     component.keyResult = keyResultOrdinalMin as unknown as KeyResultOrdinal;
     component.dialogForm = new FormGroup({
-      value: new FormControl<string>('', [Validators.required]),
+      ordinalZone: new FormControl<string>('', [Validators.required]),
       confidence: new FormControl<number>(5, [Validators.required,
         Validators.min(1),
         Validators.max(10)])
@@ -50,43 +50,43 @@ describe('CheckInFormOrdinalComponent', () => {
   });
 
   it('should set zone of check-in to fail if value is empty', waitForAsync(async() => {
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe('');
   }));
 
   it('should be able to set zone to fail', waitForAsync(async() => {
     const radioButtons = await loader.getAllHarnesses(MatRadioButtonHarness);
     await radioButtons[0].check();
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe(Zone.FAIL);
   }));
 
   it('should be able to set zone to commit', waitForAsync(async() => {
     const radioButtons = await loader.getAllHarnesses(MatRadioButtonHarness);
     await radioButtons[1].check();
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe(Zone.COMMIT);
   }));
 
   it('should be able to set zone to target', waitForAsync(async() => {
     const radioButtons = await loader.getAllHarnesses(MatRadioButtonHarness);
     await radioButtons[2].check();
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe(Zone.TARGET);
   }));
 
   it('should be able to set zone to stretch', waitForAsync(async() => {
     const radioButtons = await loader.getAllHarnesses(MatRadioButtonHarness);
     await radioButtons[3].check();
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe(Zone.STRETCH);
   }));
 
   it('should be able to switch options', waitForAsync(async() => {
     const radioButtons = await loader.getAllHarnesses(MatRadioButtonHarness);
     await radioButtons[3].check();
     await radioButtons[1].check();
-    expect(component.dialogForm.controls['value'].value)
+    expect(component.dialogForm.controls['ordinalZone'].value)
       .toBe(Zone.COMMIT);
   }));
 });