18059 plentymarkets #18232
18059 plentymarkets #18232
Conversation
…r order management, and enhance prop definitions. Introduced methods for creating, retrieving, and managing orders, including adding notes and fetching order items and documents. Added constants for order types and payment statuses. Improved utility functions for better data handling.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a PlentyONE HTTP client and multiple order-related actions: create/get orders, get order items/properties/documents, and add order notes; introduces shared constants and parse helper; adds dynamic propDefinitions and updates package metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Action
participant A as Action (Create Order)
participant P as PlentyONE App
participant API as Plentymarkets REST
U->>A: Provide order inputs
A->>P: this.plentyone.createOrder(payload)
P->>API: POST /rest/orders
API-->>P: 201 Created (order)
P-->>A: Response
A-->>U: Summary with order ID
sequenceDiagram
autonumber
participant U as User Action
participant A as Action (Get Orders)
participant P as PlentyONE App
participant API as Plentymarkets REST
U->>A: Provide filters
A->>P: this.plentyone.getOrders(params)
P->>API: GET /rest/orders?...
API-->>P: 200 OK (orders)
P-->>A: Response
A-->>U: Summary with count
sequenceDiagram
autonumber
participant U as User Action
participant A as Action (Add Order Note)
participant P as PlentyONE App
participant API as Plentymarkets REST
U->>A: orderId, text, isVisibleForContact
A->>P: this.plentyone.addOrderNote({ referenceType, referenceValue, text, isVisibleForContact })
P->>API: POST /rest/comments (or /comments)
API-->>P: 201 Created (note)
P-->>A: Response
A-->>U: Summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Poem
✨ 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/Issue comments)Type 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: 15
🧹 Nitpick comments (14)
components/plentyone/common/utils.mjs (1)
4-23: Optional: Narrow JSON.parse to likely JSON to avoid surprises.Parsing arbitrary strings like "123" to numbers may be unintended in other call sites. Consider only parsing when strings start with “{” or “[”.
- if (Array.isArray(obj)) { + if (Array.isArray(obj)) { return obj.map((item) => { if (typeof item === "string") { - try { - return JSON.parse(item); - } catch (e) { - return item; - } + const s = item.trim(); + if (s.startsWith("{") || s.startsWith("[")) { + try { return JSON.parse(s); } catch { /* fall through */ } + } + return item; } return item; }); } if (typeof obj === "string") { - try { - return JSON.parse(obj); - } catch (e) { - return obj; - } + const s = obj.trim(); + if (s.startsWith("{") || s.startsWith("[")) { + try { return JSON.parse(s); } catch { /* fall through */ } + } + return obj; }components/plentyone/actions/get-orders/get-orders.mjs (1)
145-151: Optional: Clarify “With” prop help text.Consider listing a few common relations in the description and that multiple values can be selected.
components/plentyone/actions/get-order-documents/get-order-documents.mjs (1)
19-24: Optional: Add try/catch for consistent error reporting.Match the pattern used in other actions to surface API errors in the step summary.
components/plentyone/actions/get-order-items/get-order-items.mjs (1)
28-29: Avoid potential TypeError on undefined orderItemsAccessing
response.orderItems.lengthwithout verifyingorderItemsexists can throw. The refactor above usesconst items = response?.orderItems ?? [].components/plentyone/actions/get-order/get-order.mjs (1)
25-27: Consider returning a consistent shape across actionsOther actions sometimes return
response.data(e.g., create) or a subset (items). Decide on a convention (raw API payload vs wrapper) and standardize to reduce surprises for users.components/plentyone/actions/add-order-note/add-order-note.mjs (2)
22-26: Default visibility and avoid sending undefinedSet a default for
isVisibleForContactand avoid transmittingundefinedfields.isVisibleForContact: { type: "boolean", label: "Is Visible for Contact", - description: "Whether the note is visible to the contact.", + description: "Whether the note is visible to the contact.", + default: false, },
28-41: Add failure summary and keep payload minimalProvide a failure summary and omit undefined keys in the payload.
- async run({ $ }) { - const response = await this.plentyone.addOrderNote({ - $, - data: { - referenceType: "order", - referenceValue: this.orderId, - text: this.text, - isVisibleForContact: this.isVisibleForContact, - }, - }); - - $.export("$summary", `Successfully added note to order: ${this.orderId}`); - return response; - }, + async run({ $ }) { + try { + const data = { + referenceType: "order", + referenceValue: this.orderId, + text: this.text?.trim(), + }; + if (this.isVisibleForContact !== undefined) { + data.isVisibleForContact = this.isVisibleForContact; + } + const response = await this.plentyone.addOrderNote({ $, data }); + $.export("$summary", `Successfully added note to order: ${this.orderId}`); + return response?.data ?? response; + } catch (error) { + $.export("$summary", `Failed to add note to order: ${this.orderId}`); + throw error; + } + },components/plentyone/actions/create-order/create-order.mjs (2)
20-21: Mark required props explicitly (clarity in UI)If
orderTypeIdis required, consider addingoptional: falsefor UI clarity (behaviorally default is required, but explicit is clearer).
88-90: Consistency: return payload instead of wrapperOther actions sometimes return
responsewhile here you accessresponse.data. Favor returning the API payload (response.data) across actions for consistency.components/plentyone/plentyone.app.mjs (5)
101-109: Add Accept header and confirm plenty ID source for base URL
- Add Accept: application/json for clarity.
- Verify that this.$auth.id is the Plenty system PID (e.g., “p{myPID}”) returned by OAuth; otherwise, derive/store it explicitly. The PlentyONE docs show the base host format https://p{myPID}.my.plentysystems.com/rest. (developers.plentymarkets.com)
Apply within this range:
_headers() { return { "Authorization": `Bearer ${this.$auth.oauth_access_token}`, - "Content-Type": "application/json", + "Content-Type": "application/json", + "Accept": "application/json", }; }, _baseUrl() { return `https://p${this.$auth.id}.my.plentysystems.com/rest`; },
23-40: Contact ID should be numeric; harden label buildingUse integer type and guard against missing name parts to avoid “undefined undefined”. Also include the ID in the label for disambiguation in large lists.
Apply within this range:
contactId: { - type: "string", + type: "integer", label: "Contact ID", description: "ID of the contact to retrieve orders for.", async options({ page }) { const { entries } = await this.getContacts({ params: { page: page + 1, }, }); - return entries.map(({ - plentyId: value, title, firstName, lastName, - }) => ({ - label: `${title} ${firstName} ${lastName}`, - value, - })); + return entries.map(({ plentyId: value, title, firstName, lastName }) => { + const name = [title, firstName, lastName].filter(Boolean).join(" ").trim(); + return { + label: name ? `${name} (ID ${value})` : `Contact ${value}`, + value, + }; + }); }, },
41-57: Optional: enrich order selector labelsConsider including the order number or prefixing with “Order #” for clarity when IDs appear in other contexts.
7-21: Optional: include ID in warehouse labelsLarge accounts often have similarly named warehouses; appending the ID improves UX.
Possible tweak:
- return data.map(({ id: value, name: label }) => ({ label, value })); + return data.map(({ id: value, name }) => ({ label: `${name} (ID ${value})`, value }));
119-125: Order note helper lacks order contextIf callers must pass referenceType/referenceValue in opts for /comments, consider a convenience wrapper that takes (orderId, text, isVisibleForContact=false) and builds the payload, or support /orders/{orderId}/comments if available in your target docs. The generic /comments route with referenceType "order" and referenceValue = orderId is valid for attaching comments to orders. (developers.plentymarkets.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
components/plentyone/actions/add-order-note/add-order-note.mjs(1 hunks)components/plentyone/actions/create-order/create-order.mjs(1 hunks)components/plentyone/actions/get-order-documents/get-order-documents.mjs(1 hunks)components/plentyone/actions/get-order-items/get-order-items.mjs(1 hunks)components/plentyone/actions/get-order-properties/get-order-properties.mjs(1 hunks)components/plentyone/actions/get-order/get-order.mjs(1 hunks)components/plentyone/actions/get-orders/get-orders.mjs(1 hunks)components/plentyone/common/constants.mjs(1 hunks)components/plentyone/common/utils.mjs(1 hunks)components/plentyone/package.json(2 hunks)components/plentyone/plentyone.app.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T19:23:09.039Z
Learnt from: jcortes
PR: PipedreamHQ/pipedream#14935
File: components/sailpoint/package.json:15-18
Timestamp: 2024-12-12T19:23:09.039Z
Learning: When developing Pipedream components, do not add built-in Node.js modules like `fs` to `package.json` dependencies, as they are native modules provided by the Node.js runtime.
Applied to files:
components/plentyone/package.json
🧬 Code graph analysis (8)
components/plentyone/actions/get-order-documents/get-order-documents.mjs (4)
components/plentyone/actions/get-order-items/get-order-items.mjs (1)
response(20-26)components/plentyone/actions/get-order-properties/get-order-properties.mjs (1)
response(19-22)components/plentyone/actions/get-order/get-order.mjs (1)
response(20-23)components/plentyone/actions/get-orders/get-orders.mjs (1)
response(155-180)
components/plentyone/actions/add-order-note/add-order-note.mjs (3)
components/plentyone/actions/create-order/create-order.mjs (1)
response(73-86)components/plentyone/actions/get-order-documents/get-order-documents.mjs (1)
response(19-24)components/plentyone/actions/get-order/get-order.mjs (1)
response(20-23)
components/plentyone/actions/get-order/get-order.mjs (2)
components/plentyone/actions/get-order-items/get-order-items.mjs (1)
response(20-26)components/plentyone/actions/get-order-properties/get-order-properties.mjs (1)
response(19-22)
components/plentyone/common/utils.mjs (1)
components/akeneo/akeneo.app.mjs (1)
JSON(99-110)
components/plentyone/actions/get-order-items/get-order-items.mjs (3)
components/plentyone/actions/get-order-documents/get-order-documents.mjs (1)
response(19-24)components/plentyone/actions/get-order-properties/get-order-properties.mjs (1)
response(19-22)components/plentyone/actions/get-order/get-order.mjs (1)
response(20-23)
components/plentyone/actions/create-order/create-order.mjs (1)
components/plentyone/common/constants.mjs (2)
ORDER_TYPE_OPTIONS(1-62)LOCK_STATUS_OPTIONS(303-316)
components/plentyone/actions/get-orders/get-orders.mjs (2)
components/plentyone/common/constants.mjs (3)
ORDER_TYPE_OPTIONS(1-62)PAYMENT_STATUS_OPTIONS(162-179)ORDER_WITH_OPTIONS(181-274)components/plentyone/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
components/plentyone/actions/get-order-properties/get-order-properties.mjs (3)
components/plentyone/actions/get-order-documents/get-order-documents.mjs (1)
response(19-24)components/plentyone/actions/get-order-items/get-order-items.mjs (1)
response(20-26)components/plentyone/actions/get-order/get-order.mjs (1)
response(20-23)
⏰ 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). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (10)
components/plentyone/package.json (2)
15-17: Verify dependency placement and compatibility of @pipedream/platform.Looks good to add, but confirm 3.1.0 aligns with the runtime used for PlentyONE components and whether this should live in dependencies (vs peerDependencies) per repo convention.
3-3: Semver bump acknowledged.Version bump to 0.1.0 is fine given new functionality.
components/plentyone/common/constants.mjs (1)
1-62: Confirm codes/values match Plentymarkets enums.The numeric values (order types, reason codes) look plausible, but please verify against current API docs to avoid mismatches.
components/plentyone/actions/get-orders/get-orders.mjs (1)
154-179: Confirm API param names and types.Double-check Plenty API expects
typeID(vstypeId) and whethervariationNumberis numeric or string. Ensure the client strips undefined params before sending.components/plentyone/plentyone.app.mjs (6)
132-145: LGTM: orders and single order gettersPaths and shaping look consistent with PlentyONE patterns. Keep the “with[]” expansion support in mind for later.
172-179: LGTM: order items listExplicit method: "GET" is set; pagination is delegated via opts.params.
181-186: LGTM: create orderStraightforward POST /orders; payload is supplied by caller.
58-76: Verify countries API shapeConfirm getCountries() returns an array with id and name, or adapt to the actual response shape. In some PlentyONE contexts, storefront countries are exposed under other routes. (pwa-docs.plentymarkets.com)
101-118: Small doc-aligned tweak: Accept header exampleThe PlentyONE docs include Accept in examples; your addition above aligns with this and helps versioning later if needed. (developers.plentymarkets.com)
152-157: Switch find endpoint to POST and add order-specific helper
- Change
getOrderDocumentsto usemethod: "POST"on/orders/documents/findto support complex filters (verify in PlentyONE REST API docs).- Add
getOrderDocumentsByOrder({ orderId, ...opts })callingGET /orders/${orderId}/documents(confirm this endpoint exists).
components/plentyone/actions/get-order-documents/get-order-documents.mjs
Show resolved
Hide resolved
…OrderStatuses method in PlentyONE component. Enhanced order creation by parsing order items and related properties for improved data handling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
components/plentyone/actions/create-order/create-order.mjs (1)
73-96: Build a clean payload, strictly parse arrays to objects, returnresponse.data, and surface useful error messagesForwarding raw strings (when JSON parse fails) and sending undefined keys can produce invalid payloads and poor DX. Also,
response.idis likely nested underdata.async run({ $ }) { try { - const response = await this.plentyone.createOrder({ - $, - data: { - typeId: this.orderTypeId, - plentyId: this.plentyId, - statusId: this.statusId, - ownerId: this.ownerId, - lockStatus: this.lockStatus, - orderItems: parseObject(this.orderItems), - properties: parseObject(this.properties), - addressRelations: parseObject(this.addressRelations), - relations: parseObject(this.relations), - }, - }); + const parseList = (arr, name) => { + if (!arr) return undefined; + const out = arr.map((v) => parseObject(v)); + const invalid = out.some((v) => v == null || typeof v !== "object"); + if (invalid) { + throw new ConfigurationError(`${name} must be an array of JSON objects`); + } + return out; + }; + + const data = { + typeId: this.orderTypeId, + plentyId: this.plentyId, + ...(this.statusId !== undefined && { statusId: this.statusId }), + ...(this.ownerId !== undefined && { ownerId: this.ownerId }), + ...(this.lockStatus !== undefined && { lockStatus: this.lockStatus }), + ...(this.orderItems && { orderItems: parseList(this.orderItems, "orderItems") }), + ...(this.properties && { properties: parseList(this.properties, "properties") }), + ...(this.addressRelations && { addressRelations: parseList(this.addressRelations, "addressRelations") }), + ...(this.relations && { relations: parseList(this.relations, "relations") }), + }; + + const response = await this.plentyone.createOrder({ $, data }); - $.export("$summary", `Successfully created order: ${response.id}`); - return response; + const id = response?.data?.id ?? response?.id; + $.export("$summary", `Successfully created order: ${id ?? "Unknown"}`); + return response?.data ?? response; } catch (error) { $.export("$summary", "Failed to create order"); - throw new ConfigurationError(error); + const msg = error?.response?.data ?? error?.message ?? error; + throw new ConfigurationError(typeof msg === "string" ? msg : JSON.stringify(msg)); } },components/plentyone/plentyone.app.mjs (3)
189-196: Rename togetOrderReturnsand add a globalgetReturns()for/returnsCurrent name suggests global returns but the path is per-order. Split to avoid confusion and to support both endpoints.
- getReturns({ + getOrderReturns({ orderId, ...opts }) { return this._makeRequest({ path: `/orders/${orderId}/shipping/returns`, ...opts, }); },Add alongside (outside this hunk):
// Global returns list getReturns(opts = {}) { return this._makeRequest({ path: "/returns", ...opts, }); },
77-98: Order item selector must return the order item ID (not variation ID) and should be an integer; guard when orderId is missingCurrent mapping returns
itemVariationId, which will break update/delete endpoints that expect the order item’sid.orderItemId: { - type: "string", + type: "integer", label: "Order Item ID", description: "ID of the order item to delete.", async options({ page, orderId, }) { - const { entries } = await this.getOrderItems({ + if (!orderId) return []; + const { entries } = await this.getOrderItems({ orderId, params: { page: page + 1, }, }); - return entries.map(({ - itemVariationId: value, orderItemName: label, - }) => ({ - label, - value, - })); + return entries.map(({ id, orderItemName }) => ({ + label: orderItemName ? `${orderItemName} (Order Item #${id})` : `Order Item #${id}`, + value: id, + })); }, },
129-137: Merge default and caller headers; add a sane default timeoutPrevents accidental header overwrite (e.g., Authorization) and avoids hanging calls.
- _makeRequest({ - $ = this, path, ...opts - }) { - return axios($, { - url: this._baseUrl() + path, - headers: this._headers(), - ...opts, - }); - }, + _makeRequest({ $ = this, path, ...opts }) { + const { headers, timeout, ...rest } = opts ?? {}; + return axios($, { + url: this._baseUrl() + path, + headers: { ...this._headers(), ...(headers || {}) }, + timeout: timeout ?? 20000, + ...rest, + }); + },
🧹 Nitpick comments (4)
components/plentyone/actions/create-order/create-order.mjs (1)
48-71: Consider modeling these asobject[]to match the payload shapeIf the UI allows, declaring these as
object[]reduces parsing pitfalls and aligns types with the API.orderItems: { - type: "string[]", + type: "object[]", label: "Order Items", description: "A list of objects of the order items. [See the documentation](https://developers.plentymarkets.com/en-gb/plentymarkets-rest-api/index.html#/Order/post_rest_orders) for more details.", optional: true, }, properties: { - type: "string[]", + type: "object[]", label: "Properties", description: "A list of objects of the order properties. [See the documentation](https://developers.plentymarkets.com/en-gb/plentymarkets-rest-api/index.html#/Order/post_rest_orders) for more details.", optional: true, }, addressRelations: { - type: "string[]", + type: "object[]", label: "Address Relations", description: "A list of objects of the order address relations. [See the documentation](https://developers.plentymarkets.com/en-gb/plentymarkets-rest-api/index.html#/Order/post_rest_orders) for more details.", optional: true, }, relations: { - type: "string[]", + type: "object[]", label: "Relations", description: "A list of objects of the order relations. [See the documentation](https://developers.plentymarkets.com/en-gb/plentymarkets-rest-api/index.html#/Order/post_rest_orders) for more details.", optional: true, },components/plentyone/plentyone.app.mjs (3)
22-40: Verify the identifier: usingplentyIdfor a prop named “Contact ID” is confusingIf downstream filters expect
plentyId, either rename the prop to “Plenty ID” or switchvalueto the contact’sid. Also consider making this an integer for consistency.- contactId: { - type: "string", + contactId: { + type: "integer", label: "Contact ID", description: "ID of the contact to retrieve orders for.", async options({ page }) { const { entries } = await this.getContacts({ params: { page: page + 1, }, }); - return entries.map(({ - plentyId: value, title, firstName, lastName, - }) => ({ - label: `${title} ${firstName} ${lastName}`, - value, - })); + return entries.map(({ id: value, title, firstName, lastName }) => ({ + label: [title, firstName, lastName].filter(Boolean).join(" ") || `Contact #${value}`, + value, + })); }, },
41-57: Use integer type for orderId optionsIDs are numeric; using integer avoids string/number mismatches downstream.
orderId: { - type: "string", + type: "integer", label: "Order ID", description: "ID of the order to retrieve documents for.",
99-117: Fallback label for statuses without English namesAvoids empty labels if
names.enis missing.- return entries.map(({ - statusId: value, names: { en: label }, - }) => ({ - label, - value, - })); + return entries.map(({ statusId: value, names }) => { + const label = names?.en ?? Object.values(names ?? {})[0] ?? `Status ${value}`; + return { label, value }; + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/plentyone/actions/create-order/create-order.mjs(1 hunks)components/plentyone/plentyone.app.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/plentyone/actions/create-order/create-order.mjs (2)
components/plentyone/common/constants.mjs (2)
ORDER_TYPE_OPTIONS(1-62)LOCK_STATUS_OPTIONS(303-316)components/plentyone/common/utils.mjs (2)
parseObject(1-24)parseObject(1-24)
⏰ 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). (4)
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
|
/approve |
Resolves #18059
Summary by CodeRabbit
New Features
Refactor
Chores