[Components] nextdoor - new action components #15306
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThis pull request introduces a comprehensive set of Nextdoor advertising components, including modules for creating advertisers, campaigns, ad groups, ads, and scheduled reports. The implementation provides a structured approach to interacting with the Nextdoor Advertising Manager (NAM) API, with each module designed to handle specific resource creation tasks. The changes include defining properties, methods for API interactions, and supporting utility functions in the Nextdoor application configuration. Changes
Assessment against linked issues
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (2)
components/nextdoor/nextdoor.app.mjs (1)
91-99: Improve date input validation forstartTimeandendTimeThe
startTimeandendTimeproperties expect a ZonedDateTime format, which might be complex for users to input correctly. Consider using a date-time picker or validating the input format to improve user experience and prevent errors.Also applies to: 96-99
components/nextdoor/actions/create-ad-group/create-ad-group.mjs (1)
47-51: Consider adding bid amount validation.The bid amount should be validated to ensure it meets minimum requirements and follows the correct format.
bid: { type: "object", label: "Bid", description: "The bid for the ad group.", + validate: (value) => { + if (!value.amount?.startsWith('USD ')) { + throw new Error('Bid amount must start with "USD "'); + } + const amount = parseFloat(value.amount.replace('USD ', '')); + if (isNaN(amount) || amount <= 0) { + throw new Error('Invalid bid amount'); + } + return true; + }, default: { amount: "USD 10", pricing_type: "CPM", },
📜 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 (8)
components/nextdoor/actions/create-ad-group/create-ad-group.mjs(1 hunks)components/nextdoor/actions/create-ad/create-ad.mjs(1 hunks)components/nextdoor/actions/create-advertiser/create-advertiser.mjs(1 hunks)components/nextdoor/actions/create-campaign/create-campaign.mjs(1 hunks)components/nextdoor/actions/create-scheduled-report/create-scheduled-report.mjs(1 hunks)components/nextdoor/common/constants.mjs(1 hunks)components/nextdoor/nextdoor.app.mjs(1 hunks)components/nextdoor/package.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (5)
components/nextdoor/nextdoor.app.mjs (2)
241-246: Verify the authentication token propertyIn the
getHeadersmethod, you're accessingthis.$auth.authorization_token. Please ensure thatauthorization_tokenis the correct property provided by the authentication setup. Depending on the authentication configuration, it might need to bethis.$auth.oauth_access_tokenor another appropriate property.
247-255:⚠️ Potential issueReview assignment of
$in_makeRequestmethodIn the
_makeRequestmethod, assigning$ = thismay lead to unintended behavior since$typically represents the execution context. Consider removing the default assignment to ensure the correct context is used.Apply this diff to correct the assignment:
-_makeRequest({ - $ = this, path, headers, ...args -} = {}) { +_makeRequest({ + $, path, headers, ...args +} = {}) {Likely invalid or redundant comment.
components/nextdoor/actions/create-campaign/create-campaign.mjs (1)
1-59: Well-structured implementation ofcreate-campaignactionThe
create-campaignaction is implemented correctly with clear code structure and proper use of methods and properties. The use of destructuring and method definitions aligns with best practices.components/nextdoor/package.json (2)
3-3: Version bump aligns with new features.The version increment from 0.0.1 to 0.1.0 correctly follows semantic versioning for the addition of new advertising components.
15-16: Verify @pipedream/platform version compatibility.The dependency on @pipedream/platform uses a caret range (^3.0.3) which allows compatible updates. Let's verify this version against current recommendations.
✅ Verification successful
@pipedream/platform version ^3.0.3 is appropriate
The dependency is using the latest available version (3.0.3) with a caret range, which matches current standards across recently updated components in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest @pipedream/platform version and its usage across components # Check the latest version of @pipedream/platform npm view @pipedream/platform version # Check platform versions used across other components fd package.json components -x grep -l "@pipedream/platform" {} \; -x grep -A 1 "@pipedream/platform" {}Length of output: 65740
components/nextdoor/actions/create-scheduled-report/create-scheduled-report.mjs
Show resolved
Hide resolved
components/nextdoor/actions/create-scheduled-report/create-scheduled-report.mjs
Show resolved
Hide resolved
components/nextdoor/actions/create-ad-group/create-ad-group.mjs
Outdated
Show resolved
Hide resolved
components/nextdoor/actions/create-ad-group/create-ad-group.mjs
Outdated
Show resolved
Hide resolved
jcortes
force-pushed
the
nextdoor-new-lead-instant-source
branch
from
6f243ff to
aca6e93
Compare
jcortes
force-pushed
the
nextdoor-new-lead-instant-source
branch
2 times, most recently
from
886e129 to
81c4ad1
Compare
jcortes
force-pushed
the
nextdoor-new-lead-instant-source
branch
from
81c4ad1 to
431e2e1
Compare
| frequencyCaps: { | ||
| type: "string[]", | ||
| label: "Frecuency Caps", | ||
| description: "The frecuency caps for the ad group. Where `max_impressions`, `num_timeunits`, and `timeunit` are required. Eligible `timeunit` values are `MINUTE`, `MINUTES`, `HOUR`, `HOURS`, `DAY`, `DAYS`, `WEEK`, `WEEKS`, `MONTH`, `MONTHS`.", |
There was a problem hiding this comment.
Shouldn't this be spelled frequency in the label and description?
Is it possible to split this into an individual prop for each of 'max impressions', 'num_timeunits' and 'timeunits'? Or can the object have more properties than these three?
| type: "object", | ||
| label: "Budget", | ||
| description: "The budget for the ad group. Both `amount` and `budget_type` properties are required.", | ||
| default: { |
There was a problem hiding this comment.
Is it possible to split this into two props (Budget Amount and Budget Type) or can the object have more properties?
WHY
Resolves #15251
Summary by CodeRabbit
Release Notes for Nextdoor Integration
New Features
Improvements
Version Update