Hubspot - bug fixes to actions that list resources #18432
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughVersion numbers were incremented across HubSpot actions and package.json. In four actions, the internal accumulator variable was renamed (results → posts/campaigns/emails/events), and corresponding summary strings and return values now reference the new arrays. No method signatures or control flow changed; functionality remains to fetch paginated items. Changes
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
🧹 Nitpick comments (6)
components/hubspot/actions/list-marketing-events/list-marketing-events.mjs (1)
21-23: Use dynamiclimittied tomaxResults.Avoid fetching 100 items per page when
maxResultsis smaller.Example change (outside the changed hunk):
const params = { limit: Math.min(100, this.maxResults), };components/hubspot/actions/list-marketing-emails/list-marketing-emails.mjs (2)
91-101: Setlimitto align withmaxResultsand reduce API calls.Add a per-page
limit(capped by HubSpot’s max) to minimize requests.Example change (outside the changed hunk):
const params = { createdAt: this.createdAt, createdAfter: this.createdAfter, createdBefore: this.createdBefore, updatedAt: this.updatedAt, updatedAfter: this.updatedAfter, updatedBefore: this.updatedBefore, includeStats: this.includeStats, archived: this.archived, sort: this.sort, limit: Math.min(100, this.maxResults), };
103-123: Minor pagination nit: avoid settingafterwhen there’s no next page.Harmless, but you can guard the assignment for readability.
- hasMore = paging?.next.after; - params.after = paging?.next.after; + hasMore = paging?.next?.after; + if (hasMore) params.after = hasMore;components/hubspot/actions/list-blog-posts/list-blog-posts.mjs (2)
81-82: Rename is correct; fix summary wording (“post” vs “page”).The accumulator/return change is good. The summary string should say “post(s)”.
Apply:
- `Found ${posts.length} page${posts.length === 1 + `Found ${posts.length} post${posts.length === 1 ? "" : "s"}`,Also applies to: 107-108, 119-124
85-95: Addlimitto pagination params (efficiency).Same rationale as other actions: fewer requests when
maxResultsis small.Example change (outside the changed hunk):
const params = { createdAt: this.createdAt, createdAfter: this.createdAfter, createdBefore: this.createdBefore, updatedAt: this.updatedAt, updatedAfter: this.updatedAfter, updatedBefore: this.updatedBefore, archived: this.archived, sort: this.sort, limit: Math.min(100, this.maxResults), };components/hubspot/actions/list-campaigns/list-campaigns.mjs (1)
39-41: Add per-pagelimit(cap 100) to HubSpot campaigns list paramsSet per-page limit to Math.min(100, this.maxResults) to avoid extra requests when maxResults is small — HubSpot supports ?limit=1–100 (default 50, max 100).
File: components/hubspot/actions/list-campaigns/list-campaigns.mjs (around lines 39–41)
const params = { sort: this.sort, limit: Math.min(100, this.maxResults), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/hubspot/actions/list-blog-posts/list-blog-posts.mjs(4 hunks)components/hubspot/actions/list-campaigns/list-campaigns.mjs(4 hunks)components/hubspot/actions/list-marketing-emails/list-marketing-emails.mjs(4 hunks)components/hubspot/actions/list-marketing-events/list-marketing-events.mjs(4 hunks)components/hubspot/package.json(1 hunks)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/hubspot/package.json (1)
3-3: Version bump aligns with action updates.Patch version increment to 1.7.7 looks appropriate for a bug fix release linked to #18423.
Please confirm a release note/changelog entry calls out the fix for “List Marketing Emails returns empty array”.
components/hubspot/actions/list-campaigns/list-campaigns.mjs (1)
35-36: Accumulator rename + return fix look good.Using
campaignsconsistently and returning it should prevent empty outputs from variable mismatches.Also applies to: 54-55, 66-71
components/hubspot/actions/list-marketing-events/list-marketing-events.mjs (1)
20-20: Good: consistenteventsaccumulator and return.Prevents the empty-array issue from mismatched identifiers.
Also applies to: 38-39, 50-55
components/hubspot/actions/list-marketing-emails/list-marketing-emails.mjs (1)
87-88: This likely fixes the “always empty” bug.Collecting into
emailsand returningemailsresolves the prior accumulator/return mismatch.Please run a quick smoke test on an account with known emails to confirm non-empty results and correct pagination behavior.
Also applies to: 114-115, 126-131
Resolves #18423
Summary by CodeRabbit
Chores
Refactor
End-user impact: No functional changes; actions continue to list items with pagination as before.