-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Zoho Books - New or Updated Invoice #16227
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe pull request updates version numbers across several modules, refactors the event handling in the base module by adding helper methods and revising control flow, and introduces a new module for handling new or updated invoice events alongside its test event. These changes enhance code modularity and clarity in the Zoho Books integration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InvoiceModule as NewOrUpdatedInvoiceModule
participant ZohoAPI as ZohoBooksAPI
participant Emitter
User->>InvoiceModule: Trigger invoice event
InvoiceModule->>ZohoAPI: listInvoices(getParams())
ZohoAPI-->>InvoiceModule: Return invoice data
InvoiceModule->>InvoiceModule: Process data (getTsField, getItemId, getSummary)
InvoiceModule->>Emitter: Emit invoice event
sequenceDiagram
participant Emitter
participant BaseModule
Emitter->>BaseModule: Call emitEvent(response, lastDate)
BaseModule->>BaseModule: Retrieve tsField via getTsField()
loop For each item in response
BaseModule->>BaseModule: Compare item[tsField] with lastDate
alt Not sorted
BaseModule->>BaseModule: Update maxDate if applicable
end
BaseModule->>BaseModule: Retrieve item ID via getItemId(item)
BaseModule->>Emitter: Emit processed event
end
Assessment against linked issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/zoho_books/sources/new-or-updated-invoice/test-event.mjs (1)
1-76: Sample invoice structure is comprehensive but could use more realistic test dataThe test event provides a thorough representation of a Zoho Books invoice with all expected fields. This will be valuable for users implementing with this trigger.
A few suggestions to improve the test event:
- Consider using current dates instead of future dates (2025-04-09)
- Add sample data for customer_name rather than empty string
- Include a small non-zero amount for total/balance to make the test data more realistic
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs (1)
9-9: Consider using version "0.3.0" for consistency.According to the PR context, other modules like
new-customerandnew-expensehave been updated to version "0.0.2", and this appears to be a more substantial feature addition. Consider using "0.3.0" for consistency with the overall versioning strategy across Zoho Books components.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
components/zoho_books/package.json(1 hunks)components/zoho_books/sources/common/base.mjs(1 hunks)components/zoho_books/sources/new-customer/new-customer.mjs(1 hunks)components/zoho_books/sources/new-expense/new-expense.mjs(1 hunks)components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs(1 hunks)components/zoho_books/sources/new-or-updated-invoice/test-event.mjs(1 hunks)components/zoho_books/sources/new-sales-order/new-sales-order.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (11)
components/zoho_books/package.json (1)
3-3: Version update is appropriate for new feature additionThe minor version increment from 0.2.0 to 0.3.0 correctly follows semantic versioning principles for adding new functionality (the new-or-updated-invoice feature) without breaking changes.
components/zoho_books/sources/new-sales-order/new-sales-order.mjs (1)
9-9: Version update consistent with integration improvementsThe patch version increment to 0.0.2 is appropriate, likely reflecting inherited improvements from the base module refactoring mentioned in the PR summary, while maintaining the component's API.
components/zoho_books/sources/new-customer/new-customer.mjs (1)
9-9: Version update consistent with base module refactoringThe patch version increment to 0.0.2 maintains consistency with other Zoho Books source components and appropriately reflects the event handling improvements in the shared base module.
components/zoho_books/sources/new-expense/new-expense.mjs (1)
9-9: Version update aligned with other componentsThe consistent patch version increment to 0.0.2 across all source components demonstrates good version management and properly reflects the inherited improvements from the base module refactoring.
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs (3)
31-39: Date formatting implementation looks robust.The timestamp formatting function properly handles UTC conversion and padding. It ensures consistent date formatting for the Zoho Books API.
40-42: Unique ID generation looks good.Good approach combining the invoice ID with the timestamp to ensure uniqueness between updates of the same invoice. This aligns well with the
dedupe: "unique"setting.
43-48: Effective summary generation with useful status indication.The summary effectively distinguishes between new and updated invoices by comparing the creation and modification timestamps, which provides valuable context to users.
components/zoho_books/sources/common/base.mjs (4)
22-33: Good refactoring with helper methods.Adding these helper methods improves modularity and makes the code more maintainable. Child classes can now override specific behaviors without duplicating the entire
emitEventmethod.
45-59: Improved logic for handling sorted and unsorted responses.The refactored code now handles both sorted and unsorted API responses appropriately, with different logic paths for each case. Breaking the loop for sorted responses when finding older items is efficient.
73-74: Using getItemId for emission is a good practice.Using the helper method for ID generation ensures consistency between how items are stored and how they're emitted.
66-68:❓ Verification inconclusive
Verify timestamp selection logic for sorted responses.
For sorted responses, you're setting the last date to the timestamp of the first item in the response array. Make sure this is correct for your API's sorting order. If the API returns newest-first, this works, but if it returns oldest-first, you might want to use the last item instead.
🏁 Script executed:
#!/bin/bash # Let's check the documentation or existing implementations to confirm the sorting order rg -A 3 -B 3 "isSorted" --glob "components/zoho_books/**/*.mjs"Length of output: 2272
Action: Verify API Sorting Order for Timestamp Selection
The current implementation in
components/zoho_books/sources/common/base.mjs(lines 66–68) sets the last date using:this._setLastDate(isSorted ? Date.parse(responseArray[0][tsField]) : maxDate);Since
isSorted()always returnstrue, the code assumes that API responses are sorted in descending order (newest-first), makingresponseArray[0]the most recent item. Please verify with your API documentation that responses are indeed returned in newest-first order. If they are returned in ascending (oldest-first) order, the logic should be updated to use the last item in the array instead.
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs
Outdated
Show resolved
Hide resolved
luancazarine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @michelle0927, LGTM! Ready for QA!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs (2)
31-48: Consider adding validation for date formats in formatTimestamp.While the method handles undefined timestamps well, it lacks validation for invalid date formats which could lead to unexpected behavior.
formatTimestamp(ts) { if (!ts) { return undefined; } const date = new Date(ts); + // Check if date is valid + if (isNaN(date.getTime())) { + console.error(`Invalid timestamp: ${ts}`); + return undefined; + } const pad = (n) => n.toString().padStart(2, "0"); const formatted = `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}T${pad(date.getHours())}:${pad(date.getMinutes())}:${pad(date.getSeconds())}`; const offsetMinutes = date.getTimezoneOffset(); const sign = offsetMinutes > 0 ? "-" : "+"; const absOffset = Math.abs(offsetMinutes); const offsetHours = pad(Math.floor(absOffset / 60)); const offsetMins = pad(absOffset % 60); return `${formatted}${sign}${offsetHours}${offsetMins}`; },
49-51: Add error handling for getItemId method.The current implementation might return invalid IDs if the timestamp is not parsable. Consider adding error handling to ensure valid IDs.
getItemId(item) { - return `${item.invoice_id}${Date.parse(item[this.getTsField()])}`; + const timestamp = Date.parse(item[this.getTsField()]); + if (isNaN(timestamp)) { + console.warn(`Invalid timestamp for invoice ${item.invoice_id}: ${item[this.getTsField()]}`); + // Fallback to current timestamp to ensure unique ID + return `${item.invoice_id}${Date.now()}`; + } + return `${item.invoice_id}${timestamp}`; },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/zoho_books/sources/new-or-updated-invoice/new-or-updated-invoice.mjs (7)
1-3: Imports look good!The imports are clean, minimal, and appropriate for this component. Good practice to import only what's needed.
4-11: Component metadata is well-defined.The component is properly configured with appropriate values for key, name, description, version, type, and dedupe settings. Starting with version 0.0.1 is appropriate for a new module.
12-16: Methods extension and getFunction implementation look good.Properly extends the common methods and correctly implements the getFunction method to return the appropriate Zoho Books API function.
17-25: Field accessor methods are properly implemented.The getFieldName, getFieldId, and getTsField methods correctly specify the field names used for invoice data and timestamps.
26-30: getParams implementation is correct.The method properly constructs parameters for the API call using the timestamp field.
52-58: getSummary implementation is clear and concise.Good use of ternary operator to determine if an invoice is new or updated based on timestamps.
59-60: Export with sampleEmit is correct.Properly exports the component with the sampleEmit reference for testing.
|
/approve |
Resolves #16176
Summary by CodeRabbit
New Features
Chores