Add QBORepository and related tests for effective revenue calculation #86
Conversation
- Introduced `QBORepository` class to handle QuickBooks Online integration, including methods for fetching and aggregating customer revenue data from invoices. - Added comprehensive unit tests for `QBORepository`, covering error handling, data aggregation, and retry logic for API calls. - Created supporting types for `CustomerRevenueByRef` and `Invoice` to enhance type safety and clarity in the implementation. These changes establish a robust foundation for the QBO integration, improving revenue reporting capabilities within the application.
|
Warning Rate limit exceeded@anatolyshipitz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 38 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughA new QuickBooks Online (QBO) integration module is introduced, including the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QBORepository
participant OAuth2Manager
participant Axios
participant QBO_API
Client->>QBORepository: getEffectiveRevenue()
QBORepository->>OAuth2Manager: getAccessToken()
OAuth2Manager-->>QBORepository: accessToken
loop For each page of invoices
QBORepository->>Axios: GET /invoices (with accessToken)
Axios->>QBO_API: API request
QBO_API-->>Axios: Invoice data
Axios-->>QBORepository: Invoice data
end
QBORepository->>QBORepository: aggregateInvoices()
QBORepository-->>Client: CustomerRevenueByRef
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🔍 Vulnerabilities of
|
| digest | sha256:7294e8a530264b2178c61fa140e736bea9ea269b469a277ca9e96d98d7e80d33 |
| vulnerabilities |     |
| platform | linux/amd64 |
| size | 243 MB |
| packages | 1628 |
📦 Base Image node:20-alpine
| also known as |
|
| digest | sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd |
| vulnerabilities |    |
Description
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
| ||||||||||||||||
Description
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (21)
workers/main/src/services/QBO/index.test.ts (2)
3-3: Prefer type-only imports for interfaces to avoid runtime import noiseSplit value and type imports for clarity and to prevent unintended runtime imports.
Apply this diff:
-import { CustomerRevenueByRef, Invoice, QBORepository } from './index'; +import { QBORepository } from './index'; +import type { CustomerRevenueByRef, Invoice } from './index';
11-22: Runtime tests don’t validate TypeScript types—consider consolidating or using expectTypeOfThese tests assert object shapes at runtime; they do not verify compile-time types. Consider:
- Consolidating these with types.test.ts to avoid duplication, or
- Using Vitest’s expectTypeOf for type-level assertions.
Example:
import { expectTypeOf } from 'vitest'; import type { CustomerRevenueByRef, Invoice } from './index'; expectTypeOf(revenue).toMatchTypeOf<CustomerRevenueByRef>(); expectTypeOf(invoice).toMatchTypeOf<Invoice>();Also applies to: 24-36
workers/main/src/services/QBO/index.ts (1)
2-2: Optional: explicitly re-export types to make type-only intent clearThis helps tooling and readers; keep wildcard for repository exports.
Example:
-export * from './types'; +export type { CustomerRevenueByRef, Invoice } from './types';workers/main/src/services/QBO/QBORepository.test.ts (7)
1-5: Scope ESLint rule disables; avoid file-wide blanket disablesLimit disables to where they’re needed to keep test debt low.
Apply this diff to remove the file-wide disables:
-/* eslint-disable @typescript-eslint/no-explicit-any */ -/* eslint-disable @typescript-eslint/no-unsafe-call */ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -/* eslint-disable @typescript-eslint/no-unsafe-assignment */ -/* eslint-disable @typescript-eslint/no-unsafe-return */Then either:
- Type the mocks precisely (preferred), or
- Add targeted eslint-disable-next-line where unavoidable.
Example precise typing for axios mock:import type { AxiosInstance } from 'axios'; // ... let mockAxiosInstance: Partial<AxiosInstance>; mockAxiosInstance = { get: vi.fn() };
63-63: Avoidanyin tests when easy to typeUse Partial for the axios mock instance.
Apply this diff:
- let mockAxiosInstance: any; + import type { AxiosInstance } from 'axios'; + let mockAxiosInstance: Partial<AxiosInstance>;
75-81: Narrow theanycast for OAuth2Manager mock implementationAvoid
as any; narrow to the actual surface used in the test.Apply this diff:
- mockOAuth2Manager.mockImplementation( - () => - ({ - getAccessToken: vi.fn().mockResolvedValue('test-access-token'), - }) as any, - ); + mockOAuth2Manager.mockImplementation( + () => + ({ + getAccessToken: vi.fn().mockResolvedValue('test-access-token'), + }) as unknown as Pick<InstanceType<typeof OAuth2Manager>, 'getAccessToken'>, + );
104-112: Strengthen axios-retry assertions by checking policy detailsCapture options passed to axiosRetry and assert status handling and retry behavior.
Example addition after constructing QBORepository:
const [, retryOpts] = mockAxiosRetry.mock.calls[0]; expect(retryOpts.retries).toBe(3); expect(typeof retryOpts.retryDelay).toBe('function'); expect(typeof retryOpts.retryCondition).toBe('function'); // Optionally assert retryCondition for typical retryable cases: const networkError = { code: 'ECONNRESET', isAxiosError: true }; expect(retryOpts.retryCondition(networkError as any)).toBe(true); const rateLimit = { response: { status: 429 } }; expect(retryOpts.retryCondition(rateLimit as any)).toBe(true);
152-158: Edge case: include an unpaid invoice to ensure it’s excluded from effective revenueAdd a case where Balance > 0 to confirm it doesn’t affect totals.
Example:
mockAxiosInstance.get.mockResolvedValue( createMockApiResponse([ createMockInvoice({ TotalAmt: 100, Balance: 100, CustomerRef: { value: 'customer3', name: 'Unpaid' } }), ]), ); const result = await qboRepository.getEffectiveRevenue(); expect(result.customer3).toBeUndefined();I can add this test in a follow-up commit if you want.
160-172: Avoid calling the method twice; assert both instance and message using one promiseThis reduces flakiness and speeds up the test.
Apply this diff:
- await expect(qboRepository.getEffectiveRevenue()).rejects.toThrow( - QuickBooksRepositoryError, - ); - - await expect(qboRepository.getEffectiveRevenue()).rejects.toThrow( - 'QBORepository.getEffectiveRevenue failed: QBORepository.getPaidInvoices failed: API request failed', - ); + const promise = qboRepository.getEffectiveRevenue(); + await expect(promise).rejects.toBeInstanceOf(QuickBooksRepositoryError); + await expect(promise).rejects.toThrow( + 'QBORepository.getEffectiveRevenue failed: QBORepository.getPaidInvoices failed: API request failed', + );
136-150: Optional: verify Authorization header usage with access tokenIf QBORepository attaches the token per-request, assert the header was set when calling invoices endpoint.
Example:
expect(mockOAuth2Manager).toHaveBeenCalled(); expect(mockAxiosInstance.get).toHaveBeenCalled(); const [, config] = mockAxiosInstance.get.mock.calls[0] ?? []; expect(config?.headers?.Authorization).toBe('Bearer test-access-token');If the token is set via an interceptor or default headers, adapt the assertion accordingly.
workers/main/src/services/QBO/types.test.ts (2)
3-3: Use type-only imports for interfacesMake intent explicit and avoid runtime import overhead.
Apply this diff:
-import { CustomerRevenueByRef, Invoice } from './types'; +import type { CustomerRevenueByRef, Invoice } from './types';
5-109: Prefer type-level assertions over runtime shape checks for TypeScript interfacesReplace or complement with Vitest’s expectTypeOf for compile-time verification.
Example:
import { expectTypeOf } from 'vitest'; // Inside tests: expectTypeOf<Invoice>().toMatchTypeOf({ TotalAmt: 0, Balance: 0, CustomerRef: { value: '', name: '' }, }); expectTypeOf<CustomerRevenueByRef>().toMatchTypeOf({ anyCustomer: { customerName: '', totalAmount: 0, invoiceCount: 0 }, });These fail at type-check time, offering stronger guarantees than runtime property checks.
workers/main/src/services/QBO/QBORepository.errorHandling.test.ts (3)
59-75: Capture and assert axios-retry retryCondition without unsafe castingYou can simplify the mock and ensure
retryConditionwas actually wired by asserting it was captured.- (mockAxiosRetry as ReturnType<typeof vi.fn>).mockImplementation( + vi.mocked(axiosRetry).mockImplementation( ( instance, config: { retryCondition?: (error: { response?: { status: number }; code?: string; }) => boolean; }, ) => { if (config?.retryCondition) { mockRetryCondition = config.retryCondition; } }, ); + + // Optionally, assert it's set to avoid false positives + expect(mockRetryCondition).toBeDefined();
111-127: Consider adding 'EAI_AGAIN' to retryable network errors test matrixDNS lookup transient failures often surface as
EAI_AGAIN. Adding it keeps test expectations aligned with typical retry policies.- const networkErrors = [ - 'ECONNRESET', - 'ETIMEDOUT', - 'ENOTFOUND', - 'ECONNABORTED', - ]; + const networkErrors = [ + 'ECONNRESET', + 'ETIMEDOUT', + 'ENOTFOUND', + 'ECONNABORTED', + 'EAI_AGAIN', + ];
148-166: Avoid double-executing the method under test when asserting error type and messageCurrently the call is made twice solely to assert type and message. Assert both from a single invocation for faster and clearer tests.
- await expect(qboRepository.getEffectiveRevenue()).rejects.toThrow( - QuickBooksRepositoryError, - ); - - await expect(qboRepository.getEffectiveRevenue()).rejects.toThrow( - 'QBORepository.getEffectiveRevenue failed: QBORepository.getPaidInvoices failed: Token expired', - ); + let caught: unknown; + try { + await qboRepository.getEffectiveRevenue(); + } catch (e) { + caught = e; + } + expect(caught).toBeInstanceOf(QuickBooksRepositoryError); + expect((caught as Error).message).toContain( + 'QBORepository.getEffectiveRevenue failed: QBORepository.getPaidInvoices failed: Token expired', + );workers/main/src/services/QBO/QBORepository.integration.test.ts (2)
150-179: Ensure fake timers are always restored to avoid leakage across testsRestoring timers inside the test works, but moving it to
afterEachis safer if an assertion fails beforeuseRealTimers()runs.describe('date window calculation', () => { it('should use correct date range for effective revenue months', async () => { const mockDate = new Date('2024-01-15'); - vi.useFakeTimers(); + vi.useFakeTimers(); vi.setSystemTime(mockDate); @@ - vi.useRealTimers(); }); + + afterEach(() => { + vi.useRealTimers(); + }); });
169-176: Also assert Authorization header is set with the OAuth2 bearer tokenValidates that the token retrieved from
OAuth2Manageris actually applied to the request.- expect(mockAxiosInstance.get).toHaveBeenCalledWith( + expect(mockAxiosInstance.get).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ - params: { - query: expect.stringContaining(expectedQuery) as string, - }, + params: { + query: expect.stringContaining(expectedQuery) as string, + }, + headers: expect.objectContaining({ + Authorization: 'Bearer test-access-token', + }), }), );workers/main/src/services/QBO/QBORepository.ts (4)
63-66: Broaden retryable network errors to include 'EAI_AGAIN'Transient DNS failures often surface as
EAI_AGAIN. Including it reduces avoidable hard failures.- return ['ECONNRESET', 'ETIMEDOUT', 'ENOTFOUND', 'ECONNABORTED'].includes( + return ['ECONNRESET', 'ETIMEDOUT', 'ENOTFOUND', 'ECONNABORTED', 'EAI_AGAIN'].includes( error.code || '', );
68-79: Make Retry-After parsing robust (seconds or HTTP-date per RFC 7231)Some servers send
Retry-Afteras an HTTP-date. Support both while keeping exponential backoff as a fallback. Also clamp negatives.- if (error.response?.status === 429) { - const retryAfter = error.response.headers['retry-after']; - - if (retryAfter) return parseInt(retryAfter) * 1000; - } + if (error.response?.status === 429) { + const retryAfter = error.response.headers['retry-after']; + if (retryAfter) { + // numeric seconds + if (/^\d+$/.test(retryAfter)) { + return Math.max(parseInt(retryAfter, 10) * 1000, 0); + } + // HTTP-date + const targetMs = Date.parse(retryAfter); + if (!Number.isNaN(targetMs)) { + const ms = targetMs - Date.now(); + if (ms > 0) return ms; + } + } + }
93-101: Nit: avoid quoting numeric literal in QBO query filterQBO fields like
Balanceare numeric; usingBalance = 0avoids any ambiguity. Current form likely works, but the numeric literal is cleaner.- const query = `SELECT * FROM Invoice WHERE TxnDate >= '${startDate}' AND TxnDate <= '${endDate}' AND Balance = '0' STARTPOSITION ${startPosition} MAXRESULTS ${maxResults}`; + const query = `SELECT * FROM Invoice WHERE TxnDate >= '${startDate}' AND TxnDate <= '${endDate}' AND Balance = 0 STARTPOSITION ${startPosition} MAXRESULTS ${maxResults}`;
93-110: Optional safety: prevent pathological infinite pagination loopsIf an API bug ignores
STARTPOSITION, the loop could spin. Add a simple page cap to fail fast with context.const allInvoices: Invoice[] = []; - let startPosition = 1; + let startPosition = 1; const maxResults = 100; + const maxPages = 1000; // defensive cap + let page = 0; while (true) { @@ - if (invoices.length === 0) break; + if (invoices.length === 0) break; @@ - startPosition += maxResults; + startPosition += maxResults; + page += 1; + if (page >= maxPages) { + throw new QuickBooksRepositoryError( + `QBORepository.getPaidInvoices failed: exceeded max pages (${maxPages})` + ); + } if (invoices.length < maxResults) break; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
workers/main/src/services/QBO/QBORepository.errorHandling.test.ts(1 hunks)workers/main/src/services/QBO/QBORepository.integration.test.ts(1 hunks)workers/main/src/services/QBO/QBORepository.test.ts(1 hunks)workers/main/src/services/QBO/QBORepository.ts(1 hunks)workers/main/src/services/QBO/index.test.ts(1 hunks)workers/main/src/services/QBO/index.ts(1 hunks)workers/main/src/services/QBO/types.test.ts(1 hunks)workers/main/src/services/QBO/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests are co-located with source files and should be named with the pattern *.test.ts
Files:
workers/main/src/services/QBO/index.test.tsworkers/main/src/services/QBO/QBORepository.test.tsworkers/main/src/services/QBO/types.test.tsworkers/main/src/services/QBO/QBORepository.errorHandling.test.tsworkers/main/src/services/QBO/QBORepository.integration.test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Follow the function naming pattern: prefix? + action (A) + high context (HC) + low context? (LC), using action verbs such as get, fetch, send, create, validate, handle, calculate, and boolean prefixes is, has, should
Use descriptive, unabbreviated variable names; use singular for single values and plural for collections; ensure variable names are context-specific
Files:
workers/main/src/services/QBO/index.test.tsworkers/main/src/services/QBO/index.tsworkers/main/src/services/QBO/QBORepository.test.tsworkers/main/src/services/QBO/types.test.tsworkers/main/src/services/QBO/QBORepository.errorHandling.test.tsworkers/main/src/services/QBO/QBORepository.integration.test.tsworkers/main/src/services/QBO/types.tsworkers/main/src/services/QBO/QBORepository.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to workers/main/vitest.config.ts : Test configuration should be defined in workers/main/vitest.config.ts
Applied to files:
workers/main/src/services/QBO/index.test.tsworkers/main/src/services/QBO/types.test.ts
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to workers/main/tsconfig.json : TypeScript configuration should be defined in workers/main/tsconfig.json
Applied to files:
workers/main/src/services/QBO/index.ts
📚 Learning: 2025-07-29T15:56:21.892Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-07-29T15:56:21.892Z
Learning: New workers must not duplicate logic already present in shared modules; place all shared code in 'workers-shared/' to maximize reuse and maintainability.
Applied to files:
workers/main/src/services/QBO/index.ts
📚 Learning: 2025-07-29T15:56:21.892Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: .cursor/rules/temporal-project-structure.mdc:0-0
Timestamp: 2025-07-29T15:56:21.892Z
Learning: Applies to workers/*/{workflows,activities,index.ts,README.md,types.ts} : All Temporal workers must be placed under 'workers/<worker-name>/' and include: 'workflows/' (workflow definitions), 'activities/' (activity implementations), 'index.ts' (worker entry point), 'types.ts' (optional), and 'README.md' (usage and development instructions).
Applied to files:
workers/main/src/services/QBO/index.ts
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to **/*.test.ts : Tests are co-located with source files and should be named with the pattern *.test.ts
Applied to files:
workers/main/src/services/QBO/types.test.ts
📚 Learning: 2025-08-01T13:15:19.658Z
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#78
File: workers/main/src/services/OAuth2/OAuth2TokenManager.ts:0-0
Timestamp: 2025-08-01T13:15:19.658Z
Learning: In workers/main/src/services/OAuth2/OAuth2TokenManager.ts, the user anatolyshipitz enhanced the OAuth2Error class with error code functionality, including a code property and ERROR_CODES constants. The OAuth2TokenRefreshProvider now throws typed errors with specific codes, and the OAuth2TokenManager uses these codes for conditional error handling instead of checking message text.
Applied to files:
workers/main/src/services/QBO/QBORepository.errorHandling.test.ts
📚 Learning: 2025-08-07T16:49:02.094Z
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#85
File: workers/main/src/services/OAuth2/OAuth2Manager.ts:108-126
Timestamp: 2025-08-07T16:49:02.094Z
Learning: In workers/main/src/services/OAuth2/OAuth2Manager.ts, user anatolyshipitz considers structured error code checking for OAuth2 errors redundant, preferring the simpler string matching approach with message.includes('invalid_grant') for error detection during token refresh operations.
Applied to files:
workers/main/src/services/QBO/QBORepository.errorHandling.test.ts
🧬 Code Graph Analysis (1)
workers/main/src/services/QBO/index.test.ts (1)
workers/main/src/services/QBO/types.ts (2)
CustomerRevenueByRef(1-7)Invoice(9-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (10)
workers/main/src/services/QBO/index.test.ts (1)
6-9: LGTM: export surface verification is clear and sufficientBasic export checks for QBORepository are appropriate here.
workers/main/src/services/QBO/index.ts (1)
1-2: LGTM: simple, centralized exportsFacade re-exports look good and keep imports tidy.
workers/main/src/services/QBO/QBORepository.test.ts (2)
115-151: LGTM: aggregation over paid invoices by customer is correctMocks and expectations read well; structure mirrors QBO responses.
87-95: OAuth2Manager key prefix is correctThe
QBORepositoryconstructor prefixesqboConfig.companyIdwithqbo-(line 34), so the test’s expectation of'qbo-test-company-id'aligns with the implementation. No changes required.workers/main/src/services/QBO/types.ts (1)
1-16: LGTM: concise, accurate type definitionsInterfaces match expected QBO payload shapes and the aggregation result structure.
workers/main/src/services/QBO/QBORepository.errorHandling.test.ts (1)
168-188: Good coverage for malformed/null API responses returning empty aggregatesVerifies resilience to empty/malformed payloads by asserting
{}. This matches repository behavior and keeps early-stage robustness without overengineering.workers/main/src/services/QBO/QBORepository.integration.test.ts (1)
60-147: Aggregation scenarios are solid and readableTests accurately cover multiple invoices per customer and missing customer references grouped under
unknown. Expectations match repository logic.workers/main/src/services/QBO/QBORepository.ts (3)
29-54: Constructor, token manager wiring, and axios-retry setup look solidGood separation of concerns: token acquisition, configured Axios instance with retry policy, and clear entrypoint via
getEffectiveRevenue.
136-146: Date window calculation aligns with tests and is clearUsing current date and subtracting configurable months matches the integration test’s expectations. Naming is descriptive and action-oriented per guidelines.
148-158: Error wrapping strategy is consistent and testableDouble-wrapping ensures contextual trace in messages (as asserted in tests). This aligns with your preference for simple string-based detection (per retrieved learning #85).
- Introduced `generateJitter` function in `utils.ts` to create cryptographically secure random jitter for retry delays, enhancing the reliability of retry logic. - Updated `QBORepository` to utilize the new `generateJitter` function instead of a manual jitter calculation, improving code clarity and maintainability. - Added unit tests for `generateJitter` to ensure it generates values within the expected range and behaves consistently across multiple calls. These changes enhance the utility functions available for managing delays in API calls, contributing to a more robust integration with QuickBooks Online.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
workers/main/src/common/utils.ts (1)
32-37: Guard against non-positive base delays (optional)If
baseDelayis 0 or negative, returning 0 is safer and avoids surprising negative jitter.export function generateJitter(baseDelay: number): number { + if (baseDelay <= 0) return 0; // ... }Optional: if this jitter feeds setTimeout, consider returning an integer (e.g., Math.floor) to avoid fractional ms.
workers/main/src/common/utils.test.ts (1)
83-117: Optional: make “scaling” deterministic by mocking random sourceIf you want to assert proportional scaling, mock the RNG to return the same u32 for both calls.
Example:
import { randomBytes } from 'node:crypto'; vi.spyOn(require('node:crypto'), 'randomBytes') .mockReturnValueOnce(Buffer.from([0x12,0x34,0x56,0x78])) .mockReturnValueOnce(Buffer.from([0x12,0x34,0x56,0x78])); // Now largeJitter should equal smallJitter * (largeDelay/smallDelay)This avoids relying on probabilistic behavior in unit tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
workers/main/src/common/utils.test.ts(4 hunks)workers/main/src/common/utils.ts(2 hunks)workers/main/src/services/QBO/QBORepository.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- workers/main/src/services/QBO/QBORepository.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.test.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
Tests are co-located with source files and should be named with the pattern *.test.ts
Files:
workers/main/src/common/utils.test.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.{ts,tsx}: Follow the function naming pattern: prefix? + action (A) + high context (HC) + low context? (LC), using action verbs such as get, fetch, send, create, validate, handle, calculate, and boolean prefixes is, has, should
Use descriptive, unabbreviated variable names; use singular for single values and plural for collections; ensure variable names are context-specific
Files:
workers/main/src/common/utils.test.tsworkers/main/src/common/utils.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: anatolyshipitz
PR: speedandfunction/automatization#34
File: workers/main/src/workflows/financial/FinancialReportFormatter.ts:3-7
Timestamp: 2025-05-30T17:57:21.010Z
Learning: User anatolyshipitz prefers to keep code implementations simple during early development stages rather than adding comprehensive error handling and validation. They consider extensive type annotations and error handling "redundant" when focusing on core functionality and testing.
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to workers/main/vitest.config.ts : Test configuration should be defined in workers/main/vitest.config.ts
Applied to files:
workers/main/src/common/utils.test.ts
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to **/*.test.ts : Tests are co-located with source files and should be named with the pattern *.test.ts
Applied to files:
workers/main/src/common/utils.test.ts
📚 Learning: 2025-08-05T13:42:48.295Z
Learnt from: CR
PR: speedandfunction/automatization#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-05T13:42:48.295Z
Learning: Applies to workers/main/src/configs/** : Environment validation should be implemented in workers/main/src/configs/
Applied to files:
workers/main/src/common/utils.test.tsworkers/main/src/common/utils.ts
🧬 Code Graph Analysis (1)
workers/main/src/common/utils.test.ts (1)
workers/main/src/common/utils.ts (1)
generateJitter(32-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Docker Security Scanning (n8n, Dockerfile.n8n, n8n-test:latest)
- GitHub Check: Service Availability Check
🔇 Additional comments (4)
workers/main/src/common/utils.ts (1)
1-1: Default crypto import is safe with current TypeScript settingsOur
workers/main/tsconfig.jsonalready hasesModuleInterop: true, which enables synthetic default imports for CommonJS modules. This means:
import crypto from 'crypto';will work as expected.- No change to the import or usage in
src/common/utils.tsis required.workers/main/src/common/utils.test.ts (3)
8-8: LGTM: importing generateJitter where it’s usedImport grouping and co-location align with guidelines.
23-23: LGTM: harmless formatting tweak on mockImplementationNo behavioral change.
54-55: LGTM: expected error output adjustedMatches the message constructed in utils.ts.
- Updated the `validateEnv` test to improve error message formatting and ensure proper handling of missing environment variables. - Enhanced the `generateJitter` test cases by clarifying the expected behavior and consolidating multiple tests into a single comprehensive test for different baseDelay values. - Removed redundant tests for jitter generation to streamline the testing process while maintaining coverage for expected functionality. These changes improve the clarity and reliability of unit tests for environment validation and jitter generation utilities.
- Updated the calculation of the random value in the `generateJitter` function to use a more accurate divisor (0x100000000) for converting to the [0,1) range. This change ensures that the generated jitter values are correctly scaled, enhancing the reliability of the jitter generation for retry delays. This modification improves the precision of the jitter utility, contributing to more effective delay management in API calls.
…y logic - Enhanced the `QBORepository` class by introducing dedicated methods for handling HTTP and network retryable errors, improving the clarity and maintainability of the error management logic. - Added new constants for HTTP status codes and retry configuration, ensuring consistent usage across the repository. - Updated the `types.ts` file to include new interfaces and enums for better type safety and clarity in error handling. - Refactored unit tests for `QBORepository` and related types to cover new functionality and ensure robust testing of error scenarios. These changes contribute to a more resilient integration with QuickBooks Online, enhancing the application's ability to manage API call failures effectively.
- Added a new line to the `calculateRetryDelay` method in the `QBORepository` class to improve the clarity of the rate limit delay calculation. - Included an additional line in the `getRateLimitDelay` method to ensure proper handling of the `retry-after` header, enhancing the robustness of the retry logic. These changes contribute to a more resilient integration with QuickBooks Online by refining the error handling and retry mechanisms.
|
QBORepositoryclass to handle QuickBooks Online integration, including methods for fetching and aggregating customer revenue data from invoices.QBORepository, covering error handling, data aggregation, and retry logic for API calls.CustomerRevenueByRefandInvoiceto enhance type safety and clarity in the implementation.These changes establish a robust foundation for the QBO integration, improving revenue reporting capabilities within the application.