-
Notifications
You must be signed in to change notification settings - Fork 5.5k
14336 aweber #18738
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
14336 aweber #18738
Conversation
- Added `createBroadcast` action to facilitate broadcast creation. - Introduced `New Broadcast Event` source to emit events for new broadcasts. - Updated existing actions to version 0.0.5 and adjusted descriptions for clarity. - Added new properties for list and integration options in the Aweber app. - Updated package version to 0.4.0 and dependencies to ensure compatibility.
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds a Create Broadcast action and New Broadcast Event source; extends the Aweber app with propDefinitions and methods for broadcasts, segments, and integrations; introduces test fixtures and sampleEmit; updates request header handling and generateMeta id fallback; bumps package and several action/source versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Action as Create Broadcast Action
participant App as aweber.app
participant API as AWeber API
User->>Action: Execute with props (accountId, listId, subject, body, ...)
Action->>App: createBroadcast(accountId, listId, data, { headers })
App->>API: POST /accounts/{accountId}/lists/{listId}/broadcasts
Note right of API: Content-Type: application/x-www-form-urlencoded
API-->>App: 201 Created (broadcast payload with uuid)
App-->>Action: Broadcast response
Action-->>User: Summary: Created broadcast {uuid}
sequenceDiagram
autonumber
participant Source as New Broadcast Event Source
participant App as aweber.app
participant API as AWeber API
participant PD as Pipedream Runtime
loop Poll interval
Source->>App: getBroadcasts(accountId, listId, { status })
App->>API: GET /accounts/{accountId}/lists/{listId}/broadcasts?status=...
API-->>App: List of broadcasts
App-->>Source: Broadcast list
Source->>Source: generateMeta(resource.id || resource.uuid)
alt New resource
Source->>PD: $emit(resource, {summary: "New broadcast event {subject}"})
else Already seen
Source->>Source: Skip
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)components/aweber/actions/create-broadcast/create-broadcast.mjs (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). (3)
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: 5
🧹 Nitpick comments (1)
components/aweber/sources/new-broadcast-event/new-broadcast-event.mjs (1)
23-32
: Consider enhancing the status field description.The current description "The status of the broadcast event" is brief. Consider clarifying that this field filters which broadcast events are emitted based on their status.
Apply this diff to improve clarity:
status: { type: "string", label: "Status", - description: "The status of the broadcast event.", + description: "Filter broadcast events by status. Only events matching this status will be emitted.", options: [ "draft", "scheduled", "sent", ], },
📜 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 (14)
components/aweber/actions/add-subscriber/add-subscriber.mjs
(1 hunks)components/aweber/actions/create-broadcast/create-broadcast.mjs
(1 hunks)components/aweber/actions/create-or-update-subscriber/create-or-update-subscriber.mjs
(1 hunks)components/aweber/actions/get-accounts/get-accounts.mjs
(1 hunks)components/aweber/actions/get-lists/get-lists.mjs
(1 hunks)components/aweber/actions/get-subscribers/get-subscribers.mjs
(1 hunks)components/aweber/actions/update-subscriber/update-subscriber.mjs
(1 hunks)components/aweber/aweber.app.mjs
(5 hunks)components/aweber/package.json
(2 hunks)components/aweber/sources/common.mjs
(2 hunks)components/aweber/sources/new-broadcast-event/new-broadcast-event.mjs
(1 hunks)components/aweber/sources/new-broadcast-event/test-event.mjs
(1 hunks)components/aweber/sources/subscriber-added/subscriber-added.mjs
(2 hunks)components/aweber/sources/subscriber-added/test-event.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (2)
components/aweber/actions/create-or-update-subscriber/create-or-update-subscriber.mjs (1)
response
(87-98)components/aweber/actions/update-subscriber/update-subscriber.mjs (1)
response
(74-87)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (12)
components/aweber/actions/update-subscriber/update-subscriber.mjs (1)
9-9
: LGTM!Version bump is consistent with the broader package updates in this PR.
components/aweber/sources/common.mjs (2)
1-1
: LGTM!Import organization improvement.
67-67
: Good defensive fallback for broadcast events.The fallback to
resource.uuid
accommodates broadcast resources that useuuid
as their primary identifier instead ofid
. This aligns with the test fixture innew-broadcast-event/test-event.mjs
which usesuuid
.components/aweber/package.json (2)
3-3
: LGTM!Version bump aligns with the new features and updates in this PR.
13-13
: Resolve — @pipedream/platform 3.1.0 verified for this component.
Aweber imports axios, ConfigurationError and DEFAULT_POLLING_SOURCE_TIMER_INTERVAL (components/aweber/aweber.app.mjs, common/utils.mjs, sources/common.mjs); platform v3.1.0 (platform/dist/index.js and .d.ts) exports these symbols. No changes required.components/aweber/sources/subscriber-added/subscriber-added.mjs (2)
3-3
: LGTM!Good pattern for wiring test fixtures to the source component. The sampleEmit field provides clear example payloads for testing and documentation.
Also applies to: 31-31
11-11
: LGTM!Version bump is consistent with the addition of the sampleEmit field.
components/aweber/sources/new-broadcast-event/test-event.mjs (1)
1-9
: LGTM!The broadcast event test fixture provides a comprehensive sample payload with all relevant fields (uuid, broadcast_id, timestamps, status, subject).
components/aweber/sources/new-broadcast-event/new-broadcast-event.mjs (1)
1-11
: LGTM!The source implementation follows the established pattern from other Aweber sources. The structure is clean, methods are well-defined, and the resource fetching logic is appropriate.
Also applies to: 34-54
components/aweber/aweber.app.mjs (3)
110-217
: LGTM!The three new propDefinitions (listSelfLink, integrations, segmentSelfLink) follow consistent patterns with existing prop definitions. They provide async options fetching with pagination support and clear labeling. The segmentSelfLink description is particularly helpful in explaining the optional nature and default behavior.
270-275
: LGTM!Cleaner headers handling by accepting headers as an explicit parameter rather than extracting from otherArgs. This improves the method signature clarity.
304-361
: LGTM!The four new methods (getBroadcasts, getSegments, getIntegrations, createBroadcast) follow the established patterns in the codebase. They correctly construct API paths and propagate arguments.
components/aweber/actions/create-broadcast/create-broadcast.mjs
Outdated
Show resolved
Hide resolved
components/aweber/actions/create-broadcast/create-broadcast.mjs
Outdated
Show resolved
Hide resolved
components/aweber/actions/create-broadcast/create-broadcast.mjs
Outdated
Show resolved
Hide resolved
- Corrected "HMTL" to "HTML" in the `bodyText` description for clarity. - Fixed the spelling of `clickTrackingEnabeld` to `clickTrackingEnabled` in the properties.
- Changed readOnlyHint from true to false in the Aweber create-broadcast action to enable editing capabilities.
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: 1
♻️ Duplicate comments (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (1)
155-157
: Content-Type header should beapplication/json
.The header is still set to
"application/x-www-form-urlencoded"
but the payload is a JavaScript object that axios serializes as JSON. This issue was previously flagged and remains unresolved.Apply this diff to fix the Content-Type header:
headers: { - "Content-Type": "application/x-www-form-urlencoded", + "Content-Type": "application/json", },Alternatively, remove the explicit
Content-Type
header and let axios set it automatically based on the request body type.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (2)
components/aweber/actions/create-or-update-subscriber/create-or-update-subscriber.mjs (1)
response
(87-98)components/aweber/actions/update-subscriber/update-subscriber.mjs (1)
response
(74-87)
⏰ 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: Publish TypeScript components
- GitHub Check: Verify TypeScript components
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.
Just one comment about the Body HTML and Body Text properties - I think this needs to be revised before moving forward
- Marked both `bodyHtml` and `bodyText` properties as optional in the create-broadcast action to enhance flexibility in message creation.
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: 1
♻️ Duplicate comments (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (1)
157-159
: Content-Type header is still incorrect.As flagged in a previous review, the
Content-Type
header is set toapplication/x-www-form-urlencoded
, but thedata
object is a JavaScript object that axios will serialize as JSON. The header should beapplication/json
or omitted entirely to let axios set it automatically.Apply this diff to fix the header:
}, headers: { - "Content-Type": "application/x-www-form-urlencoded", + "Content-Type": "application/json", }, });Alternatively, remove the explicit
Content-Type
header:}, - headers: { - "Content-Type": "application/x-www-form-urlencoded", - }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (2)
components/aweber/actions/create-or-update-subscriber/create-or-update-subscriber.mjs (1)
response
(87-98)components/aweber/actions/update-subscriber/update-subscriber.mjs (1)
response
(74-87)
⏰ 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: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/aweber/actions/create-broadcast/create-broadcast.mjs (1)
9-13
: LGTM! Annotations are correct.The
readOnlyHint: false
annotation is now correct for this write operation, and the other annotations are appropriate for a create action.
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.
Hi @luancazarine lgtm! I've just added a couple of suggestions. Ready for QA.
components/aweber/actions/create-broadcast/create-broadcast.mjs
Outdated
Show resolved
Hide resolved
components/aweber/sources/new-broadcast-event/new-broadcast-event.mjs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jorge Cortes <[email protected]>
…ent.mjs Co-authored-by: Jorge Cortes <[email protected]>
/approve |
Resolves #14336
Summary by CodeRabbit