-
Notifications
You must be signed in to change notification settings - Fork 5.5k
[Components] nextdoor - new action components #15306
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
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 (
|
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: 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
Outdated
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
6f243ff to
aca6e93
Compare
81c4ad1 to
431e2e1
Compare
GTFalcao
left a 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.
Hi @jcortes , I have a few suggestions (not sure if they are feasible, depends on the app specifications). Please take a look at them before we move this forward
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
5a4c18a to
0c194ec
Compare
GTFalcao
left a 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.
I commented on one of the new props, but moving it to QA nonetheless
components/nextdoor/actions/create-ad-group/create-ad-group.mjs
Outdated
Show resolved
Hide resolved
0d1a312 to
4e83e43
Compare
4e83e43 to
ebc16ca
Compare
|
/approve |
WHY
Resolves #15251
Summary by CodeRabbit
Release Notes for Nextdoor Integration
New Features
Improvements
Version Update