Skip to content
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

[Microsoft integration] getFullMessageList #9544

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

guillim
Copy link
Contributor

@guillim guillim commented Jan 10, 2025

Creation of the GmailGetMessageListService
Implementation of the driver to MS Graph API getFullMessageList

@guillim guillim self-assigned this Jan 10, 2025
Copy link

github-actions bot commented Jan 10, 2025

TODOs/FIXMEs:

  • // TODO: Placeholder: packages/twenty-server/src/modules/messaging/message-import-manager/services/messaging-get-message-list.service.ts

Generated by 🚫 dangerJS against 7284e96

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Implemented Microsoft Graph API integration for message fetching, adding support for retrieving email messages from Microsoft 365 alongside the existing Gmail integration.

  • Fixed incorrect MESSAGING_MICROSOFT_USERS_MESSAGES_LIST_MAX_RESULT value in /packages/twenty-server/src/modules/messaging/message-import-manager/drivers/microsoft/services/microsoft-get-message-list.service.ts (set to 1 instead of 1000)
  • Missing error handling for Microsoft Graph API calls in microsoft-get-message-list.service.ts
  • Using beta API version instead of stable in Microsoft Graph API calls
  • Incomplete getPartialMessageList implementation for Microsoft provider returns empty values
  • Incorrect redirect URIs in documentation referencing Google endpoints instead of Microsoft ones

7 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +34 to +40
const response: PageCollection = await microsoftClient
.api(syncCursor || '/me/mailfolders/inbox/messages/delta?$select=id')
.version('beta')
.headers({
Prefer: `odata.maxpagesize=${MESSAGING_MICROSOFT_USERS_MESSAGES_LIST_MAX_RESULT}`,
})
.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: No try-catch block around API call that could fail due to network issues or invalid tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only interesting thing we might talk out loud

Copy link
Contributor Author

@guillim guillim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Successfully merging this pull request may close these issues.

2 participants