Add search action for spotify #18791
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
WalkthroughAdds a new Spotify Search action that validates inputs, performs a GET to Spotify's /search with provided params, aggregates items across requested types, and returns the API response. Also bumps the Spotify component package version to 0.8.0. Changes
Sequence DiagramsequenceDiagram
participant User
participant Action as Spotify Search Action
participant Spotify as Spotify API
User->>Action: invoke(query, types, market, limit, offset, includeExternal)
Action->>Action: validate query and types
Action->>Spotify: GET /search?q=...&type=...&market=...&limit=...&offset=...&include_external=...
Spotify-->>Action: per-type search payloads
Action->>Action: extract & aggregate items, build summary
Action-->>User: return aggregated response and summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
components/spotify/actions/search/search.mjs (1)
68-76: Consider removing redundant description text.The description on line 73 states "Default: 20, Maximum: 50" but this information is already captured by the
default: 20andmax: 50properties. While the redundancy doesn't cause issues and may help users, it creates a maintenance burden if these values need to change.Apply this diff to remove the redundant information:
- description: "The maximum number of results to return per type. Default: 20, Maximum: 50.", + description: "The maximum number of results to return per type.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/spotify/actions/search/search.mjs(1 hunks)components/spotify/package.json(1 hunks)
🔇 Additional comments (7)
components/spotify/package.json (2)
3-3: LGTM! Appropriate version bump for new feature.The minor version increment from 0.7.3 to 0.8.0 correctly follows semantic versioning for the addition of the new search action.
18-18: LGTM! Secure lodash version.The lodash dependency at version 4.17.21 includes the security fix for CVE-2021-23337 (command injection in _.template) and is a stable release. Based on learnings.
components/spotify/actions/search/search.mjs (5)
1-7: LGTM! Efficient import strategy.Using
lodash/get.jsfor a single function is a good practice for tree-shaking and keeping bundle size minimal. Based on learnings.
9-19: LGTM! Appropriate metadata and annotations.The metadata correctly describes this as a read-only search operation with appropriate hints for the platform.
122-125: LGTM! Safe property access with aggregation.Using
lodash.getwith a fallback empty array ensures safe access to nested response properties and correctly handles missing data. The reduce properly aggregates items across all requested types.
127-134: LGTM! Clear and informative summary construction.The typeLabel logic correctly handles both single and multiple type searches, with appropriate pluralization. The summary message clearly communicates results to users.
135-135: LGTM! Returning full API response provides flexibility.Returning the complete API response (rather than just aggregated items) gives users access to pagination metadata and other useful information from the Spotify API.
bontaq
force-pushed
the
add-search-for-spotify
branch
from
8c8d735 to
ea5fab0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/spotify/actions/search/search.mjs (2)
106-108: Empty query validation added — LGTM.Good use of
ConfigurationErrorand trimming to prevent empty searches. This addresses earlier feedback.
110-110: Removed array normalization — consistent with other Spotify actions.Using
typedirectly matches patterns elsewhere in this package.
🧹 Nitpick comments (2)
components/spotify/actions/search/search.mjs (2)
112-123: Add a request timeout for resiliency.Prevent hanging runs on network stalls by setting a conservative timeout.
- const res = await axios($, this.spotify._getAxiosParams({ + const res = await axios($, this.spotify._getAxiosParams({ method: "GET", path: "/search", params: { q: query, type: types.join(","), market, limit, offset, include_external: includeExternal, }, - })); + timeout: 15000, + }));
125-129: UseflatMapto simplify aggregation.Minor readability win; behavior unchanged.
- const allItems = types.reduce((accumulator, itemType) => ( - accumulator.concat(get(res, `${ITEM_TYPES_RESULT_NAME[itemType]}.items`, [])) - ), []); + const allItems = types.flatMap( + (itemType) => get(res, `${ITEM_TYPES_RESULT_NAME[itemType]}.items`, []), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/spotify/actions/search/search.mjs(1 hunks)components/spotify/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/spotify/package.json
bontaq
force-pushed
the
add-search-for-spotify
branch
from
ea5fab0 to
490562b
Compare
bontaq
force-pushed
the
add-search-for-spotify
branch
from
490562b to
199e016
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/spotify/actions/search/search.mjs (2)
25-29: Consider adding prop-level validation for better UX.While the runtime validation at lines 106-108 works, adding
required: trueto the query prop would provide earlier feedback (at configuration time rather than execution time) and improve the user experience by preventing empty workflows from being saved.Apply this diff to add prop-level validation:
query: { type: "string", label: "Search Query", description: "Search query keywords and optional field filters. [See the Spotify docs for query syntax](https://developer.spotify.com/documentation/web-api/reference/search)", + required: true, },Note: You can keep the runtime trim validation at line 106 for whitespace-only inputs even with
required: true.
129-143: LGTM! Response aggregation and summary logic work correctly.The use of
flatMapwithlodash/getelegantly aggregates items across all requested types, and the summary message provides useful feedback. The typeLabel logic correctly handles singular/plural forms.The typeLabel logic could be slightly simplified for readability:
- const typeLabel = types.length > 1 - ? "items" - : types[0] + (allItems.length !== 1 - ? "s" - : ""); + const typeLabel = types.length === 1 + ? `${types[0]}${allItems.length === 1 ? "" : "s"}` + : "items";This flips the condition to check for the simpler case first and uses a template literal, but both versions are functionally equivalent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/spotify/actions/search/search.mjs(1 hunks)components/spotify/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/spotify/package.json
🔇 Additional comments (5)
components/spotify/actions/search/search.mjs (5)
1-10: LGTM! All imports are used and appropriate.The imports are clean and all utilized within the action. Using
lodash/getfor safe nested property access in the response aggregation is a solid choice.
12-22: LGTM! Metadata follows Pipedream conventions.The component key follows the required format, the version is appropriate for a new component, and the annotations accurately reflect the search action's characteristics (non-destructive, external API call, read-only).
30-63: LGTM! Well-structured type selection with sensible defaults.The prop correctly uses a
string[]type with all six Spotify search types represented. The default value of[ITEM_TYPES.TRACK]ensures users always have at least one type selected, which works well with the validation at lines 111-113.
64-94: LGTM! Props follow best practices.Good use of
propDefinitionto reuse common props from the app file. The limit max of 50 and default of 20 align with Spotify API constraints. TheincludeExternalprop correctly supports the "audio" option per the Spotify API specification.
96-127: LGTM! Solid validation and API request structure.The validation logic correctly guards against empty queries (including whitespace-only) and empty type arrays, preventing both poor UX and API errors. The API call is well-structured with appropriate parameters, and the 15-second timeout is sensible for an external search API.
3 participants
WHY
The main impetus for this is to provide an action LLMs can call before calling other Spotify actions, since they need to get proper IDs to pass in. I imagine it's handy for other things too :p
Summary by CodeRabbit
New Features
Chores