Skip to content

Conversation

@michelle0927
Copy link
Collaborator

@michelle0927 michelle0927 commented Nov 6, 2025

Resolves #18965

Summary by CodeRabbit

  • Chores

    • Updated package version and dependencies
  • Changes

    • Clarified the New Call event source description
    • Adjusted the initial data lookback window for the New Call source to provide more focused event retrieval

@vercel
Copy link

vercel bot commented Nov 6, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
pipedream-docs Ignored Ignored Nov 6, 2025 9:01pm
pipedream-docs-redirect-do-not-edit Ignored Ignored Nov 6, 2025 9:01pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

This change addresses indefinite pagination in the Gong "New Call" trigger by introducing a deployment-time hook that limits initial event retrieval to 25 items, narrowing the data lookback window from three months to one day, and updating related dependencies.

Changes

Cohort / File(s) Summary
Package & Dependencies
components/gong/package.json
Version bumped to 0.3.2; @pipedream/platform dependency updated from ^3.0.3 to ^3.1.0
Polling Mechanism
components/gong/sources/common/polling.mjs
processResources() method signature updated to accept optional max parameter for limiting results; new hooks.deploy implementation added to fetch resources with hard-coded max of 25 events
New Call Source
components/gong/sources/new-call/new-call.mjs
Version bumped to 0.0.4; description updated; initial data retrieval window narrowed from three months to one day using oneDayAgo instead of threeMonthsAgo; getResourceFnArgs and default fromDateTime parameter adjusted accordingly

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Deploy as Deploy Hook
    participant Run as Run Flow
    participant API as Gong API

    rect rgb(240, 248, 255)
    Note over User,API: Initial Deployment
    User->>Deploy: Trigger source deployment
    Deploy->>API: Fetch resources (paginated)
    API-->>Deploy: Return resources
    Deploy->>Deploy: processResources(resources, max=25)
    Deploy->>Deploy: Slice to 25 items max
    Deploy-->>User: Emit 25 events
    end

    rect rgb(240, 248, 255)
    Note over User,API: Subsequent Runs
    User->>Run: Trigger on schedule
    Run->>API: Fetch resources (filtered by oneDayAgo)
    API-->>Run: Return recent resources
    Run->>Run: processResources(resources)
    Run-->>User: Emit new events
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Method signature change: The optional max parameter in processResources() requires verification that all call sites are compatible and that the slicing logic correctly handles edge cases (empty arrays, max larger than array length)
  • New deployment hook: Confirm that hooks.deploy executes at the correct lifecycle stage and properly interacts with the existing polling mechanism
  • Time window reduction: Validate that changing from three-month to one-day lookback in getResourceFnArgs won't cause missed events in production and that timestamp filtering logic in subsequent runs is robust

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is minimal and only contains a reference to issue #18965, lacking substantive explanation of changes or adherence to the 'WHY' template section. Expand the description to explain why pagination changes were needed and summarize what changes were made to fix the indefinite pagination issue.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Gong "New Call" trigger update pagination' clearly summarizes the main change - updating the pagination behavior of the Gong New Call trigger.
Linked Issues check ✅ Passed The PR successfully addresses all requirements from #18965: deployment is limited to 25 events via the new deploy hook with max=25, the one-day lookback replaces the three-month window for better timestamp filtering, and version bump reflects the update.
Out of Scope Changes check ✅ Passed All changes in the PR are directly related to fixing the indefinite pagination issue in the Gong new-call trigger; there are no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-18965

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aebf7b0 and 8e93fa6.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • components/gong/package.json (2 hunks)
  • components/gong/sources/common/polling.mjs (2 hunks)
  • components/gong/sources/new-call/new-call.mjs (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-23T03:55:15.166Z
Learnt from: GTFalcao
Repo: PipedreamHQ/pipedream PR: 15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.

Applied to files:

  • components/gong/sources/new-call/new-call.mjs
🧬 Code graph analysis (1)
components/gong/sources/common/polling.mjs (1)
components/zep/actions/get-threads/get-threads.mjs (1)
  • max (39-39)
⏰ 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
🔇 Additional comments (5)
components/gong/package.json (1)

3-3: LGTM: Version and dependency updates are appropriate.

The version bump and platform dependency update align with the feature changes in this PR.

Also applies to: 16-16

components/gong/sources/common/polling.mjs (1)

42-58: LGTM: processResources correctly limits emission while maintaining timestamp tracking.

The logic properly reverses resources to descending order, slices to the newest N events when max is provided, and updates lastCreatedAt to the newest emitted resource. This ensures subsequent runs fetch only newer events.

components/gong/sources/new-call/new-call.mjs (3)

7-7: LGTM: Description is clear and specific.

The updated description explicitly states the trigger behavior and includes relevant API documentation.


9-9: LGTM: Version bump is appropriate.

Patch version increment is correct for this bug fix.


20-24: Key fix: Lookback window reduced from 3 months to 1 day.

This change directly addresses the indefinite pagination issue reported in #18965. By reducing the default lookback window from 3 months to 1 day, the initial deployment will fetch far fewer historical calls, preventing the emission of 1000+ events. Combined with the deploy hook's 25-event emission limit, this ensures a controlled initial deployment.

The 1-day window strikes a good balance between avoiding excessive historical data and capturing recent events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Gong "New Call" trigger indefinite pagination

2 participants