Address code review

libs/shared/base-abstract-classes/src/service/editors/editor.service.ts

@@ -84,7 +84,6 @@ export abstract class EditorService<T extends TableRow> extends SubscriptionHand
   protected initForm(): void {
     const defaultValues = new this._entityClass();
 
-    // Initialize the FormGroup
     this._form = new FormGroup<ModelForm<T>>({} as any);
 
     // Loop through the fields and initialize controls with default values

libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts

@@ -161,7 +161,6 @@ describe('SingleRowEditorService', () => {
   });
 
   describe('updateFormAfterReload()', () => {
-    let consoleErrorSpy: Spy;
     let consoleWarnSpy: Spy;
     let mockForm: any;
 
@@ -179,8 +178,7 @@ describe('SingleRowEditorService', () => {
       // Mock originalValue
       service['_originalValue'] = { id: 123, name: 'Test Name', guid: 456 }; // Add guid value
 
-      // Spy on console.error and console.warn
-      consoleErrorSpy = spyOn(console, 'error');
+      // Spy on console.warn
       consoleWarnSpy = spyOn(console, 'warn');
 
       // Temporarily override `fields` for testing
@@ -198,17 +196,6 @@ describe('SingleRowEditorService', () => {
       expect(mockForm.controls.guid.setValue).toHaveBeenCalledWith(456);
     });
 
-    it('should log an error for missing controls', () => {
-      // Override `fields` to include an invalid field
-      Object.defineProperty(service, 'fields', {
-        value: ['id', 'missingField'],
-      });
-
-      service['updateFormAfterReload']();
-
-      expect(consoleErrorSpy).toHaveBeenCalledWith("Control 'missingField' does not exist!");
-    });
-
     it('should log a warning for invalid field types', () => {
       service['updateFormAfterReload']();
 

libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts

@@ -20,7 +20,7 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
     this.initForm();
   }
 
-  protected override initForm() {
+  protected override initForm(): void {
     super.initForm();
 
     this.subscriptions.push(
@@ -57,7 +57,7 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
   /*
    *  ****** onReloadSuccessful() and its helpers ******
    */
-  protected onLoadedExistingEntity(entity: T) {
+  protected onLoadedExistingEntity(entity: T): void {
     this._originalValue = entity;
     this._isNew = false;
 
@@ -71,7 +71,7 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
     }
   }
 
-  protected onCreatingNewEntity(id: string | number) {
+  protected onCreatingNewEntity(id: string | number): void {
     this._originalValue = new this._entityClass();
 
     // TODO: get rid of this type hack, see: https://github.com/microsoft/TypeScript/issues/32704
@@ -80,11 +80,11 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
     this._isNew = true;
   }
 
-  protected setLoadedEntity() {
+  protected setLoadedEntity(): void {
     this._loadedEntityId = this._originalValue[this._entityIdField];
   }
 
-  protected updateFormAfterReload() {
+  protected updateFormAfterReload(): void {
     this._loading = true;
 
     for (const field of this.fields) {
@@ -95,12 +95,6 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
         if (control) {
           const value = this._originalValue[field as keyof T]; // Ensure type safety here
           control.setValue(value as T[typeof field]);
-        } else {
-          console.error(`Control '${field}' does not exist!`);
-          console.log(`----------- DEBUG CONTROL KEYS:`);
-          for (const k of Object.keys(this._form.controls)) {
-            console.log(k);
-          }
         }
       } else {
         console.warn(`Field '${String(field)}' is not a valid string key.`);
@@ -110,7 +104,7 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
     this._loading = false;
   }
 
-  protected onReloadSuccessful(data: T[], id: string | number) {
+  protected onReloadSuccessful(data: T[], id: string | number): void {
     if (data.length > 0) {
       // we are loading an existing entity
       this.onLoadedExistingEntity(data[0]);
@@ -122,5 +116,6 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS
     this.setLoadedEntity();
     this.updateFullQuery();
   }
+
   /* ****** */
 }

libs/shared/common-services/src/validation.service.spec.ts

@@ -3,28 +3,32 @@ import { ValidationService } from './validation.service';
 import { take } from 'rxjs/operators';
 
 describe('ValidationService', () => {
-  let service: ValidationService;
-
-  beforeEach(() => {
+  beforeEach(() =>
     TestBed.configureTestingModule({
       providers: [ValidationService],
-    });
+    }),
+  );
 
-    service = TestBed.inject(ValidationService);
-  });
+  function setup(): { service: ValidationService } {
+    const service = TestBed.inject(ValidationService);
+    return { service };
+  }
 
   it('should be created', () => {
+    const { service } = setup();
     expect(service).toBeTruthy();
   });
 
   it('should have a default value of true for validationPassed$', (done: DoneFn) => {
+    const { service } = setup();
     service.validationPassed$.pipe(take(1)).subscribe((value) => {
       expect(value).toBe(true);
       done();
     });
   });
 
   it('should set control validity and update validationPassed$', (done: DoneFn) => {
+    const { service } = setup();
     service.setControlValidity('control1', false);
 
     service.validationPassed$.pipe(take(1)).subscribe((value) => {
@@ -41,6 +45,7 @@ describe('ValidationService', () => {
   });
 
   it('should handle removing non-existing controls gracefully', (done: DoneFn) => {
+    const { service } = setup();
     service.removeControl('nonExistentControl');
 
     service.validationPassed$.pipe(take(1)).subscribe((value) => {
@@ -50,6 +55,7 @@ describe('ValidationService', () => {
   });
 
   it('should correctly update validation state with multiple controls', (done: DoneFn) => {
+    const { service } = setup();
     service.setControlValidity('control1', true);
     service.setControlValidity('control2', true);
     service.setControlValidity('control3', false);

libs/shared/common-services/src/validation.service.ts

@@ -5,7 +5,7 @@ import { BehaviorSubject } from 'rxjs';
   providedIn: 'root',
 })
 export class ValidationService {
-  private controlsValidityMap = new Map<any, boolean>();
+  private readonly controlsValidityMap = new Map<any, boolean>();
   readonly validationPassed$: BehaviorSubject<boolean> = new BehaviorSubject(true);
 
   setControlValidity(control: any, isValid: boolean): void {

libs/shared/directives/src/validate-input.directive.spec.ts

@@ -1,5 +1,5 @@
-import { Component, DebugElement } from '@angular/core';
-import { TestBed, ComponentFixture, fakeAsync, tick } from '@angular/core/testing';
+import { Component } from '@angular/core';
+import { TestBed, fakeAsync, tick } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 import { ReactiveFormsModule, FormControl, FormsModule, NgControl } from '@angular/forms';
 import { InputValidationDirective } from './validate-input.directive';
@@ -19,22 +19,22 @@ class TestComponent {
 }
 
 describe('InputValidationDirective', () => {
-  let fixture: ComponentFixture<TestComponent>;
-  let debugElement: DebugElement;
-
-  beforeEach(() => {
+  function setup() {
     TestBed.configureTestingModule({
       declarations: [TestComponent],
       imports: [ReactiveFormsModule, FormsModule, InputValidationDirective],
       providers: [ValidationService],
     }).compileComponents();
 
-    fixture = TestBed.createComponent(TestComponent);
-    debugElement = fixture.debugElement.query(By.directive(InputValidationDirective));
+    const fixture = TestBed.createComponent(TestComponent);
+    const debugElement = fixture.debugElement.query(By.directive(InputValidationDirective));
     fixture.detectChanges();
-  });
+
+    return { fixture, debugElement };
+  }
 
   it('should display error message when control is invalid and touched', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
     control?.setValidators(() => ({ required: true }));
     control?.markAsTouched();
@@ -49,6 +49,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should remove error message when control becomes valid', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
     control?.setValidators(() => ({ required: true }));
     control?.markAsTouched();
@@ -72,6 +73,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should handle multiple error types and display first error', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
 
     control?.setValidators(() => ({ required: true, minlength: true }));
@@ -87,6 +89,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should mark control as touched when invalid', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
     spyOn(control!, 'markAsTouched').and.callThrough();
 
@@ -101,6 +104,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should safely remove errorDiv if already exists', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
 
     // Set invalid state to create an errorDiv
@@ -126,6 +130,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should display correct error message for "required" error', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
 
     control?.setValidators(() => ({ required: true }));
@@ -141,6 +146,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should display "Invalid field" for non-required errors', fakeAsync(() => {
+    const { fixture, debugElement } = setup();
     const control = debugElement.injector.get(NgControl).control;
 
     control?.setValidators(() => ({ minlength: { requiredLength: 5, actualLength: 3 } }));
@@ -156,6 +162,7 @@ describe('InputValidationDirective', () => {
   }));
 
   it('should return early if control is null (explicit)', () => {
+    const { debugElement } = setup();
     const directive = debugElement.injector.get(InputValidationDirective);
     const ngControl = debugElement.injector.get(NgControl);