test: intacct export settings watchers #1002
Conversation
WalkthroughThe pull request introduces modifications to the test suites for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
🧹 Outside diff range and nitpick comments (5)
src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts (5)
14-14: Correct the class name 'PaginatedintacctDestinationAttribute' to match naming conventionsThe class name
PaginatedintacctDestinationAttributeshould bePaginatedIntacctDestinationAttributeto adhere to CamelCase naming conventions and improve readability.Apply the following diff to correct the class name:
- import { IntacctDestinationAttribute, PaginatedintacctDestinationAttribute } from 'src/app/core/models/intacct/db/destination-attribute.model'; + import { IntacctDestinationAttribute, PaginatedIntacctDestinationAttribute } from 'src/app/core/models/intacct/db/destination-attribute.model';
65-68: Update class name to 'PaginatedIntacctDestinationAttribute'Reflect the corrected class name in the usage to maintain consistency.
Apply this diff:
- of(copy.ACCOUNT as unknown as PaginatedintacctDestinationAttribute), + of(copy.ACCOUNT as unknown as PaginatedIntacctDestinationAttribute), - of(copy.EXPENSE_PAYMENT_TYPE as unknown as PaginatedintacctDestinationAttribute), + of(copy.EXPENSE_PAYMENT_TYPE as unknown as PaginatedIntacctDestinationAttribute), - of(copy.VENDOR as unknown as PaginatedintacctDestinationAttribute), + of(copy.VENDOR as unknown as PaginatedIntacctDestinationAttribute), - of(copy.CHARGE_CARD_NUMBER as unknown as PaginatedintacctDestinationAttribute) + of(copy.CHARGE_CARD_NUMBER as unknown as PaginatedIntacctDestinationAttribute)
351-353: Use the corrected class name 'PaginatedIntacctDestinationAttribute'Ensure that the updated class name is used consistently throughout the code.
Apply this diff:
- mappingService.getPaginatedDestinationAttributes.and.returnValue( - of(mockPaginatedDestinationAttributes.EXPENSE_PAYMENT_TYPE as unknown as PaginatedintacctDestinationAttribute) + mappingService.getPaginatedDestinationAttributes.and.returnValue( + of(mockPaginatedDestinationAttributes.EXPENSE_PAYMENT_TYPE as unknown as PaginatedIntacctDestinationAttribute)
172-177: Update toast message to reflect all synchronized dimensionsThe method
refreshDimensionsrefreshes both Sage Intacct and Fyle dimensions, but the toast message states: 'Syncing data dimensions from Sage Intacct'. Consider updating the toast message to accurately reflect that data dimensions from both platforms are being synchronized for clarity.Apply this diff to update the toast message:
- expect(toastService.displayToastMessage).toHaveBeenCalledWith(ToastSeverity.SUCCESS, 'Syncing data dimensions from Sage Intacct'); + expect(toastService.displayToastMessage).toHaveBeenCalledWith(ToastSeverity.SUCCESS, 'Syncing data dimensions from Fyle and Sage Intacct');
294-297: Ensure consistency in date label: 'Spend date' vs. 'Spent date'The label 'Spend date' might be inconsistent with the corresponding enum value
ExportDateType.SPENT_AT. Consider updating the label to 'Spent date' for consistency and clarity.Apply this diff to correct the label:
- label: 'Spend date', + label: 'Spent date',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.spec.ts (1 hunks)
- src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts (1 hunks)
- src/app/integrations/intacct/intacct.fixture.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/integrations/intacct/intacct-main/intacct-dashboard/intacct-dashboard.component.spec.ts
🔇 Additional comments (3)
src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts (1)
389-389: Verify the brand ID value 'co' in 'C1 Specific Behavior' testThe
brandIdis set to'co'in the test for 'C1 Specific Behavior'. Please verify if'co'is the correct brand ID for C1. If'c1'is intended, consider updating thebrandIdto ensure the test behaves as expected.src/app/integrations/intacct/intacct.fixture.ts (2)
478-478: Verify thesplit_expense_groupingbeing set tonull.In the
mockExportSettings, thesplit_expense_groupingis assignednull:split_expense_grouping: nullPlease confirm if this is intentional. If
split_expense_groupingis an optional field, assigningnullmay be acceptable. Otherwise, consider setting it to a validSplitExpenseGroupingenum value to prevent potential runtime errors.
633-634: Ensure email addresses in test data are fictitious.The
"email": "[email protected]"Please verify that this email is fictitious and used solely for testing purposes. Using real email addresses can lead to privacy concerns and unintended data exposure.
...ons/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts
Show resolved
Hide resolved
| export const mockPaginatedDestinationAttributes = { | ||
| ACCOUNT: { | ||
| "count": 4, | ||
| "next": "http://intacct-api.staging-integrations:8000/api/workspaces/366/sage_intacct/paginated_destination_attributes/?attribute_type=ACCOUNT&limit=100&offset=100", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding absolute URLs in test fixtures.
The next field in mockPaginatedDestinationAttributes contains a hardcoded absolute URL:
"next": "http://intacct-api.staging-integrations:8000/api/workspaces/366/sage_intacct/paginated_destination_attributes/?attribute_type=ACCOUNT&limit=100&offset=100",Hardcoding environment-specific URLs can cause issues when running tests in different environments. Consider using a relative URL or a placeholder to make the test fixture more flexible and environment-agnostic.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts (1)
285-288: Ensure consistent cleanup in testsIn the
Custom Watchersdescribe block, thebeforeEachat lines 286-288 setsbrandingConfig.brandId = 'fyle';, but there is no corresponding cleanup within this block. While there is anafterAllin theC1 Specific Behaviorblock, it's important to maintain test isolation within each describe block.Consider adding an
afterEachto resetbrandingConfig.brandIdto its original value to ensure test independence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts (4 hunks)
🧰 Additional context used
🪛 GitHub Check: lint
src/app/integrations/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts
[failure] 316-316:
["setCCExpenseDateOptions"] is better written in dot notation
[failure] 359-359:
Too many nested callbacks (5). Maximum allowed is 4
[failure] 377-377:
Too many nested callbacks (5). Maximum allowed is 4
| tick(1000); | ||
|
|
||
| expect(mappingService.getPaginatedDestinationAttributes).toHaveBeenCalledWith('EXPENSE_PAYMENT_TYPE', 'test'); | ||
| expect(component.destinationOptions.EXPENSE_PAYMENT_TYPE.every(option => (option.detail ? option.detail.is_reimbursable : true))).toBeTrue(); |
There was a problem hiding this comment.
Reduce nested callbacks to enhance readability
At line 359, there's a nesting depth of 5 callbacks, which exceeds the maximum recommended depth of 4. This can make the test harder to read and maintain.
Consider refactoring the test to reduce the nesting level. You might extract some of the inner functions or use async/await if applicable.
🧰 Tools
🪛 GitHub Check: lint
[failure] 359-359:
Too many nested callbacks (5). Maximum allowed is 4
...ons/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts
Show resolved
Hide resolved
...ons/intacct/intacct-shared/intacct-export-settings/intacct-export-settings.component.spec.ts
Show resolved
Hide resolved
JustARatherRidiculouslyLongUsername
added
the
deploy
JustARatherRidiculouslyLongUsername
temporarily deployed
to
Staging
GitHub Actions
Inactive
Clickup
https://app.clickup.com/t/86cwh86da
Summary by CodeRabbit
New Features
IntacctExportSettingsComponent, including new test cases for form initialization, validation, and dynamic behavior based on user interactions.Bug Fixes
IntacctDashboardComponenttest suite for better clarity.Documentation