-
Notifications
You must be signed in to change notification settings - Fork 5.5k
17438 afosto #18476
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
base: master
Are you sure you want to change the base?
17438 afosto #18476
Conversation
- Added new actions for managing carts: create, add item, add information, add note, and confirm cart. - Introduced a new utility function for parsing objects. - Updated the Afosto component to include a Cart ID property definition. - Bumped version to 0.1.0 and added dependencies for the platform.
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds Afosto app HTTP helpers and GraphQL mutation methods, a parseObject utility, five new cart-related action modules (create cart, add item, add information, add note, confirm cart), mutation templates, and updates package version plus dependency on @pipedream/platform. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Afosto Action
participant App as Afosto App
participant API as Afosto GraphQL API
User->>Action: Invoke action with props
Action->>App: call method (e.g., addItemToCart/addInformationToCart)
App->>API: POST /graphql (Authorization, Content-Type)
API-->>App: Response (data / errors)
App-->>Action: Response
alt Success
Action-->>User: $.export(summary) and return data
else Error
Action-->>User: Throw ConfigurationError with first GraphQL error
end
sequenceDiagram
autonumber
participant Action as Add Information to Cart
participant App as Afosto App
participant API as Afosto GraphQL API
Note over Action: Build inputs: customer, phone, shipping, billing (uses parseObject)
Action->>App: addInformationToCart({ variables })
App->>API: POST batched mutation
API-->>App: Response with cart payload or errors
App-->>Action: data / errors
alt Success
Action-->>User: $.export("Successfully added information to cart with ID: {cartId}") and return cart
else Error
Action-->>User: Throw ConfigurationError with first GraphQL error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✨ 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. 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.
Actionable comments posted: 10
🧹 Nitpick comments (5)
components/afosto/package.json (1)
1-18
: PR objectives gap: sources and REST actions still missing.Issue #17438 requests sources (order.shipped, order.cancelled) and REST actions (create/update shipment, fetch invoices). This PR currently ships only cart GraphQL actions. Plan to include those endpoints or split into separate PRs?
I can scaffold the two sources and three actions (with props, auth, endpoints, and sample responses). Want me to push a commit or open follow-up issues?
components/afosto/common/utils.mjs (1)
16-22
: Optional: trim before JSON.parse.Safer for inputs like
" { \"a\": 1 } "
.Apply this diff:
- if (typeof obj === "string") { + if (typeof obj === "string") { try { - return JSON.parse(obj); + return JSON.parse(obj.trim()); } catch (e) { return obj; } }components/afosto/afosto.app.mjs (1)
6-12
: Consider adding propDefinitions for upcoming actions.Given issue #17438, adding
orderId
andshipmentId
propDefinitions here can standardize action props across shipment/invoice actions.components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
16-20
: Constrain quantity input.Add a minimum and default to prevent invalid values.
Apply this diff:
quantity: { type: "integer", label: "Quantity", description: "The quantity of the item to add to the cart.", + min: 1, + default: 1, },components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
33-35
: Header likely unnecessary here.
DisablePreParseMultipartForm
isn’t needed for JSON GraphQL requests; safe to remove unless you’ve observed a specific issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
(1 hunks)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
(1 hunks)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs
(1 hunks)components/afosto/actions/confirm-cart/confirm-cart.mjs
(1 hunks)components/afosto/actions/create-cart/create-cart.mjs
(1 hunks)components/afosto/afosto.app.mjs
(1 hunks)components/afosto/common/utils.mjs
(1 hunks)components/afosto/package.json
(2 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/afosto/package.json
🧬 Code graph analysis (6)
components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
response
(334-444)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
response
(30-58)components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
response
(20-37)components/afosto/actions/create-cart/create-cart.mjs (1)
response
(14-33)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (2)
variables
(25-28)response
(30-58)
components/afosto/common/utils.mjs (1)
components/akeneo/akeneo.app.mjs (1)
JSON
(99-110)
components/afosto/actions/confirm-cart/confirm-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
response
(334-444)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
response
(30-70)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
response
(30-58)components/afosto/actions/create-cart/create-cart.mjs (1)
response
(14-33)
components/afosto/actions/create-cart/create-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
response
(334-444)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
response
(30-70)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
response
(30-58)components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
response
(20-37)
components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (2)
variables
(312-332)response
(334-444)
⏰ 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). (1)
- GitHub Check: pnpm publish
🔇 Additional comments (3)
components/afosto/package.json (1)
15-17
: Dependency addition looks correct; version bump is fine.
@pipedream/platform
is appropriate for axios usage in the app methods. No issues spotted.components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
33-57
: Sanity-checkaddItemsToCart
response shape Confirm thatcart.items
is a plain array in Afosto’s schema (not a connection). If it’s a connection, update the query to selectitems { edges { node { … } } }
and adjust the response parsing accordingly.components/afosto/afosto.app.mjs (1)
21-23
: No change needed:https://afosto.app/graphql
is the correct Storefront GraphQL endpoint.
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
Outdated
Show resolved
Hide resolved
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
Outdated
Show resolved
Hide resolved
components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
Outdated
Show resolved
Hide resolved
components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
Outdated
Show resolved
Hide resolved
components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs
Outdated
Show resolved
Hide resolved
…r order in pickup points query
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.
Hey @luancazarine I think we need to improve the file and code organization a bit on this one.
Also not sure what the point of the try/catch blocks is, the thrown errors have different structures across components, wouldn't it be better to let our native error handling deal with the responses? Or are we intentionally formatting them differently here?
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
Show resolved
Hide resolved
components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
Outdated
Show resolved
Hide resolved
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
Outdated
Show resolved
Hide resolved
- Introduced new mutations for adding information, items, notes, confirming, and creating carts. - Refactored existing actions to leverage the new mutations for improved clarity and maintainability. - Updated the shipping and billing address fields to accept structured data objects. - Added error handling for API responses in actions.
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: 8
🧹 Nitpick comments (8)
components/afosto/common/mutations.mjs (1)
103-128
: Trim selection sets to essentials to reduce payload size.You’re fetching many fields (e.g., created_at, hs_code, url) that aren’t used by actions. Consider requesting only what you surface/need to minimize response time and payload.
Also applies to: 130-149, 151-160, 162-176
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (2)
105-125
: Avoid sending undefined phone data inside contact; add a simple guard.If you keep phone optional in the future, ensure the contact.phone_numbers array is omitted unless both values are present.
Apply this diff:
- const phoneNumberData = { - country_code: this.countryCode, - number: this.number, - }; + const phoneNumberData = { + country_code: this.countryCode, + number: this.number, + }; + const hasPhone = Boolean(this.countryCode && this.number); @@ - contact: { + contact: { email: this.email, is_guest: this.isGuest, given_name: this.givenName, additional_name: this.additionalName, family_name: this.familyName, - phone_numbers: [ - phoneNumberData, - ], + phone_numbers: hasPhone ? [phoneNumberData] : undefined, }, @@ - phoneNumberInput: { - cart_id: cartId, - phone_number: phoneNumberData, - }, + phoneNumberInput: { + cart_id: cartId, + phone_number: phoneNumberData, + },Note: With the separate mutation requiring input non-null, keeping phone required (change above) is the safer path. If you want truly optional phones, split the phone mutation into a conditional separate request.
Also applies to: 139-154
161-167
: Aggregate error messages; guard undefined data.Slightly clearer errors and safer access.
- if (response.errors) { - throw new ConfigurationError(JSON.stringify(response.errors[0])); - } - - $.export("$summary", `Successfully added information to cart with ID: ${this.cartId}`); - return response.data; + if (response?.errors?.length) { + throw new ConfigurationError(response.errors.map((e) => e.message).join("; ")); + } + $.export("$summary", `Successfully added information to cart with ID: ${this.cartId}`); + return response?.data;components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
27-33
: Harden error handling and null guards.Mirror the pattern used elsewhere: aggregate messages and verify order presence.
- if (response.errors) { - throw new ConfigurationError(JSON.stringify(response.errors[0])); - } - - $.export("$summary", `Successfully confirmed cart with ID: ${response.data.confirmCart.order.id}`); - return response.data.confirmCart.order; + if (response?.errors?.length) { + throw new ConfigurationError(response.errors.map((e) => e.message).join("; ")); + } + const order = response?.data?.confirmCart?.order; + if (!order?.id) { + throw new ConfigurationError("GraphQL error: no order returned"); + } + $.export("$summary", `Successfully confirmed order with ID: ${order.id}`); + return order;components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
35-41
: Consistent GraphQL error handling and safe access.Aggregate messages and guard return path.
- if (response.errors) { - throw new ConfigurationError(JSON.stringify(response.errors[0])); - } - - $.export("$summary", `Successfully added note to cart with ID: ${this.cartId}`); - return response.data.setNoteForCart.cart; + if (response?.errors?.length) { + throw new ConfigurationError(response.errors.map((e) => e.message).join("; ")); + } + $.export("$summary", `Successfully added note to cart with ID: ${this.cartId}`); + return response?.data?.setNoteForCart?.cart;components/afosto/afosto.app.mjs (3)
15-21
: Add a guard for missing auth and minor header tweaks.
Throw early if auth is absent; optionally include Accept._headers(headers = {}) { - return { - "Authorization": `Bearer ${this.$auth.auth_token}`, + if (!this?.$auth?.auth_token) { + throw new Error("Missing Afosto auth_token in $auth"); + } + return { + "Accept": "application/json", + "Authorization": `Bearer ${this.$auth.auth_token}`, "Content-Type": "application/json", ...headers, }; },Please confirm the $auth field is actually auth_token for Afosto in this app connection.
41-92
: Scope gap vs PR objectives (shipments/invoices sources/actions missing).
PR calls for order.shipped and order.cancelled sources, and actions for shipments/invoices. This module currently focuses on cart mutations only. Confirm if shipment/invoice endpoints and sources will be added in this PR or a follow-up.I can scaffold REST helpers and actions for:
- POST /orders/{order_id}/shipments
- PUT/PATCH /orders/{order_id}/shipments/{shipment_id}
- GET /orders/{order_id}/invoices
22-24
: Externalize GraphQL base URL
The hardcoded URLhttps://afosto.app/graphql
is confirmed. Consider exposing it via a prop or environment variable for future flexibility (e.g., account- or region-specific endpoints).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
(1 hunks)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
(1 hunks)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs
(1 hunks)components/afosto/actions/confirm-cart/confirm-cart.mjs
(1 hunks)components/afosto/actions/create-cart/create-cart.mjs
(1 hunks)components/afosto/afosto.app.mjs
(1 hunks)components/afosto/common/mutations.mjs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs
🧰 Additional context used
🧬 Code graph analysis (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
components/afosto/common/utils.mjs (2)
parseObject
(1-24)parseObject
(1-24)
components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (2)
variables
(112-154)response
(156-159)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
response
(30-43)components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
response
(20-25)components/afosto/actions/create-cart/create-cart.mjs (1)
response
(14-16)
components/afosto/actions/confirm-cart/confirm-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
response
(156-159)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
response
(30-43)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
response
(30-33)components/afosto/actions/create-cart/create-cart.mjs (1)
response
(14-16)
components/afosto/actions/create-cart/create-cart.mjs (4)
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs (1)
response
(156-159)components/afosto/actions/add-item-to-cart/add-item-to-cart.mjs (1)
response
(30-43)components/afosto/actions/add-note-to-cart/add-note-to-cart.mjs (1)
response
(30-33)components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
response
(20-25)
⏰ 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: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
🔇 Additional comments (6)
components/afosto/common/mutations.mjs (1)
80-97
: pickup_points variable mapping looks correct now.postal_code receives $postalCode and country_code receives $countryCode. LGTM.
If the upstream API expects shipping address for pickup_points (not billing), confirm the right source of postal/country inputs in the calling action.
components/afosto/actions/create-cart/create-cart.mjs (1)
4-12
: Scope check vs Issue #17438 (shipments/invoices).This action adds cart creation, but the linked issue also requests shipment create/update and invoice fetch, plus sources for order.shipped/cancelled. Are those planned in this PR or separate follow‑ups?
components/afosto/actions/confirm-cart/confirm-cart.mjs (1)
31-33
: Summary refers to “cart ID” but the API returns an order ID.Change label for accuracy.
- $.export("$summary", `Successfully confirmed cart with ID: ${response.data.confirmCart.order.id}`); + $.export("$summary", `Successfully confirmed order with ID: ${response.data.confirmCart.order.id}`);components/afosto/afosto.app.mjs (3)
7-13
: Clarify requiredness of cartId and intended reuse.
If actions rely on this propDefinition, consider marking it required (optional: false) or document when it’s needed.
25-34
: Do not default $ to this in _makeRequest; make $ required and pass it from callers.
Defaulting masks missing caller context and will break axios($, …) at runtime.- _makeRequest({ - $ = this, path = "", headers, ...opts - }) { - return axios($, { + _makeRequest({ + $, path = "", headers, ...opts + }) { + if (!$) throw new Error("Missing Pipedream $ context"); + return axios($, { method: "POST", url: this._baseUrl() + path, headers: this._headers(headers), ...opts, }); },#!/bin/bash # Find call sites to ensure $ is passed rg -nP '\.(createCart|addInformationToCart|addItemToCart|addNoteToCart|confirmCart)\s*\(' -C2Based on learnings.
1-2
: Approve: Dependency pinned correctly
Thecomponents/afosto/package.json
lists@pipedream/platform
at^3.1.0
, satisfying the ≥ 3.1.0 requirement.
components/afosto/actions/add-information-to-cart/add-information-to-cart.mjs
Show resolved
Hide resolved
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.
LGTM! Ready for QA
Resolves #17438
Summary by CodeRabbit
New Features
Improvements
Chores