feat: branding based conditions to feature flag #1179
Conversation
WalkthroughThe changes update the application’s logic by replacing static brand ID checks with dynamic feature flag checks. Numerous components, models, configuration files, and templates have been updated to import and use a new object, Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant BL as Business Logic
participant FF as FeatureFlagConfig
participant BC as Branding Config Module
UI->>BL: User action triggers event (e.g., login, mapping)
BL->>BC: Import brandingConfig and brandingFeatureConfig
BL->>FF: Check specific feature flag (e.g., loginRedirectUri, sentenseCaseConversion)
FF-->>BL: Return flag value (true/false)
BL->>UI: Execute conditional logic based on flag value
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (24)
src/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration-routing.module.ts (1)
26-26: Consider extracting the component selection logic for better readability.While the ternary operator works, extracting this logic into a getter or function would improve readability and maintainability.
- component: !brandingFeatureConfig.featureFlags.importSettings.intacctC1ImportSettings ? IntacctImportSettingsComponent : IntacctC1ImportSettingsComponent + component: this.getImportSettingsComponent() + } + ] + } +]; + +private getImportSettingsComponent(): any { + return brandingFeatureConfig.featureFlags.importSettings.intacctC1ImportSettings + ? IntacctC1ImportSettingsComponent + : IntacctImportSettingsComponent;src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.ts (1)
43-43: Consider extracting the label transformation logic for better readability.The current implementation with nested method calls in a ternary operator is hard to read.
- label: brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage), + label: this.transformLabel(mappingPage), + +private transformLabel(text: string): string { + return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion + ? new SentenceCasePipe().transform(text) + : new TitleCasePipe().transform(text); +}src/app/core/models/branding/feature-configuration.model.ts (1)
1-66: Consider adding JSDoc comments for feature flags.The feature configuration type would benefit from documentation explaining the purpose and impact of each feature flag, especially for complex settings like
cccDateConfiguration,isEmployeeMappingIsEmployee, etc.Example documentation:
/** * Configuration type defining feature flags for different aspects of the application. */ export type FeatureConfiguration = { [brandingId: string]: { /** * Controls whether illustrations are displayed in the UI. */ illustrationsAllowed: boolean; // ... add similar documentation for other properties } }src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.ts (1)
29-31: Remove redundant declarations.The
brandingConfigandbrandingFeatureConfigdeclarations are redundant as they're already imported and can be used directly.- readonly brandingConfig = brandingConfig; - - readonly brandingFeatureConfig = brandingFeatureConfig;src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.ts (2)
47-47: Consider extracting the case conversion logic into a helper function.The conditional expression could be simplified by moving it to a dedicated helper function, improving readability and reusability across components.
+private getFormattedLabel(text: string): string { + return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion + ? new SentenceCasePipe().transform(text) + : new TitleCasePipe().transform(text); +} + -label: brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage), +label: this.getFormattedLabel(mappingPage),
53-55: Consider using early return pattern for cleaner code.The employee mapping feature flag check could be simplified using an early return pattern.
-if (!brandingFeatureConfig.featureFlags.mapEmployees) { - this.mappingPages.splice(0, 1); -} +if (brandingFeatureConfig.featureFlags.mapEmployees) { + this.router.navigateByUrl(this.mappingPages[0].routerLink); + this.isLoading = false; + return; +} + +this.mappingPages.splice(0, 1); this.router.navigateByUrl(this.mappingPages[0].routerLink); this.isLoading = false;src/app/shared/components/si/core/intacct-connector/intacct-connector.component.ts (1)
4-4: Consider destructuring imports for better readability.The import statement could be more readable with destructuring.
-import { brandingConfig, brandingContent, brandingFeatureConfig, brandingKbArticles } from 'src/app/branding/branding-config'; +import { + brandingConfig, + brandingContent, + brandingFeatureConfig, + brandingKbArticles +} from 'src/app/branding/branding-config';src/app/shared/components/core/svg-icon/svg-icon.component.ts (1)
99-114: Improve type safety and code organization.A few suggestions to enhance the code:
- Remove the
@ts-ignoreby properly typing the ICON_MAPPING.- Extract the style class manipulation into a separate method.
+type IconMappingKey = keyof typeof ICON_MAPPING; +private getIconMapping(key: string): string { + return ICON_MAPPING[key as IconMappingKey] || key; +} +private updateStyleClasses(): void { + if (this.isTextColorAllowed) { + return; + } + + this.styleClasses = this.styleClasses + .split(' ') + .filter((styleClass: string) => !styleClass.startsWith('tw-text-')) + .concat(['tw-text-icon-primary']) + .join(' '); +} private setupProperties(): void { if (brandingFeatureConfig.featureFlags.c1Icon) { - if (this.c1SvgSource) { - this.svgSource = this.c1SvgSource.toString(); - } else { - // @ts-ignore - this.svgSource = ICON_MAPPING[this.svgSource]; - } + this.svgSource = this.c1SvgSource?.toString() || this.getIconMapping(this.svgSource); + this.updateStyleClasses(); - if (!this.isTextColorAllowed) { - this.styleClasses = this.styleClasses.split(' ').filter((styleClass: string) => (!styleClass.startsWith('tw-text-') ? styleClass: '')).join(' '); - this.styleClasses += ' tw-text-icon-primary'; - } } }src/app/core/models/intacct/intacct-configuration/import-settings.model.ts (1)
91-95: LGTM! Improved feature control using feature flags.The change from static brand ID checks to feature flags enhances configurability and maintainability.
This architectural improvement:
- Decouples feature control from brand identity
- Makes it easier to enable/disable features across different brands
- Provides better flexibility for feature management
🧰 Tools
🪛 Biome (1.9.4)
[error] 94-94: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
src/app/core/models/xero/xero-configuration/xero-import-settings.model.ts (1)
68-77: LGTM! Enhanced customer field control using feature flags.The implementation correctly handles the customer source field visibility based on feature flags.
This change:
- Provides granular control over field visibility
- Allows for dynamic field management without code changes
- Maintains backward compatibility with existing customer data
src/app/auth/login/login.component.ts (1)
54-54: Consider improving the ternary operator readability.While the logic is correct, the ternary operator makes the code less readable.
Consider this more readable alternative:
- brandingFeatureConfig.loginRedirectUri ? this.router.navigate([redirectUri], { queryParams: { code: code } }) : this.router.navigate([redirectUri]); + if (brandingFeatureConfig.loginRedirectUri) { + this.router.navigate([redirectUri], { queryParams: { code: code } }); + } else { + this.router.navigate([redirectUri]); + }src/app/branding/c1-branding-config.ts (1)
155-156: Address TODO comments in demo video links.There are TODO comments indicating that video links need to be updated for Sage 300 and MS Dynamics.
Would you like me to create issues to track these TODO items?
Also applies to: 159-160
src/app/branding/fyle-branding-config.ts (2)
103-104: Address TODO comments in KB articles.There are TODO comments indicating that KB articles need to be updated for Sage 300 and Xero.
Would you like me to create issues to track these TODO items?
Also applies to: 134-135
158-159: Address TODO comment in demo video links.There is a TODO comment indicating that the video link needs to be updated for MS Dynamics.
Would you like me to create an issue to track this TODO item?
src/app/integrations/netsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts (4)
121-123: Fix typo in feature flag name.The feature flag name contains a typo:
sentenseCaseConversionshould besentenceCaseConversion.Apply this diff to fix the typo:
- return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? ' \(optional\)' : ''; + return brandingFeatureConfig.featureFlags.exportSettings.sentenceCaseConversion ? ' \(optional\)' : '';
121-123: Remove unnecessary escape characters in the string.The parentheses in the string don't need to be escaped as they have no special meaning in this context.
- return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? ' \(optional\)' : ''; + return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? ' (optional)' : '';
129-134: Optimize TitleCasePipe usage.The TitleCasePipe is instantiated multiple times unnecessarily. Consider creating a single instance and reusing it.
getCreateVendorLabel(): string { + const titleCasePipe = new TitleCasePipe(); if (this.workspaceGeneralSettings.employee_field_mapping === EmployeeFieldMapping.VENDOR) { - return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? EmployeeFieldMapping.VENDOR.toLowerCase() : new TitleCasePipe().transform(EmployeeFieldMapping.VENDOR); + return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? EmployeeFieldMapping.VENDOR.toLowerCase() : titleCasePipe.transform(EmployeeFieldMapping.VENDOR); } - return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? EmployeeFieldMapping.EMPLOYEE.toLowerCase() : new TitleCasePipe().transform(EmployeeFieldMapping.EMPLOYEE); + return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? EmployeeFieldMapping.EMPLOYEE.toLowerCase() : titleCasePipe.transform(EmployeeFieldMapping.EMPLOYEE); }
136-138: Optimize TitleCasePipe usage.Similar to
getCreateVendorLabel(), this function also creates a new TitleCasePipe instance unnecessarily.getCreateMerchantLabel(): string { - return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? NameInJournalEntry.MERCHANT.toLowerCase() : new TitleCasePipe().transform(NameInJournalEntry.MERCHANT); + const titleCasePipe = new TitleCasePipe(); + return brandingFeatureConfig.featureFlags.exportSettings.lowerCaseConversion ? NameInJournalEntry.MERCHANT.toLowerCase() : titleCasePipe.transform(NameInJournalEntry.MERCHANT); }src/app/core/models/netsuite/netsuite-configuration/netsuite-export-setting.model.ts (1)
266-268: Improve feature flag naming.The feature flag name
isEmployeeMappingIsEmployeeis redundant with "is" appearing twice. Consider renaming it toemployeeMappingIsEmployeeorisEmployeeMappingEmployee.src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (4)
38-39: Standardize Label Formatting UsingsentenceCasePipeWithin the dropdown’s item template, the option’s label is now processed through a
sentenceCasepipe:<span> {{ (option.label | sentenceCase) }} </span>This ensures a consistent presentation format for dropdown items.
45-47: Ensure Consistent Formatting for Selected Item LabelsSimilarly, the selected item template now displays the label using the
sentenceCasepipe:<span class="selected-item-text"> {{ (item.label | sentenceCase) }} </span>This improves the visual consistency of dropdown selections.
116-117: Apply Sentence Case Formatting to Option Names in Default FieldsIn the default fields dropdown template, the option name is now formatted via the
sentenceCasepipe:{{ (option.name | sentenceCase) }}This standardizes the presentation of option names.
121-122: Ensure Consistent Formatting for Selected Default Field ItemsSimilarly, the selected item for default fields now uses the
sentenceCasepipe:{{ (item.name | sentenceCase) }}This reinforces consistent UI text formatting.
src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts (1)
278-280: Consider extracting C1-specific logic.The C1-specific logic in
setupPagecould be extracted into a separate method for better maintainability.+ private setupC1Fields(response: any[]): void { + if (brandingFeatureConfig.featureFlags.c1Options) { + this.xeroExpenseFields = response[2]; + } + } + setupPage() { // ... existing code ... - if (brandingFeatureConfig.featureFlags.c1Options) { - this.xeroExpenseFields = response[2]; - } + this.setupC1Fields(response); // ... rest of the code ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
src/app/auth/login/login.component.ts(2 hunks)src/app/branding/c1-branding-config.ts(3 hunks)src/app/branding/fyle-branding-config.ts(3 hunks)src/app/core/models/branding/feature-configuration.model.ts(3 hunks)src/app/core/models/common/advanced-settings.model.ts(2 hunks)src/app/core/models/intacct/intacct-configuration/import-settings.model.ts(2 hunks)src/app/core/models/netsuite/netsuite-configuration/netsuite-advanced-settings.model.ts(2 hunks)src/app/core/models/netsuite/netsuite-configuration/netsuite-export-setting.model.ts(2 hunks)src/app/core/models/xero/xero-configuration/xero-import-settings.model.ts(2 hunks)src/app/core/services/common/helper.service.ts(2 hunks)src/app/integrations/intacct/intacct-main/intacct-configuration/intacct-configuration-routing.module.ts(2 hunks)src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.ts(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.html(1 hunks)src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.ts(3 hunks)src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.ts(1 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts(2 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.html(2 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.ts(2 hunks)src/app/integrations/netsuite/netsuite-shared/netsuite-import-settings/netsuite-custom-segment-dialog/netsuite-custom-segment-dialog.component.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-mapping/qbd-direct-mapping.component.ts(1 hunks)src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.ts(1 hunks)src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.html(1 hunks)src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts(1 hunks)src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.html(1 hunks)src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts(1 hunks)src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.ts(1 hunks)src/app/integrations/xero/xero-shared/xero-export-settings/xero-export-settings.component.ts(1 hunks)src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts(2 hunks)src/app/shared/components/configuration/configuration-confirmation-dialog/configuration-confirmation-dialog.component.ts(2 hunks)src/app/shared/components/configuration/configuration-connector/configuration-connector.component.html(1 hunks)src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html(1 hunks)src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html(6 hunks)src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.ts(2 hunks)src/app/shared/components/configuration/configuration-step-footer/configuration-step-footer.component.html(1 hunks)src/app/shared/components/core/svg-icon/svg-icon.component.ts(2 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html(2 hunks)src/app/shared/components/helper/mandatory-error-message/mandatory-error-message.component.html(1 hunks)src/app/shared/components/helper/mandatory-error-message/mandatory-error-message.component.ts(2 hunks)src/app/shared/components/helper/onboarding-done/onboarding-done.component.html(2 hunks)src/app/shared/components/helper/onboarding-done/onboarding-done.component.ts(1 hunks)src/app/shared/components/si/core/intacct-connector/intacct-connector.component.html(1 hunks)src/app/shared/components/si/core/intacct-connector/intacct-connector.component.ts(2 hunks)
🔇 Additional comments (54)
src/app/shared/components/helper/mandatory-error-message/mandatory-error-message.component.ts (2)
2-2: LGTM! Clean import addition.The import statement is properly updated to include the new
brandingFeatureConfig.
17-17: LGTM! Well-structured property addition.The new
brandingFeatureConfigproperty follows good practices:
- Marked as readonly for immutability
- Follows consistent pattern with existing
brandingConfigpropertysrc/app/shared/components/helper/onboarding-done/onboarding-done.component.ts (1)
19-19: LGTM! Property follows established patterns.The new
brandingFeatureConfigproperty is correctly marked asreadonlyand follows the same pattern as other branding-related properties in the class.src/app/shared/components/configuration/configuration-confirmation-dialog/configuration-confirmation-dialog.component.ts (1)
55-55: LGTM! Clean and concise implementation.The
brandIconpath construction uses feature flags appropriately with a clear ternary operator.src/app/integrations/qbo/qbo-main/qbo-mapping/qbo-mapping.component.ts (1)
49-51: LGTM! Clear and concise feature flag check.The employee mapping feature flag check is well-implemented and easy to understand.
src/app/integrations/xero/xero-main/xero-mapping/xero-mapping.component.ts (1)
43-43: Update property name once the typo is fixed.Once the property name is corrected in the feature configuration model, update this line to use
sentenceCaseConversion.- label: brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage), + label: brandingFeatureConfig.featureFlags.exportSettings.sentenceCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage),src/app/integrations/netsuite/netsuite-main/netsuite-mapping/netsuite-mapping.component.ts (1)
43-43: Update property name once the typo is fixed.Once the property name is corrected in the feature configuration model, update this line to use
sentenceCaseConversion.- label: brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage), + label: brandingFeatureConfig.featureFlags.exportSettings.sentenceCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage),✅ Verification successful
Update Feature Configuration Property Name to
sentenceCaseConversion
The diff is correct. Once the feature configuration model is updated to use the correctly spelled property, please update this line to refer tosentenceCaseConversioninstead of the currentsentenseCaseConversion.src/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.ts (1)
45-45: Update property name once the typo is fixed.Once the property name is corrected in the feature configuration model, update this line to use
sentenceCaseConversion.- label: brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage), + label: brandingFeatureConfig.featureFlags.exportSettings.sentenceCaseConversion ? new SentenceCasePipe().transform(mappingPage) : new TitleCasePipe().transform(mappingPage),✅ Verification successful
Consistent Naming for Feature Flag Update
The proposed change is correct. Once the feature configuration model is updated to use the correctly spelled property (
sentenceCaseConversion), please update the filesrc/app/integrations/intacct/intacct-main/intacct-mapping/intacct-mapping.component.tson line 45 accordingly. This ensures consistency across the codebase.src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.ts (1)
112-112: LGTM! Added feature configuration property.The readonly property is correctly added for feature flag access.
src/app/integrations/xero/xero-shared/xero-export-settings/xero-export-settings.component.ts (1)
114-114: LGTM! Improved configurability with feature flags.The change from static brand ID checks to feature flags enhances configurability and maintainability by making the condition more specific and feature-focused.
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts (1)
76-76: LGTM! Well-structured feature configuration.The
brandingFeatureConfigproperty is correctly marked as readonly, preventing accidental modifications.src/app/branding/c1-branding-config.ts (4)
17-17: LGTM! Well-organized feature flags.The new feature flags are logically grouped and clearly named.
Also applies to: 21-23
28-35: LGTM! Comprehensive export settings configuration.The export settings feature flags provide granular control over various export functionalities.
45-48: LGTM! Enhanced import settings configuration.The import settings feature flags provide fine-grained control over import functionalities.
59-60: LGTM! Extended advanced settings configuration.The advanced settings feature flags provide additional control over merchant creation and memo formatting.
src/app/branding/fyle-branding-config.ts (4)
17-17: LGTM! Consistent feature flag structure.The feature flags mirror the C1 configuration structure, maintaining consistency across branding configurations.
Also applies to: 21-23
28-35: LGTM! Comprehensive export settings configuration.The export settings feature flags provide consistent control over export functionalities.
45-48: LGTM! Enhanced import settings configuration.The import settings feature flags maintain consistency with C1 configuration.
59-60: LGTM! Extended advanced settings configuration.The advanced settings feature flags align with C1 configuration structure.
src/app/integrations/netsuite/netsuite-shared/netsuite-advanced-settings/netsuite-advanced-settings.component.ts (1)
129-138: LGTM! Clean refactor to feature flags.The refactoring from brand ID checks to feature flags improves the code's configurability. The implementation correctly handles label case formatting based on the feature flags.
src/app/core/models/netsuite/netsuite-configuration/netsuite-export-setting.model.ts (2)
298-301: LGTM! Clean refactor to feature flags.The refactoring from brand ID checks to feature flags improves the code's configurability. The implementation correctly patches form controls based on the feature flag.
267-268: LGTM! Clean transition to feature flag-based logic.The changes effectively replace brand-based logic with feature flag-based logic while maintaining the same functionality.
Also applies to: 298-301
src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.ts (3)
57-57: LGTM! Clean refactor to feature flags.The refactoring from static types to conditional types based on feature flags improves the code's configurability.
359-360: LGTM! Clean refactor to feature flags.The refactoring from brand ID checks to feature flags improves the code's configurability. The implementation correctly handles date configuration based on credit card export type.
57-57: LGTM! Consistent feature flag implementation.The changes maintain consistency with the feature flag transition pattern seen across other files.
Also applies to: 359-360
src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts (2)
359-360: LGTM! Clean refactor to feature flags.The refactoring from brand ID checks to feature flags improves the code's configurability. The implementation correctly handles date configuration based on credit card export type.
359-360: LGTM! Consistent feature flag implementation.The changes maintain consistency with the feature flag transition pattern seen across other files.
src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.ts (2)
395-401: LGTM! Feature flag check for CCC export group configuration.The code correctly uses the feature flag to control CCC export group configuration behavior.
487-490: LGTM! Feature flag check for employee mapping configuration.The code correctly uses the feature flag to control employee mapping behavior, setting credit card expense to true and employee field mapping to vendor when the feature flag is disabled.
src/app/shared/components/helper/mandatory-error-message/mandatory-error-message.component.html (1)
2-2: LGTM! Feature flag check for C1 icon display.The code correctly uses the feature flag to control the display of the SVG icon.
src/app/shared/components/helper/onboarding-done/onboarding-done.component.html (3)
10-12: LGTM! Feature flag check for congratulatory message.The code correctly uses the feature flag to control the display of the congratulatory message.
17-19: LGTM! Feature flag check for C1 hint text.The code correctly uses the feature flag to control the display of the hint text for C1.
28-30: LGTM! Feature flag check for non-C1 hint text.The code correctly uses the feature flag to control the display of the hint text for non-C1.
src/app/shared/components/configuration/configuration-step-footer/configuration-step-footer.component.html (2)
18-20: LGTM! Feature flag check for C1 button text.The code correctly uses the feature flag to control the display of the button text for C1, applying sentence case formatting.
21-23: LGTM! Feature flag check for non-C1 button text.The code correctly uses the feature flag to control the display of the button text for non-C1, preserving the original text case.
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.html (1)
27-27: Check Feature Flag Property Name inswitchLinkTextBindingThe updated binding now uses
[switchLinkText]="brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? 'Disconnect company' : 'Disconnect'"to determine the disconnect link text based on a feature flag. Please verify that the property name
sentenseCaseConversionis correctly spelled (i.e. if it was intended to besentenceCaseConversion), to avoid any subtle bugs in production.✅ Verification successful
🏁 Script executed:
#!/bin/bash echo "Searching for 'sentenseCaseConversion' occurrences:" rg "sentenseCaseConversion" -n . echo "" echo "Searching for 'sentenceCaseConversion' occurrences:" rg "sentenceCaseConversion" -n .Length of output: 20075
Consistent Property Name Usage Confirmed
After searching the codebase, it's clear that the property is consistently defined and used as
sentenseCaseConversionacross multiple files (e.g., in the branding configuration model and various component files). This indicates that the binding on line 27 inqbo-onboarding-connector.component.htmlis intentionally referencingsentenseCaseConversion. No change is necessary.src/app/shared/components/si/core/intacct-connector/intacct-connector.component.html (1)
37-42: Transition to Feature Flag for Conditional RenderingThe conditional blocks for rendering a styled information container have been updated:
- When
brandingFeatureConfig.featureFlags.allowForC1is true, a div with specialized styling is rendered.- When the flag is false, an alternative layout is used.
This change effectively replaces the brand ID check and improves configurability.
src/app/shared/components/configuration/configuration-connector/configuration-connector.component.html (1)
49-56: Conditional Display Based onallowForC1Feature FlagThe section inside the connector component now conditionally renders the accounting company’s name and a loading message using:
<h3 *ngIf="accountingCompanyName && brandingFeatureConfig.featureFlags.allowForC1" …> {{ accountingCompanyName }} </h3> <p … *ngIf="brandingFeatureConfig.featureFlags.allowForC1"> Fetching data from your {{appName}} account… </p> <div [ngClass]="{'tw-mt-16-px': !brandingFeatureConfig.featureFlags.allowForC1}"> <app-loader …></app-loader> </div>This refactoring demonstrates a clean shift away from static brand checks. Ensure that these conditions meet the intended UX flows for both configurations.
src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html (3)
5-5: Render Logo Based on Feature FlagThe image element for displaying the logo is now conditionally rendered only when the
c1Iconflag is true:<img *ngIf="brandingFeatureConfig.featureFlags.c1Icon" src="{{logoPath}}" width="logoWidth" class="tw-mr-24-px tw-h-44-px"/>This change correctly ties the logo’s visibility to a feature flag.
16-21: Display Export Zero-State Message for Non-C1 ConfigurationThe first
<h5>block now appears only whenbrandingFeatureConfig.featureFlags.allowForC1is false:<h5 … *ngIf="!isExportInProgress && !exportableAccountingExportIds.length && !accountingExportSummary && !brandingFeatureConfig.featureFlags.allowForC1"> No new expenses to export right now. <p class="tw-pt-4-px"> Once expenses are {{ importStates }}, they'll automatically be available here for export. </p> </h5>This ensures users see the appropriate message when the C1 configuration is not active.
22-26: Display Export Zero-State Message for C1 ConfigurationWhen
brandingFeatureConfig.featureFlags.allowForC1is true, a variant message is shown:<h5 … *ngIf="!isExportInProgress && !exportableAccountingExportIds.length && !accountingExportSummary && brandingFeatureConfig.featureFlags.allowForC1"> <p class="tw-pt-4-px"> Once expenses are {{ importStates }}, they'll automatically be available in the accounting dashboard. </p> </h5>This conditional update delivers a tailored message for the configured environment.
src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html (2)
54-56: Update Preview Text Based on Feature FlagThe preview text now changes based on the
allowForC1flag:<span *ngIf="!brandingFeatureConfig.featureFlags.allowForC1"> how it looks on {{uiExposedAppName}}</span> <span *ngIf="brandingFeatureConfig.featureFlags.allowForC1"> in {{uiExposedAppName}}</span>This subtle change enhances clarity in the UI.
11-13: Refine Conditional Rendering for Export Type MessagingThe
<h5>block that renders a message for certain export types now includes a refined condition:<h5 *ngIf="(formControllerName === 'reimbursableExportType' || formControllerName === 'cccExportType' || formControllerName === 'creditCardExportType') && appName !== AppName.QBO && subLabel === undefined && !(formControllerName === 'cccExportType' && brandingFeatureConfig.featureFlags.allowForC1 && exportConfigurationIconPath && appName === AppName.INTACCT)" class="!tw-text-text-muted tw-text-14-px !tw-font-400 !tw-leading-4"> Choose the type of transaction record that you would like to create in {{ uiExposedAppName }} while exporting expenses from {{brandingConfig.brandName}}. <span *ngIf="exportConfigurationIconPath && appName === AppName.INTACCT"> Click<a (click)="showExportTable()" class="tw-cursor-pointer tw-text-link-primary"> here</a> for more details. </span> </h5>Verify that this complex conditional accurately reflects the intended business logic across the different scenarios.
✅ Verification successful
🏁 Script executed:
#!/bin/bash echo "=== Displaying the first 50 lines of configuration-select-field.component.html ===" sed -n '1,50p' src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html echo "" echo "=== Searching for usages of ngIf with <h5> in configuration-select-field.component.html ===" rg "<h5 \\*ngIf=" src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.html echo "" echo "=== Displaying the first 100 lines of configuration-select-field.component.ts (if exists) to review property definitions ===" if [ -f src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.ts ]; then sed -n '1,100p' src/app/shared/components/configuration/configuration-select-field/configuration-select-field.component.ts else echo "File configuration-select-field.component.ts not found." fi echo "" echo "=== Searching for references to AppName.INTACCT ===" rg "AppName\\.INTACCT" -n .Length of output: 19390
Conditional Rendering Verified
The complex condition in the
<h5>is structured to only render the generic export messaging when:
- The field is one of the export types (
reimbursableExportType,cccExportType, orcreditCardExportType).- The app is not QBO.
- No custom label (
subLabel) has been provided.- And—importantly—it excludes the specific scenario for
cccExportTypewhen the C1 feature is allowed (i.e. whenbrandingFeatureConfig.featureFlags.allowForC1is true, an export icon exists, and the app is Intacct).This aligns with the intended business logic so that when the
cccExportTypecondition with Intacct and C1 is met, a different message (rendered in a separate<h5>block) is shown. Each branch (for the different export types) is clearly differentiated, ensuring proper messaging across scenarios.src/app/shared/components/configuration/configuration-import-field/configuration-import-field.component.html (1)
46-47: Feature Flag Conditional Update for Field Description
The modified code now usesbrandingFeatureConfig.featureFlags.allowForC1to determine which text to display for the imported field. This replaces the old static brand id check and enables more dynamic behavior via feature flags. Please verify that the displayed text is correct for both states (when the flag is enabled versus disabled) and that it meets the UX requirements.src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.html (1)
15-17: Conditional Visibility Update in Intacct Export Settings
The wrapping<div>for the reimbursable expenses section now uses!brandingFeatureConfig.featureFlags.allowForC1to control its visibility. This change shifts the condition from a static brand id check to a dynamic feature flag check. Please verify that this inversion correctly reflects the intended behavior for users based on their configuration.src/app/integrations/netsuite/netsuite-shared/netsuite-export-settings/netsuite-export-settings.component.html (1)
1-389: Consistency Check for Branding Conditions in NetSuite Export Settings
While the feature flag approach is used in some parts of the codebase, this file still relies on static checks likebrandingConfig.brandId(e.g., in the[ngClass]expressions) and uses helper methods (such ashelperService.isFieldRequired) to decide field visibility. Given the PR objective to transition to feature flag–driven logic, please verify whether additional updates are required here—such as incorporating flags likebrandingFeatureConfig.featureFlags.exportSettings.allowAccountsPayableInCoCCCandallowBankAccountInCoCCC—to ensure full consistency across integrations.src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.html (1)
105-119: Feature Flag Update for Employee Settings in QBO Export
The employee mapping section now uses!brandingFeatureConfig.featureFlags.allowForC1to control its rendering. This update shifts the decision from an older brand-specific check to a dynamic, feature flag–based condition. Please verify that showing the employee settings form only whenallowForC1is false meets the requirements for the QuickBooks Online integration and that no unintended side effects occur.src/app/core/models/common/advanced-settings.model.ts (2)
8-8: LGTM! Import statement updated to use feature flags.The import statement has been updated to include
brandingFeatureConfigandbrandingContent, which aligns with the transition to feature flag-based configuration.
91-96: Verify the feature flag configuration.The logic for filtering memo options now depends on the
excludeCardNumberAndEmployeeNameInMemofeature flag. Please ensure that:
- The feature flag is properly configured in all environments
- The default value of this flag matches the previous behavior for backward compatibility
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check feature flag configuration across environments rg -A 5 "excludeCardNumberAndEmployeeNameInMemo"Length of output: 3228
Feature Flag Configuration Verified
The flag is defined with the appropriate boolean values across environments. In the branding configuration files, the
excludeCardNumberAndEmployeeNameInMemoflag is set as expected (e.g., true inc1-branding-config.tsand false infyle-branding-config.ts), and the model interfaces confirm its type. This behavior aligns with the intended logic and backward compatibility.src/app/core/models/netsuite/netsuite-configuration/netsuite-advanced-settings.model.ts (2)
7-7: LGTM! Import statement updated consistently.The import statement has been updated to include
brandingFeatureConfig, maintaining consistency with other files.
78-80: Consistent implementation of feature flag check.The memo options filtering logic matches the implementation in
advanced-settings.model.ts, ensuring consistent behavior across integrations.src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts (1)
217-250: Verify the impact of C1 feature flag on customer import functionality.The customer import watcher logic now depends on the
c1Optionsfeature flag. This change affects how customer fields are handled in the form array.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for other components that might be affected by this feature flag rg -A 5 "c1Options.*customer.*import"Length of output: 39
Let's gather some additional context before drawing definite conclusions. The initial command didn’t return any output, so we need to verify the usage of the
c1Optionsflag and its effect on customer import functionality directly in thexero-import-settings.component.tsfile. I'll run a few targeted searches for the feature flag check, the usage of theimportCustomerscontrol, and the subscriptions to value changes in this file.Once these scripts run, we should see that:
- The component checks the
c1Optionsflag.- There are two distinct valueChanges subscriptions: one in the
ifbranch (whenc1Optionsis enabled) handling theimport_to_fylecontrol, and the other in theelsebranch handling theimportCustomerscontrol.- The branch with
c1Optionsenabled updates the form control and filters the expense fields accordingly.This confirms that the customer import watcher logic indeed changes based on the
c1Optionsflag. Please run the above scripts to verify the file's contents and confirm that the impact of the C1 feature flag is as expected.If the output from these searches confirms the intended behavior, no further changes are necessary.
🏁 Script executed:
#!/bin/bash # Search for the c1Options flag check in the file echo "Searching for c1Options flag check:" rg -n "brandingFeatureConfig\.featureFlags\.c1Options" src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts # Search for usage of the importCustomers control echo -e "\nSearching for 'importCustomers' usage:" rg -n "importCustomers" src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.ts # Search for subscriptions to valueChanges to check the two code paths echo -e "\nSearching for valueChanges subscriptions:" rg -n "valueChanges.subscribe" src/app/integrations/xero/xero-shared/xero-import-settings/xero-import-settings.component.tsLength of output: 1612
Impact of C1 Feature Flag on Customer Import Functionality Verified
- The component checks the state of
brandingFeatureConfig.featureFlags.c1Optionsat two distinct points.- When enabled, the logic subscribes to value changes on the
import_to_fylecontrol within the expense fields array, updating theimportCustomerscontrol and modifyingfyleExpenseFieldsaccordingly.- When disabled, the subscription is attached directly to the
importCustomerscontrol, with similar albeit slightly varied alterations tofyleExpenseFields.- These outcomes confirm that customer import behavior now conditionally depends on the
c1Optionsfeature flag.src/app/core/services/common/helper.service.ts (2)
12-12: LGTM! Import statement updated consistently.The import statement has been updated to include
brandingFeatureConfig, maintaining consistency with other files.
261-264: Verify the impact of sentence case conversion feature flag.The sentence case conversion now depends on the
sentenseCaseConversionfeature flag. This change could affect text formatting across the application.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for components using sentenseCaseConversion rg -A 5 "sentenseCaseConversion" # Check for direct usage of SentenceCasePipe ast-grep --pattern $'import { SentenceCasePipe } from'Length of output: 55559
Impact of Sentence Case Conversion Feature Flag Verified
The shell script results confirm that our helper method—dependent on the flag
brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion—is consistently used across the codebase. When enabled (as in the C1 branding configuration), the helper applies theSentenceCasePipetransformation following a conversion from snake to space case, which can affect label and placeholder texts. Conversely, for configurations like Fyle where the flag is false, that additional sentence-case conversion is skipped. This conditional behavior correctly controls text formatting in various UI components.
| cccDateConfiguration: boolean; | ||
| cccExportGroupConfiguration: boolean; | ||
| isEmployeeMappingIsEmployee: boolean; | ||
| sentenseCaseConversion: boolean; |
There was a problem hiding this comment.
Fix typo in property name.
The property name sentenseCaseConversion contains a typo and should be sentenceCaseConversion.
- sentenseCaseConversion: boolean;
+ sentenceCaseConversion: boolean;📝 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.
| sentenseCaseConversion: boolean; | |
| sentenceCaseConversion: boolean; |
| return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? 'Choose custom field type' : 'Choose Custom Field Type'; | ||
| } | ||
|
|
||
| getInternalIDLabel(): string { | ||
| if (brandingConfig.brandId === 'co') { | ||
| if (brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion) { | ||
| return new SentenceCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform('Enter ' + this.form.controls.customFieldType.value)) + ' internal ID'; | ||
| } | ||
| return 'Enter ' + new TitleCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform(this.form.controls.customFieldType.value)) + ' Internal ID'; | ||
| } | ||
|
|
||
| getTransactionIdLabel(): string { | ||
| return this.brandingConfig.brandId === 'co' ? new SentenceCasePipe().transform('Enter Transaction Line Field') + ' ID' : 'Enter Transaction Line Field ID'; | ||
| return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform('Enter Transaction Line Field') + ' ID' : 'Enter Transaction Line Field ID'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate case conversion logic into a shared utility.
The case conversion logic is duplicated across multiple methods. Consider creating a shared utility function to handle this common operation.
+private getFormattedLabel(text: string, suffix: string = ''): string {
+ const formattedText = brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion
+ ? new SentenceCasePipe().transform(text)
+ : new TitleCasePipe().transform(text);
+ return formattedText + suffix;
+}
-getCustomSegmentTypeLabel(): string {
- return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? 'Choose custom field type' : 'Choose Custom Field Type';
+getCustomSegmentTypeLabel(): string {
+ return this.getFormattedLabel('Choose custom field type');
}
-getInternalIDLabel(): string {
- if (brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion) {
- return new SentenceCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform('Enter ' + this.form.controls.customFieldType.value)) + ' internal ID';
- }
- return 'Enter ' + new TitleCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform(this.form.controls.customFieldType.value)) + ' Internal ID';
+getInternalIDLabel(): string {
+ const baseText = new SnakeCaseToSpaceCasePipe().transform('Enter ' + this.form.controls.customFieldType.value);
+ return this.getFormattedLabel(baseText, ' internal ID');
}
-getTransactionIdLabel(): string {
- return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform('Enter Transaction Line Field') + ' ID' : 'Enter Transaction Line Field ID';
+getTransactionIdLabel(): string {
+ return this.getFormattedLabel('Enter Transaction Line Field', ' ID');
}📝 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.
| return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? 'Choose custom field type' : 'Choose Custom Field Type'; | |
| } | |
| getInternalIDLabel(): string { | |
| if (brandingConfig.brandId === 'co') { | |
| if (brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion) { | |
| return new SentenceCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform('Enter ' + this.form.controls.customFieldType.value)) + ' internal ID'; | |
| } | |
| return 'Enter ' + new TitleCasePipe().transform(new SnakeCaseToSpaceCasePipe().transform(this.form.controls.customFieldType.value)) + ' Internal ID'; | |
| } | |
| getTransactionIdLabel(): string { | |
| return this.brandingConfig.brandId === 'co' ? new SentenceCasePipe().transform('Enter Transaction Line Field') + ' ID' : 'Enter Transaction Line Field ID'; | |
| return brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion ? new SentenceCasePipe().transform('Enter Transaction Line Field') + ' ID' : 'Enter Transaction Line Field ID'; | |
| private getFormattedLabel(text: string, suffix: string = ''): string { | |
| const formattedText = brandingFeatureConfig.featureFlags.exportSettings.sentenseCaseConversion | |
| ? new SentenceCasePipe().transform(text) | |
| : new TitleCasePipe().transform(text); | |
| return formattedText + suffix; | |
| } | |
| getCustomSegmentTypeLabel(): string { | |
| return this.getFormattedLabel('Choose custom field type'); | |
| } | |
| getInternalIDLabel(): string { | |
| const baseText = new SnakeCaseToSpaceCasePipe().transform('Enter ' + this.form.controls.customFieldType.value); | |
| return this.getFormattedLabel(baseText, ' internal ID'); | |
| } | |
| getTransactionIdLabel(): string { | |
| return this.getFormattedLabel('Enter Transaction Line Field', ' ID'); | |
| } |
Description
feat: branding based conditions to feature flag
Clickup
https://app.clickup.com/t/
Summary by CodeRabbit
New Features
Refactor