Skip to content
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.

Sign up for GitHub

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

fix: form value bug fixes #1022

Merged
merged 4 commits into from
Oct 17, 2024
Merged

fix: form value bug fixes #1022

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dfa6a6c
form value bug fixes
DhaaraniCIT Oct 15, 2024
4359975
Merge branch 'master' into integrstion-form-value-access-changes-fix
DhaaraniCIT Oct 15, 2024
575bd5d
fixes in import and advanced settings
DhaaraniCIT Oct 17, 2024
716d720
Merge branch 'master' into integrstion-form-value-access-changes-fix
DhaaraniCIT Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ...ared/business-central-advanced-settings/business-central-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,16 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">Preview of the Description
[skipExportForm]="skipExportForm"
[expenseFilter]="expenseFilters"
[conditionFieldOptions]="conditionFieldOptions"
(deleteSkipExportForm)="deleteExpenseFilter($event)">
(deleteSkipExportForm)="deleteExpenseFilter($event)"
(invalidSkipExportForm)="invalidSkipExportForm($event)">
</app-configuration-skip-export>
</div>
</div>
</div>
<app-configuration-step-footer
[ctaText] = "!isSaveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING"
[showBackButton] = "isOnboarding"
[isButtonDisabled]="!advancedSettingForm.valid || !skipExportForm.valid || !getSkipExportValue()"
[isButtonDisabled]="!advancedSettingForm.valid || !skipExportForm.valid || !getSkipExportValue() || isSkipExportFormInvalid"
(save)="save()"
(navigateToPreviousStep)="navigateBack()">
</app-configuration-step-footer>
Expand Down
6 changes: 6 additions & 0 deletions ...shared/business-central-advanced-settings/business-central-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ export class BusinessCentralAdvancedSettingsComponent implements OnInit {

readonly brandingConfig = brandingConfig;

isSkipExportFormInvalid: boolean;

constructor(
private advancedSettingsService: BusinessCentralAdvancedSettingsService,
private helper: HelperService,
Expand All @@ -70,6 +72,10 @@ export class BusinessCentralAdvancedSettingsComponent implements OnInit {
private router: Router
) { }

invalidSkipExportForm($event: boolean) {
this.isSkipExportFormInvalid = $event;
}

getSkipExportValue() {
if (this.advancedSettingForm.controls.skipExport) {
if (this.skipExportForm.controls.condition1.value) {
Expand Down
5 changes: 3 additions & 2 deletions ...intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@
#skipExportChild
[enableSkipExport]="advancedSettingsForm.get('skipSelectiveExpenses')?.value"
[skipExportForm]="skipExportForm"
(skipExportFormChange)="updateForm($event)">
(skipExportFormChange)="updateForm($event)"
(invalidSkipExportForm)="invalidSkipExportForm($event)">
</app-skip-export>
</div>
</div>
Expand Down Expand Up @@ -238,6 +239,6 @@
</div>
</div>
</div>
<app-configuration-step-footer (navigateToPreviousStep)="navigateToPreviousStep()" [ctaText] = "!saveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING" (save)="save()" [isButtonDisabled]="!advancedSettingsForm.valid || (advancedSettingsForm.get('skipSelectiveExpenses')?.value ? !skipExportForm.valid : false)" [showBackButton]="isOnboarding ? true : false"></app-configuration-step-footer>
<app-configuration-step-footer (navigateToPreviousStep)="navigateToPreviousStep()" [ctaText] = "!saveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING" (save)="save()" [isButtonDisabled]="isSkipExportFormInvalid || !advancedSettingsForm.valid || (advancedSettingsForm.get('skipSelectiveExpenses')?.value ? !skipExportForm.valid : false)" [showBackButton]="isOnboarding ? true : false"></app-configuration-step-footer>
</form>
</div>
5 changes: 5 additions & 0 deletions ...s/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { DestinationAttribute } from 'src/app/core/models/db/destination-attribu
})

export class IntacctAdvancedSettingsComponent implements OnInit {
isSkipExportFormInvalid: boolean;

@ViewChild('skipExportChild') skipExportChild: SkipExportComponent;

Expand Down Expand Up @@ -129,6 +130,10 @@ export class IntacctAdvancedSettingsComponent implements OnInit {
private mappingService: SiMappingsService
) { }

invalidSkipExportForm($event: boolean) {
this.isSkipExportFormInvalid = $event;
}

navigateToPreviousStep(): void {
this.router.navigate([`/integrations/intacct/onboarding/import_settings`]);
}
Expand Down
3 changes: 3 additions & 0 deletions ...intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ export class IntacctC1ImportSettingsComponent implements OnInit {
}

closeModel() {
this.customFieldControl.patchValue({
source_field: null
});
this.customFieldForm.reset();
this.showDialog = false;
}
Expand Down
9 changes: 8 additions & 1 deletion ...tions/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ export class IntacctImportSettingsComponent implements OnInit {
}

closeModel() {
this.customFieldControl.patchValue({
source_field: null
});
Comment on lines +261 to +263
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add null check for 'customFieldControl' before calling 'patchValue'

In the closeModel() method, ensure that this.customFieldControl is not null or undefined before calling patchValue to prevent potential runtime errors.

Consider updating the code as follows:

+ if (this.customFieldControl) {
    this.customFieldControl.patchValue({
        source_field: null
    });
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.customFieldControl.patchValue({
source_field: null
});
if (this.customFieldControl) {
this.customFieldControl.patchValue({
source_field: null
});
}

this.customFieldForm.reset();
this.showDialog = false;
}
Expand Down Expand Up @@ -337,13 +340,17 @@ export class IntacctImportSettingsComponent implements OnInit {

this.importSettingsForm.controls.isDependentImportEnabled.valueChanges.subscribe((isDependentImportEnabled) => {
if (isDependentImportEnabled) {
this.importSettingsForm.controls.costCodes.patchValue(null);
this.importSettingsForm.controls.costTypes.patchValue(null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is common for if and else, move it out

this.importSettingsForm.controls.costCodes.enable();
this.importSettingsForm.controls.costTypes.enable();
this.importSettingsForm.controls.costCodes.setValidators(Validators.required);
this.importSettingsForm.controls.costTypes.setValidators(Validators.required);
} else {
this.importSettingsForm.controls.costCodes.patchValue(null);
this.importSettingsForm.controls.costTypes.patchValue(null);
this.importSettingsForm.controls.costCodes.disable();
this.importSettingsForm.controls.costTypes.disable();
this.importSettingsForm.controls.costTypes.disable();
this.importSettingsForm.controls.costCodes.clearValidators();
this.importSettingsForm.controls.costTypes.clearValidators();
}
Expand Down
5 changes: 3 additions & 2 deletions ...uite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">{{brandingContent.previewD
[skipExportForm]="skipExportForm"
[expenseFilter]="expenseFilters"
[conditionFieldOptions]="conditionFieldOptions"
(deleteSkipExportForm)="deleteExpenseFilter($event)">
(deleteSkipExportForm)="deleteExpenseFilter($event)"
(invalidSkipExportForm)="invalidSkipExportForm($event)">
</app-configuration-skip-export>
</div>
</div>
Expand Down Expand Up @@ -246,7 +247,7 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">{{brandingContent.previewD
</div>
<app-configuration-step-footer
[ctaText] = "!isSaveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING"
[isButtonDisabled]="!advancedSettingForm.valid"
[isButtonDisabled]="!advancedSettingForm.valid || isSkipExportFormInvalid"
[showBackButton]="isOnboarding ? true : false"
(save)="save()"
(navigateToPreviousStep)="navigateToPreviousStep()">
Expand Down
6 changes: 6 additions & 0 deletions ...tsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ export class NetsuiteAdvancedSettingsComponent implements OnInit {

readonly brandingContent = brandingContent.netsuite.configuration.advancedSettings;

isSkipExportFormInvalid: boolean;

constructor(
private advancedSettingsService: NetsuiteAdvancedSettingsService,
private configurationService: ConfigurationService,
Expand All @@ -107,6 +109,10 @@ export class NetsuiteAdvancedSettingsComponent implements OnInit {
return brandingConfig.brandId === 'co' ? ' \(optional\)' : '';
}

invalidSkipExportForm($event: boolean) {
this.isSkipExportFormInvalid = $event;
}

getCreateVendorLabel(): string {
if (this.workspaceGeneralSettings.employee_field_mapping === EmployeeFieldMapping.VENDOR) {
return brandingConfig.brandId === 'co' ? EmployeeFieldMapping.VENDOR.toLowerCase() : new TitleCasePipe().transform(EmployeeFieldMapping.VENDOR);
Expand Down
5 changes: 3 additions & 2 deletions ...pp/integrations/qbo/qbo-shared/qbo-advanced-settings/qbo-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,15 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">{{brandingContent.previewD
[skipExportForm]="skipExportForm"
[expenseFilter]="expenseFilters"
[conditionFieldOptions]="conditionFieldOptions"
(deleteSkipExportForm)="deleteExpenseFilter($event)">
(deleteSkipExportForm)="deleteExpenseFilter($event)"
(invalidSkipExportForm)="invalidSkipExportForm($event)">
</app-configuration-skip-export>
</div>
</div>
</div>
<app-configuration-step-footer
[ctaText] = "!isSaveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING"
[isButtonDisabled]="!advancedSettingForm.valid"
[isButtonDisabled]="!advancedSettingForm.valid || isSkipExportFormInvalid"
[showBackButton]="isOnboarding ? true : false"
(save)="save()"
(navigateToPreviousStep)="navigateToPreviousStep()"></app-configuration-step-footer>
Expand Down
6 changes: 6 additions & 0 deletions src/app/integrations/qbo/qbo-shared/qbo-advanced-settings/qbo-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ export class QboAdvancedSettingsComponent implements OnInit {

readonly brandingContent = brandingContent.configuration.advancedSettings;

isSkipExportFormInvalid: boolean;

constructor(
private advancedSettingsService: QboAdvancedSettingsService,
private configurationService: ConfigurationService,
Expand All @@ -94,6 +96,10 @@ export class QboAdvancedSettingsComponent implements OnInit {
this.router.navigate([`/integrations/qbo/onboarding/import_settings`]);
}

invalidSkipExportForm($event: boolean) {
this.isSkipExportFormInvalid = $event;
}

private saveSkipExportFields(): void {
if (!this.skipExportForm.valid) {
return;
Expand Down
5 changes: 3 additions & 2 deletions ...sage300/sage300-shared/sage300-advanced-settings/sage300-advanced-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">Preview of the Description
[skipExportForm]="skipExportForm"
[expenseFilter]="expenseFilters"
[conditionFieldOptions]="conditionFieldOptions"
(deleteSkipExportForm)="deleteExpenseFilter($event)">
(deleteSkipExportForm)="deleteExpenseFilter($event)"
(invalidSkipExportForm)="invalidSkipExportForm($event)">
</app-configuration-skip-export>
</div>
</div>
Expand All @@ -94,6 +95,6 @@ <h4 class="tw-text-form-label-text-color tw-mb-12-px">Preview of the Description
(save)="save()"
[showBackButton] = "isOnboarding"
(navigateToPreviousStep)="navigateBack()"
[isButtonDisabled]="!advancedSettingForm.valid || !skipExportForm.valid || !getSkipExportValue()"></app-configuration-step-footer>
[isButtonDisabled]="!advancedSettingForm.valid || !skipExportForm.valid || !getSkipExportValue() || isSkipExportFormInvalid"></app-configuration-step-footer>
</form>
</div>
6 changes: 6 additions & 0 deletions ...s/sage300/sage300-shared/sage300-advanced-settings/sage300-advanced-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ export class Sage300AdvancedSettingsComponent implements OnInit {

readonly brandingConfig = brandingConfig;

isSkipExportFormInvalid: boolean;

constructor(
private advancedSettingsService: Sage300AdvancedSettingsService,
private helper: HelperService,
Expand All @@ -79,6 +81,10 @@ export class Sage300AdvancedSettingsComponent implements OnInit {
private router: Router
) { }

invalidSkipExportForm($event: boolean) {
this.isSkipExportFormInvalid = $event;
}

private formatMemoPreview(): void {
const time = Date.now();
const today = new Date(time);
Expand Down
3 changes: 3 additions & 0 deletions ...tions/sage300/sage300-shared/sage300-import-settings/sage300-import-settings.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export class Sage300ImportSettingsComponent implements OnInit {
}

closeModel() {
this.customFieldControl.patchValue({
source_field: null
});
Comment on lines +128 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure customFieldControl is initialized before usage

In the closeModel() method, this.customFieldControl might be undefined if it hasn't been initialized, which could lead to runtime errors. Consider adding a null check before calling patchValue() to prevent potential issues.

Apply this diff to add a null check:

closeModel() {
+   if (this.customFieldControl) {
      this.customFieldControl.patchValue({
        source_field: null
      });
+   }
    this.customFieldForm.reset();
    this.showCustomFieldDialog = false;
}

Committable suggestion was skipped due to low confidence.

this.customFieldForm.reset();
this.showCustomFieldDialog = false;
}
Expand Down
2 changes: 1 addition & 1 deletion .../integrations/xero/xero-onboarding/xero-clone-settings/xero-clone-settings.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@
[form]="advancedSettingForm"
[isFieldMandatory]="true"
[formControllerName]="'billPaymentAccount'"
[dropdownDisplayKey]="'value'"
[dropdownDisplayKey]="'name'"
[tooltipText]="'Once the payment for the reimbursable expense is complete in ' + brandingConfig.brandName + ', the payment entries will be posted to the selected Payment account in Xero.'">
</app-clone-setting-field>
</div>
Expand Down
5 changes: 4 additions & 1 deletion ...components/configuration/configuration-skip-export/configuration-skip-export.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export class ConfigurationSkipExportComponent implements OnInit {

@Output() deleteSkipExportForm = new EventEmitter<number>();

@Output() invalidSkipExportForm = new EventEmitter<boolean>();

isLoading: boolean = true;

showExpenseFilters: boolean;
Expand Down Expand Up @@ -209,11 +211,12 @@ export class ConfigurationSkipExportComponent implements OnInit {
if (this.showAdditionalCondition) {
if (condition1.valid && condition2.valid) {
if (condition1.value?.field_name === condition2.value?.field_name) {
this.skipExportForm.controls.operator2.setValue(null);
this.invalidSkipExportForm.emit(true);
return true;
}
}
}
this.invalidSkipExportForm.emit(false);
return false;
Comment on lines +214 to 220
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Review the logic in checkValidationCondition method

The current implementation of checkValidationCondition seems to have a logical issue:

  1. It emits true (indicating an invalid form) when both conditions are valid and have the same field_name.
  2. It emits false (indicating a valid form) in all other cases.

This logic appears counterintuitive. Typically, we would expect the form to be considered invalid when conditions are not met, rather than when they are. Please review and clarify if this is the intended behavior.

Consider revising the logic to align with expected form validation behavior. For example:

checkValidationCondition() {
  const condition1 = this.skipExportForm.controls.condition1;
  const condition2 = this.skipExportForm.controls.condition2;
  if (this.showAdditionalCondition) {
    if (condition1.valid && condition2.valid) {
      if (condition1.value?.field_name === condition2.value?.field_name) {
        this.invalidSkipExportForm.emit(false);
        return false; // Form is valid when conditions are different
      }
    }
  }
  this.invalidSkipExportForm.emit(true);
  return true; // Form is invalid in other cases
}

Please review and adjust according to the intended validation logic.

}

Expand Down
5 changes: 2 additions & 3 deletions src/app/shared/components/si/helper/skip-export/skip-export.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@
</p-dropdown>
<app-mandatory-error-message *ngIf="skipExportForm.controls.condition2.touched && !skipExportForm.controls.condition2.valid"
[customErrorMessage]="'Please select the condition'"></app-mandatory-error-message>
<app-mandatory-error-message *ngIf="checkValidationCondition()"
[customErrorMessage]="'Condition selected should be distinct.'"></app-mandatory-error-message>
</div>
<div class="tw-mr-24-px">
<p-dropdown appendTo="body" [options]="operatorFieldOptions2" formControlName="operator2" placeholder="Select Operator"></p-dropdown>
Expand All @@ -117,9 +119,6 @@
<app-svg-icon [svgSource]="'bin'" [width]="'24px'" [height]="'24px'" [styleClasses]="'tw-text-sub-text-color tw-text-center tw-cursor-pointer'" [tooltipText]="'Remove Condition'" (iconClick)="remCondition()"></app-svg-icon>
</div>
</div>
<div class=" tw-text-mandatory-field-color" *ngIf="checkValidationCondition()">
<p>*Condition selected should be distinct.</p>
</div>
</div>
<div class="tw-flex items-center tw-text-mandatory-field-color tw-pt-12-px tw-pb-24-px" *ngIf="showAddButton">
<app-svg-icon [svgSource]="'plus-square-medium'" [width]="'18px'" [height]="'18px'" (iconClick)="updateAdditionalFilterVisibility(true)"></app-svg-icon>
Expand Down
10 changes: 7 additions & 3 deletions src/app/shared/components/si/helper/skip-export/skip-export.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { forkJoin } from 'rxjs';
import { constructPayload1, constructPayload2 } from 'src/app/core/models/intacct/misc/skip-export.model';
import { ConditionField, CustomOperatorOption, ExpenseFilterResponse, JoinOptions, SkipExport } from 'src/app/core/models/intacct/intacct-configuration/advanced-settings.model';
import { SiAdvancedSettingService } from 'src/app/core/services/si/si-configuration/si-advanced-setting.service';
import { RxwebValidators } from '@rxweb/reactive-form-validators';

@Component({
selector: 'app-skip-export',
Expand All @@ -18,6 +19,8 @@ export class SkipExportComponent implements OnInit {

@Output() skipExportFormChange = new EventEmitter<FormGroup>();

@Output() invalidSkipExportForm = new EventEmitter<boolean>();

isLoading: boolean = true;

expenseFilters: SkipExport[];
Expand Down Expand Up @@ -226,7 +229,7 @@ export class SkipExportComponent implements OnInit {
this.showAddButton = !show;
if (this.showAdditionalCondition) {
this.skipExportForm.controls.join_by.setValidators(Validators.required);
this.skipExportForm.controls.condition2.setValidators(Validators.required);
this.skipExportForm.controls.condition2.setValidators([Validators.required, RxwebValidators.unique()]);
this.skipExportForm.controls.operator2.setValidators(Validators.required);
if (this.valueOption2.length===0) {
this.skipExportForm.controls.value2.setValidators(Validators.required);
Expand Down Expand Up @@ -254,11 +257,12 @@ export class SkipExportComponent implements OnInit {
if (this.showAdditionalCondition) {
if (condition1.valid && condition2.valid) {
if (condition1.value?.field_name === condition2.value?.field_name) {
this.skipExportForm.controls.operator2.setValue(null);
this.invalidSkipExportForm.emit(true);
return true;
}
}
}
this.invalidSkipExportForm.emit(false);
return false;
}

Expand Down Expand Up @@ -567,7 +571,7 @@ export class SkipExportComponent implements OnInit {
if (response.count === 2) {
this.showAdditionalCondition = true;
this.showAddButton = false;
this.skipExportForm.controls.condition2.setValidators(Validators.required);
this.skipExportForm.controls.condition2.setValidators([Validators.required, RxwebValidators.unique()]);
this.skipExportForm.controls.operator2.setValidators(Validators.required);
this.skipExportForm.controls.join_by.setValidators(Validators.required);
if (!this.valueOption2.length && !(selectedOperator2 === 'is_empty' || selectedOperator2 === 'is_not_empty')) {
Expand Down
Loading