-
-
Notifications
You must be signed in to change notification settings - Fork 180
Add logic to allow warnings for input fields #3256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 15 commits
f6a29cd
e5cbaaa
3b99c72
b661cd0
0051ff3
51ae79f
e3a6a85
4ded5c1
8febd6f
78899c7
4196e13
b009f20
9262e7d
25817b0
02a4661
3a53132
8f8cda6
46a70b3
04346e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -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<T extends TableRow> extends SubscriptionHand | |||
| } | ||||
|
|
||||
| protected initForm(): void { | ||||
| // Create an instance of the entity class to access default values | ||||
| const defaultValues = new this._entityClass(); | ||||
|
|
||||
| // Initialize the FormGroup | ||||
|
Exitare marked this conversation as resolved.
Outdated
|
||||
| this._form = new FormGroup<ModelForm<T>>({} as any); | ||||
|
|
||||
| // Loop through the fields and initialize controls with default values | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need this comment, IMHO the code seems already quite self-explanatory :)
Suggested change
|
||||
| for (const field of this.fields) { | ||||
| this._form.addControl(field, new FormControl()); | ||||
| const defaultValue = defaultValues[field]; // Get the default value dynamically | ||||
|
Exitare marked this conversation as resolved.
Outdated
|
||||
| this._form.addControl( | ||||
| field, | ||||
| new FormControl(defaultValue, [Validators.required]), // Use the default value | ||||
| ); | ||||
| } | ||||
|
|
||||
| this.disableEntityIdField(); | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,8 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS | |
| protected override handlerService: HandlerService<T>, | ||
| ) { | ||
| super(_entityClass, _entityTable, _entityIdField, handlerService); | ||
| console.log('Entity Class'); | ||
| console.log(_entityClass); | ||
|
Exitare marked this conversation as resolved.
Outdated
|
||
| this.initForm(); | ||
| } | ||
|
|
||
|
|
@@ -86,19 +88,27 @@ export abstract class SingleRowEditorService<T extends TableRow> extends EditorS | |
|
|
||
| protected updateFormAfterReload() { | ||
|
Exitare marked this conversation as resolved.
Outdated
|
||
| 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` | ||
|
Exitare marked this conversation as resolved.
|
||
| if (typeof field === 'string') { | ||
| const control = this._form.controls[field]; | ||
|
Comment on lines
+92
to
+93
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this change needed for the original purpose of the PR? in general, I'd prefer to split changes in several PRs, for example:
|
||
|
|
||
| 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); | ||
| } | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure about this code inside the else block, it seems more a debug code , we could keep it, but could you give me more insight about it?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Exitare please do not mark comments as resolved without answering or addressing them
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I resolved it, because I removed it initially. But then one of the tests failed and I added it back, but didn't unresolve the issue here. |
||
| } else { | ||
| console.warn(`Field '${String(field)}' is not a valid string key.`); | ||
| } | ||
| } | ||
|
|
||
| this._loading = false; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| export * from './electron.service'; | ||
| export * from './config.service'; | ||
| export * from './location.service'; | ||
| export * from './validation.service'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
|
Exitare marked this conversation as resolved.
Outdated
|
||
|
|
||
| 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(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import { Injectable } from '@angular/core'; | ||
| import { BehaviorSubject } from 'rxjs'; | ||
|
|
||
| @Injectable({ | ||
| providedIn: 'root', | ||
| }) | ||
| export class ValidationService { | ||
| public validationPassed$: BehaviorSubject<boolean> = new BehaviorSubject(true); | ||
|
Exitare marked this conversation as resolved.
Outdated
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.