-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Microsoft Teams - update trigger pagination #18703
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 GitHub. 2 Skipped Deployments
|
WalkthroughUpdates Microsoft Teams component package version and dependency. In the shared base pagination helper, adds early loop termination when encountering a non-new resource. Individual Microsoft Teams source files bump their internal version metadata with no logic changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Trigger Source
participant Base Helper (getNewPaginatedResources)
participant MS Graph API
Trigger Source->>Base Helper (getNewPaginatedResources): start with last checkpoint
loop Paginate pages
Base Helper->>MS Graph API: fetch next page of resources
MS Graph API-->>Base Helper: page results (ordered)
alt resource is new
Base Helper->>Base Helper: collect resource
else resource is not new
Note over Base Helper: New behavior: break out early
break Stop pagination
Base Helper-->>Trigger Source: return collected new resources
end
end
end
Base Helper-->>Trigger Source: return collected new resources
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/microsoft_teams/sources/common/base.mjs (1)
28-41: Enforce explicit descending ordering on all Graph API list endpoints
Add an$orderbyparameter to each paginated request to guarantee newest-first sorting before breaking early. For example:
- Messages:
$orderby=lastModifiedDateTime desc- Chats and channels:
$orderby=createdDateTime desc(orlastModifiedDateTime)- Teams, channels list, team members: if supported, use
$orderby=createdDateTime desc; otherwise remove the earlybreakand filter post-fetch.
This ensuresgetNewPaginatedResourcesnever skips newer items when the API’s default order is unspecified.
🧹 Nitpick comments (1)
components/microsoft_teams/sources/new-team/new-team.mjs (1)
20-26: Consider using resource creation timestamp for metadata.The
generateMetamethod usesDate.now()for the timestamp (line 24), which doesn't reflect the actual creation time of the team resource. If the team resource has acreatedDateTimefield, consider using it instead for more accurate metadata, similar to how other sources likenew-channelandnew-chathandle this.This is a pre-existing issue, not introduced by this PR.
generateMeta(team) { return { id: team.id, summary: team.displayName, - ts: Date.now(), + ts: team.createdDateTime ? Date.parse(team.createdDateTime) : Date.now(), }; },
📜 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/microsoft_teams/package.json(2 hunks)components/microsoft_teams/sources/common/base.mjs(1 hunks)components/microsoft_teams/sources/new-channel-message/new-channel-message.mjs(1 hunks)components/microsoft_teams/sources/new-channel/new-channel.mjs(1 hunks)components/microsoft_teams/sources/new-chat-message/new-chat-message.mjs(1 hunks)components/microsoft_teams/sources/new-chat/new-chat.mjs(1 hunks)components/microsoft_teams/sources/new-team-member/new-team-member.mjs(1 hunks)components/microsoft_teams/sources/new-team/new-team.mjs(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: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (7)
components/microsoft_teams/sources/new-channel-message/new-channel-message.mjs (1)
8-8: Version bump looks good.The version update aligns with the package-level version bump and the pagination optimization in base.mjs.
components/microsoft_teams/sources/new-team/new-team.mjs (1)
8-8: Version bump looks good.The version update is consistent with the package-level changes.
components/microsoft_teams/sources/new-channel/new-channel.mjs (1)
8-8: Version bump looks good.The version update aligns with the package-level version bump and the pagination optimization in base.mjs.
components/microsoft_teams/sources/new-chat/new-chat.mjs (1)
8-8: Version bump looks good.The version update aligns with the package-level version bump and the pagination optimization in base.mjs.
components/microsoft_teams/sources/new-chat-message/new-chat-message.mjs (1)
8-8: Version bump looks good.The version update aligns with the package-level version bump and the pagination optimization in base.mjs.
components/microsoft_teams/package.json (1)
18-18: Verify compatibility with @pipedream/platform 3.x
The jump from ^1.x to ^3.x includes breaking changes. Confirm that your Microsoft Teams components:
- Initialize and call the SDK via new PipedreamClient() and use namespaced methods (e.g. client.apps.list())
- Handle pagination with getNextPage()/hasNextPage() instead of raw page_info
- Use the props-based component model and the new HTTP/auth helpers ($ context or axios wrapper)
- Align TypeScript types with the v3 SDK surface
Refer to Pipedream’s SDK Migration Guide (v1→v2+) and the “Migrating from Legacy Actions to Component Actions” docs to update your code accordingly.
components/microsoft_teams/sources/new-team-member/new-team-member.mjs (1)
22-29: Verify pagination behavior for team members:listTeamMembersreturns conversationMember objects without a creation timestamp, sogetNewPaginatedResourceswill paginate through all members on every run—confirm this is intended or switch to the/teams/{teamId}/members/deltaendpoint for incremental sync.
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.
LGTM!
|
Thank you for working on this! I also want to point out the related #18632 if you haven't already seen it |
Resolves #18634
Summary by CodeRabbit