feat: expose real-time exports and import with code for c1#1760
Open
ashwin1111 wants to merge 4 commits intomasterfrom
Open
feat: expose real-time exports and import with code for c1#1760ashwin1111 wants to merge 4 commits intomasterfrom
ashwin1111 wants to merge 4 commits intomasterfrom
Conversation
Contributor
WalkthroughRemoved two branding feature flags ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/app/core/services/common/advanced-settings.service.ts (1)
25-33: Unnecessary double-spread around theReal-timeentry.
...[{ label: 'Real-time', value: 0 }]spreads a single-element array that was just constructed inline — it's equivalent to a plain object literal in this position.♻️ Proposed cleanup
static getHoursOptions(): SelectFormOption[] { return [ - ...[{ label: 'Real-time', value: 0 }], + { label: 'Real-time', value: 0 }, ...[...Array(24).keys()].map(hour => ({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/common/advanced-settings.service.ts` around lines 25 - 33, The getHoursOptions function currently uses an unnecessary double-spread for the "Real-time" entry (...[{ label: 'Real-time', value: 0 }]); remove the outer spread and return the options as a single array where the Real-time object is included directly before the spread of the hours array (e.g., include { label: 'Real-time', value: 0 } followed by ...[...Array(24).keys()].map(...) ) so the output shape is unchanged but the redundant spread is eliminated.src/app/core/services/common/accounting-export.service.ts (1)
272-287: Extract the allowlist to a named constant to reduce duplication.The inline array
[AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300]is duplicated across the codebase in at least three locations:
src/app/core/services/common/accounting-export.service.ts:274src/app/core/services/common/export-log.service.ts:87src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts:110, 115A shared constant would make it easier to update when adding new apps and improve code maintainability. All current callers of
getAccountingExportSummary()use the correct signature—no old-style three-argument calls remain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/core/services/common/accounting-export.service.ts` around lines 272 - 287, Extract the inline app allowlist into a named exported constant (e.g., ALLOWED_EXPORT_START_DATE_APPS: AppName[]) and use that constant instead of the inline array in getAccountingExportSummary (method in AccountingExportService); then import and replace the same inline array usages in ExportLogService (reference to its method using that array) and DashboardExportSummarySectionComponent (both occurrences) so all three locations reference the single exported constant and remove the duplicated literal arrays.src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts (1)
108-118: Extract the allowed-apps list into a single source of truth.The list is duplicated across component logic and the template, which risks drift. Consider a
readonly Set<AppName>+isRepurposedExportSummaryAppgetter and reuse it in both TS and HTML.♻️ Suggested refactor
+ private readonly repurposedExportApps = new Set<AppName>([ + AppName.XERO, + AppName.QBO, + AppName.NETSUITE, + AppName.INTACCT, + AppName.QBD_DIRECT, + AppName.SAGE300 + ]); + + get isRepurposedExportSummaryApp(): boolean { + return this.repurposedExportApps.has(this.appName); + } ... - if ([AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300].includes(this.appName)) { + if (this.isRepurposedExportSummaryApp) { lastExportedAt = this.accountingExportSummary.repurposed_last_exported_at; } else { lastExportedAt = this.accountingExportSummary.last_exported_at; } - } else if (status === AccountingExportStatus.FAILED && [AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300].includes(this.appName)) { + } else if (status === AccountingExportStatus.FAILED && this.isRepurposedExportSummaryApp) { lastUpdatedAt = this.accountingExportSummary.repurposed_last_exported_at; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts` around lines 108 - 118, The duplicated allowed-apps array should be centralized: add a readonly Set<AppName> (e.g., repurposedExportApps) and an isRepurposedExportSummaryApp getter on the DashboardExportSummarySectionComponent that returns repurposedExportApps.has(this.appName); update the TS logic (replace the inline array checks in the block that sets lastExportedAt/lastUpdatedAt) to call isRepurposedExportSummaryApp, and update the component template to use the same getter instead of repeating the literal list so both template and class share a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html`:
- Line 5: The header currently uses literal user-facing strings; replace them
with Transloco translations by using the transloco pipe for the two branches
instead of raw text. In the h4 that references appName and AppName (in
dashboard-export-summary-section.component.html), change the conditional text to
use translation keys (e.g. "dashboard.exportOverview" and
"dashboard.yourLastExport") and pipe them through Transloco (e.g. {{
'dashboard.exportOverview' | transloco }} or via property binding like
[innerText]="('dashboard.exportOverview' | transloco)"), preserving the existing
conditional that checks [AppName.XERO, AppName.QBO, ...]. Ensure both branches
use the transloco pipe and update translation files with the new keys.
---
Nitpick comments:
In `@src/app/core/services/common/accounting-export.service.ts`:
- Around line 272-287: Extract the inline app allowlist into a named exported
constant (e.g., ALLOWED_EXPORT_START_DATE_APPS: AppName[]) and use that constant
instead of the inline array in getAccountingExportSummary (method in
AccountingExportService); then import and replace the same inline array usages
in ExportLogService (reference to its method using that array) and
DashboardExportSummarySectionComponent (both occurrences) so all three locations
reference the single exported constant and remove the duplicated literal arrays.
In `@src/app/core/services/common/advanced-settings.service.ts`:
- Around line 25-33: The getHoursOptions function currently uses an unnecessary
double-spread for the "Real-time" entry (...[{ label: 'Real-time', value: 0 }]);
remove the outer spread and return the options as a single array where the
Real-time object is included directly before the spread of the hours array
(e.g., include { label: 'Real-time', value: 0 } followed by
...[...Array(24).keys()].map(...) ) so the output shape is unchanged but the
redundant spread is eliminated.
In
`@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts`:
- Around line 108-118: The duplicated allowed-apps array should be centralized:
add a readonly Set<AppName> (e.g., repurposedExportApps) and an
isRepurposedExportSummaryApp getter on the
DashboardExportSummarySectionComponent that returns
repurposedExportApps.has(this.appName); update the TS logic (replace the inline
array checks in the block that sets lastExportedAt/lastUpdatedAt) to call
isRepurposedExportSummaryApp, and update the component template to use the same
getter instead of repeating the literal list so both template and class share a
single source of truth.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/app/branding/c1/branding-config.tssrc/app/branding/fyle/branding-config.tssrc/app/core/models/branding/feature-configuration.model.tssrc/app/core/services/common/accounting-export.service.tssrc/app/core/services/common/advanced-settings.service.tssrc/app/core/services/netsuite/netsuite-configuration/netsuite-advanced-settings.service.tssrc/app/core/services/qbd-direct/qbd-direct-configuration/qbd-direct-advanced-settings.service.tssrc/app/core/services/qbo/qbo-configuration/qbo-advanced-settings.service.tssrc/app/core/services/sage300/sage300-configuration/sage300-advanced-settings.service.tssrc/app/core/services/xero/xero-configuration/xero-advanced-settings.service.tssrc/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.tssrc/app/integrations/intacct/intacct-shared/intacct-advanced-settings/intacct-advanced-settings.component.tssrc/app/integrations/netsuite/netsuite-main/netsuite-dashboard/netsuite-dashboard.component.tssrc/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.tssrc/app/integrations/qbo/qbo-main/qbo-dashboard/qbo-dashboard.component.tssrc/app/integrations/sage300/sage300-main/sage300-dashboard/sage300-dashboard.component.tssrc/app/integrations/xero/xero-main/xero-dashboard/xero-dashboard.component.tssrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.htmlsrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.ts
💤 Files with no reviewable changes (3)
- src/app/branding/c1/branding-config.ts
- src/app/core/models/branding/feature-configuration.model.ts
- src/app/branding/fyle/branding-config.ts
...s/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html (1)
5-11: Transloco header keys addressed — LGTM.
dashboardExportSummarySection.todaysExportOverviewanddashboardExportSummarySection.yourLastExportcorrectly follow the i18n naming convention (top-level key = camelCase component folder name, semantic sub-keys). This resolves the previous review comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html` around lines 5 - 11, The transloco keys used in the dashboard header are correct and require no changes: leave the conditional header binding in DashboardExportSummarySectionComponent (the h4 that checks [AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300].includes(appName) and uses 'dashboardExportSummarySection.todaysExportOverview' or 'dashboardExportSummarySection.yourLastExport') as-is; no code updates are needed other than keeping the existing transloco keys and their usage intact.
🧹 Nitpick comments (3)
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.html (1)
69-69:[isDisabled]is a dead expression — alwaysfalseinside this@ifblock.The parent
@ifon line 56 already assertsQbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT]to be truthy, so!QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT]can never betruehere. Consider simplifying to[isDisabled]="false"or removing the binding entirely iffalseis the default.♻️ Proposed simplification
- [isDisabled]="!QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT]"(Remove the binding;
isDisableddefaults tofalseinapp-configuration-select-field.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.html` at line 69, The template binding [isDisabled]="!QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT]" is redundant because an enclosing `@if` already guarantees QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT] is truthy; remove the dead expression by deleting the [isDisabled] attribute (or replace it with [isDisabled]="false" if explicitness is desired) on the app-configuration-select-field to leave isDisabled at its default false state and reference QbdDirectImportCodeFieldCodeConfig, DefaultImportFields.ACCOUNT and the isDisabled input to locate the element to change.src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html (1)
276-327: Redundant inner@if— remove the extra nesting level.The outer block at line 254 already guarantees
uiExposedAppName === AppName.SAGE300, so the inner@if (uiExposedAppName === AppName.SAGE300)at line 276 is alwaystrueand can never gate anything. Before this PR the inner condition included theallowImportCodeflag, which provided an independent gate; now only the app-name check remains, making it a no-op that only adds indentation.♻️ Proposed fix
- `@if` (uiExposedAppName === AppName.SAGE300) { - <div> - `@for` (defaultField of defaultImportFields; ... - ... - <div formArrayName="expenseFields"> - ... - </div> - </div> - } + <div> + `@for` (defaultField of defaultImportFields; ... + ... + <div formArrayName="expenseFields"> + ... + </div> + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html` around lines 276 - 327, Remove the redundant inner "@if (uiExposedAppName === AppName.SAGE300)" block around the defaultImportFields/expenseFields rendering in configuration-import-field.component.html: locate the inner conditional that currently wraps the defaultImportFields loop and the <div formArrayName="expenseFields"> section and delete that `@if` and its extra wrapper div so the content directly lives under the existing outer app-name check; ensure you keep all existing bindings (defaultImportFields, defaultField.import_code, importCodeFieldConfig, getImportCodeSelectorOptions, expenseFieldsGetter, showImportCodeSection, getFormGroup, getImportSubLabel, etc.) and preserve the original logic and indentation for those elements.src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html (1)
7-7: Inline array literal duplicated 4× — extract to a component property.
[AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300]is copy-pasted at lines 7, 22, 38, and 55. Adding or removing an app from this set will require four synchronised edits, making it a maintenance hazard.Declare a single readonly property in the component class and reference it in the template:
♻️ Proposed fix
In the component
.tsfile:+ readonly repurposedExportSummaryApps = [ + AppName.XERO, AppName.QBO, AppName.NETSUITE, + AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300, + ] as const;In the template (apply to all 4 occurrences):
-[AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300].includes(appName) +repurposedExportSummaryApps.includes(appName)Also applies to: 22-22, 38-38, 55-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html` at line 7, The template repeats the same inline array of AppName values in multiple places; add a single readonly array property on the DashboardExportSummarySectionComponent (e.g. supportedApps or SUPPORTED_EXPORT_APPS) containing [AppName.XERO, AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300] and replace all four template expressions with supportedApps.includes(appName) so future changes are made in one place; update the component class (constructor or as a top-level readonly field) and change each template occurrence to reference that property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.html`:
- Line 287: In the template condition referencing importSettingsForm, remove the
unnecessary "this." and make the length access null-safe; change the check using
importSettingsForm.get('importCodeFields') to use optional chaining for both
value and length (e.g.,
importSettingsForm.get('importCodeFields')?.value?.length > 0) so the form
control and its value are guarded, targeting the importSettingsForm and the
'importCodeFields' control in the template.
---
Duplicate comments:
In
`@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html`:
- Around line 5-11: The transloco keys used in the dashboard header are correct
and require no changes: leave the conditional header binding in
DashboardExportSummarySectionComponent (the h4 that checks [AppName.XERO,
AppName.QBO, AppName.NETSUITE, AppName.INTACCT, AppName.QBD_DIRECT,
AppName.SAGE300].includes(appName) and uses
'dashboardExportSummarySection.todaysExportOverview' or
'dashboardExportSummarySection.yourLastExport') as-is; no code updates are
needed other than keeping the existing transloco keys and their usage intact.
---
Nitpick comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.html`:
- Line 69: The template binding
[isDisabled]="!QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT]"
is redundant because an enclosing `@if` already guarantees
QbdDirectImportCodeFieldCodeConfig[DefaultImportFields.ACCOUNT] is truthy;
remove the dead expression by deleting the [isDisabled] attribute (or replace it
with [isDisabled]="false" if explicitness is desired) on the
app-configuration-select-field to leave isDisabled at its default false state
and reference QbdDirectImportCodeFieldCodeConfig, DefaultImportFields.ACCOUNT
and the isDisabled input to locate the element to change.
In
`@src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html`:
- Around line 276-327: Remove the redundant inner "@if (uiExposedAppName ===
AppName.SAGE300)" block around the defaultImportFields/expenseFields rendering
in configuration-import-field.component.html: locate the inner conditional that
currently wraps the defaultImportFields loop and the <div
formArrayName="expenseFields"> section and delete that `@if` and its extra wrapper
div so the content directly lives under the existing outer app-name check;
ensure you keep all existing bindings (defaultImportFields,
defaultField.import_code, importCodeFieldConfig, getImportCodeSelectorOptions,
expenseFieldsGetter, showImportCodeSection, getFormGroup, getImportSubLabel,
etc.) and preserve the original logic and indentation for those elements.
In
`@src/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.html`:
- Line 7: The template repeats the same inline array of AppName values in
multiple places; add a single readonly array property on the
DashboardExportSummarySectionComponent (e.g. supportedApps or
SUPPORTED_EXPORT_APPS) containing [AppName.XERO, AppName.QBO, AppName.NETSUITE,
AppName.INTACCT, AppName.QBD_DIRECT, AppName.SAGE300] and replace all four
template expressions with supportedApps.includes(appName) so future changes are
made in one place; update the component class (constructor or as a top-level
readonly field) and change each template occurrence to reference that property.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/app/branding/c1/branding-config.tssrc/app/branding/fyle/branding-config.tssrc/app/core/models/branding/feature-configuration.model.tssrc/app/core/services/common/advanced-settings.service.tssrc/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.htmlsrc/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.htmlsrc/app/integrations/qbo/qbo-shared/qbo-import-settings/qbo-import-settings.component.htmlsrc/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.htmlsrc/app/shared/components/dashboard/dashboard-export-summary-section/dashboard-export-summary-section.component.htmlsrc/app/shared/components/helper/mapping/generic-mapping-table/generic-mapping-table.component.html
💤 Files with no reviewable changes (3)
- src/app/core/models/branding/feature-configuration.model.ts
- src/app/branding/c1/branding-config.ts
- src/app/branding/fyle/branding-config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/core/services/common/advanced-settings.service.ts
| } | ||
| </div> | ||
| @if (brandingFeatureConfig.featureFlags.importSettings.allowImportCode && this.importSettingsForm.get('importCodeFields')?.value.length > 0 ) { | ||
| @if (this.importSettingsForm.get('importCodeFields')?.value.length > 0 ) { |
Contributor
There was a problem hiding this comment.
Remove this. and add null-safe chaining on .length.
Two issues in the condition:
this.in template — Angular templates never need thethis.prefix; every otherimportSettingsFormreference in this file (lines 17, 49, 118, 200, 221, 272, etc.) omits it. This is inconsistent.- Unsafe
.lengthaccess —?.only guards.valueagainst anullcontrol reference; if the control is absent,.valueshort-circuits toundefinedand the subsequent.lengthaccess throws aTypeError. Add a second?.before.length.
🛡️ Proposed fix
- `@if` (this.importSettingsForm.get('importCodeFields')?.value.length > 0 ) {
+ `@if` (importSettingsForm.get('importCodeFields')?.value?.length > 0) {📝 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
| @if (this.importSettingsForm.get('importCodeFields')?.value.length > 0 ) { | |
| `@if` (importSettingsForm.get('importCodeFields')?.value?.length > 0) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/integrations/intacct/intacct-shared/intacct-import-settings/intacct-import-settings.component.html`
at line 287, In the template condition referencing importSettingsForm, remove
the unnecessary "this." and make the length access null-safe; change the check
using importSettingsForm.get('importCodeFields') to use optional chaining for
both value and length (e.g.,
importSettingsForm.get('importCodeFields')?.value?.length > 0) so the form
control and its value are guarded, targeting the importSettingsForm and the
'importCodeFields' control in the template.
ashwin1111
changed the title
ashwin1111
added
c1_staging_deploy
🔁 Code Duplication Report - Angular
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.ts`:
- Around line 179-201: The import payload is being built from two unsynchronized
sources (the legacy importCodeField control and the new importCodeFields
FormArray), so remove the legacy control usage and make the payload builder
derive import-code entries exclusively from the importCodeFields FormArray;
update addImportCodeField (and any initialization) to only add/remove entries
from this.importSettingsForm.get('importCodeFields') and remove/stop
initializing the legacy importCodeField control, and modify the payload
construction in the import-settings.model (the function that currently
concatenates importCodeField and importCodeFields) to read and map values solely
from the importCodeFields FormArray so user changes are reflected correctly.
In `@src/assets/i18n/en.json`:
- Around line 893-908: The i18n child keys under intacctC1ImportSettings are out
of alphabetical order; reorder the keys (for example the entries like
"category", "configureImportMethodTitle", "configureImportMethodDescription",
"exampleCodesAndNamesConstruction", "exampleCodesAndNamesFinance",
"exampleCodesAndNamesMeals", "exampleNamesOnlyConstruction",
"exampleNamesOnlyFinance", "exampleNamesOnlyMeals", "howToImportLabel",
"howToImportSubLabel", "importCodesAndNames", "importNamesOnly",
"oneTimeSetupNote", "selectImportMethodError", "selectImportMethodPlaceholder")
into strict alphabetical order within the intacctC1ImportSettings object in
src/assets/i18n/en.json so the child keys are sorted to reduce future merge
conflicts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.htmlsrc/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.tssrc/assets/i18n/en.json
Comment on lines
+179
to
+201
| addImportCodeField(event: any, sourceField: string): void { | ||
| const importCodeFieldsArray = this.importSettingsForm.get('importCodeFields') as FormArray; | ||
|
|
||
| if (sourceField === IntacctCategoryDestination.GL_ACCOUNT || sourceField === 'GENERAL_LEDGER_ACCOUNT') { | ||
| sourceField = IntacctCategoryDestination.ACCOUNT; | ||
| } | ||
|
|
||
| const index = importCodeFieldsArray.controls.findIndex(control => control?.get('source_field')?.value === sourceField); | ||
|
|
||
| if (event.checked) { | ||
| if (this.acceptedImportCodeField.includes(sourceField) && this.intacctImportCodeConfig[sourceField] && index === -1) { | ||
| const value = this.formBuilder.group({ | ||
| source_field: [sourceField], | ||
| import_code: [this.importSettingsService.getImportCodeField(this.importSettings!.configurations.import_code_fields, sourceField, this.intacctImportCodeConfig), Validators.required] | ||
| }); | ||
| importCodeFieldsArray.push(value); | ||
| } | ||
| } else { | ||
| if (index !== -1) { | ||
| importCodeFieldsArray.removeAt(index); | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect payload construction and where import_code_fields is sourced
rg -n --type=ts "constructPayload" src/app/core/models/intacct/intacct-configuration
rg -n --type=ts "importCodeFields|importCodeField|import_code_fields" src/appRepository: fylein/fyle-integrations-app
Length of output: 29537
🏁 Script executed:
sed -n '130,155p' src/app/core/models/intacct/intacct-configuration/import-settings.model.tsRepository: fylein/fyle-integrations-app
Length of output: 1775
Resolve conflicting sources in import-code payload construction.
The payload builder reads and merges two unsynchronized controls: the legacy importCodeField (initialized once, never updated) and the new importCodeFields FormArray (mutated by addImportCodeField). Line 140 in import-settings.model.ts concatenates both, causing initial values to persist in the payload even after user modifications. Refactor to use importCodeFields FormArray alone, remove the legacy control, and adjust the payload logic accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/app/integrations/intacct/intacct-shared/intacct-c1-import-settings/intacct-c1-import-settings.component.ts`
around lines 179 - 201, The import payload is being built from two
unsynchronized sources (the legacy importCodeField control and the new
importCodeFields FormArray), so remove the legacy control usage and make the
payload builder derive import-code entries exclusively from the importCodeFields
FormArray; update addImportCodeField (and any initialization) to only add/remove
entries from this.importSettingsForm.get('importCodeFields') and remove/stop
initializing the legacy importCodeField control, and modify the payload
construction in the import-settings.model (the function that currently
concatenates importCodeField and importCodeFields) to read and map values solely
from the importCodeFields FormArray so user changes are reflected correctly.
Comment on lines
+893
to
+908
| "category": "Category", | ||
| "configureImportMethodTitle": "Configure import method", | ||
| "configureImportMethodDescription": "You have the option to import either just the names of your values or both names and code, based on how your employees identify these values.", | ||
| "oneTimeSetupNote": "This is a one-time setup and cannot be modified once the values are imported to {{brandName}}.", | ||
| "selectImportMethodPlaceholder": "Select how to import {{field}}", | ||
| "howToImportLabel": "How to import the {{field}} from {{appName}}?", | ||
| "howToImportSubLabel": "You have the option to import either just the names of your {{field}} or both the names and codes.", | ||
| "selectImportMethodError": "Please select how the {{field}} should be imported", | ||
| "importCodesAndNames": "Import codes + names", | ||
| "importNamesOnly": "Import names only", | ||
| "exampleCodesAndNamesMeals": "Example: 4567: Meals & Entertainment", | ||
| "exampleNamesOnlyMeals": "Example: Meals & Entertainment", | ||
| "exampleCodesAndNamesFinance": "Example: 24: Finance", | ||
| "exampleNamesOnlyFinance": "Example: Finance", | ||
| "exampleCodesAndNamesConstruction": "Example: 12-00-201: PCL Construction", | ||
| "exampleNamesOnlyConstruction": "Example: PCL Construction" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check the new keys and their alphabetical order in en.json
# 2. Verify presence in other locale files
python - <<'PY'
import json
from pathlib import Path
# Load en.json
en_path = Path("src/assets/i18n/en.json")
en_data = json.load(en_path.open())
intacct_keys = en_data.get("intacctC1ImportSettings", {})
# Check alphabetical ordering
keys_list = list(intacct_keys.keys())
sorted_keys = sorted(keys_list)
is_sorted = keys_list == sorted_keys
print("=== en.json intacctC1ImportSettings ===")
print(f"Keys in order: {keys_list}")
print(f"Alphabetically sorted: {is_sorted}")
if not is_sorted:
print(f"Should be: {sorted_keys}")
# Check other locale files
print("\n=== Checking other locales ===")
en_keys = set(intacct_keys.keys())
locales = {}
for path in sorted(Path("src/assets/i18n").glob("*.json")):
if path.name == "en.json":
continue
try:
data = json.load(path.open())
other_keys = set(data.get("intacctC1ImportSettings", {}).keys())
missing = sorted(en_keys - other_keys)
locales[path.name] = {
"has_section": "intacctC1ImportSettings" in data,
"missing_keys": missing if missing else []
}
except Exception as e:
locales[path.name] = {"error": str(e)}
for locale, info in locales.items():
if "error" in info:
print(f"{locale}: ERROR - {info['error']}")
else:
status = "✓ complete" if not info["missing_keys"] and info["has_section"] else "✗ incomplete"
print(f"{locale}: {status}", end="")
if info["missing_keys"]:
print(f" - missing: {info['missing_keys']}")
else:
print()
PYRepository: fylein/fyle-integrations-app
Length of output: 1558
🏁 Script executed:
#!/bin/bash
# Check what locale files exist and their intacctC1ImportSettings sections
cd src/assets/i18n
echo "=== Available locale files ==="
ls -la *.json
echo -e "\n=== Checking intacctC1ImportSettings presence ==="
for file in *.json; do
if grep -q '"intacctC1ImportSettings"' "$file"; then
echo "$file: ✓ has section"
else
echo "$file: ✗ missing section"
fi
done
echo -e "\n=== Key count comparison ==="
python - <<'PY'
import json
from pathlib import Path
for path in sorted(Path(".").glob("*.json")):
data = json.load(path.open())
count = len(data.get("intacctC1ImportSettings", {}))
print(f"{path.name}: {count} keys")
PYRepository: fylein/fyle-integrations-app
Length of output: 282
Alphabetize intacctC1ImportSettings keys in en.json.
The new keys added to intacctC1ImportSettings are not alphabetically sorted. Reorder them alphabetically to comply with the coding guideline: "Keep i18n child keys alphabetically sorted to reduce merge conflicts."
Note: Other locale files (es.json, etc.) do not currently exist in the repository, so propagation to additional locales is not applicable at this time. When additional locale files are introduced, ensure they include the same keys.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/i18n/en.json` around lines 893 - 908, The i18n child keys under
intacctC1ImportSettings are out of alphabetical order; reorder the keys (for
example the entries like "category", "configureImportMethodTitle",
"configureImportMethodDescription", "exampleCodesAndNamesConstruction",
"exampleCodesAndNamesFinance", "exampleCodesAndNamesMeals",
"exampleNamesOnlyConstruction", "exampleNamesOnlyFinance",
"exampleNamesOnlyMeals", "howToImportLabel", "howToImportSubLabel",
"importCodesAndNames", "importNamesOnly", "oneTimeSetupNote",
"selectImportMethodError", "selectImportMethodPlaceholder") into strict
alphabetical order within the intacctC1ImportSettings object in
src/assets/i18n/en.json so the child keys are sorted to reduce future merge
conflicts.
DhaaraniCIT
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Please add PR description here, add screenshots if needed
Clickup
https://app.clickup.com/
Summary by CodeRabbit
Refactor
New Features
Bug Fixes