From f6a29cd4040889cfba94e84f47dd7c16b0de1899 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sun, 12 Jan 2025 12:53:20 -0800 Subject: [PATCH 01/13] Fix duplicate disable & component not inheriting style from parent --- apps/keira/src/app/scss/_utils.scss | 4 ++-- .../creature/src/npc-vendor/npc-vendor.component.html | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/keira/src/app/scss/_utils.scss b/apps/keira/src/app/scss/_utils.scss index de32d05261..a0dca10ead 100644 --- a/apps/keira/src/app/scss/_utils.scss +++ b/apps/keira/src/app/scss/_utils.scss @@ -52,8 +52,8 @@ input[type='radio'] { } .item-extended-cost { display: inline-block; - background-color: #080d22; - color: #fff; + background-color: inherit; + color: inherit; padding: 4px 10px 4px 0px; } .item-extended-cost img { diff --git a/libs/features/creature/src/npc-vendor/npc-vendor.component.html b/libs/features/creature/src/npc-vendor/npc-vendor.component.html index 0bcaca76e7..139d24e8be 100644 --- a/libs/features/creature/src/npc-vendor/npc-vendor.component.html +++ b/libs/features/creature/src/npc-vendor/npc-vendor.component.html @@ -21,7 +21,6 @@ From e5cbaaab30bd16bb36179b4035f7b9ff9baa9125 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sun, 12 Jan 2025 12:53:31 -0800 Subject: [PATCH 02/13] Revert "Fix duplicate disable & component not inheriting style from parent" This reverts commit f6a29cd4040889cfba94e84f47dd7c16b0de1899. --- apps/keira/src/app/scss/_utils.scss | 4 ++-- .../creature/src/npc-vendor/npc-vendor.component.html | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/keira/src/app/scss/_utils.scss b/apps/keira/src/app/scss/_utils.scss index a0dca10ead..de32d05261 100644 --- a/apps/keira/src/app/scss/_utils.scss +++ b/apps/keira/src/app/scss/_utils.scss @@ -52,8 +52,8 @@ input[type='radio'] { } .item-extended-cost { display: inline-block; - background-color: inherit; - color: inherit; + background-color: #080d22; + color: #fff; padding: 4px 10px 4px 0px; } .item-extended-cost img { diff --git a/libs/features/creature/src/npc-vendor/npc-vendor.component.html b/libs/features/creature/src/npc-vendor/npc-vendor.component.html index 139d24e8be..0bcaca76e7 100644 --- a/libs/features/creature/src/npc-vendor/npc-vendor.component.html +++ b/libs/features/creature/src/npc-vendor/npc-vendor.component.html @@ -21,6 +21,7 @@ From 0051ff31184255e070521822182b7d6d62f43a7d Mon Sep 17 00:00:00 2001 From: Exitare Date: Fri, 17 Jan 2025 20:20:01 -0800 Subject: [PATCH 03/13] Add logic to allow warnings for input fields --- apps/keira/src/app/scss/_editor.scss | 4 +++ .../creature-template.component.html | 15 ++++++--- .../creature-template.component.ts | 6 ++-- .../quest-preview.service.spec.ts | 2 +- .../src/service/editors/editor.service.ts | 13 ++++++-- .../editors/single-row-editor.service.ts | 28 +++++++++++----- libs/shared/directives/.eslintrc.json | 33 +++++++++++++++++++ libs/shared/directives/README.md | 3 ++ libs/shared/directives/karma.conf.js | 15 +++++++++ libs/shared/directives/project.json | 24 ++++++++++++++ libs/shared/directives/src/index.ts | 1 + .../src/validate-input.directive.ts | 24 ++++++++++++++ libs/shared/directives/tsconfig.json | 29 ++++++++++++++++ libs/shared/directives/tsconfig.lib.json | 12 +++++++ libs/shared/directives/tsconfig.spec.json | 9 +++++ libs/shared/error-templates/.eslintrc.json | 33 +++++++++++++++++++ libs/shared/error-templates/README.md | 3 ++ libs/shared/error-templates/karma.conf.js | 15 +++++++++ libs/shared/error-templates/project.json | 24 ++++++++++++++ libs/shared/error-templates/src/index.ts | 1 + .../src/input-validation-error.ts | 24 ++++++++++++++ libs/shared/error-templates/tsconfig.json | 29 ++++++++++++++++ libs/shared/error-templates/tsconfig.lib.json | 12 +++++++ .../shared/error-templates/tsconfig.spec.json | 9 +++++ tsconfig.base.json | 2 ++ 25 files changed, 352 insertions(+), 18 deletions(-) create mode 100644 libs/shared/directives/.eslintrc.json create mode 100644 libs/shared/directives/README.md create mode 100644 libs/shared/directives/karma.conf.js create mode 100644 libs/shared/directives/project.json create mode 100644 libs/shared/directives/src/index.ts create mode 100644 libs/shared/directives/src/validate-input.directive.ts create mode 100644 libs/shared/directives/tsconfig.json create mode 100644 libs/shared/directives/tsconfig.lib.json create mode 100644 libs/shared/directives/tsconfig.spec.json create mode 100644 libs/shared/error-templates/.eslintrc.json create mode 100644 libs/shared/error-templates/README.md create mode 100644 libs/shared/error-templates/karma.conf.js create mode 100644 libs/shared/error-templates/project.json create mode 100644 libs/shared/error-templates/src/index.ts create mode 100644 libs/shared/error-templates/src/input-validation-error.ts create mode 100644 libs/shared/error-templates/tsconfig.json create mode 100644 libs/shared/error-templates/tsconfig.lib.json create mode 100644 libs/shared/error-templates/tsconfig.spec.json diff --git a/apps/keira/src/app/scss/_editor.scss b/apps/keira/src/app/scss/_editor.scss index a059ffd3a6..e7d6d25879 100644 --- a/apps/keira/src/app/scss/_editor.scss +++ b/apps/keira/src/app/scss/_editor.scss @@ -1,5 +1,9 @@ @import 'variables'; +input.ng-invalid.ng-touched { + border-color: red; +} + .top-bar { background-color: #000; color: $content-block-bg-color; diff --git a/libs/features/creature/src/creature-template/creature-template.component.html b/libs/features/creature/src/creature-template/creature-template.component.html index 44bd7d0869..36989f8c6e 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.html +++ b/libs/features/creature/src/creature-template/creature-template.component.html @@ -47,9 +47,15 @@
- - - + + +
@@ -231,7 +237,8 @@ Models are now available in the Creature Template Model editor.Models are now available in the + Creature Template Model editor.
diff --git a/libs/features/creature/src/creature-template/creature-template.component.ts b/libs/features/creature/src/creature-template/creature-template.component.ts index f311c11baf..23108297db 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.ts +++ b/libs/features/creature/src/creature-template/creature-template.component.ts @@ -25,7 +25,6 @@ import { } from '@keira/shared/acore-world-model'; import { SingleRowEditorComponent } from '@keira/shared/base-abstract-classes'; import { QueryOutputComponent, TopBarComponent } from '@keira/shared/base-editor-components'; -import { Model3DViewerComponent } from '@keira/shared/model-3d-viewer'; import { BooleanOptionSelectorComponent, CreatureSelectorBtnComponent, @@ -41,6 +40,8 @@ import { TooltipModule } from 'ngx-bootstrap/tooltip'; import { CreatureHandlerService } from '../creature-handler.service'; import { CreatureTemplateService } from './creature-template.service'; import { RouterLink } from '@angular/router'; +import { InputValidationDirective } from '@keira/shared/directives'; +import { ValidationFeedbackComponent } from '@keira/shared/error-templates'; @Component({ changeDetection: ChangeDetectionStrategy.OnPush, @@ -59,11 +60,12 @@ import { RouterLink } from '@angular/router'; FlagsSelectorBtnComponent, SpellSelectorBtnComponent, CreatureSelectorBtnComponent, - Model3DViewerComponent, GenericOptionSelectorComponent, BooleanOptionSelectorComponent, IconSelectorComponent, RouterLink, + InputValidationDirective, + ValidationFeedbackComponent, ], }) export class CreatureTemplateComponent extends SingleRowEditorComponent { diff --git a/libs/features/quest/src/quest-preview/quest-preview.service.spec.ts b/libs/features/quest/src/quest-preview/quest-preview.service.spec.ts index 0f7581d301..9dc93a8f2a 100644 --- a/libs/features/quest/src/quest-preview/quest-preview.service.spec.ts +++ b/libs/features/quest/src/quest-preview/quest-preview.service.spec.ts @@ -169,7 +169,7 @@ describe('QuestPreviewService', () => { expect(mysqlQueryService.getItemNameByStartQuest).toHaveBeenCalledTimes(1); expect(mysqlQueryService.getItemNameByStartQuest).toHaveBeenCalledWith(mockID); expect(mysqlQueryService.getReputationRewardByFaction).toHaveBeenCalledTimes(1); - expect(mysqlQueryService.getReputationRewardByFaction).toHaveBeenCalledWith(null as any); + expect(mysqlQueryService.getReputationRewardByFaction).toHaveBeenCalledWith(0); }); it('sqliteQuery', async () => { diff --git a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts index 1ce8a18cf9..97035412b7 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts @@ -1,7 +1,7 @@ import { QueryError } from 'mysql2'; import { ToastrService } from 'ngx-toastr'; import { Observable } from 'rxjs'; -import { FormControl, FormGroup } from '@angular/forms'; +import { FormControl, FormGroup, Validators } from '@angular/forms'; import { Class, StringKeys, TableRow } from '@keira/shared/constants'; import { MysqlQueryService } from '@keira/shared/db-layer'; @@ -82,10 +82,19 @@ export abstract class EditorService extends SubscriptionHand } protected initForm(): void { + // Create an instance of the entity class to access default values + const defaultValues = new this._entityClass(); + + // Initialize the FormGroup this._form = new FormGroup>({} as any); + // Loop through the fields and initialize controls with default values for (const field of this.fields) { - this._form.addControl(field, new FormControl()); + const defaultValue = defaultValues[field]; // Get the default value dynamically + this._form.addControl( + field, + new FormControl(defaultValue, [Validators.required]), // Use the default value + ); } this.disableEntityIdField(); diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts index d9375a82dd..ae6a2dc307 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts @@ -17,6 +17,8 @@ export abstract class SingleRowEditorService extends EditorS protected override handlerService: HandlerService, ) { super(_entityClass, _entityTable, _entityIdField, handlerService); + console.log('Entity Class'); + console.log(_entityClass); this.initForm(); } @@ -86,19 +88,27 @@ export abstract class SingleRowEditorService extends EditorS protected updateFormAfterReload() { this._loading = true; + for (const field of this.fields) { - const control = this._form.controls[field]; - /* istanbul ignore else */ - if (control) { - control.setValue(this._originalValue[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); + // Ensure `field` is of type `string` + if (typeof field === 'string') { + const control = this._form.controls[field]; + + 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.`); } } + this._loading = false; } diff --git a/libs/shared/directives/.eslintrc.json b/libs/shared/directives/.eslintrc.json new file mode 100644 index 0000000000..a20cf65c22 --- /dev/null +++ b/libs/shared/directives/.eslintrc.json @@ -0,0 +1,33 @@ +{ + "extends": ["../../../.eslintrc.json"], + "ignorePatterns": ["!**/*"], + "overrides": [ + { + "files": ["*.ts"], + "extends": ["plugin:@nx/angular", "plugin:@angular-eslint/template/process-inline-templates"], + "rules": { + "@angular-eslint/directive-selector": [ + "error", + { + "type": "attribute", + "prefix": "keira", + "style": "camelCase" + } + ], + "@angular-eslint/component-selector": [ + "error", + { + "type": "element", + "prefix": "keira", + "style": "kebab-case" + } + ] + } + }, + { + "files": ["*.html"], + "extends": ["plugin:@nx/angular-template"], + "rules": {} + } + ] +} diff --git a/libs/shared/directives/README.md b/libs/shared/directives/README.md new file mode 100644 index 0000000000..94a2767b15 --- /dev/null +++ b/libs/shared/directives/README.md @@ -0,0 +1,3 @@ +# directives + +This library was generated with [Nx](https://nx.dev). diff --git a/libs/shared/directives/karma.conf.js b/libs/shared/directives/karma.conf.js new file mode 100644 index 0000000000..0571e426e4 --- /dev/null +++ b/libs/shared/directives/karma.conf.js @@ -0,0 +1,15 @@ +const { join } = require('path'); +const getBaseKarmaConfig = require('../../../karma.conf'); + +module.exports = function (config) { + const baseConfig = getBaseKarmaConfig(config); + config.set({ + ...baseConfig, + frameworks: [...baseConfig.frameworks], + plugins: [...baseConfig.plugins], + coverageReporter: { + ...baseConfig.coverageReporter, + dir: join(__dirname, '../../../coverage/libs/shared/directives'), + }, + }); +}; diff --git a/libs/shared/directives/project.json b/libs/shared/directives/project.json new file mode 100644 index 0000000000..fe7bf55ad9 --- /dev/null +++ b/libs/shared/directives/project.json @@ -0,0 +1,24 @@ +{ + "name": "keira-shared-directives", + "$schema": "../../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "libs/shared/directives/src", + "prefix": "keira", + "tags": ["scope:shared"], + "projectType": "library", + "targets": { + "test": { + "executor": "@angular-devkit/build-angular:karma", + "options": { + "tsConfig": "libs/shared/directives/tsconfig.spec.json", + "karmaConfig": "libs/shared/directives/karma.conf.js", + "polyfills": ["zone.js", "zone.js/testing"], + "sourceMap": true, + "codeCoverage": true, + "scripts": [] + } + }, + "lint": { + "executor": "@nx/eslint:lint" + } + } +} diff --git a/libs/shared/directives/src/index.ts b/libs/shared/directives/src/index.ts new file mode 100644 index 0000000000..78f2fd9021 --- /dev/null +++ b/libs/shared/directives/src/index.ts @@ -0,0 +1 @@ +export * from './validate-input.directive'; diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts new file mode 100644 index 0000000000..e4af19476d --- /dev/null +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -0,0 +1,24 @@ +import { Directive, ElementRef, HostBinding, Input, OnInit } from '@angular/core'; +import { AbstractControl } from '@angular/forms'; + +@Directive({ + selector: '[keiraInputValidation]', + standalone: true, +}) +export class InputValidationDirective implements OnInit { + @Input('keiraInputValidation') control!: AbstractControl | null; + + constructor(private el: ElementRef) {} + + ngOnInit(): void { + if (!this.control) { + console.warn('ValidationDirective: No control provided.'); + return; + } + } + + @HostBinding('class.is-invalid') + get isInvalid(): boolean { + return !!this.control?.invalid && !!this.control?.touched; + } +} diff --git a/libs/shared/directives/tsconfig.json b/libs/shared/directives/tsconfig.json new file mode 100644 index 0000000000..5cf0a16564 --- /dev/null +++ b/libs/shared/directives/tsconfig.json @@ -0,0 +1,29 @@ +{ + "compilerOptions": { + "target": "es2022", + "useDefineForClassFields": false, + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true + }, + "files": [], + "include": [], + "references": [ + { + "path": "./tsconfig.lib.json" + }, + { + "path": "./tsconfig.spec.json" + } + ], + "extends": "../../../tsconfig.base.json", + "angularCompilerOptions": { + "enableI18nLegacyMessageIdFormat": false, + "strictInjectionParameters": true, + "strictInputAccessModifiers": true, + "strictTemplates": true + } +} diff --git a/libs/shared/directives/tsconfig.lib.json b/libs/shared/directives/tsconfig.lib.json new file mode 100644 index 0000000000..f68063a517 --- /dev/null +++ b/libs/shared/directives/tsconfig.lib.json @@ -0,0 +1,12 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "declaration": true, + "declarationMap": true, + "inlineSources": true, + "types": [] + }, + "exclude": ["src/**/*.spec.ts", "jest.config.ts", "src/**/*.test.ts"], + "include": ["src/**/*.ts"] +} diff --git a/libs/shared/directives/tsconfig.spec.json b/libs/shared/directives/tsconfig.spec.json new file mode 100644 index 0000000000..b864ec66ae --- /dev/null +++ b/libs/shared/directives/tsconfig.spec.json @@ -0,0 +1,9 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../out-tsc/spec", + "types": ["jasmine", "node"] + }, + "include": ["**/*.spec.ts", "**/*.d.ts"], + "exclude": ["dist", "release", "node_modules"] +} diff --git a/libs/shared/error-templates/.eslintrc.json b/libs/shared/error-templates/.eslintrc.json new file mode 100644 index 0000000000..a20cf65c22 --- /dev/null +++ b/libs/shared/error-templates/.eslintrc.json @@ -0,0 +1,33 @@ +{ + "extends": ["../../../.eslintrc.json"], + "ignorePatterns": ["!**/*"], + "overrides": [ + { + "files": ["*.ts"], + "extends": ["plugin:@nx/angular", "plugin:@angular-eslint/template/process-inline-templates"], + "rules": { + "@angular-eslint/directive-selector": [ + "error", + { + "type": "attribute", + "prefix": "keira", + "style": "camelCase" + } + ], + "@angular-eslint/component-selector": [ + "error", + { + "type": "element", + "prefix": "keira", + "style": "kebab-case" + } + ] + } + }, + { + "files": ["*.html"], + "extends": ["plugin:@nx/angular-template"], + "rules": {} + } + ] +} diff --git a/libs/shared/error-templates/README.md b/libs/shared/error-templates/README.md new file mode 100644 index 0000000000..4a2b5d63f7 --- /dev/null +++ b/libs/shared/error-templates/README.md @@ -0,0 +1,3 @@ +# error-templates + +This library was generated with [Nx](https://nx.dev). diff --git a/libs/shared/error-templates/karma.conf.js b/libs/shared/error-templates/karma.conf.js new file mode 100644 index 0000000000..c8eb83219f --- /dev/null +++ b/libs/shared/error-templates/karma.conf.js @@ -0,0 +1,15 @@ +const { join } = require('path'); +const getBaseKarmaConfig = require('../../../karma.conf'); + +module.exports = function (config) { + const baseConfig = getBaseKarmaConfig(config); + config.set({ + ...baseConfig, + frameworks: [...baseConfig.frameworks], + plugins: [...baseConfig.plugins], + coverageReporter: { + ...baseConfig.coverageReporter, + dir: join(__dirname, '../../../coverage/libs/shared/error-templates'), + }, + }); +}; diff --git a/libs/shared/error-templates/project.json b/libs/shared/error-templates/project.json new file mode 100644 index 0000000000..15ad7f284a --- /dev/null +++ b/libs/shared/error-templates/project.json @@ -0,0 +1,24 @@ +{ + "name": "keira-shared-error-templates", + "$schema": "../../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "libs/shared/error-templates/src", + "prefix": "keira", + "tags": ["scope:shared"], + "projectType": "library", + "targets": { + "test": { + "executor": "@angular-devkit/build-angular:karma", + "options": { + "tsConfig": "libs/shared/error-templates/tsconfig.spec.json", + "karmaConfig": "libs/shared/error-templates/karma.conf.js", + "polyfills": ["zone.js", "zone.js/testing"], + "sourceMap": true, + "codeCoverage": true, + "scripts": [] + } + }, + "lint": { + "executor": "@nx/eslint:lint" + } + } +} diff --git a/libs/shared/error-templates/src/index.ts b/libs/shared/error-templates/src/index.ts new file mode 100644 index 0000000000..42da3af908 --- /dev/null +++ b/libs/shared/error-templates/src/index.ts @@ -0,0 +1 @@ +export * from './input-validation-error'; diff --git a/libs/shared/error-templates/src/input-validation-error.ts b/libs/shared/error-templates/src/input-validation-error.ts new file mode 100644 index 0000000000..43c648bb2c --- /dev/null +++ b/libs/shared/error-templates/src/input-validation-error.ts @@ -0,0 +1,24 @@ +import { Component, Input } from '@angular/core'; +import { AbstractControl } from '@angular/forms'; + +@Component({ + selector: 'keira-validation-feedback', + standalone: true, + template: ` + @if (control?.invalid && control?.touched) { +
+ {{ errorMessage }} +
+ } + `, +}) +export class ValidationFeedbackComponent { + @Input() control!: AbstractControl | null; + + get errorMessage(): string { + if (this.control?.errors?.['required']) { + return 'This field is required'; + } + return 'Invalid field'; + } +} diff --git a/libs/shared/error-templates/tsconfig.json b/libs/shared/error-templates/tsconfig.json new file mode 100644 index 0000000000..5cf0a16564 --- /dev/null +++ b/libs/shared/error-templates/tsconfig.json @@ -0,0 +1,29 @@ +{ + "compilerOptions": { + "target": "es2022", + "useDefineForClassFields": false, + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true + }, + "files": [], + "include": [], + "references": [ + { + "path": "./tsconfig.lib.json" + }, + { + "path": "./tsconfig.spec.json" + } + ], + "extends": "../../../tsconfig.base.json", + "angularCompilerOptions": { + "enableI18nLegacyMessageIdFormat": false, + "strictInjectionParameters": true, + "strictInputAccessModifiers": true, + "strictTemplates": true + } +} diff --git a/libs/shared/error-templates/tsconfig.lib.json b/libs/shared/error-templates/tsconfig.lib.json new file mode 100644 index 0000000000..f68063a517 --- /dev/null +++ b/libs/shared/error-templates/tsconfig.lib.json @@ -0,0 +1,12 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "declaration": true, + "declarationMap": true, + "inlineSources": true, + "types": [] + }, + "exclude": ["src/**/*.spec.ts", "jest.config.ts", "src/**/*.test.ts"], + "include": ["src/**/*.ts"] +} diff --git a/libs/shared/error-templates/tsconfig.spec.json b/libs/shared/error-templates/tsconfig.spec.json new file mode 100644 index 0000000000..b864ec66ae --- /dev/null +++ b/libs/shared/error-templates/tsconfig.spec.json @@ -0,0 +1,9 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../out-tsc/spec", + "types": ["jasmine", "node"] + }, + "include": ["**/*.spec.ts", "**/*.d.ts"], + "exclude": ["dist", "release", "node_modules"] +} diff --git a/tsconfig.base.json b/tsconfig.base.json index 610ba0de7d..26b25c3095 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -54,6 +54,8 @@ "@keira/shared/switch-language": ["libs/shared/switch-language/src/index.ts"], "@keira/shared/test-utils": ["libs/shared/test-utils/src/index.ts"], "@keira/shared/utils": ["libs/shared/utils/src/index.ts"], + "@keira/shared/directives": ["libs/shared/directives/src/index.ts"], + "@keira/shared/error-templates": ["libs/shared/error-templates/src/index.ts"], "texts": ["libs/features/texts/src/index.ts"] }, "rootDir": "." From 8febd6f57985d269fcae13e4ec5d3859a2d438fb Mon Sep 17 00:00:00 2001 From: Exitare Date: Sat, 25 Jan 2025 12:11:05 -0800 Subject: [PATCH 04/13] Add improved directive. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Stefano Borzì --- .../creature-template.component.html | 11 +---- .../creature-template.component.ts | 2 - .../src/validate-input.directive.ts | 46 +++++++++++++++---- libs/shared/error-templates/.eslintrc.json | 33 ------------- libs/shared/error-templates/README.md | 3 -- libs/shared/error-templates/karma.conf.js | 15 ------ libs/shared/error-templates/project.json | 24 ---------- libs/shared/error-templates/src/index.ts | 1 - .../src/input-validation-error.ts | 24 ---------- libs/shared/error-templates/tsconfig.json | 29 ------------ libs/shared/error-templates/tsconfig.lib.json | 12 ----- .../shared/error-templates/tsconfig.spec.json | 9 ---- 12 files changed, 38 insertions(+), 171 deletions(-) delete mode 100644 libs/shared/error-templates/.eslintrc.json delete mode 100644 libs/shared/error-templates/README.md delete mode 100644 libs/shared/error-templates/karma.conf.js delete mode 100644 libs/shared/error-templates/project.json delete mode 100644 libs/shared/error-templates/src/index.ts delete mode 100644 libs/shared/error-templates/src/input-validation-error.ts delete mode 100644 libs/shared/error-templates/tsconfig.json delete mode 100644 libs/shared/error-templates/tsconfig.lib.json delete mode 100644 libs/shared/error-templates/tsconfig.spec.json diff --git a/libs/features/creature/src/creature-template/creature-template.component.html b/libs/features/creature/src/creature-template/creature-template.component.html index 36989f8c6e..99962a3397 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.html +++ b/libs/features/creature/src/creature-template/creature-template.component.html @@ -47,15 +47,8 @@
- - - + +
diff --git a/libs/features/creature/src/creature-template/creature-template.component.ts b/libs/features/creature/src/creature-template/creature-template.component.ts index 23108297db..aa17661ddd 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.ts +++ b/libs/features/creature/src/creature-template/creature-template.component.ts @@ -41,7 +41,6 @@ import { CreatureHandlerService } from '../creature-handler.service'; import { CreatureTemplateService } from './creature-template.service'; import { RouterLink } from '@angular/router'; import { InputValidationDirective } from '@keira/shared/directives'; -import { ValidationFeedbackComponent } from '@keira/shared/error-templates'; @Component({ changeDetection: ChangeDetectionStrategy.OnPush, @@ -65,7 +64,6 @@ import { ValidationFeedbackComponent } from '@keira/shared/error-templates'; IconSelectorComponent, RouterLink, InputValidationDirective, - ValidationFeedbackComponent, ], }) export class CreatureTemplateComponent extends SingleRowEditorComponent { diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index e4af19476d..7ca6b2698b 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -1,24 +1,50 @@ -import { Directive, ElementRef, HostBinding, Input, OnInit } from '@angular/core'; -import { AbstractControl } from '@angular/forms'; +import { Directive, ElementRef, inject, OnInit, Renderer2 } from '@angular/core'; +import { AbstractControl, NgControl } from '@angular/forms'; +import { SubscriptionHandler } from '@keira/shared/utils'; @Directive({ selector: '[keiraInputValidation]', standalone: true, }) -export class InputValidationDirective implements OnInit { - @Input('keiraInputValidation') control!: AbstractControl | null; +export class InputValidationDirective extends SubscriptionHandler implements OnInit { + private readonly el: ElementRef = inject(ElementRef); + private readonly renderer: Renderer2 = inject(Renderer2); + private readonly ngControl: NgControl = inject(NgControl); - constructor(private el: ElementRef) {} + private errorDiv: HTMLElement | null = null; ngOnInit(): void { - if (!this.control) { - console.warn('ValidationDirective: No control provided.'); + const control = this.ngControl.control; + + if (!control) { return; } + + this.subscriptions.push( + control.statusChanges?.subscribe(() => { + console.log('control', control); + this.updateErrorMessage(control); + }), + ); } - @HostBinding('class.is-invalid') - get isInvalid(): boolean { - return !!this.control?.invalid && !!this.control?.touched; + private updateErrorMessage(control: AbstractControl): void { + if (this.errorDiv) { + this.renderer.removeChild(this.el.nativeElement.parentNode, this.errorDiv); + this.errorDiv = null; + } + + if (control?.touched && control?.invalid) { + console.log('control.errors', control.errors); + this.errorDiv = this.renderer.createElement('div'); + this.renderer.addClass(this.errorDiv, 'error-message'); + const errorMessage = control?.errors?.['required'] ? 'This field is required' : 'Invalid field'; + + const text = this.renderer.createText(errorMessage); + this.renderer.appendChild(this.errorDiv, text); + + const parent = this.el.nativeElement.parentNode; + this.renderer.appendChild(parent, this.errorDiv); + } } } diff --git a/libs/shared/error-templates/.eslintrc.json b/libs/shared/error-templates/.eslintrc.json deleted file mode 100644 index a20cf65c22..0000000000 --- a/libs/shared/error-templates/.eslintrc.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "extends": ["../../../.eslintrc.json"], - "ignorePatterns": ["!**/*"], - "overrides": [ - { - "files": ["*.ts"], - "extends": ["plugin:@nx/angular", "plugin:@angular-eslint/template/process-inline-templates"], - "rules": { - "@angular-eslint/directive-selector": [ - "error", - { - "type": "attribute", - "prefix": "keira", - "style": "camelCase" - } - ], - "@angular-eslint/component-selector": [ - "error", - { - "type": "element", - "prefix": "keira", - "style": "kebab-case" - } - ] - } - }, - { - "files": ["*.html"], - "extends": ["plugin:@nx/angular-template"], - "rules": {} - } - ] -} diff --git a/libs/shared/error-templates/README.md b/libs/shared/error-templates/README.md deleted file mode 100644 index 4a2b5d63f7..0000000000 --- a/libs/shared/error-templates/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# error-templates - -This library was generated with [Nx](https://nx.dev). diff --git a/libs/shared/error-templates/karma.conf.js b/libs/shared/error-templates/karma.conf.js deleted file mode 100644 index c8eb83219f..0000000000 --- a/libs/shared/error-templates/karma.conf.js +++ /dev/null @@ -1,15 +0,0 @@ -const { join } = require('path'); -const getBaseKarmaConfig = require('../../../karma.conf'); - -module.exports = function (config) { - const baseConfig = getBaseKarmaConfig(config); - config.set({ - ...baseConfig, - frameworks: [...baseConfig.frameworks], - plugins: [...baseConfig.plugins], - coverageReporter: { - ...baseConfig.coverageReporter, - dir: join(__dirname, '../../../coverage/libs/shared/error-templates'), - }, - }); -}; diff --git a/libs/shared/error-templates/project.json b/libs/shared/error-templates/project.json deleted file mode 100644 index 15ad7f284a..0000000000 --- a/libs/shared/error-templates/project.json +++ /dev/null @@ -1,24 +0,0 @@ -{ - "name": "keira-shared-error-templates", - "$schema": "../../../node_modules/nx/schemas/project-schema.json", - "sourceRoot": "libs/shared/error-templates/src", - "prefix": "keira", - "tags": ["scope:shared"], - "projectType": "library", - "targets": { - "test": { - "executor": "@angular-devkit/build-angular:karma", - "options": { - "tsConfig": "libs/shared/error-templates/tsconfig.spec.json", - "karmaConfig": "libs/shared/error-templates/karma.conf.js", - "polyfills": ["zone.js", "zone.js/testing"], - "sourceMap": true, - "codeCoverage": true, - "scripts": [] - } - }, - "lint": { - "executor": "@nx/eslint:lint" - } - } -} diff --git a/libs/shared/error-templates/src/index.ts b/libs/shared/error-templates/src/index.ts deleted file mode 100644 index 42da3af908..0000000000 --- a/libs/shared/error-templates/src/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './input-validation-error'; diff --git a/libs/shared/error-templates/src/input-validation-error.ts b/libs/shared/error-templates/src/input-validation-error.ts deleted file mode 100644 index 43c648bb2c..0000000000 --- a/libs/shared/error-templates/src/input-validation-error.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { Component, Input } from '@angular/core'; -import { AbstractControl } from '@angular/forms'; - -@Component({ - selector: 'keira-validation-feedback', - standalone: true, - template: ` - @if (control?.invalid && control?.touched) { -
- {{ errorMessage }} -
- } - `, -}) -export class ValidationFeedbackComponent { - @Input() control!: AbstractControl | null; - - get errorMessage(): string { - if (this.control?.errors?.['required']) { - return 'This field is required'; - } - return 'Invalid field'; - } -} diff --git a/libs/shared/error-templates/tsconfig.json b/libs/shared/error-templates/tsconfig.json deleted file mode 100644 index 5cf0a16564..0000000000 --- a/libs/shared/error-templates/tsconfig.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "compilerOptions": { - "target": "es2022", - "useDefineForClassFields": false, - "forceConsistentCasingInFileNames": true, - "strict": true, - "noImplicitOverride": true, - "noPropertyAccessFromIndexSignature": true, - "noImplicitReturns": true, - "noFallthroughCasesInSwitch": true - }, - "files": [], - "include": [], - "references": [ - { - "path": "./tsconfig.lib.json" - }, - { - "path": "./tsconfig.spec.json" - } - ], - "extends": "../../../tsconfig.base.json", - "angularCompilerOptions": { - "enableI18nLegacyMessageIdFormat": false, - "strictInjectionParameters": true, - "strictInputAccessModifiers": true, - "strictTemplates": true - } -} diff --git a/libs/shared/error-templates/tsconfig.lib.json b/libs/shared/error-templates/tsconfig.lib.json deleted file mode 100644 index f68063a517..0000000000 --- a/libs/shared/error-templates/tsconfig.lib.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "extends": "./tsconfig.json", - "compilerOptions": { - "outDir": "../../../dist/out-tsc", - "declaration": true, - "declarationMap": true, - "inlineSources": true, - "types": [] - }, - "exclude": ["src/**/*.spec.ts", "jest.config.ts", "src/**/*.test.ts"], - "include": ["src/**/*.ts"] -} diff --git a/libs/shared/error-templates/tsconfig.spec.json b/libs/shared/error-templates/tsconfig.spec.json deleted file mode 100644 index b864ec66ae..0000000000 --- a/libs/shared/error-templates/tsconfig.spec.json +++ /dev/null @@ -1,9 +0,0 @@ -{ - "extends": "./tsconfig.json", - "compilerOptions": { - "outDir": "../out-tsc/spec", - "types": ["jasmine", "node"] - }, - "include": ["**/*.spec.ts", "**/*.d.ts"], - "exclude": ["dist", "release", "node_modules"] -} From 78899c70c30f1af1feed8b5ec9cf402221db8116 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sat, 25 Jan 2025 12:33:48 -0800 Subject: [PATCH 05/13] Fix initial wrong state of a field being not touched when it is touched. --- .../src/creature-template/creature-template.component.html | 2 +- libs/shared/directives/src/validate-input.directive.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/features/creature/src/creature-template/creature-template.component.html b/libs/features/creature/src/creature-template/creature-template.component.html index 99962a3397..57efd1e90f 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.html +++ b/libs/features/creature/src/creature-template/creature-template.component.html @@ -44,7 +44,7 @@
- +
diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 7ca6b2698b..949934dec9 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -22,7 +22,6 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI this.subscriptions.push( control.statusChanges?.subscribe(() => { - console.log('control', control); this.updateErrorMessage(control); }), ); @@ -34,6 +33,8 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI this.errorDiv = null; } + if (control?.invalid) control?.markAsTouched(); + if (control?.touched && control?.invalid) { console.log('control.errors', control.errors); this.errorDiv = this.renderer.createElement('div'); From 4196e1323e3a1e1d3fa5392863d69bf4035a9394 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sun, 26 Jan 2025 16:14:55 -0800 Subject: [PATCH 06/13] Add new validation service to pass validation status to subscribers. --- .../query-output/query-output.component.html | 14 ++++++++++---- .../src/query-output/query-output.component.ts | 18 ++++++++++++++---- libs/shared/common-services/src/index.ts | 1 + .../common-services/src/validation.service.ts | 9 +++++++++ .../directives/src/validate-input.directive.ts | 9 +++++++-- 5 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 libs/shared/common-services/src/validation.service.ts diff --git a/libs/shared/base-editor-components/src/query-output/query-output.component.html b/libs/shared/base-editor-components/src/query-output/query-output.component.html index 60cc672070..0e3006ebb7 100644 --- a/libs/shared/base-editor-components/src/query-output/query-output.component.html +++ b/libs/shared/base-editor-components/src/query-output/query-output.component.html @@ -24,16 +24,22 @@
- - - -
diff --git a/libs/shared/base-editor-components/src/query-output/query-output.component.ts b/libs/shared/base-editor-components/src/query-output/query-output.component.ts index c3261fdd6a..133315a957 100644 --- a/libs/shared/base-editor-components/src/query-output/query-output.component.ts +++ b/libs/shared/base-editor-components/src/query-output/query-output.component.ts @@ -1,5 +1,4 @@ -import { ChangeDetectionStrategy, ChangeDetectorRef, Component, EventEmitter, inject, Input, Output } from '@angular/core'; - +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, EventEmitter, inject, Input, OnInit, Output } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { EditorService } from '@keira/shared/base-abstract-classes'; import { TableRow } from '@keira/shared/constants'; @@ -11,6 +10,7 @@ import { filter } from 'rxjs'; import { HighlightjsWrapperComponent } from '../highlightjs-wrapper/highlightjs-wrapper.component'; import { ModalConfirmComponent } from '../modal-confirm/modal-confirm.component'; import { QueryErrorComponent } from './query-error/query-error.component'; +import { ValidationService } from '@keira/shared/common-services'; @Component({ // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @@ -21,18 +21,28 @@ import { QueryErrorComponent } from './query-error/query-error.component'; standalone: true, imports: [FormsModule, HighlightjsWrapperComponent, QueryErrorComponent, TranslateModule], }) -export class QueryOutputComponent extends SubscriptionHandler { +export class QueryOutputComponent extends SubscriptionHandler implements OnInit { private readonly clipboardService = inject(ClipboardService); private readonly modalService = inject(BsModalService); + private readonly validationService = inject(ValidationService); @Input() docUrl!: string; @Input() editorService!: EditorService; @Output() executeQuery = new EventEmitter(); selectedQuery: 'diff' | 'full' = 'diff'; private modalRef!: BsModalRef; - + protected validationPassed: boolean = true; private readonly changeDetectorRef = inject(ChangeDetectorRef); + ngOnInit() { + this.subscriptions.push( + this.validationService.validationPassed$.subscribe((validationPassed: boolean) => { + this.validationPassed = validationPassed; + this.changeDetectorRef.detectChanges(); + }), + ); + } + showFullQuery(): boolean { return this.editorService.isNew || this.selectedQuery === 'full'; } diff --git a/libs/shared/common-services/src/index.ts b/libs/shared/common-services/src/index.ts index 51bf180fd3..6c883567d3 100644 --- a/libs/shared/common-services/src/index.ts +++ b/libs/shared/common-services/src/index.ts @@ -1,3 +1,4 @@ export * from './electron.service'; export * from './config.service'; export * from './location.service'; +export * from './validation.service'; diff --git a/libs/shared/common-services/src/validation.service.ts b/libs/shared/common-services/src/validation.service.ts new file mode 100644 index 0000000000..0e3da574b1 --- /dev/null +++ b/libs/shared/common-services/src/validation.service.ts @@ -0,0 +1,9 @@ +import { Injectable } from '@angular/core'; +import { BehaviorSubject } from 'rxjs'; + +@Injectable({ + providedIn: 'root', +}) +export class ValidationService { + public validationPassed$: BehaviorSubject = new BehaviorSubject(true); +} diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 949934dec9..79dab3325b 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -1,6 +1,7 @@ import { Directive, ElementRef, inject, OnInit, Renderer2 } from '@angular/core'; import { AbstractControl, NgControl } from '@angular/forms'; import { SubscriptionHandler } from '@keira/shared/utils'; +import { ValidationService } from '@keira/shared/common-services'; @Directive({ selector: '[keiraInputValidation]', @@ -10,6 +11,7 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI private readonly el: ElementRef = inject(ElementRef); private readonly renderer: Renderer2 = inject(Renderer2); private readonly ngControl: NgControl = inject(NgControl); + private readonly validationService = inject(ValidationService); private errorDiv: HTMLElement | null = null; @@ -33,10 +35,11 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI this.errorDiv = null; } - if (control?.invalid) control?.markAsTouched(); + if (control?.invalid) { + control?.markAsTouched(); + } if (control?.touched && control?.invalid) { - console.log('control.errors', control.errors); this.errorDiv = this.renderer.createElement('div'); this.renderer.addClass(this.errorDiv, 'error-message'); const errorMessage = control?.errors?.['required'] ? 'This field is required' : 'Invalid field'; @@ -47,5 +50,7 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI const parent = this.el.nativeElement.parentNode; this.renderer.appendChild(parent, this.errorDiv); } + + this.validationService.validationPassed$.next(control?.valid); } } From b009f20c1865696b275c821ace56606e01758aa3 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sun, 26 Jan 2025 16:53:42 -0800 Subject: [PATCH 07/13] Add tests for directive and single editor. --- .../editors/single-row-editor.service.spec.ts | 77 ++++++- .../src/validation.service.spec.ts | 36 ++++ .../src/validate-input.directive.spec.ts | 196 ++++++++++++++++++ .../src/validate-input.directive.ts | 30 +-- 4 files changed, 324 insertions(+), 15 deletions(-) create mode 100644 libs/shared/common-services/src/validation.service.spec.ts create mode 100644 libs/shared/directives/src/validate-input.directive.spec.ts diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts index 83b6098d79..93faab5d94 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts @@ -1,6 +1,4 @@ import { TestBed } from '@angular/core/testing'; -import { RouterTestingModule } from '@angular/router/testing'; - import { ToastrService } from 'ngx-toastr'; import { instance, mock } from 'ts-mockito'; import { MysqlQueryService } from '@keira/shared/db-layer'; @@ -14,7 +12,7 @@ describe('SingleRowEditorService', () => { beforeEach(() => TestBed.configureTestingModule({ - imports: [RouterTestingModule], + imports: [], providers: [ { provide: MysqlQueryService, useValue: instance(mock(MysqlQueryService)) }, { provide: ToastrService, useValue: instance(mock(ToastrService)) }, @@ -162,4 +160,77 @@ describe('SingleRowEditorService', () => { expect(updateFullQuerySpy).toHaveBeenCalledTimes(1); }); }); + + describe('updateFormAfterReload()', () => { + let consoleErrorSpy: Spy; + let consoleWarnSpy: Spy; + let mockForm: any; + + beforeEach(() => { + // Mock the form and its controls + mockForm = { + controls: { + id: { setValue: jasmine.createSpy('setValue') }, + name: { setValue: jasmine.createSpy('setValue') }, + guid: { setValue: jasmine.createSpy('setValue') }, // Add guid control + }, + }; + service['_form'] = mockForm; + + // 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'); + consoleWarnSpy = spyOn(console, 'warn'); + + // Temporarily override `fields` for testing + Object.defineProperty(service, 'fields', { + value: ['id', 'name', 'guid', 123 as any, null as any], + writable: true, + }); + }); + + it('should set values for valid fields in the form', () => { + service['updateFormAfterReload'](); + + expect(mockForm.controls.id.setValue).toHaveBeenCalledWith(123); // Valid field + expect(mockForm.controls.name.setValue).toHaveBeenCalledWith('Test Name'); // Valid field + expect(mockForm.controls.guid.setValue).toHaveBeenCalledWith(456); // Valid field + }); + + 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'](); + + expect(consoleWarnSpy).toHaveBeenCalledWith("Field '123' is not a valid string key."); + expect(consoleWarnSpy).toHaveBeenCalledWith("Field 'null' is not a valid string key."); + }); + + it('should not throw errors for valid but empty fields', () => { + Object.defineProperty(service, 'fields', { + value: [], // No fields to iterate + }); + + expect(() => service['updateFormAfterReload']()).not.toThrow(); + }); + + it('should reset loading to false after execution', () => { + service['_loading'] = true; + + service['updateFormAfterReload'](); + + expect(service['_loading']).toBe(false); // Ensure loading is reset + }); + }); }); diff --git a/libs/shared/common-services/src/validation.service.spec.ts b/libs/shared/common-services/src/validation.service.spec.ts new file mode 100644 index 0000000000..da1601a185 --- /dev/null +++ b/libs/shared/common-services/src/validation.service.spec.ts @@ -0,0 +1,36 @@ +import { TestBed } from '@angular/core/testing'; +import { ValidationService } from './validation.service'; + +describe('ValidationService', () => { + let service: ValidationService; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ValidationService], + }); + + service = TestBed.inject(ValidationService); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + it('should have a default value of true for validationPassed$', (done: DoneFn) => { + service.validationPassed$.subscribe((value) => { + expect(value).toBe(true); + done(); + }); + }); + + it('should emit the updated value when validationPassed$ changes', (done: DoneFn) => { + // Emit a new value + service.validationPassed$.next(false); + + // Subscribe and verify the updated value + service.validationPassed$.subscribe((value) => { + expect(value).toBe(false); + done(); + }); + }); +}); diff --git a/libs/shared/directives/src/validate-input.directive.spec.ts b/libs/shared/directives/src/validate-input.directive.spec.ts new file mode 100644 index 0000000000..384344ef28 --- /dev/null +++ b/libs/shared/directives/src/validate-input.directive.spec.ts @@ -0,0 +1,196 @@ +import { Component, DebugElement } from '@angular/core'; +import { TestBed, ComponentFixture } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; +import { ReactiveFormsModule, FormControl, FormsModule, NgControl } from '@angular/forms'; +import { InputValidationDirective } from './validate-input.directive'; +import { ValidationService } from '@keira/shared/common-services'; +import { take } from 'rxjs'; + +@Component({ + template: ` +
+
+ +
+
+ `, +}) +class TestComponent { + testControl = new FormControl(''); +} + +describe('InputValidationDirective', () => { + let fixture: ComponentFixture; + let validationService: ValidationService; + let debugElement: DebugElement; + + beforeEach(() => { + TestBed.configureTestingModule({ + declarations: [TestComponent], + imports: [ReactiveFormsModule, FormsModule, InputValidationDirective], // Add the directive to imports + providers: [ValidationService], + }).compileComponents(); + + fixture = TestBed.createComponent(TestComponent); + validationService = TestBed.inject(ValidationService); + debugElement = fixture.debugElement.query(By.directive(InputValidationDirective)); + fixture.detectChanges(); + }); + + it('should create an instance', () => { + const directive = debugElement.injector.get(InputValidationDirective); + expect(directive).toBeTruthy(); + }); + + it('should display error message when control is invalid and touched', () => { + const control = debugElement.injector.get(NgControl).control; + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + expect(errorDiv.textContent).toBe('This field is required'); + }); + + it('should remove error message when control becomes valid', () => { + const control = debugElement.injector.get(NgControl).control; + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + let errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + + // Make the control valid + control?.clearValidators(); + control?.setValue('Valid value'); // Set a valid value + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); + }); + + it('should handle empty error object gracefully', () => { + const control = debugElement.injector.get(NgControl).control; + control?.setValidators(() => ({})); // No errors + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); + }); + + it('should handle multiple error types gracefully', () => { + const control = debugElement.injector.get(NgControl).control; + control?.setValidators(() => ({ required: true, minlength: true })); // Multiple errors + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + expect(errorDiv.textContent).toBe('This field is required'); // Test only the first error message + }); + + it('should not throw when control is null', () => { + const ngControl = debugElement.injector.get(NgControl); + spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); // Mock control as null + + expect(() => { + fixture.detectChanges(); + }).not.toThrow(); + }); + + it('should not create duplicate error messages if errorDiv already exists', () => { + const control = debugElement.injector.get(NgControl).control; + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const initialErrorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(initialErrorDiv).toBeTruthy(); + + // Trigger the updateErrorMessage logic again + fixture.detectChanges(); + + const updatedErrorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(updatedErrorDiv).toBe(initialErrorDiv); // Same errorDiv should remain + }); + + it('should not add error message if parentNode is null', () => { + const control = debugElement.injector.get(NgControl).control; + + // Mock parentNode as null + spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); + + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode?.querySelector('.error-message'); + expect(errorDiv).toBeFalsy(); // Error message should not be added + }); + + it('should update validationPassed$ in ValidationService', (done: DoneFn) => { + const control = debugElement.injector.get(NgControl).control; + + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + validationService.validationPassed$ + .pipe(take(1)) // Take only the first emission + .subscribe((isValid) => { + expect(isValid).toBe(false); // Initially invalid + done(); + }); + + // Test invalid state + control?.setValue(''); + fixture.detectChanges(); + + // Test valid state + control?.setValue('Valid value'); + control?.updateValueAndValidity(); + fixture.detectChanges(); + }); + + it('should set touched when control is invalid', () => { + const control = debugElement.injector.get(NgControl).control; + + spyOn(control!, 'markAsTouched'); + control?.setValidators(() => ({ required: true })); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + expect(control?.markAsTouched).toHaveBeenCalled(); + }); + + it('should not create errorDiv if parentNode is null', () => { + const control = debugElement.injector.get(NgControl).control; + spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); + + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + expect(debugElement.nativeElement.parentNode?.querySelector('.error-message')).toBeFalsy(); + }); +}); diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 79dab3325b..15081b4298 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -29,28 +29,34 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI ); } - private updateErrorMessage(control: AbstractControl): void { + private updateErrorMessage(control: AbstractControl | null): void { + // Safely remove the existing errorDiv if it exists if (this.errorDiv) { - this.renderer.removeChild(this.el.nativeElement.parentNode, this.errorDiv); + const parent = this.el.nativeElement.parentNode; + if (parent) { + this.renderer.removeChild(parent, this.errorDiv); + } this.errorDiv = null; } if (control?.invalid) { - control?.markAsTouched(); + control.markAsTouched(); } - if (control?.touched && control?.invalid) { - this.errorDiv = this.renderer.createElement('div'); - this.renderer.addClass(this.errorDiv, 'error-message'); - const errorMessage = control?.errors?.['required'] ? 'This field is required' : 'Invalid field'; + if (control?.touched && control?.invalid && control.errors && Object.keys(control.errors).length && this.el.nativeElement.parentNode) { + const parent = this.el.nativeElement.parentNode; + if (parent) { + this.errorDiv = this.renderer.createElement('div'); + this.renderer.addClass(this.errorDiv, 'error-message'); + const errorMessage = control.errors?.['required'] ? 'This field is required' : 'Invalid field'; - const text = this.renderer.createText(errorMessage); - this.renderer.appendChild(this.errorDiv, text); + const text = this.renderer.createText(errorMessage); + this.renderer.appendChild(this.errorDiv, text); - const parent = this.el.nativeElement.parentNode; - this.renderer.appendChild(parent, this.errorDiv); + this.renderer.appendChild(parent, this.errorDiv); + } } - this.validationService.validationPassed$.next(control?.valid); + this.validationService.validationPassed$.next(!!control?.valid); } } From 9262e7d3d268f568e723aaae4d817fac6c9b92f9 Mon Sep 17 00:00:00 2001 From: Exitare Date: Sun, 26 Jan 2025 17:17:37 -0800 Subject: [PATCH 08/13] Add tests --- .../src/validate-input.directive.spec.ts | 134 +++++++++++------- .../src/validate-input.directive.ts | 8 +- 2 files changed, 86 insertions(+), 56 deletions(-) diff --git a/libs/shared/directives/src/validate-input.directive.spec.ts b/libs/shared/directives/src/validate-input.directive.spec.ts index 384344ef28..b7724c6001 100644 --- a/libs/shared/directives/src/validate-input.directive.spec.ts +++ b/libs/shared/directives/src/validate-input.directive.spec.ts @@ -1,10 +1,10 @@ import { Component, DebugElement } from '@angular/core'; import { TestBed, ComponentFixture } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; -import { ReactiveFormsModule, FormControl, FormsModule, NgControl } from '@angular/forms'; +import { ReactiveFormsModule, FormControl, FormsModule, NgControl, FormControlStatus } from '@angular/forms'; import { InputValidationDirective } from './validate-input.directive'; import { ValidationService } from '@keira/shared/common-services'; -import { take } from 'rxjs'; +import { Observable, take } from 'rxjs'; @Component({ template: ` @@ -27,7 +27,7 @@ describe('InputValidationDirective', () => { beforeEach(() => { TestBed.configureTestingModule({ declarations: [TestComponent], - imports: [ReactiveFormsModule, FormsModule, InputValidationDirective], // Add the directive to imports + imports: [ReactiveFormsModule, FormsModule, InputValidationDirective], providers: [ValidationService], }).compileComponents(); @@ -66,9 +66,8 @@ describe('InputValidationDirective', () => { let errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); - // Make the control valid control?.clearValidators(); - control?.setValue('Valid value'); // Set a valid value + control?.setValue('Valid value'); control?.updateValueAndValidity(); fixture.detectChanges(); @@ -79,19 +78,21 @@ describe('InputValidationDirective', () => { it('should handle empty error object gracefully', () => { const control = debugElement.injector.get(NgControl).control; - control?.setValidators(() => ({})); // No errors - control?.markAsTouched(); + + control?.setValidators(() => null); // Simulate no errors + control?.setValue(''); control?.updateValueAndValidity(); fixture.detectChanges(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); + expect(errorDiv).toBeNull(); // Ensure no error message is created }); - it('should handle multiple error types gracefully', () => { + it('should handle multiple error types and display first error', () => { const control = debugElement.injector.get(NgControl).control; - control?.setValidators(() => ({ required: true, minlength: true })); // Multiple errors + + control?.setValidators(() => ({ required: true, minlength: true })); control?.markAsTouched(); control?.updateValueAndValidity(); @@ -99,92 +100,125 @@ describe('InputValidationDirective', () => { const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); - expect(errorDiv.textContent).toBe('This field is required'); // Test only the first error message - }); - - it('should not throw when control is null', () => { - const ngControl = debugElement.injector.get(NgControl); - spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); // Mock control as null - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); + expect(errorDiv.textContent).toBe('This field is required'); }); - it('should not create duplicate error messages if errorDiv already exists', () => { + it('should not create errorDiv if parentNode is null', () => { const control = debugElement.injector.get(NgControl).control; + spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); + control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); fixture.detectChanges(); - const initialErrorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(initialErrorDiv).toBeTruthy(); + expect(debugElement.nativeElement.parentNode?.querySelector('.error-message')).toBeFalsy(); + }); + + it('should mark control as touched when invalid', () => { + const control = debugElement.injector.get(NgControl).control; + spyOn(control!, 'markAsTouched').and.callThrough(); + + control?.setValidators(() => ({ required: true })); + control?.setValue(''); + control?.updateValueAndValidity(); - // Trigger the updateErrorMessage logic again fixture.detectChanges(); - const updatedErrorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(updatedErrorDiv).toBe(initialErrorDiv); // Same errorDiv should remain + expect(control?.markAsTouched).toHaveBeenCalled(); }); - it('should not add error message if parentNode is null', () => { + it('should update validationPassed$ with correct value', (done: DoneFn) => { const control = debugElement.injector.get(NgControl).control; - // Mock parentNode as null - spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); - control?.setValidators(() => ({ required: true })); - control?.markAsTouched(); + control?.setValue(''); control?.updateValueAndValidity(); fixture.detectChanges(); - const errorDiv = debugElement.nativeElement.parentNode?.querySelector('.error-message'); - expect(errorDiv).toBeFalsy(); // Error message should not be added + validationService.validationPassed$.pipe(take(1)).subscribe((isValid) => { + expect(isValid).toBe(false); + done(); + }); + + control?.setValue('Valid value'); + control?.updateValueAndValidity(); + + fixture.detectChanges(); }); - it('should update validationPassed$ in ValidationService', (done: DoneFn) => { + it('should not throw error when ngControl.control is null', () => { + const ngControl = debugElement.injector.get(NgControl); + spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); + + expect(() => { + fixture.detectChanges(); + }).not.toThrow(); + }); + + it('should safely remove errorDiv if already exists', () => { const control = debugElement.injector.get(NgControl).control; + // Set invalid state to create an errorDiv control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); - validationService.validationPassed$ - .pipe(take(1)) // Take only the first emission - .subscribe((isValid) => { - expect(isValid).toBe(false); // Initially invalid - done(); - }); - - // Test invalid state - control?.setValue(''); fixture.detectChanges(); - // Test valid state - control?.setValue('Valid value'); + let errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + + // Re-trigger the update with no errors + control?.clearValidators(); control?.updateValueAndValidity(); + fixture.detectChanges(); + + errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); }); - it('should set touched when control is invalid', () => { + it('should not throw error if errorDiv is already null', () => { + const directive = debugElement.injector.get(InputValidationDirective); + directive['errorDiv'] = null; // Manually set to null const control = debugElement.injector.get(NgControl).control; - spyOn(control!, 'markAsTouched'); control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); control?.updateValueAndValidity(); - fixture.detectChanges(); + expect(() => fixture.detectChanges()).not.toThrow(); + }); - expect(control?.markAsTouched).toHaveBeenCalled(); + it('should handle statusChanges being null gracefully', () => { + const control = debugElement.injector.get(NgControl).control; + + // Mock the statusChanges property as null + spyOnProperty(control!, 'statusChanges', 'get').and.returnValue(null as unknown as Observable); + + expect(() => { + fixture.detectChanges(); + control?.updateValueAndValidity(); + }).not.toThrow(); }); - it('should not create errorDiv if parentNode is null', () => { + it('should not throw error if control.errors is null', () => { const control = debugElement.injector.get(NgControl).control; + spyOnProperty(control!, 'errors', 'get').and.returnValue(null); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); + }); + + it('should not add errorDiv if parentNode is null', () => { spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); + const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 15081b4298..61a937a53d 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -30,12 +30,8 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI } private updateErrorMessage(control: AbstractControl | null): void { - // Safely remove the existing errorDiv if it exists if (this.errorDiv) { - const parent = this.el.nativeElement.parentNode; - if (parent) { - this.renderer.removeChild(parent, this.errorDiv); - } + this.renderer.removeChild(this.el.nativeElement.parentNode, this.errorDiv); this.errorDiv = null; } @@ -43,7 +39,7 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI control.markAsTouched(); } - if (control?.touched && control?.invalid && control.errors && Object.keys(control.errors).length && this.el.nativeElement.parentNode) { + if (control?.touched && control?.invalid && control.errors) { const parent = this.el.nativeElement.parentNode; if (parent) { this.errorDiv = this.renderer.createElement('div'); From 25817b0923b639fc0cfcb4374fa0738ea8377b47 Mon Sep 17 00:00:00 2001 From: Exitare Date: Mon, 27 Jan 2025 15:35:42 -0800 Subject: [PATCH 09/13] Update tests for full coverage --- .../src/validate-input.directive.spec.ts | 131 ++++++++++++++++-- 1 file changed, 122 insertions(+), 9 deletions(-) diff --git a/libs/shared/directives/src/validate-input.directive.spec.ts b/libs/shared/directives/src/validate-input.directive.spec.ts index b7724c6001..63435d1440 100644 --- a/libs/shared/directives/src/validate-input.directive.spec.ts +++ b/libs/shared/directives/src/validate-input.directive.spec.ts @@ -1,10 +1,10 @@ import { Component, DebugElement } from '@angular/core'; import { TestBed, ComponentFixture } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; -import { ReactiveFormsModule, FormControl, FormsModule, NgControl, FormControlStatus } from '@angular/forms'; +import { ReactiveFormsModule, FormControl, FormsModule, NgControl, AbstractControl } from '@angular/forms'; import { InputValidationDirective } from './validate-input.directive'; import { ValidationService } from '@keira/shared/common-services'; -import { Observable, take } from 'rxjs'; +import { take } from 'rxjs'; @Component({ template: ` @@ -194,22 +194,29 @@ describe('InputValidationDirective', () => { }); it('should handle statusChanges being null gracefully', () => { - const control = debugElement.injector.get(NgControl).control; + const control = debugElement.injector.get(NgControl).control as AbstractControl; - // Mock the statusChanges property as null - spyOnProperty(control!, 'statusChanges', 'get').and.returnValue(null as unknown as Observable); + // Mock the control to have statusChanges as null + Object.defineProperty(control, 'statusChanges', { + get: () => null, + }); expect(() => { fixture.detectChanges(); - control?.updateValueAndValidity(); }).not.toThrow(); }); it('should not throw error if control.errors is null', () => { - const control = debugElement.injector.get(NgControl).control; - spyOnProperty(control!, 'errors', 'get').and.returnValue(null); + const control = debugElement.injector.get(NgControl).control as AbstractControl; - fixture.detectChanges(); + // Mock the control to have errors as null + Object.defineProperty(control, 'errors', { + get: () => null, + }); + + expect(() => { + fixture.detectChanges(); + }).not.toThrow(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeNull(); @@ -227,4 +234,110 @@ describe('InputValidationDirective', () => { expect(debugElement.nativeElement.parentNode?.querySelector('.error-message')).toBeFalsy(); }); + + it('should return early if control is null', () => { + const ngControl = debugElement.injector.get(NgControl); + spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); + + expect(() => fixture.detectChanges()).not.toThrow(); + }); + + it('should display correct error message for "required" error', () => { + const control = debugElement.injector.get(NgControl).control; + + control?.setValidators(() => ({ required: true })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + expect(errorDiv.textContent).toBe('This field is required'); + }); + + it('should display "Invalid field" for non-required errors', () => { + const control = debugElement.injector.get(NgControl).control; + + control?.setValidators(() => ({ minlength: { requiredLength: 5, actualLength: 3 } })); + control?.markAsTouched(); + control?.updateValueAndValidity(); + + fixture.detectChanges(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeTruthy(); + expect(errorDiv.textContent).toBe('Invalid field'); + }); + + it('should handle control.errors being null without creating errorDiv', () => { + const ngControl = debugElement.injector.get(NgControl); + + // Create a FormControl instance to mock AbstractControl + const controlMock = new FormControl(); + + // Directly assign null to the errors property + Object.defineProperty(controlMock, 'errors', { + get: () => null, + configurable: true, + }); + + // Replace the ngControl's control with the mock control + spyOnProperty(ngControl, 'control', 'get').and.returnValue(controlMock); + + expect(() => { + fixture.detectChanges(); + }).not.toThrow(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); // Verify no errorDiv is created + }); + + it('should not throw error if control.errors is undefined', () => { + const control = debugElement.injector.get(NgControl).control as AbstractControl; + + // Mock the control to have errors as undefined + Object.defineProperty(control, 'errors', { + get: () => undefined, + }); + + expect(() => { + fixture.detectChanges(); + }).not.toThrow(); + + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); + }); + + it('should return early if control is null', () => { + const ngControl = debugElement.injector.get(NgControl); + + // Mock ngControl.control to be null + spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); + + expect(() => { + fixture.detectChanges(); // Triggers ngOnInit + }).not.toThrow(); + + // Verify no errorDiv is created + const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); + expect(errorDiv).toBeNull(); + }); + + it('should return early if control is null (explicit)', () => { + const directive = debugElement.injector.get(InputValidationDirective); + const ngControl = debugElement.injector.get(NgControl); + + // Mock ngControl.control to be null + spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); + + // Spy on the updateErrorMessage method to ensure it's not called + const updateErrorMessageSpy = spyOn(directive, 'updateErrorMessage'); + + // Call the method explicitly + directive.ngOnInit(); + + // Expect the method to exit early and not proceed further + expect(updateErrorMessageSpy).not.toHaveBeenCalled(); + }); }); From 02a4661fb7a3edc7662b2fd142bdfe84a041c022 Mon Sep 17 00:00:00 2001 From: Exitare Date: Mon, 27 Jan 2025 15:44:57 -0800 Subject: [PATCH 10/13] Add distinctUntilChanged to pipe --- libs/shared/directives/src/validate-input.directive.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 61a937a53d..25010a1e85 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -2,6 +2,7 @@ import { Directive, ElementRef, inject, OnInit, Renderer2 } from '@angular/core' import { AbstractControl, NgControl } from '@angular/forms'; import { SubscriptionHandler } from '@keira/shared/utils'; import { ValidationService } from '@keira/shared/common-services'; +import { distinctUntilChanged } from 'rxjs'; @Directive({ selector: '[keiraInputValidation]', @@ -23,7 +24,7 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI } this.subscriptions.push( - control.statusChanges?.subscribe(() => { + control.statusChanges?.pipe(distinctUntilChanged()).subscribe(() => { this.updateErrorMessage(control); }), ); From 3a53132fa3a51adc1879a715c26b2283d221b367 Mon Sep 17 00:00:00 2001 From: Exitare Date: Mon, 27 Jan 2025 16:36:10 -0800 Subject: [PATCH 11/13] Address issue that was caused by not taking all validators into account --- .../creature-template.component.ts | 2 + .../src/validation.service.spec.ts | 47 ++++++++++++++++--- .../common-services/src/validation.service.ts | 16 +++++++ .../src/validate-input.directive.ts | 16 +++++-- 4 files changed, 69 insertions(+), 12 deletions(-) diff --git a/libs/features/creature/src/creature-template/creature-template.component.ts b/libs/features/creature/src/creature-template/creature-template.component.ts index aa17661ddd..c01d76ff40 100644 --- a/libs/features/creature/src/creature-template/creature-template.component.ts +++ b/libs/features/creature/src/creature-template/creature-template.component.ts @@ -41,6 +41,7 @@ import { CreatureHandlerService } from '../creature-handler.service'; import { CreatureTemplateService } from './creature-template.service'; import { RouterLink } from '@angular/router'; import { InputValidationDirective } from '@keira/shared/directives'; +import { ValidationService } from '@keira/shared/common-services'; @Component({ changeDetection: ChangeDetectionStrategy.OnPush, @@ -65,6 +66,7 @@ import { InputValidationDirective } from '@keira/shared/directives'; RouterLink, InputValidationDirective, ], + providers: [ValidationService], }) export class CreatureTemplateComponent extends SingleRowEditorComponent { protected readonly UNIT_FLAGS = UNIT_FLAGS; diff --git a/libs/shared/common-services/src/validation.service.spec.ts b/libs/shared/common-services/src/validation.service.spec.ts index da1601a185..bf7f8b33ed 100644 --- a/libs/shared/common-services/src/validation.service.spec.ts +++ b/libs/shared/common-services/src/validation.service.spec.ts @@ -1,5 +1,6 @@ import { TestBed } from '@angular/core/testing'; import { ValidationService } from './validation.service'; +import { take } from 'rxjs/operators'; describe('ValidationService', () => { let service: ValidationService; @@ -17,20 +18,52 @@ describe('ValidationService', () => { }); it('should have a default value of true for validationPassed$', (done: DoneFn) => { - service.validationPassed$.subscribe((value) => { + service.validationPassed$.pipe(take(1)).subscribe((value) => { expect(value).toBe(true); done(); }); }); - it('should emit the updated value when validationPassed$ changes', (done: DoneFn) => { - // Emit a new value - service.validationPassed$.next(false); + it('should set control validity and update validationPassed$', (done: DoneFn) => { + service.setControlValidity('control1', false); - // Subscribe and verify the updated value - service.validationPassed$.subscribe((value) => { - expect(value).toBe(false); + service.validationPassed$.pipe(take(1)).subscribe((value) => { + expect(value).toBe(false); // Validation should be false + + // Update control to valid + service.setControlValidity('control1', true); + + service.validationPassed$.pipe(take(1)).subscribe((newValue) => { + expect(newValue).toBe(true); // Validation should be true + done(); + }); + }); + }); + + it('should handle removing non-existing controls gracefully', (done: DoneFn) => { + service.removeControl('nonExistentControl'); + + service.validationPassed$.pipe(take(1)).subscribe((value) => { + expect(value).toBe(true); // No change in validation state done(); }); }); + + it('should correctly update validation state with multiple controls', (done: DoneFn) => { + service.setControlValidity('control1', true); + service.setControlValidity('control2', true); + service.setControlValidity('control3', false); + + service.validationPassed$.pipe(take(1)).subscribe((value) => { + expect(value).toBe(false); // validation should be false because control3 is invalid + + // Update control3 to valid + service.setControlValidity('control3', true); + + service.validationPassed$.pipe(take(1)).subscribe((newValue) => { + expect(newValue).toBe(true); // validation should now be true + done(); + }); + }); + }); }); diff --git a/libs/shared/common-services/src/validation.service.ts b/libs/shared/common-services/src/validation.service.ts index 0e3da574b1..66c13406d9 100644 --- a/libs/shared/common-services/src/validation.service.ts +++ b/libs/shared/common-services/src/validation.service.ts @@ -5,5 +5,21 @@ import { BehaviorSubject } from 'rxjs'; providedIn: 'root', }) export class ValidationService { + private controlsValidityMap = new Map(); public validationPassed$: BehaviorSubject = new BehaviorSubject(true); + + setControlValidity(control: any, isValid: boolean): void { + this.controlsValidityMap.set(control, isValid); + this.updateValidationState(); + } + + removeControl(control: any): void { + this.controlsValidityMap.delete(control); + this.updateValidationState(); + } + + private updateValidationState(): void { + const allValid = Array.from(this.controlsValidityMap.values()).every((isValid) => isValid); + this.validationPassed$.next(allValid); + } } diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index 25010a1e85..df827f7da4 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -1,14 +1,14 @@ -import { Directive, ElementRef, inject, OnInit, Renderer2 } from '@angular/core'; +import { Directive, ElementRef, inject, OnInit, OnDestroy, Renderer2 } from '@angular/core'; import { AbstractControl, NgControl } from '@angular/forms'; import { SubscriptionHandler } from '@keira/shared/utils'; -import { ValidationService } from '@keira/shared/common-services'; import { distinctUntilChanged } from 'rxjs'; +import { ValidationService } from '@keira/shared/common-services'; @Directive({ selector: '[keiraInputValidation]', standalone: true, }) -export class InputValidationDirective extends SubscriptionHandler implements OnInit { +export class InputValidationDirective extends SubscriptionHandler implements OnInit, OnDestroy { private readonly el: ElementRef = inject(ElementRef); private readonly renderer: Renderer2 = inject(Renderer2); private readonly ngControl: NgControl = inject(NgControl); @@ -23,13 +23,21 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI return; } + this.validationService.setControlValidity(this, control.valid); + this.subscriptions.push( control.statusChanges?.pipe(distinctUntilChanged()).subscribe(() => { this.updateErrorMessage(control); + this.validationService.setControlValidity(this, control.valid); }), ); } + override ngOnDestroy(): void { + this.validationService.removeControl(this); + super.ngOnDestroy(); + } + private updateErrorMessage(control: AbstractControl | null): void { if (this.errorDiv) { this.renderer.removeChild(this.el.nativeElement.parentNode, this.errorDiv); @@ -53,7 +61,5 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI this.renderer.appendChild(parent, this.errorDiv); } } - - this.validationService.validationPassed$.next(!!control?.valid); } } From 8f8cda69be8694744a21c31a486cb46eb944a6b7 Mon Sep 17 00:00:00 2001 From: Exitare Date: Mon, 27 Jan 2025 16:58:27 -0800 Subject: [PATCH 12/13] Address code review. --- .../service/editors/editor.service.spec.ts | 4 - .../src/service/editors/editor.service.ts | 8 +- .../editors/single-row-editor.service.spec.ts | 7 +- .../editors/single-row-editor.service.ts | 2 - .../query-output/query-output.component.html | 26 ++- .../query-output/query-output.component.ts | 19 +- .../common-services/src/validation.service.ts | 2 +- .../src/validate-input.directive.spec.ts | 219 ++---------------- .../src/validate-input.directive.ts | 4 +- 9 files changed, 60 insertions(+), 231 deletions(-) diff --git a/libs/shared/base-abstract-classes/src/service/editors/editor.service.spec.ts b/libs/shared/base-abstract-classes/src/service/editors/editor.service.spec.ts index 803fd714a2..db7b20482b 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/editor.service.spec.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/editor.service.spec.ts @@ -1,13 +1,10 @@ import { TestBed } from '@angular/core/testing'; -import { RouterTestingModule } from '@angular/router/testing'; - import { QueryError } from 'mysql2'; import { ToastrService } from 'ngx-toastr'; import { of, throwError } from 'rxjs'; import { instance, mock } from 'ts-mockito'; import { MysqlQueryService } from '@keira/shared/db-layer'; import { EditorService } from './editor.service'; - import { mockChangeDetectorRef } from '@keira/shared/test-utils'; import { MockEntity, MockSingleRowEditorService } from '../../core.mock'; import Spy = jasmine.Spy; @@ -18,7 +15,6 @@ describe('EditorService', () => { beforeEach(() => TestBed.configureTestingModule({ - imports: [RouterTestingModule], providers: [ { provide: MysqlQueryService, useValue: instance(mock(MysqlQueryService)) }, { provide: ToastrService, useValue: instance(mock(ToastrService)) }, diff --git a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts index 97035412b7..9823f97626 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts @@ -82,7 +82,6 @@ export abstract class EditorService extends SubscriptionHand } protected initForm(): void { - // Create an instance of the entity class to access default values const defaultValues = new this._entityClass(); // Initialize the FormGroup @@ -90,11 +89,8 @@ export abstract class EditorService extends SubscriptionHand // Loop through the fields and initialize controls with default values for (const field of this.fields) { - const defaultValue = defaultValues[field]; // Get the default value dynamically - this._form.addControl( - field, - new FormControl(defaultValue, [Validators.required]), // Use the default value - ); + const defaultValue = defaultValues[field]; + this._form.addControl(field, new FormControl(defaultValue, [Validators.required])); } this.disableEntityIdField(); diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts index 93faab5d94..9aec317ab6 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts @@ -12,7 +12,6 @@ describe('SingleRowEditorService', () => { beforeEach(() => TestBed.configureTestingModule({ - imports: [], providers: [ { provide: MysqlQueryService, useValue: instance(mock(MysqlQueryService)) }, { provide: ToastrService, useValue: instance(mock(ToastrService)) }, @@ -194,9 +193,9 @@ describe('SingleRowEditorService', () => { it('should set values for valid fields in the form', () => { service['updateFormAfterReload'](); - expect(mockForm.controls.id.setValue).toHaveBeenCalledWith(123); // Valid field - expect(mockForm.controls.name.setValue).toHaveBeenCalledWith('Test Name'); // Valid field - expect(mockForm.controls.guid.setValue).toHaveBeenCalledWith(456); // Valid field + expect(mockForm.controls.id.setValue).toHaveBeenCalledWith(123); + expect(mockForm.controls.name.setValue).toHaveBeenCalledWith('Test Name'); + expect(mockForm.controls.guid.setValue).toHaveBeenCalledWith(456); }); it('should log an error for missing controls', () => { diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts index ae6a2dc307..b82436af95 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts @@ -17,8 +17,6 @@ export abstract class SingleRowEditorService extends EditorS protected override handlerService: HandlerService, ) { super(_entityClass, _entityTable, _entityIdField, handlerService); - console.log('Entity Class'); - console.log(_entityClass); this.initForm(); } diff --git a/libs/shared/base-editor-components/src/query-output/query-output.component.html b/libs/shared/base-editor-components/src/query-output/query-output.component.html index 0e3006ebb7..dfeb627dba 100644 --- a/libs/shared/base-editor-components/src/query-output/query-output.component.html +++ b/libs/shared/base-editor-components/src/query-output/query-output.component.html @@ -24,10 +24,22 @@
- - -
diff --git a/libs/shared/base-editor-components/src/query-output/query-output.component.ts b/libs/shared/base-editor-components/src/query-output/query-output.component.ts index 133315a957..0f9302bea0 100644 --- a/libs/shared/base-editor-components/src/query-output/query-output.component.ts +++ b/libs/shared/base-editor-components/src/query-output/query-output.component.ts @@ -1,4 +1,4 @@ -import { ChangeDetectionStrategy, ChangeDetectorRef, Component, EventEmitter, inject, Input, OnInit, Output } from '@angular/core'; +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, EventEmitter, inject, Input, Output } from '@angular/core'; import { FormsModule } from '@angular/forms'; import { EditorService } from '@keira/shared/base-abstract-classes'; import { TableRow } from '@keira/shared/constants'; @@ -11,6 +11,7 @@ import { HighlightjsWrapperComponent } from '../highlightjs-wrapper/highlightjs- import { ModalConfirmComponent } from '../modal-confirm/modal-confirm.component'; import { QueryErrorComponent } from './query-error/query-error.component'; import { ValidationService } from '@keira/shared/common-services'; +import { AsyncPipe } from '@angular/common'; @Component({ // eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @@ -19,30 +20,20 @@ import { ValidationService } from '@keira/shared/common-services'; templateUrl: './query-output.component.html', styleUrls: ['./query-output.component.scss'], standalone: true, - imports: [FormsModule, HighlightjsWrapperComponent, QueryErrorComponent, TranslateModule], + imports: [FormsModule, HighlightjsWrapperComponent, QueryErrorComponent, TranslateModule, AsyncPipe], }) -export class QueryOutputComponent extends SubscriptionHandler implements OnInit { +export class QueryOutputComponent extends SubscriptionHandler { private readonly clipboardService = inject(ClipboardService); private readonly modalService = inject(BsModalService); - private readonly validationService = inject(ValidationService); + protected readonly validationService = inject(ValidationService); @Input() docUrl!: string; @Input() editorService!: EditorService; @Output() executeQuery = new EventEmitter(); selectedQuery: 'diff' | 'full' = 'diff'; private modalRef!: BsModalRef; - protected validationPassed: boolean = true; private readonly changeDetectorRef = inject(ChangeDetectorRef); - ngOnInit() { - this.subscriptions.push( - this.validationService.validationPassed$.subscribe((validationPassed: boolean) => { - this.validationPassed = validationPassed; - this.changeDetectorRef.detectChanges(); - }), - ); - } - showFullQuery(): boolean { return this.editorService.isNew || this.selectedQuery === 'full'; } diff --git a/libs/shared/common-services/src/validation.service.ts b/libs/shared/common-services/src/validation.service.ts index 66c13406d9..eedf4b38b4 100644 --- a/libs/shared/common-services/src/validation.service.ts +++ b/libs/shared/common-services/src/validation.service.ts @@ -6,7 +6,7 @@ import { BehaviorSubject } from 'rxjs'; }) export class ValidationService { private controlsValidityMap = new Map(); - public validationPassed$: BehaviorSubject = new BehaviorSubject(true); + readonly validationPassed$: BehaviorSubject = new BehaviorSubject(true); setControlValidity(control: any, isValid: boolean): void { this.controlsValidityMap.set(control, isValid); diff --git a/libs/shared/directives/src/validate-input.directive.spec.ts b/libs/shared/directives/src/validate-input.directive.spec.ts index 63435d1440..2abdf92dd3 100644 --- a/libs/shared/directives/src/validate-input.directive.spec.ts +++ b/libs/shared/directives/src/validate-input.directive.spec.ts @@ -1,10 +1,9 @@ import { Component, DebugElement } from '@angular/core'; -import { TestBed, ComponentFixture } from '@angular/core/testing'; +import { TestBed, ComponentFixture, fakeAsync, tick } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; -import { ReactiveFormsModule, FormControl, FormsModule, NgControl, AbstractControl } from '@angular/forms'; +import { ReactiveFormsModule, FormControl, FormsModule, NgControl } from '@angular/forms'; import { InputValidationDirective } from './validate-input.directive'; import { ValidationService } from '@keira/shared/common-services'; -import { take } from 'rxjs'; @Component({ template: ` @@ -21,7 +20,6 @@ class TestComponent { describe('InputValidationDirective', () => { let fixture: ComponentFixture; - let validationService: ValidationService; let debugElement: DebugElement; beforeEach(() => { @@ -32,35 +30,31 @@ describe('InputValidationDirective', () => { }).compileComponents(); fixture = TestBed.createComponent(TestComponent); - validationService = TestBed.inject(ValidationService); debugElement = fixture.debugElement.query(By.directive(InputValidationDirective)); fixture.detectChanges(); }); - it('should create an instance', () => { - const directive = debugElement.injector.get(InputValidationDirective); - expect(directive).toBeTruthy(); - }); - - it('should display error message when control is invalid and touched', () => { + it('should display error message when control is invalid and touched', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); expect(errorDiv.textContent).toBe('This field is required'); - }); + })); - it('should remove error message when control becomes valid', () => { + it('should remove error message when control becomes valid', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); let errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); @@ -70,53 +64,29 @@ describe('InputValidationDirective', () => { control?.setValue('Valid value'); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime again fixture.detectChanges(); errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeNull(); - }); - - it('should handle empty error object gracefully', () => { - const control = debugElement.injector.get(NgControl).control; - - control?.setValidators(() => null); // Simulate no errors - control?.setValue(''); - control?.updateValueAndValidity(); - - fixture.detectChanges(); - - const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); // Ensure no error message is created - }); + })); - it('should handle multiple error types and display first error', () => { + it('should handle multiple error types and display first error', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ required: true, minlength: true })); control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); expect(errorDiv.textContent).toBe('This field is required'); - }); - - it('should not create errorDiv if parentNode is null', () => { - const control = debugElement.injector.get(NgControl).control; - spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); - - control?.setValidators(() => ({ required: true })); - control?.markAsTouched(); - control?.updateValueAndValidity(); - - fixture.detectChanges(); + })); - expect(debugElement.nativeElement.parentNode?.querySelector('.error-message')).toBeFalsy(); - }); - - it('should mark control as touched when invalid', () => { + it('should mark control as touched when invalid', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; spyOn(control!, 'markAsTouched').and.callThrough(); @@ -124,41 +94,13 @@ describe('InputValidationDirective', () => { control?.setValue(''); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); expect(control?.markAsTouched).toHaveBeenCalled(); - }); - - it('should update validationPassed$ with correct value', (done: DoneFn) => { - const control = debugElement.injector.get(NgControl).control; - - control?.setValidators(() => ({ required: true })); - control?.setValue(''); - control?.updateValueAndValidity(); - - fixture.detectChanges(); - - validationService.validationPassed$.pipe(take(1)).subscribe((isValid) => { - expect(isValid).toBe(false); - done(); - }); - - control?.setValue('Valid value'); - control?.updateValueAndValidity(); - - fixture.detectChanges(); - }); + })); - it('should not throw error when ngControl.control is null', () => { - const ngControl = debugElement.injector.get(NgControl); - spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); - }); - - it('should safely remove errorDiv if already exists', () => { + it('should safely remove errorDiv if already exists', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; // Set invalid state to create an errorDiv @@ -166,6 +108,7 @@ describe('InputValidationDirective', () => { control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); let errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); @@ -175,154 +118,42 @@ describe('InputValidationDirective', () => { control?.clearValidators(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeNull(); - }); + })); - it('should not throw error if errorDiv is already null', () => { - const directive = debugElement.injector.get(InputValidationDirective); - directive['errorDiv'] = null; // Manually set to null - const control = debugElement.injector.get(NgControl).control; - - control?.setValidators(() => ({ required: true })); - control?.markAsTouched(); - control?.updateValueAndValidity(); - - expect(() => fixture.detectChanges()).not.toThrow(); - }); - - it('should handle statusChanges being null gracefully', () => { - const control = debugElement.injector.get(NgControl).control as AbstractControl; - - // Mock the control to have statusChanges as null - Object.defineProperty(control, 'statusChanges', { - get: () => null, - }); - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); - }); - - it('should not throw error if control.errors is null', () => { - const control = debugElement.injector.get(NgControl).control as AbstractControl; - - // Mock the control to have errors as null - Object.defineProperty(control, 'errors', { - get: () => null, - }); - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); - - const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); - }); - - it('should not add errorDiv if parentNode is null', () => { - spyOnProperty(debugElement.nativeElement, 'parentNode', 'get').and.returnValue(null); - - const control = debugElement.injector.get(NgControl).control; - control?.setValidators(() => ({ required: true })); - control?.markAsTouched(); - control?.updateValueAndValidity(); - - fixture.detectChanges(); - - expect(debugElement.nativeElement.parentNode?.querySelector('.error-message')).toBeFalsy(); - }); - - it('should return early if control is null', () => { - const ngControl = debugElement.injector.get(NgControl); - spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); - - expect(() => fixture.detectChanges()).not.toThrow(); - }); - - it('should display correct error message for "required" error', () => { + it('should display correct error message for "required" error', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ required: true })); control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); expect(errorDiv.textContent).toBe('This field is required'); - }); + })); - it('should display "Invalid field" for non-required errors', () => { + it('should display "Invalid field" for non-required errors', fakeAsync(() => { const control = debugElement.injector.get(NgControl).control; control?.setValidators(() => ({ minlength: { requiredLength: 5, actualLength: 3 } })); control?.markAsTouched(); control?.updateValueAndValidity(); + tick(500); // Simulate debounceTime fixture.detectChanges(); const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); expect(errorDiv).toBeTruthy(); expect(errorDiv.textContent).toBe('Invalid field'); - }); - - it('should handle control.errors being null without creating errorDiv', () => { - const ngControl = debugElement.injector.get(NgControl); - - // Create a FormControl instance to mock AbstractControl - const controlMock = new FormControl(); - - // Directly assign null to the errors property - Object.defineProperty(controlMock, 'errors', { - get: () => null, - configurable: true, - }); - - // Replace the ngControl's control with the mock control - spyOnProperty(ngControl, 'control', 'get').and.returnValue(controlMock); - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); - - const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); // Verify no errorDiv is created - }); - - it('should not throw error if control.errors is undefined', () => { - const control = debugElement.injector.get(NgControl).control as AbstractControl; - - // Mock the control to have errors as undefined - Object.defineProperty(control, 'errors', { - get: () => undefined, - }); - - expect(() => { - fixture.detectChanges(); - }).not.toThrow(); - - const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); - }); - - it('should return early if control is null', () => { - const ngControl = debugElement.injector.get(NgControl); - - // Mock ngControl.control to be null - spyOnProperty(ngControl, 'control', 'get').and.returnValue(null); - - expect(() => { - fixture.detectChanges(); // Triggers ngOnInit - }).not.toThrow(); - - // Verify no errorDiv is created - const errorDiv = debugElement.nativeElement.parentNode.querySelector('.error-message'); - expect(errorDiv).toBeNull(); - }); + })); it('should return early if control is null (explicit)', () => { const directive = debugElement.injector.get(InputValidationDirective); diff --git a/libs/shared/directives/src/validate-input.directive.ts b/libs/shared/directives/src/validate-input.directive.ts index df827f7da4..d99c0efc4b 100644 --- a/libs/shared/directives/src/validate-input.directive.ts +++ b/libs/shared/directives/src/validate-input.directive.ts @@ -1,7 +1,7 @@ import { Directive, ElementRef, inject, OnInit, OnDestroy, Renderer2 } from '@angular/core'; import { AbstractControl, NgControl } from '@angular/forms'; import { SubscriptionHandler } from '@keira/shared/utils'; -import { distinctUntilChanged } from 'rxjs'; +import { debounceTime, distinctUntilChanged } from 'rxjs'; import { ValidationService } from '@keira/shared/common-services'; @Directive({ @@ -26,7 +26,7 @@ export class InputValidationDirective extends SubscriptionHandler implements OnI this.validationService.setControlValidity(this, control.valid); this.subscriptions.push( - control.statusChanges?.pipe(distinctUntilChanged()).subscribe(() => { + control.statusChanges?.pipe(distinctUntilChanged(), debounceTime(500)).subscribe(() => { this.updateErrorMessage(control); this.validationService.setControlValidity(this, control.valid); }), From 46a70b30cf756d43bdb2ef7fadb7e7d29e5b8d16 Mon Sep 17 00:00:00 2001 From: Exitare Date: Tue, 28 Jan 2025 16:43:08 -0800 Subject: [PATCH 13/13] Address code review --- .../src/service/editors/editor.service.ts | 1 - .../editors/single-row-editor.service.spec.ts | 15 +---------- .../editors/single-row-editor.service.ts | 19 ++++++-------- .../src/validation.service.spec.ts | 18 ++++++++----- .../common-services/src/validation.service.ts | 2 +- .../src/validate-input.directive.spec.ts | 25 ++++++++++++------- 6 files changed, 37 insertions(+), 43 deletions(-) diff --git a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts index 9823f97626..6ce1da278c 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/editor.service.ts @@ -84,7 +84,6 @@ export abstract class EditorService extends SubscriptionHand protected initForm(): void { const defaultValues = new this._entityClass(); - // Initialize the FormGroup this._form = new FormGroup>({} as any); // Loop through the fields and initialize controls with default values diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts index 9aec317ab6..4ba9ac30af 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.spec.ts +++ b/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'](); diff --git a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts index b82436af95..d74d73ea03 100644 --- a/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts +++ b/libs/shared/base-abstract-classes/src/service/editors/single-row-editor.service.ts @@ -20,7 +20,7 @@ export abstract class SingleRowEditorService extends EditorS this.initForm(); } - protected override initForm() { + protected override initForm(): void { super.initForm(); this.subscriptions.push( @@ -57,7 +57,7 @@ export abstract class SingleRowEditorService 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 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 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 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 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 extends EditorS this.setLoadedEntity(); this.updateFullQuery(); } + /* ****** */ } diff --git a/libs/shared/common-services/src/validation.service.spec.ts b/libs/shared/common-services/src/validation.service.spec.ts index bf7f8b33ed..5da3d7adb7 100644 --- a/libs/shared/common-services/src/validation.service.spec.ts +++ b/libs/shared/common-services/src/validation.service.spec.ts @@ -3,21 +3,24 @@ 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(); @@ -25,6 +28,7 @@ describe('ValidationService', () => { }); 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); diff --git a/libs/shared/common-services/src/validation.service.ts b/libs/shared/common-services/src/validation.service.ts index eedf4b38b4..d8fd48f347 100644 --- a/libs/shared/common-services/src/validation.service.ts +++ b/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(); + private readonly controlsValidityMap = new Map(); readonly validationPassed$: BehaviorSubject = new BehaviorSubject(true); setControlValidity(control: any, isValid: boolean): void { diff --git a/libs/shared/directives/src/validate-input.directive.spec.ts b/libs/shared/directives/src/validate-input.directive.spec.ts index 2abdf92dd3..c482728453 100644 --- a/libs/shared/directives/src/validate-input.directive.spec.ts +++ b/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; - 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);