New Components - picqer #17044
New Components - picqer #17044
Conversation
luancazarine
added
the
ai-assisted
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughA comprehensive Picqer integration was implemented, introducing a fully functional app module, multiple action modules for orders, returns, customers, and picklists, as well as webhook source modules for event handling. Utilities and constants were added to support the integration, and package metadata was updated to reflect new dependencies and versioning. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action
participant PicqerApp
participant PicqerAPI
User->>Action: Trigger (e.g., Create Order)
Action->>PicqerApp: Call method (e.g., createOrder)
PicqerApp->>PicqerAPI: HTTP request (e.g., POST /orders)
PicqerAPI-->>PicqerApp: API response
PicqerApp-->>Action: Return data
Action-->>User: Output result/summary
sequenceDiagram
participant PicqerAPI
participant WebhookSource
participant User
PicqerAPI->>WebhookSource: Send webhook event (with signature)
WebhookSource->>WebhookSource: Verify signature
alt Valid signature
WebhookSource->>User: Emit event (with summary)
else Invalid signature
WebhookSource->>PicqerAPI: Respond 401 Unauthorized
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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/picqer/actions/add-order-comment/add-order-comment.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/picqer/actions/add-return-comment/add-return-comment.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/picqer/actions/create-order/create-order.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
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. 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 (
|
Sources - New Event (Instant) Actions - Add Order Comment - Add Return Comment - Create Order - Get Customer - Get Order - Get Picklist - Get Status Per Order Line - Search Orders - Update Order
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (10)
components/picqer/package.json (1)
15-17: Ensure dependency list is deterministicConsider alphabetising the keys in
"dependencies"(e.g.crypto-jsbefore@pipedream/platform) or adding a lint rule that enforces order. A consistent order avoids unnecessary diff-noise on future bumps.components/picqer/common/utils.mjs (1)
4-23: Optional: deep-parse plain objectsAt the moment only top-level strings / arrays are parsed. If callers pass an object whose property values are JSON strings (common with dynamic fields), they stay unparsed. A recursive walk would make
parseObjectmore generally reusable:if (typeof obj === "string") { try { return JSON.parse(obj); } catch (e) { return obj; } } - return obj; + if (typeof obj === "object") { + return Object.fromEntries( + Object.entries(obj).map(([k, v]) => [k, parseObject(v)]), + ); + } + return obj;components/picqer/actions/get-picklist/get-picklist.mjs (1)
7-8: Component version mismatchAction version is
0.0.1while the package was bumped to0.1.0. Keeping them aligned helps consumers understand compatibility at a glance.- version: "0.0.1", + version: "0.1.0",components/picqer/actions/add-order-comment/add-order-comment.mjs (1)
7-8: Align action version with packageSame as for Get Picklist – bump to
0.1.0for consistency.components/picqer/actions/search-orders/search-orders.mjs (1)
30-41: Consider stronger typing or helper UI for date inputs
sinceDateanduntilDateare free-form strings. Wrongly-formatted timestamps will silently propagate to the API and are hard to debug.
Pipedream supports the"datetime"prop-type which renders a date-time picker in the UI.- sinceDate: { - type: "string", + sinceDate: { + type: "datetime",Same for
untilDate.
This gives users immediate feedback and prevents malformed requests.components/picqer/actions/get-customer/get-customer.mjs (1)
18-26: Handle 404 / empty responses gracefully
getCustomermight returnnullor throw when the ID is unknown. Emit a meaningful summary instead of “Successfully retrieved customer X”.try { const response = await this.picqer.getCustomer({ $, customerId: this.customerId }); - $.export("$summary", `Successfully retrieved customer ${this.customerId}`); - return response; + if (!response) $.export("$summary", `No customer found with ID ${this.customerId}`); + else $.export("$summary", `Customer ${this.customerId} retrieved`); + return response; } catch (err) { this.picqer.logError(err, $); throw err; }Prevents misleading success messages.
components/picqer/actions/update-order/update-order.mjs (1)
234-272: Consider extracting address field visibility logic.The repetitive pattern of hiding/showing address fields could be extracted to reduce duplication and improve maintainability.
Consider refactoring to:
const addressFields = [ 'deliveryName', 'deliveryContactName', 'deliveryAddress', 'deliveryAddress2', 'deliveryZipcode', 'deliveryCity', 'deliveryRegion', 'deliveryCountry', 'invoiceName', 'invoiceContactName', 'invoiceAddress', 'invoiceAddress2', 'invoiceZipcode', 'invoiceCity', 'invoiceRegion', 'invoiceCountry' ]; addressFields.forEach(field => { fixedProps[field].hidden = hasCustomerId; });components/picqer/actions/create-order/create-order.mjs (1)
310-315: Consider more descriptive error messages.The error messages could be enhanced to guide users better.
Apply this diff for clearer error messages:
- throw new ConfigurationError("Delivery Name is required if **Customer Id** is not provided"); + throw new ConfigurationError("Either Customer ID or Delivery Name must be provided. Please select a customer or enter delivery details."); } if (!customerId && !deliveryCountry) { - throw new ConfigurationError("Delivery Country is required if **Customer Id** is not provided"); + throw new ConfigurationError("Delivery Country is required when creating an order without a Customer ID. Please provide a valid ISO 3166 2-character country code.");components/picqer/picqer.app.mjs (2)
194-238: Consider adding country code validation.The delivery and invoice country fields require ISO 3166 2-char codes, but there's no validation or helper to ensure correct format.
Consider adding validation or providing a dropdown with country codes:
deliveryCountry: { type: "string", label: "Delivery Country", description: "Country of delivery address (needs to be ISO 3166 2-char code). Required if **Customer Id** is not provided.", + options: async () => { + // Return common country codes or fetch from an API + return [ + { label: "United States", value: "US" }, + { label: "Canada", value: "CA" }, + { label: "United Kingdom", value: "GB" }, + // ... more countries + ]; + }, },Alternatively, add validation in the action components that use these fields.
301-452: Consider adding JSDoc comments for better developer experience.The methods section would benefit from JSDoc comments to document parameters, return values, and potential errors. This would improve the developer experience when using this app component.
Example for one method:
/** * Create a new order in Picqer * @param {Object} opts - Request options * @param {Object} opts.$ - Pipedream context object * @param {Object} opts.data - Order data * @returns {Promise<Object>} Created order object * @throws {Error} If the API request fails */ createOrder(opts = {}) { return this._makeRequest({ method: "POST", path: "/orders", ...opts, }); },
📜 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 (16)
components/picqer/actions/add-order-comment/add-order-comment.mjs(1 hunks)components/picqer/actions/add-return-comment/add-return-comment.mjs(1 hunks)components/picqer/actions/create-order/create-order.mjs(1 hunks)components/picqer/actions/get-customer/get-customer.mjs(1 hunks)components/picqer/actions/get-order/get-order.mjs(1 hunks)components/picqer/actions/get-picklist/get-picklist.mjs(1 hunks)components/picqer/actions/get-status-per-order-line/get-status-per-order-line.mjs(1 hunks)components/picqer/actions/search-orders/search-orders.mjs(1 hunks)components/picqer/actions/update-order/update-order.mjs(1 hunks)components/picqer/common/constants.mjs(1 hunks)components/picqer/common/utils.mjs(1 hunks)components/picqer/package.json(2 hunks)components/picqer/picqer.app.mjs(1 hunks)components/picqer/sources/common/base.mjs(1 hunks)components/picqer/sources/new-event-instant/new-event-instant.mjs(1 hunks)components/picqer/sources/new-event-instant/test-event.mjs(1 hunks)
🔇 Additional comments (8)
components/picqer/package.json (1)
15-17: Re-evaluate version rangesUsing caret ranges (
^3.1.0,^4.2.0) automatically picks up new minor / patch versions at deploy time. If Picqer components are sensitive to upstream changes (especiallycrypto-jsfor signature verification), consider pinning exact versions or adding an automated lockfile-refresh step to surface breaking changes early.components/picqer/sources/new-event-instant/test-event.mjs (1)
1-43: Looks good – representative sample eventThe static payload is well-structured and mirrors the Picqer docs. No issues found.
components/picqer/actions/get-picklist/get-picklist.mjs (1)
18-24: Verify underlying method existsAssuming
picqer.app.mjsexposesgetPicklist, but this isn’t guaranteed at compile-time. Please confirm the method name and signature match; a typo will only surface at runtime.components/picqer/actions/get-status-per-order-line/get-status-per-order-line.mjs (1)
19-26: Pagination / large-response advisory
getStatusPerOrderLinecan return many lines for big orders. Ensure the underlyingpicqerhelper streams or paginates; otherwise responses might exceed Pipedream’s 6 MB limit.components/picqer/actions/get-order/get-order.mjs (1)
1-27: LGTM!Clean and straightforward implementation of the get order action. Good use of propDefinition for consistency.
components/picqer/actions/update-order/update-order.mjs (1)
309-355: Well-implemented data transformation logic.Good handling of:
- Float parsing for discount
- Array joining for multi-value custom fields
- Proper field name mapping to API format
components/picqer/common/constants.mjs (1)
1-230: Well-organized constants.Good structure with clear option objects and comprehensive event types coverage.
components/picqer/actions/create-order/create-order.mjs (1)
253-272: Good handling of required custom fields.Nice improvement over the update action by including the
requiredproperty for custom order fields.
| export const parseObject = (obj) => { | ||
| if (!obj) return undefined; | ||
|
|
There was a problem hiding this comment.
!obj swallows valid falsy values
0, false, and the empty string "" are all valid inputs but will currently return undefined.
-export const parseObject = (obj) => {
- if (!obj) return undefined;
+export const parseObject = (obj) => {
+ if (obj === null || obj === undefined) return undefined;This keeps the original semantics for null / undefined while preserving other falsy values.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const parseObject = (obj) => { | |
| if (!obj) return undefined; | |
| export const parseObject = (obj) => { | |
| if (obj === null || obj === undefined) return undefined; |
🤖 Prompt for AI Agents
In components/picqer/common/utils.mjs around lines 1 to 3, the check `if (!obj)`
incorrectly returns undefined for valid falsy values like 0, false, and "".
Change the condition to explicitly check for null or undefined (e.g., `obj ==
null`) to preserve these valid falsy inputs while still handling null or
undefined correctly.
| const response = await this.picqer.addOrderComment({ | ||
| $, | ||
| orderId: this.orderId, | ||
| data: { | ||
| body: this.body, | ||
| show_at_related: this.showAtRelated, | ||
| is_visible_fulfilment: this.isVisibleFulfillment, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid sending undefined fields to the API
If showAtRelated or isVisibleFulfillment are left empty, they’re transmitted as null/undefined, which some APIs reject. Build the payload conditionally:
- data: {
- body: this.body,
- show_at_related: this.showAtRelated,
- is_visible_fulfilment: this.isVisibleFulfillment,
- },
+ data: (() => {
+ const data = { body: this.body };
+ if (this.showAtRelated !== undefined)
+ data.show_at_related = this.showAtRelated;
+ if (this.isVisibleFulfillment !== undefined)
+ data.is_visible_fulfilment = this.isVisibleFulfillment;
+ return data;
+ })(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await this.picqer.addOrderComment({ | |
| $, | |
| orderId: this.orderId, | |
| data: { | |
| body: this.body, | |
| show_at_related: this.showAtRelated, | |
| is_visible_fulfilment: this.isVisibleFulfillment, | |
| }, | |
| }); | |
| const response = await this.picqer.addOrderComment({ | |
| $, | |
| orderId: this.orderId, | |
| data: (() => { | |
| const data = { body: this.body }; | |
| if (this.showAtRelated !== undefined) | |
| data.show_at_related = this.showAtRelated; | |
| if (this.isVisibleFulfillment !== undefined) | |
| data.is_visible_fulfilment = this.isVisibleFulfillment; | |
| return data; | |
| })(), | |
| }); |
🤖 Prompt for AI Agents
In components/picqer/actions/add-order-comment/add-order-comment.mjs around
lines 37 to 45, the payload sent to the API includes fields showAtRelated and
isVisibleFulfillment even when they are undefined, which can cause API
rejection. Modify the code to conditionally add these fields to the data object
only if they have defined values, ensuring no undefined or null fields are sent
in the request.
| async run({ $ }) { | ||
| const response = await this.picqer.searchOrders({ | ||
| $, | ||
| params: { | ||
| search: this.search, | ||
| sinceid: this.sinceId, | ||
| beforeid: this.beforeId, | ||
| sincedate: this.sinceDate, | ||
| untildate: this.untilDate, | ||
| status: this.status, | ||
| reference: this.reference, | ||
| emailaddress: this.emailAddress, | ||
| idcustomer: this.customerId, | ||
| idwarehouse: this.warehouseId, | ||
| idfulfilment_customer: this.fulfillmentCustomerId, | ||
| }, | ||
| }); | ||
|
|
||
| $.export("$summary", `Orders found: ${response.length}`); | ||
| return response; |
There was a problem hiding this comment.
Guard against non-array responses before using .length
searchOrders() can return a paged object ({ data: [ … ], ... }) when pagination is enabled, or an error object. Directly reading response.length risks undefined or misleading summaries.
- $.export("$summary", `Orders found: ${response.length}`);
- return response;
+const orders = Array.isArray(response)
+ ? response
+ : response?.data ?? [];
+
+$.export("$summary", `Orders found: ${orders.length}`);
+return response;This keeps the action resilient to future API changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async run({ $ }) { | |
| const response = await this.picqer.searchOrders({ | |
| $, | |
| params: { | |
| search: this.search, | |
| sinceid: this.sinceId, | |
| beforeid: this.beforeId, | |
| sincedate: this.sinceDate, | |
| untildate: this.untilDate, | |
| status: this.status, | |
| reference: this.reference, | |
| emailaddress: this.emailAddress, | |
| idcustomer: this.customerId, | |
| idwarehouse: this.warehouseId, | |
| idfulfilment_customer: this.fulfillmentCustomerId, | |
| }, | |
| }); | |
| $.export("$summary", `Orders found: ${response.length}`); | |
| return response; | |
| async run({ $ }) { | |
| const response = await this.picqer.searchOrders({ | |
| $, | |
| params: { | |
| search: this.search, | |
| sinceid: this.sinceId, | |
| beforeid: this.beforeId, | |
| sincedate: this.sinceDate, | |
| untildate: this.untilDate, | |
| status: this.status, | |
| reference: this.reference, | |
| emailaddress: this.emailAddress, | |
| idcustomer: this.customerId, | |
| idwarehouse: this.warehouseId, | |
| idfulfilment_customer: this.fulfillmentCustomerId, | |
| }, | |
| }); | |
| const orders = Array.isArray(response) | |
| ? response | |
| : response?.data ?? []; | |
| $.export("$summary", `Orders found: ${orders.length}`); | |
| return response; |
🤖 Prompt for AI Agents
In components/picqer/actions/search-orders/search-orders.mjs around lines 86 to
105, the code assumes response is always an array and uses response.length
directly, but searchOrders() may return a paged object or an error object. To
fix this, check if response is an array before accessing length; if it is a
paged object, use the length of its data array; if neither, handle the case
gracefully to avoid errors and provide an accurate summary.
| search: this.search, | ||
| sinceid: this.sinceId, | ||
| beforeid: this.beforeId, | ||
| sincedate: this.sinceDate, | ||
| untildate: this.untilDate, | ||
| status: this.status, | ||
| reference: this.reference, | ||
| emailaddress: this.emailAddress, | ||
| idcustomer: this.customerId, | ||
| idwarehouse: this.warehouseId, | ||
| idfulfilment_customer: this.fulfillmentCustomerId, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Filter out undefined query params before sending
Every prop is included even when unset, producing ?search=&sinceid=&…. Some APIs treat empty strings as filters, altering behaviour. A quick cleanup avoids surprises:
- params: {
- search: this.search,
- sinceid: this.sinceId,
- ...
- },
+ params: Object.fromEntries(
+ Object.entries({
+ search: this.search,
+ sinceid: this.sinceId,
+ beforeid: this.beforeId,
+ sincedate: this.sinceDate,
+ untildate: this.untilDate,
+ status: this.status,
+ reference: this.reference,
+ emailaddress: this.emailAddress,
+ idcustomer: this.customerId,
+ idwarehouse: this.warehouseId,
+ idfulfilment_customer: this.fulfillmentCustomerId,
+ }).filter(([, v]) => v !== undefined && v !== "")
+ ),Cleaner URLs and fewer edge-cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| search: this.search, | |
| sinceid: this.sinceId, | |
| beforeid: this.beforeId, | |
| sincedate: this.sinceDate, | |
| untildate: this.untilDate, | |
| status: this.status, | |
| reference: this.reference, | |
| emailaddress: this.emailAddress, | |
| idcustomer: this.customerId, | |
| idwarehouse: this.warehouseId, | |
| idfulfilment_customer: this.fulfillmentCustomerId, | |
| }, | |
| params: Object.fromEntries( | |
| Object.entries({ | |
| search: this.search, | |
| sinceid: this.sinceId, | |
| beforeid: this.beforeId, | |
| sincedate: this.sinceDate, | |
| untildate: this.untilDate, | |
| status: this.status, | |
| reference: this.reference, | |
| emailaddress: this.emailAddress, | |
| idcustomer: this.customerId, | |
| idwarehouse: this.warehouseId, | |
| idfulfilment_customer: this.fulfillmentCustomerId, | |
| }).filter(([, v]) => v !== undefined && v !== "") | |
| ), |
🤖 Prompt for AI Agents
In components/picqer/actions/search-orders/search-orders.mjs around lines 90 to
101, the query parameters include all properties even when they are undefined or
empty, resulting in URLs with empty query values. To fix this, filter out any
properties from the query object that are undefined or empty before sending the
request, ensuring only set parameters are included. This will produce cleaner
URLs and prevent unintended API filtering behavior.
| async run({ $ }) { | ||
| const response = await this.picqer.addReturnComment({ | ||
| $, | ||
| returnId: this.returnId, | ||
| data: { | ||
| body: this.body, | ||
| show_at_related: this.showAtRelated, | ||
| is_visible_fulfilment: this.isVisibleFulfillment, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid sending undefined in request payload
If showAtRelated / isVisibleFulfillment are left blank, the current body transmits null/undefined, which Picqer may reject.
- data: {
- body: this.body,
- show_at_related: this.showAtRelated,
- is_visible_fulfilment: this.isVisibleFulfillment,
- },
+ data: {
+ body: this.body,
+ ...(this.showAtRelated !== undefined && { show_at_related: this.showAtRelated }),
+ ...(this.isVisibleFulfillment !== undefined && { is_visible_fulfilment: this.isVisibleFulfillment }),
+ },Keeps the payload minimal and complies with API expectations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async run({ $ }) { | |
| const response = await this.picqer.addReturnComment({ | |
| $, | |
| returnId: this.returnId, | |
| data: { | |
| body: this.body, | |
| show_at_related: this.showAtRelated, | |
| is_visible_fulfilment: this.isVisibleFulfillment, | |
| }, | |
| }); | |
| async run({ $ }) { | |
| const response = await this.picqer.addReturnComment({ | |
| $, | |
| returnId: this.returnId, | |
| data: { | |
| body: this.body, | |
| ...(this.showAtRelated !== undefined && { show_at_related: this.showAtRelated }), | |
| ...(this.isVisibleFulfillment !== undefined && { is_visible_fulfilment: this.isVisibleFulfillment }), | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In components/picqer/actions/add-return-comment/add-return-comment.mjs around
lines 36 to 45, the request payload includes properties show_at_related and
is_visible_fulfilment even when their values are undefined or null, which may
cause the Picqer API to reject the request. Modify the code to only include
these properties in the data object if their corresponding values (showAtRelated
and isVisibleFulfillment) are defined, thereby keeping the payload minimal and
compliant with API expectations.
| }); | ||
|
|
||
| return data.map(({ | ||
| idtemplate: value, name: label, | ||
| }) => ({ | ||
| label, | ||
| value, | ||
| })); | ||
| }, | ||
| }, | ||
| shippingProviderId: { | ||
| type: "string", | ||
| label: "Shipping Provider Id", | ||
| description: "The shipping provider id for creating a new order.", | ||
| async options({ page }) { | ||
| const data = await this.listShippingProviders({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
| return data.map(({ | ||
| idshippingprovider: value, name: label, | ||
| }) => ({ | ||
| label, | ||
| value, | ||
| })); | ||
| }, | ||
| }, | ||
| warehouseId: { | ||
| type: "string", | ||
| label: "Warehouse ID", | ||
| description: "Warehouses that can be used to fulfil this order.", | ||
| async options({ page }) { | ||
| const warehouses = await this.listWarehouses({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
| return warehouses.map(({ | ||
| idwarehouse: value, name: label, | ||
| }) => ({ | ||
| label, | ||
| value, | ||
| })); | ||
| }, | ||
| }, | ||
| fulfillmentCustomerId: { | ||
| type: "string", | ||
| label: "Fulfillment Customer ID", | ||
| description: "Fulfillment customer for this order.", | ||
| async options({ page }) { | ||
| const fulfillmentCustomers = await this.listFulfillmentCustomers({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
| return fulfillmentCustomers.map(({ | ||
| idcustomer: value, name: label, | ||
| }) => ({ | ||
| label, | ||
| value, | ||
| })); | ||
| }, | ||
| }, | ||
| returnId: { | ||
| type: "string", | ||
| label: "Return ID", | ||
| description: "The return id for searching returns.", | ||
| async options({ page }) { | ||
| const data = await this.searchReturns({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
|
|
||
| return data.map(({ | ||
| idreturn: value, returnid, name, | ||
| }) => ({ | ||
| label: `${returnid} - ${name}`, | ||
| value, | ||
| })); | ||
| }, | ||
| }, | ||
| picklistId: { | ||
| type: "string", | ||
| label: "Picklist ID", | ||
| description: "The picklist id for searching picklists.", | ||
| async options({ page }) { | ||
| const data = await this.listPicklists({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
| return data.map(({ | ||
| idpicklist: value, picklistid, deliveryname, | ||
| }) => ({ | ||
| label: `${picklistid} - ${deliveryname}`, | ||
| value, | ||
| })); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add consistent error handling across all async options methods.
All async options methods follow the same pattern but lack error handling and null checks. This could cause the options loading to fail silently or crash.
Consider creating a helper method to standardize this pattern:
Add this helper method in the methods section:
async _fetchOptions(apiMethod, params, valueKey, labelKey, formatLabel) {
try {
const data = await this[apiMethod]({
params: {
offset: params.page * LIMIT,
...params.additionalParams,
},
});
return data?.map((item) => ({
label: formatLabel ? formatLabel(item) : item[labelKey] || `${valueKey}: ${item[valueKey]}`,
value: item[valueKey],
})) || [];
} catch (error) {
console.error(`Failed to fetch options for ${apiMethod}:`, error);
return [];
}
}Then refactor each async options method to use this helper. For example:
- async options({ page }) {
- const data = await this.searchOrders({
- params: {
- offset: page * LIMIT,
- },
- });
-
- return data.map(({
- idorder: value, invoicename: label,
- }) => ({
- label,
- value,
- }));
- },
+ async options({ page }) {
+ return this._fetchOptions("searchOrders", { page }, "idorder", "invoicename");
+ },🤖 Prompt for AI Agents
In components/picqer/picqer.app.mjs from lines 15 to 157, all async options
methods lack error handling and null checks, risking silent failures or crashes.
To fix this, add a helper method named _fetchOptions in the methods section that
wraps the API call in a try-catch block, handles pagination offset, maps the
results safely with optional label formatting, and returns an empty array on
error. Then refactor each async options method to call this helper with
appropriate parameters instead of duplicating the logic.
| _baseUrl() { | ||
| return `https://${this.$auth.subdomain}.picqer.com/api/v1`; | ||
| }, | ||
| _auth() { | ||
| return { | ||
| username: `${this.$auth.api_key}`, | ||
| password: "", | ||
| }; | ||
| }, |
There was a problem hiding this comment.
Validate subdomain to prevent potential security issues.
The subdomain is directly interpolated into the URL without validation. This could potentially lead to subdomain takeover or SSRF attacks if the subdomain value is not properly validated.
Add subdomain validation:
_baseUrl() {
+ // Validate subdomain format to prevent injection attacks
+ if (!this.$auth.subdomain || !/^[a-zA-Z0-9-]+$/.test(this.$auth.subdomain)) {
+ throw new Error("Invalid subdomain format");
+ }
return `https://${this.$auth.subdomain}.picqer.com/api/v1`;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _baseUrl() { | |
| return `https://${this.$auth.subdomain}.picqer.com/api/v1`; | |
| }, | |
| _auth() { | |
| return { | |
| username: `${this.$auth.api_key}`, | |
| password: "", | |
| }; | |
| }, | |
| _baseUrl() { | |
| // Validate subdomain format to prevent injection attacks | |
| if (!this.$auth.subdomain || !/^[a-zA-Z0-9-]+$/.test(this.$auth.subdomain)) { | |
| throw new Error("Invalid subdomain format"); | |
| } | |
| return `https://${this.$auth.subdomain}.picqer.com/api/v1`; | |
| }, | |
| _auth() { | |
| return { | |
| username: `${this.$auth.api_key}`, | |
| password: "", | |
| }; | |
| }, |
🤖 Prompt for AI Agents
In components/picqer/picqer.app.mjs around lines 302 to 310, the subdomain used
in the _baseUrl method is directly interpolated without validation, which can
lead to security risks like subdomain takeover or SSRF. To fix this, add
validation logic to ensure the subdomain only contains allowed characters (e.g.,
alphanumeric and hyphens) and reject or sanitize any invalid input before
constructing the URL. This validation should be done before returning the URL
string in _baseUrl.
| _makeRequest({ | ||
| $ = this, path, ...opts | ||
| }) { | ||
| return axios($, { | ||
| url: this._baseUrl() + path, | ||
| auth: this._auth(), | ||
| ...opts, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and response validation to _makeRequest.
The generic request method lacks error handling and response validation, which could lead to unhandled errors propagating to action components.
- _makeRequest({
- $ = this, path, ...opts
- }) {
- return axios($, {
- url: this._baseUrl() + path,
- auth: this._auth(),
- ...opts,
- });
- },
+ async _makeRequest({
+ $ = this, path, ...opts
+ }) {
+ try {
+ const response = await axios($, {
+ url: this._baseUrl() + path,
+ auth: this._auth(),
+ ...opts,
+ });
+ return response;
+ } catch (error) {
+ if (error.response) {
+ throw new Error(`Picqer API error (${error.response.status}): ${error.response.data?.message || error.message}`);
+ }
+ throw error;
+ }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _makeRequest({ | |
| $ = this, path, ...opts | |
| }) { | |
| return axios($, { | |
| url: this._baseUrl() + path, | |
| auth: this._auth(), | |
| ...opts, | |
| }); | |
| }, | |
| async _makeRequest({ | |
| $ = this, path, ...opts | |
| }) { | |
| try { | |
| const response = await axios($, { | |
| url: this._baseUrl() + path, | |
| auth: this._auth(), | |
| ...opts, | |
| }); | |
| return response; | |
| } catch (error) { | |
| if (error.response) { | |
| throw new Error( | |
| `Picqer API error (${error.response.status}): ` + | |
| `${error.response.data?.message || error.message}` | |
| ); | |
| } | |
| throw error; | |
| } | |
| }, |
🤖 Prompt for AI Agents
In components/picqer/picqer.app.mjs around lines 311 to 319, the _makeRequest
method lacks error handling and response validation, risking unhandled errors in
calling components. Modify _makeRequest to wrap the axios call in a try-catch
block to catch and handle errors gracefully. After receiving the response,
validate its structure or status to ensure it meets expected criteria before
returning it. If validation fails, throw a descriptive error to prevent silent
failures.
| orderId: { | ||
| type: "string", | ||
| label: "Order ID", | ||
| description: "The order id for searching orders.", | ||
| async options({ page }) { | ||
| const data = await this.searchOrders({ | ||
| params: { | ||
| offset: page * LIMIT, | ||
| }, | ||
| }); | ||
|
|
||
| return data.map(({ | ||
| idorder: value, invoicename: label, | ||
| }) => ({ | ||
| label, | ||
| value, | ||
| })); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify description and add error handling.
The description "The order id for searching orders" is ambiguous. Consider updating to "Select an existing order" or similar for clarity.
Additionally, the async options method lacks error handling. If the API call fails, it could crash the options loading.
Apply this diff to improve the implementation:
- description: "The order id for searching orders.",
+ description: "Select an existing order.",
async options({ page }) {
- const data = await this.searchOrders({
- params: {
- offset: page * LIMIT,
- },
- });
-
- return data.map(({
- idorder: value, invoicename: label,
- }) => ({
- label,
- value,
- }));
+ try {
+ const data = await this.searchOrders({
+ params: {
+ offset: page * LIMIT,
+ },
+ });
+
+ return data?.map(({
+ idorder: value, invoicename: label,
+ }) => ({
+ label: label || `Order ${value}`,
+ value,
+ })) || [];
+ } catch (error) {
+ console.error("Failed to fetch orders:", error);
+ return [];
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| orderId: { | |
| type: "string", | |
| label: "Order ID", | |
| description: "The order id for searching orders.", | |
| async options({ page }) { | |
| const data = await this.searchOrders({ | |
| params: { | |
| offset: page * LIMIT, | |
| }, | |
| }); | |
| return data.map(({ | |
| idorder: value, invoicename: label, | |
| }) => ({ | |
| label, | |
| value, | |
| })); | |
| }, | |
| }, | |
| orderId: { | |
| type: "string", | |
| label: "Order ID", | |
| description: "Select an existing order.", | |
| async options({ page }) { | |
| try { | |
| const data = await this.searchOrders({ | |
| params: { | |
| offset: page * LIMIT, | |
| }, | |
| }); | |
| return data?.map(({ | |
| idorder: value, invoicename: label, | |
| }) => ({ | |
| label: label || `Order ${value}`, | |
| value, | |
| })) || []; | |
| } catch (error) { | |
| console.error("Failed to fetch orders:", error); | |
| return []; | |
| } | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In components/picqer/picqer.app.mjs around lines 11 to 29, update the orderId
field description to a clearer phrase like "Select an existing order" to improve
user understanding. Also, add error handling in the async options method by
wrapping the API call in a try-catch block; in case of failure, return an empty
array or a fallback value to prevent crashes during options loading.
| deleteHook(hookId) { | ||
| return this._makeRequest({ | ||
| method: "DELETE", | ||
| path: `/hooks/${hookId}`, | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Fix inconsistent method signature for deleteHook.
The deleteHook method doesn't accept an options object like other methods, making it inconsistent with the API pattern and preventing the use of the $ parameter for logging.
- deleteHook(hookId) {
- return this._makeRequest({
- method: "DELETE",
- path: `/hooks/${hookId}`,
- });
+ deleteHook({
+ hookId, ...opts
+ }) {
+ return this._makeRequest({
+ method: "DELETE",
+ path: `/hooks/${hookId}`,
+ ...opts,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deleteHook(hookId) { | |
| return this._makeRequest({ | |
| method: "DELETE", | |
| path: `/hooks/${hookId}`, | |
| }); | |
| }, | |
| deleteHook({ | |
| hookId, ...opts | |
| }) { | |
| return this._makeRequest({ | |
| method: "DELETE", | |
| path: `/hooks/${hookId}`, | |
| ...opts, | |
| }); | |
| }, |
🤖 Prompt for AI Agents
In components/picqer/picqer.app.mjs around lines 447 to 452, the deleteHook
method lacks an options parameter, making its signature inconsistent with other
methods and preventing use of the $ parameter for logging. Modify deleteHook to
accept an options object as a second parameter with a default empty object, and
pass this options object to the _makeRequest call to maintain consistency and
enable logging.
Resolves #16959.
Summary by CodeRabbit