-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New Components - DHL #18181
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?
New Components - DHL #18181
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughIntroduces a DHL app HTTP wrapper using axios, adds a Get Tracking Information action that calls the DHL tracking endpoint, defines shared shipping service constants, and updates the DHL package to include @pipedream/platform and a new version. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Flow
participant A as Action: dhl-get-tracking
participant APP as DHL App
participant AX as axios
participant API as DHL Tracking API
U->>A: Run with tracking params
A->>APP: getTracking({ params })
APP->>APP: _baseUrl()
APP->>AX: _makeRequest(path: "/track/shipments", headers:{dhl-api-key})
AX->>API: GET /track/shipments?query
API-->>AX: 200 Response (tracking data)
AX-->>APP: Response
APP-->>A: Response
A-->>U: Return data (+ $summary)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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). (5)
✨ 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 (
|
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: 4
🧹 Nitpick comments (13)
components/dhl/common/constants.mjs (1)
39-44
: Optional: Also export named constants for clearer imports and tree-shakingKeeping the default export is fine; adding named exports improves DX without breaking existing imports.
Apply on top of the previous change:
export default { SHIPPING_SERVICES, STATUS_OPTIONS, SUBSCRIPTION_TYPES, MODE_OF_TRANSPORT, SHIPPIMG_SERVICES, }; + +export { + SHIPPING_SERVICES, + STATUS_OPTIONS, + SUBSCRIPTION_TYPES, + MODE_OF_TRANSPORT, +};components/dhl/actions/list-subscriptions/list-subscriptions.mjs (1)
16-16
: Make the summary robust to different response shapesIf the API returns a count field or a differently named array, the current expression may show 0. Minor guard improves UX without altering behavior.
Apply:
- $.export("$summary", `Successfully listed ${response.subscriptions?.length || 0} subscriptions`); + const count = Array.isArray(response?.subscriptions) + ? response.subscriptions.length + : (response?.totalCount ?? response?.count ?? 0); + $.export("$summary", `Successfully listed ${count} subscriptions`);components/dhl/actions/get-subscription-details/get-subscription-details.mjs (1)
19-23
: Standardize the subscription ID parameter name across the DHL integrationTo avoid confusion between the external action prop (
subscriptionId
) and the internal app-method argument (subscriberId
), I recommend aligning onsubscriptionId
everywhere, with a simple alias for the HTTP call. Specifically:• In
components/dhl/dhl.app.mjs
, update each method signature:- getSubscriptionDetails({ subscriberId, ...opts }) { - return this._makeRequest({ - path: `/dgff/push/subscription/timestamp/${subscriberId}`, + getSubscriptionDetails({ subscriptionId, ...opts }) { + const subscriberId = subscriptionId; + return this._makeRequest({ + path: `/dgff/push/subscription/timestamp/${subscriberId}`,(and similarly for
updateSubscriptionStatus
andupdateSubscriptionFilters
)• In each action (e.g.
components/dhl/actions/get-subscription-details/get-subscription-details.mjs
), pass the prop directly:- const response = await this.dhl.getSubscriptionDetails({ - $, - subscriberId: this.subscriptionId, + const response = await this.dhl.getSubscriptionDetails({ + $, + subscriptionId: this.subscriptionId,(and likewise in the other DHL actions)
Optional refactor: if you prefer not to alias inside the methods, you can keep the HTTP‐path variable named
subscriberId
by destructuring with a rename:getSubscriptionDetails({ subscriptionId, ...opts }) { return this._makeRequest({ path: `/dgff/push/subscription/timestamp/${subscriptionId}`, ...opts, }); }This change ensures a single, consistent property name (
subscriptionId
) exposed to action users while still satisfying DHL’s API expectations.components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (2)
54-59
: Docs nit: fix “GET Started” phrasing and spacing in descriptionMinor copy edit for clarity and consistency across actions.
- description: "Event codes. Please refer to [GET Started Page](https://developer.dhl.com/api-reference/dgf-push-api#get-started-section/timestamp-push-api-v2-_28in-testing_29) for the event codes. Example:`A30`", + description: "Event codes. Please refer to the [Get Started page](https://developer.dhl.com/api-reference/dgf-push-api#get-started-section/timestamp-push-api-v2-_28in-testing_29) for the event codes. Example: `A30`",
74-83
: Only include callback-details when a URL is providedPrevents sending null/undefined values to the API.
- const response = await this.dhl.updateSubscriptionFilters({ - $, - subscriberId: this.subscriptionId, - data: { - filters, - "callback-details": { - "callback-url": this.callbackUrl, - }, - }, - }); + const data = { filters }; + if (this.callbackUrl) { + data["callback-details"] = { + "callback-url": this.callbackUrl, + }; + } + const response = await this.dhl.updateSubscriptionFilters({ + $, + subscriberId: this.subscriptionId, + data, + });components/dhl/actions/create-subscription/create-subscription.mjs (3)
18-27
: Use a boolean prop for onlyNotifyAPIBookings instead of string "true"/"false"A boolean prop improves UX and avoids accidental string mismatches. Convert to boolean on send.
- onlyNotifyAPIBookings: { - type: "string", - label: "Only Notify API Bookings", - description: "Enable to receive timestamp or tracking notifications exclusively for shipments booked through the API", - options: [ - "true", - "false", - ], - optional: true, - }, + onlyNotifyAPIBookings: { + type: "boolean", + label: "Only Notify API Bookings", + description: "Receive notifications exclusively for shipments booked through the API", + optional: true, + },
71-76
: Docs nit: fix “GET Started” phrasing and spacing in descriptionKeep wording consistent with the other action comment.
- description: "Event codes. Please refer to [GET Started Page](https://developer.dhl.com/api-reference/dgf-push-api#get-started-section/timestamp-push-api-v2-_28in-testing_29) for the event codes. Example:`A30`", + description: "Event codes. Please refer to the [Get Started page](https://developer.dhl.com/api-reference/dgf-push-api#get-started-section/timestamp-push-api-v2-_28in-testing_29) for the event codes. Example: `A30`",
91-101
: Construct payload conditionally and coerce onlyNotifyAPIBookings to booleanAvoid sending undefined fields; ensure correct type for the API.
- const response = await this.dhl.createSubscription({ - $, - data: { - "status": this.status, - "onlyNotifyAPIBookings": this.onlyNotifyAPIBookings, - "subscription-type": this.subscriptionType, - filters, - "callback-details": { - "callback-url": this.callbackUrl, - }, - }, - }); + const data = { + status: this.status, + filters, + "callback-details": { + "callback-url": this.callbackUrl, + }, + }; + if (this.onlyNotifyAPIBookings !== undefined) { + data.onlyNotifyAPIBookings = Boolean(this.onlyNotifyAPIBookings); + } + if (this.subscriptionType) { + data["subscription-type"] = this.subscriptionType; + } + const response = await this.dhl.createSubscription({ + $, + data, + });components/dhl/actions/get-tracking/get-tracking.mjs (2)
36-41
: Label mismatch: “Requester Postal Code” vs recipientPostalCode propAlign the label with the prop/description to avoid user confusion.
- label: "Requester Postal Code", + label: "Recipient Postal Code",
48-59
: Optional: set sensible defaults for limit/offset in the UIDefaults are mentioned in the description; reflect them via default values.
limit: { type: "integer", label: "Limit", description: "Maximum number of events to be returned in the response. Default: 5", optional: true, + default: 5, }, offset: { type: "integer", label: "Offset", description: "Offset of the first event to be returned in the response. Default: 0", optional: true, + default: 0, },components/dhl/dhl.app.mjs (3)
13-15
: Improve subscriptionId options UX by returning label/value pairsReturning objects yields friendlier dropdowns and allows future display customization without changing values.
- const { subscriptions } = await this.listSubscriptions(); - return subscriptions?.map(({ subscriptionID }) => subscriptionID) || []; + const { subscriptions } = await this.listSubscriptions(); + return subscriptions?.map(({ subscriptionID }) => ({ + label: subscriptionID, + value: subscriptionID, + })) || [];
35-43
: More robust URL join and explicit JSON headersPrevents double slashes or missing slashes if api_url is configured inconsistently; explicit headers are harmless and sometimes required by APIs.
- _makeRequest({ - $ = this, path, ...opts - }) { - return axios($, { - url: `${this._baseUrl()}${path}`, - headers: { - "dhl-api-key": `${this.$auth.api_key}`, - }, - ...opts, - }); - }, + _makeRequest({ + $ = this, path, ...opts + }) { + const url = new URL(path, this._baseUrl()).toString(); + return axios($, { + url, + headers: { + "dhl-api-key": `${this.$auth.api_key}`, + "accept": "application/json", + "content-type": "application/json", + }, + ...opts, + }); + },
39-41
: Use the canonical DHL header casing: “DHL-API-Key”
- File: components/dhl/dhl.app.mjs (lines 39–41)
Update the header to match DHL’s examples:- headers: { - "dhl-api-key": `${this.$auth.api_key}`, - }, + headers: { + "DHL-API-Key": `${this.$auth.api_key}`, + },HTTP headers are case-insensitive, but DHL’s official Shipment Tracking API reference and their integration guides use “DHL-API-Key” for the API key header, which can aid debugging against their examples. (developer.dhl.com, parabola.io)
📜 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.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
components/dhl/actions/create-subscription/create-subscription.mjs
(1 hunks)components/dhl/actions/get-subscription-details/get-subscription-details.mjs
(1 hunks)components/dhl/actions/get-tracking/get-tracking.mjs
(1 hunks)components/dhl/actions/list-subscriptions/list-subscriptions.mjs
(1 hunks)components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs
(1 hunks)components/dhl/actions/update-subscription-status/update-subscription-status.mjs
(1 hunks)components/dhl/common/constants.mjs
(1 hunks)components/dhl/dhl.app.mjs
(1 hunks)components/dhl/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/dhl/package.json
🧬 Code graph analysis (6)
components/dhl/actions/list-subscriptions/list-subscriptions.mjs (5)
components/dhl/actions/create-subscription/create-subscription.mjs (1)
response
(91-102)components/dhl/actions/get-subscription-details/get-subscription-details.mjs (1)
response
(19-22)components/dhl/actions/get-tracking/get-tracking.mjs (1)
response
(62-74)components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (1)
response
(74-83)components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
response
(25-31)
components/dhl/actions/get-tracking/get-tracking.mjs (5)
components/dhl/actions/create-subscription/create-subscription.mjs (1)
response
(91-102)components/dhl/actions/get-subscription-details/get-subscription-details.mjs (1)
response
(19-22)components/dhl/actions/list-subscriptions/list-subscriptions.mjs (1)
response
(13-15)components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (1)
response
(74-83)components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
response
(25-31)
components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (2)
components/dhl/actions/create-subscription/create-subscription.mjs (4)
i
(54-54)i
(82-82)filters
(81-81)response
(91-102)components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
response
(25-31)
components/dhl/actions/update-subscription-status/update-subscription-status.mjs (3)
components/dhl/actions/create-subscription/create-subscription.mjs (1)
response
(91-102)components/dhl/actions/get-subscription-details/get-subscription-details.mjs (1)
response
(19-22)components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (1)
response
(74-83)
components/dhl/actions/get-subscription-details/get-subscription-details.mjs (3)
components/dhl/actions/list-subscriptions/list-subscriptions.mjs (1)
response
(13-15)components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (1)
response
(74-83)components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
response
(25-31)
components/dhl/actions/create-subscription/create-subscription.mjs (2)
components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs (4)
i
(37-37)i
(65-65)filters
(64-64)response
(74-83)components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
response
(25-31)
⏰ 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: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (6)
components/dhl/package.json (1)
3-17
: Version bump and platform dependency look appropriateMinor version bump aligns with adding new actions. The addition of @pipedream/platform is expected for axios/requests and Pipedream helpers. Also confirms the retrieved learning: no built-in Node modules added to dependencies.
components/dhl/actions/list-subscriptions/list-subscriptions.mjs (1)
6-6
: Confirm the docs link matches the “list subscriptions” endpointThe anchor references a specific operation; verify it points to the collection/list endpoint (GET /subscriptions) and not the single-subscription GET. If it doesn’t, please adjust the link to the correct section.
components/dhl/actions/update-subscription-status/update-subscription-status.mjs (1)
24-33
: LGTM: Status update call and summaryGood use of propDefinitions and a concise PATCH payload. Assuming dhl.app.mjs handles error mapping, this action is solid.
components/dhl/actions/create-subscription/create-subscription.mjs (1)
104-106
: LGTM on summary usageUsing response.subscriptionID in the success message is consistent with the API shape indicated elsewhere.
components/dhl/actions/get-tracking/get-tracking.mjs (1)
61-77
: LGTM on request assemblyParameter names and mapping to this.dhl.getTracking({ params }) look consistent.
components/dhl/dhl.app.mjs (1)
45-89
: LGTM on method surface and pathsEndpoints and verbs look consistent with the actions and linked API docs.
components/dhl/actions/create-subscription/create-subscription.mjs
Outdated
Show resolved
Hide resolved
components/dhl/actions/update-subscription-filters/update-subscription-filters.mjs
Outdated
Show resolved
Hide resolved
/approve |
/approve |
Resolves #18058
@vunguyenhung The "subscription" related components are untested. I added the Push Subscription V2 (DHL Global Forwarding) API to the account in 1PW, but it still shows as Pending, and the endpoints return a 401 error.
Summary by CodeRabbit
New Features
Refactor
Chores