fix: Add an "Export Date" option to QBO CCP date options #1148
Conversation
WalkthroughThe pull request introduces enhancements to the QuickBooks Online (QBO) integration's export settings components. Specifically, it adds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant QBOComponent
participant ExportDateOptions
User->>QBOComponent: Select Credit Card Expense Object
QBOComponent->>ExportDateOptions: Update Date Options
alt Credit Card Purchase Selected
ExportDateOptions->>ExportDateOptions: Add Current Date Option
end
QBOComponent->>QBOComponent: Validate Export Date
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
🪧 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 (
|
JustARatherRidiculouslyLongUsername
requested review from
anishfyle,
ashwin1111 and
DhaaraniCIT
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.ts (1)
327-332: Consider adding error handling for invalid date values.The conditional logic for adding the CURRENT_DATE option is correct, but consider adding error handling for potential invalid date values.
if (selectedValue === QBOCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE) { + try { this.cccExpenseGroupingDateOptions.push({ label: brandingContent.common.currentDate, value: ExportDateType.CURRENT_DATE }); + } catch (error) { + console.error('Failed to add current date option:', error); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.ts(2 hunks)src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts(2 hunks)
🔇 Additional comments (3)
src/app/integrations/qbo/qbo-onboarding/qbo-clone-settings/qbo-clone-settings.component.ts (2)
12-12: LGTM! Import statement updated correctly.Added ExportDateType to the imports for handling the new export date option.
340-343: LGTM! Validation logic is well implemented.The validation logic correctly handles invalid creditCardExportDate values by resetting them to null.
src/app/integrations/qbo/qbo-shared/qbo-export-settings/qbo-export-settings.component.ts (1)
9-9: LGTM! Import statement updated correctly.Added ExportDateType to the imports consistently with the clone settings component.
| if (selectedValue === QBOCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE) { | ||
| this.cccExpenseGroupingDateOptions.push({ | ||
| label: brandingContent.common.currentDate, | ||
| value: ExportDateType.CURRENT_DATE | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refactoring duplicate code into a shared service.
The updateCCCExpenseGroupingDateOptions method implementation is duplicated between QboCloneSettingsComponent and QboExportSettingsComponent. Consider extracting this logic into a shared service.
Create a new service (e.g., QboExportDateService) with the following implementation:
@Injectable({
providedIn: 'root'
})
export class QboExportDateService {
updateCCCExpenseGroupingDateOptions(
selectedValue: QBOCorporateCreditCardExpensesObject,
options: SelectFormOption[],
form: FormGroup
): SelectFormOption[] {
let updatedOptions = [...options];
if (selectedValue === QBOCorporateCreditCardExpensesObject.CREDIT_CARD_PURCHASE) {
try {
updatedOptions.push({
label: brandingContent.common.currentDate,
value: ExportDateType.CURRENT_DATE
});
} catch (error) {
console.error('Failed to add current date option:', error);
}
}
const allowedValues = updatedOptions.map(option => option.value);
if (!allowedValues.includes(form.get('creditCardExportDate')?.value)) {
form.get('creditCardExportDate')?.setValue(null);
}
return updatedOptions;
}
}This refactoring will:
- Eliminate code duplication
- Centralize the export date logic
- Make it easier to maintain and update the logic in one place
- Add proper error handling
Also applies to: 330-333
(cherry picked from commit c80e1ba)
Description
Add the "Export Date" option to QBO Credit Card Purchases. Also fix a small bug where an invalid option can remain in the
creditCardExportDateafter changing the export type.Example:
Post date remains in the form and can be submitted, although it is invalid for bills
Clickup
https://app.clickup.com/t/86cxhjzhf
Summary by CodeRabbit
New Features
Improvements