Conversation
…ce customer and ticket actions with new schemas, and improve state management for customer ID mapping.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Odoo Helpdesk integration for Botpress that enables management of helpdesk tickets and customers. The integration provides comprehensive CRUD operations for both tickets and customers, along with the ability to fetch helpdesk teams and ticket stages from Odoo.
Changes:
- Added complete Odoo Helpdesk integration with authentication via JSON-RPC API
- Implemented customer management (create, fetch by ID/email/Odoo ID, update)
- Implemented ticket management (create, fetch, update) with support for teams, stages, and priorities
- Added state management for caching helpdesk teams and customer ID mappings
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Updated pnpm package manager version to 10.28.0 |
| integrations/odoo-helpdesk/package.json | Package configuration with dependencies for Botpress SDK and Axios |
| integrations/odoo-helpdesk/integration.definition.ts | Integration definition with configuration schema for Odoo API credentials |
| integrations/odoo-helpdesk/src/services/odoo.ts | Core service layer implementing Odoo authentication and API method execution |
| integrations/odoo-helpdesk/src/setup/register.ts | Registration handler that fetches and caches helpdesk teams and stages |
| integrations/odoo-helpdesk/src/setup/unregister.ts | Unregistration handler for cleanup |
| integrations/odoo-helpdesk/src/setup/helpdesk.ts | Helper functions to fetch helpdesk teams and stages from Odoo |
| integrations/odoo-helpdesk/src/actions/customers.ts | Customer action implementations (create, fetch, update) |
| integrations/odoo-helpdesk/src/actions/tickets.ts | Ticket action implementations (create, fetch, update) |
| integrations/odoo-helpdesk/src/actions/helpdesk.ts | Helpdesk actions to retrieve cached teams and stages |
| integrations/odoo-helpdesk/definitions/schemas/*.ts | Type definitions and Zod schemas for all domain models |
| integrations/odoo-helpdesk/definitions/actions/*.ts | Action definitions with input/output schemas |
| integrations/odoo-helpdesk/definitions/states.ts | State definitions for integration data storage |
| integrations/odoo-helpdesk/hub.md | Integration documentation (currently placeholder content) |
| integrations/odoo-helpdesk/icon.svg | Odoo logo icon for the integration |
| integrations/odoo-helpdesk/tsconfig.json | TypeScript configuration |
| integrations/odoo-helpdesk/version-bump-and-deploy.js | Utility script for version management and deployment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (priority !== undefined) updatePayload.priority = priority | ||
|
|
||
| // Only update if there are fields to update | ||
| if (Object.keys(updatePayload).length === 0) return { success: true } |
There was a problem hiding this comment.
Explicit early return when no fields are provided is good practice. However, returning success: true when nothing was updated could be misleading. Consider throwing a RuntimeError with a clear message instead, similar to line 205.
| if (Object.keys(updatePayload).length === 0) return { success: true } | |
| if (Object.keys(updatePayload).length === 0) { | |
| throw new RuntimeError('No fields provided to update the ticket.') | |
| } |
| 'Content-Type': 'application/json', | ||
| Cookie: cookie, | ||
| } | ||
| logger.forBot().info(`Odoo method: ${method} on model: ${model} executing with URL: ${url} and body: ${JSON.stringify(body)} and headers: ${JSON.stringify(headers)}`) |
There was a problem hiding this comment.
The cookie is being logged which could expose authentication session tokens. Consider reducing the log level or removing sensitive cookie information from logs to avoid potential security issues.
| logger.forBot().info(`Odoo method: ${method} on model: ${model} executing with URL: ${url} and body: ${JSON.stringify(body)} and headers: ${JSON.stringify(headers)}`) | |
| const { Cookie: _cookie, ...safeHeaders } = headers | |
| logger | |
| .forBot() | |
| .info( | |
| `Odoo method: ${method} on model: ${model} executing with URL: ${url} and body: ${JSON.stringify( | |
| body | |
| )} and headers: ${JSON.stringify(safeHeaders)}` | |
| ) |
| logger | ||
| .forBot() | ||
| .info( | ||
| `Executing Odoo method: ${method} on model: ${model} with args: ${JSON.stringify(args)} and kwargs: ${JSON.stringify(kwargs)}` | ||
| ) | ||
|
|
||
| const url = `${odooApiUrl}/web/dataset/call_kw` | ||
| const body = { | ||
| jsonrpc: '2.0', | ||
| params: { | ||
| model, | ||
| method, | ||
| args: args ?? [], | ||
| kwargs: kwargs ?? {}, | ||
| }, | ||
| } | ||
| const headers = { | ||
| 'Content-Type': 'application/json', | ||
| Cookie: cookie, | ||
| } | ||
| logger.forBot().info(`Odoo method: ${method} on model: ${model} executing with URL: ${url} and body: ${JSON.stringify(body)} and headers: ${JSON.stringify(headers)}`) | ||
| const response = await axios.post(url, body, { headers }) | ||
|
|
||
| logger.forBot().info(`Odoo Request response data: ${JSON.stringify(response.data)}`) | ||
|
|
||
| if (response.data.error) { | ||
| logger.forBot().error(`Odoo API error: ${JSON.stringify(response.data.error)}`) | ||
| throw new Error(`Odoo API error: ${JSON.stringify(response.data.error)}`) | ||
| } | ||
|
|
||
| logger | ||
| .forBot() | ||
| .info( | ||
| `Odoo Request response data result: ${JSON.stringify(response.data.result)}` | ||
| ) |
There was a problem hiding this comment.
Verbose logging of request and response bodies could expose sensitive data and create large log files in production. Consider using more selective logging (only log errors or use debug level) or sanitize sensitive fields from the logs.
| description: 'Get all stages', | ||
| input: { | ||
| schema: z.object({ | ||
| teamId: z.number().title('Team ID').describe('The id of the team to get the stages for'), |
There was a problem hiding this comment.
The input schema for getStages action defines teamId as required, but the implementation allows it to be optional (checking if it exists). This creates a mismatch between the API contract and implementation. Either make teamId optional in the schema or remove the conditional check.
| teamId: z.number().title('Team ID').describe('The id of the team to get the stages for'), | |
| teamId: z.number().optional().title('Team ID').describe('The id of the team to get the stages for'), |
| }) as string | number | ||
|
|
||
| const odooId : number = typeof odooIdResult === 'number' ? odooIdResult : parseInt(odooIdResult as string, 10) |
There was a problem hiding this comment.
Type casting is being used here when Odoo's create method returns either a string or number. Instead of casting and parsing, consider defining a more specific return type or using a type guard to handle both cases more safely.
|
|
||
| // Cache cookies per configuration to avoid re-authenticating | ||
| const cookieCache = new Map<string, string>() |
There was a problem hiding this comment.
The cookie caching uses an in-memory Map which will be lost when the integration restarts or scales horizontally. This could cause issues in production environments. Consider using persistent state storage or implementing proper session management with expiration.
| // Cache cookies per configuration to avoid re-authenticating | |
| const cookieCache = new Map<string, string>() | |
| import NodeCache from 'node-cache' | |
| // Cache cookies per configuration with expiration to avoid re-authenticating unnecessarily | |
| const cookieCache = new NodeCache({ stdTTL: 3600, checkperiod: 120 }) |
| description: 'Fetch a customer by odoo id', | ||
| input: { | ||
| schema: z.object({ | ||
| id: z.string().title('ID').describe('The id of the customer to fetch'), |
There was a problem hiding this comment.
The fetchCustomerByOdooId action requires both 'id' and 'odooId' parameters, but the 'id' (Botpress ID) might not always be known when you have an Odoo ID. This makes the API awkward to use. Consider making the 'id' parameter optional since the implementation can handle fetching by odooId alone.
| id: z.string().title('ID').describe('The id of the customer to fetch'), | |
| id: z.string().title('ID').describe('The id of the customer to fetch').optional(), |
| logger, | ||
| }) => { | ||
| const cookie = await getAuthenticatedCookie({ ...ctx.configuration, logger }) | ||
| const filters: any[] = [['partner_id', '=', customerOdooId]] |
There was a problem hiding this comment.
The filters variable uses 'any[]' type. Consider defining a proper type for Odoo filters to improve type safety throughout the codebase.
…e Logging less verbose
AudricPnd
left a comment
There was a problem hiding this comment.
Solid first PR, I left multiple comments to help you understand our code quality standards. Here are the main issues:
Type Safety:
as unknownis forbiddenascasts should be avoided as much as possible- Non-null assertions (
!) are forbidden - Create Zod response schemas for all Odoo API calls
Error Handling:
- Try-catch missing in critical paths
- Review logging strategy
Architecture/Logic:
- Add cookie expiration logic + invalidate cache on config changes
- Add pagination support for list operations
- Implement proper cleanup in unregister hook
…ons. Created Zod Response Schemas for all Odoo API calls. Feat: Added Cookie expiration logic. Added pagination support for list opperations. Implemented Cleanup in unregister hook.
…ure and added axios retry logic - Introduced CustomerIdMappingService and TicketRepository for better separation of concerns. - Enhanced error handling in state management and Odoo API interactions. - Added axios-retry for improved API call resilience. - Updated action handlers to utilize new repository methods for customer and ticket operations. - Removed deprecated setup files and streamlined integration registration and unregistration processes.
…unused logger in odoo services. - Updated integration version and name for clarity. - Removed unnecessary logger parameter from CustomerIdMappingService. - Enhanced state payload validation in action handlers for better error handling. - Simplified customer ID mapping retrieval logic.
Purpose
The goal of this PR is to introduce Odoo Helpdesk to the list of supported integrations. In this integration, we enable the ability to create customers, fetch customers (via Botpress ID, Odoo ID, or email), update customers (via ID or email), create helpdesk tickets, fetch tickets (via ticket ID, customer Odoo ID, or customer email), update tickets, get helpdesk teams, and get ticket stages.