fix: unit tests for migration of /v2/corporate_card_transactions #3510
Conversation
|
🚨 gitStream Monthly Automation Limit Reached 🚨 Your organization has exceeded the number of pull requests allowed for automation with gitStream. To continue automating your PR workflows and unlock additional features, please contact LinearB. |
WalkthroughThis pull request removes an outdated mock data file and updates several test suites to reflect a new naming convention and data structure for corporate card transactions. The deleted file contained immutable mock data, now replaced in tests by the new constant Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant CorporateCreditCardExpenseService
participant SpenderPlatformV1ApiService
Component->>+CorporateCreditCardExpenseService: getCorporateCardTransactions(params)
CorporateCreditCardExpenseService->>+SpenderPlatformV1ApiService: get(updatedParams)
SpenderPlatformV1ApiService-->>-CorporateCreditCardExpenseService: ccTransactionResponseData
CorporateCreditCardExpenseService-->>-Component: ccTransactionResponseData
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (6)
✨ 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: 1
🔭 Outside diff range comments (2)
src/app/core/services/corporate-credit-card-expense.service.spec.ts (1)
159-178: 🧹 Nitpick (assertive)Mind it! Add more test cases for error scenarios, macha!
The test suite for
getCorporateCardTransactionsneeds additional test cases to handle:
- API error responses
- Network failures
- Invalid parameters
Add these test cases to make the test suite more robust:
it('should handle API errors in getCorporateCardTransactions', (done) => { const error = new Error('API Error'); spenderPlatformV1ApiService.get.and.returnValue(throwError(() => error)); const param = { offset: 0, queryParams: {}, limit: 1, }; cccExpenseService.getCorporateCardTransactions(param).subscribe({ error: (err) => { expect(err).toBe(error); done(); } }); }); it('should handle invalid parameters in getCorporateCardTransactions', (done) => { const param = { offset: -1, // Invalid offset queryParams: {}, limit: 0, // Invalid limit }; cccExpenseService.getCorporateCardTransactions(param).subscribe({ error: (err) => { expect(err instanceof Error).toBe(true); expect(err.message).toContain('Invalid parameters'); done(); } }); });src/app/core/services/merge-expenses.service.spec.ts (1)
488-533: 🧹 Nitpick (assertive)Style ah? Let's make it more stylish with comprehensive test coverage!
The test suite for
getCorporateCardTransactionsis good but needs more test cases for edge scenarios.Add these test cases to make it more powerful:
it('should handle invalid group_id in corporate card transactions', (done) => { const params = { queryParams: { group_id: null }, offset: 0, limit: 1 }; customInputsService.getAll.withArgs(true).and.returnValue(of(customInputData)); corporateCreditCardExpenseService.getCorporateCardTransactions .withArgs(params) .and.returnValue(throwError(() => new Error('Invalid group_id'))); mergeExpensesService.getCorporateCardTransactions([undefined]).subscribe({ error: (err) => { expect(err.message).toEqual('Invalid group_id'); done(); } }); }); it('should handle pagination in corporate card transactions', (done) => { const params = { queryParams: { group_id: ['in.(,)'] }, offset: 10, limit: 5 }; customInputsService.getAll.withArgs(true).and.returnValue(of(customInputData)); corporateCreditCardExpenseService.getCorporateCardTransactions .withArgs(params) .and.returnValue(of(ccTransactionResponseData)); mergeExpensesService.getCorporateCardTransactions(expensesDataWithCC, 10, 5).subscribe((res) => { expect(res).toEqual(ccTransactionResponseData.data); done(); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/app/core/mock-data/corporate-card-expense.data.ts(0 hunks)src/app/core/services/corporate-credit-card-expense.service.spec.ts(1 hunks)src/app/core/services/merge-expenses.service.spec.ts(4 hunks)src/app/fyle/merge-expense/merge-expense-3.page.spec.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/app/core/mock-data/corporate-card-expense.data.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (12.x)
| it('onPaymentModeChanged(): should call mergeExpensesService.getCorporateCardTransactions() once and assign CCCTxns correctly', () => { | ||
| component.expenses = expenseList2; | ||
| mergeExpensesService.getCorporateCardTransactions.and.returnValue(of(apiCardV2Transactions.data)); | ||
| mergeExpensesService.getCorporateCardTransactions.and.returnValue(of(ccTransactionResponseData.data)); | ||
| component.onPaymentModeChanged(); | ||
| expect(mergeExpensesService.getCorporateCardTransactions).toHaveBeenCalledOnceWith(expenseList2); | ||
| expect(component.CCCTxns).toEqual(apiCardV2Transactions.data); | ||
| expect(component.CCCTxns).toEqual(ccTransactionResponseData.data); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Thalaiva says: Add more test cases for payment mode changes!
The test for onPaymentModeChanged only covers the happy path. We need to test different payment modes and error scenarios.
Add these test cases to make it more robust:
it('should handle empty expense list in onPaymentModeChanged', () => {
component.expenses = [];
component.onPaymentModeChanged();
expect(mergeExpensesService.getCorporateCardTransactions).not.toHaveBeenCalled();
expect(component.CCCTxns).toEqual([]);
});
it('should handle errors in onPaymentModeChanged', () => {
component.expenses = expenseList2;
mergeExpensesService.getCorporateCardTransactions.and.returnValue(throwError(() => new Error('API Error')));
component.onPaymentModeChanged();
expect(mergeExpensesService.getCorporateCardTransactions).toHaveBeenCalledWith(expenseList2);
expect(component.CCCTxns).toBeUndefined();
});
|
* feat: Migrate /v2/corporate_card_transactions to platform * fix: unit tests for migration of /v2/corporate_card_transactions (#3510)
Clickup
https://app.clickup.com/t/86cy0fqm2
Code Coverage
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit
Chores
Refactor
Tests