feat: remove Revert.dev dependency from Pipedrive integration #22492
Conversation
- Replace Revert API calls with direct Pipedrive OAuth2 and REST API - Update OAuth flow to use Pipedrive's authorization endpoint - Implement token exchange in callback handler - Map Revert's unified API format to Pipedrive's native API structure - Remove REVERT_* environment variables from configuration - Update app config to reflect direct Pipedrive integration - Follow patterns from other CRM integrations like HubSpot Breaking change: Requires PIPEDRIVE_CLIENT_ID and PIPEDRIVE_CLIENT_SECRET environment variables instead of REVERT_* variables Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
✅ No security or compliance issues detected. Reviewed everything up to 9b9d86b. Security Overview
Detected Code Changes
Reply to this PR with |
- Add token refresh implementation following Pipedrive OAuth2 specification - Use proper Basic Auth for token refresh requests - Handle token refresh errors with appropriate logging - Update access token and expiry date after successful refresh - Add note about database credential update requirement Co-Authored-By: [email protected] <[email protected]>
- Import updateTokenObjectInDb utility for proper credential management - Store credentialId in constructor for database updates - Update token refresh logic to persist refreshed tokens to database - Follow Cal.com patterns for OAuth credential management - Ensure tokens are properly updated after successful refresh Co-Authored-By: [email protected] <[email protected]>
…drive integration
Originally by: anikdhabal
# Remove Revert.dev dependency from Pipedrive integration
## Summary
This PR removes the dependency on Revert.dev from the Pipedrive CRM integration and implements direct OAuth2 authentication with Pipedrive's native API. The changes follow the same patterns used by other CRM integrations in the codebase (particularly HubSpot).
**Key Changes:**
- **OAuth Flow**: Replaced Revert redirect with direct Pipedrive OAuth2 authorization
- **API Integration**: Converted all API calls from Revert's unified format to Pipedrive's native REST API
- **Token Management**: Implemented OAuth token storage and basic refresh logic
- **Environment Variables**: Removed `REVERT_*` variables, now requires `PIPEDRIVE_CLIENT_ID` and `PIPEDRIVE_CLIENT_SECRET`
- **Data Mapping**: Mapped contact and activity data between Revert's unified format and Pipedrive's native structure
**Breaking Change**: Existing Pipedrive integrations will need to be reconfigured with new OAuth credentials.
## Review & Testing Checklist for Human
- [ ] **Test OAuth flow end-to-end** - Create a Pipedrive OAuth app and verify the complete authorization flow works
- [ ] **Test contact creation and search** - Verify contacts can be created and searched properly with real data
- [ ] **Test activity/meeting management** - Verify activities can be created, updated, and deleted correctly
- [ ] **Verify token refresh logic** - The current implementation is incomplete and may need proper refresh token handling
- [ ] **Test error handling** - Verify proper error responses for failed API calls and invalid tokens
**Recommended Test Plan:**
1. Set up a Pipedrive developer account and create an OAuth app
2. Configure `PIPEDRIVE_CLIENT_ID` and `PIPEDRIVE_CLIENT_SECRET` in environment
3. Test the integration setup flow in Cal.com
4. Create a test booking and verify it creates contacts and activities in Pipedrive
5. Test updating and canceling bookings
---
### Diagram
```mermaid
%%{ init : { "theme" : "default" }}%%
graph TB
subgraph "OAuth Flow"
A["api/add.ts<br/>OAuth Authorization"]:::major-edit
B["api/callback.ts<br/>Token Exchange"]:::major-edit
end
subgraph "CRM Integration"
C["lib/CrmService.ts<br/>Pipedrive API Client"]:::major-edit
D["config.json<br/>App Configuration"]:::minor-edit
end
subgraph "Configuration"
E["turbo.json<br/>Environment Variables"]:::minor-edit
F[".env.appStore.example<br/>Environment Template"]:::minor-edit
end
subgraph "External APIs"
G["Pipedrive OAuth API<br/>oauth.pipedrive.com"]:::context
H["Pipedrive REST API<br/>api.pipedrive.com"]:::context
end
A --> B
B --> C
C --> H
A --> G
B --> G
C --> D
subgraph Legend
L1[Major Edit]:::major-edit
L2[Minor Edit]:::minor-edit
L3[Context/No Edit]:::context
end
classDef major-edit fill:#90EE90
classDef minor-edit fill:#ADD8E6
classDef context fill:#FFFFFF
```
### Notes
- **Token Refresh**: The current `getValidAccessToken()` implementation is incomplete - it doesn't actually refresh expired tokens, just returns the existing one. This should be addressed before production use.
- **OAuth Scopes**: Using comprehensive scopes: `deals:read,deals:write,persons:read,persons:write,activities:read,activities:write`
- **API Mapping**: Activities are used for meetings/events instead of a separate events API, following Pipedrive's data model
- **Error Handling**: Basic error handling is implemented but may need enhancement for production edge cases
**Session Info**:
- Link to Devin run: https://app.devin.ai/sessions/ca8c7c0a8b9b4b87ae92d5db2d23039a
- Requested by: @anikdhabal
|
This PR is being marked as stale due to inactivity. |
github-actions
bot
added
$50
community
hariombalhara
left a comment
hariombalhara
left a comment
There was a problem hiding this comment.
@anikdhabal We have OAuthManager that we use for Google Calendar and Microsoft Office. I believe zoom too.
Can you check if we can use that here.
The benefit with OAuthManager approach is that it automatically handles invalidation of credential , plus supports credential sync feature out of the box.
|
❌ Cannot revive Devin session - the session is too old. Please start a new session instead. |
- Refactor CrmService.ts to use OAuthManager instead of manual token refresh - Create getPipedriveAppKeys helper function for client credentials - Update callback.ts to use the new helper function - Add requestPipedrive helper method for all API calls - Implement proper token refresh callbacks (isTokenObjectUnusable, isAccessTokenUnusable) This addresses the review comments from hariom and cubic on PR #22492. Co-Authored-By: [email protected] <[email protected]>
Devin AI is completing this stale PRThis PR by @anikdhabal has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
…ipedrive-1752520608
Summary of Completed WorkI've reviewed and completed this PR to remove the Revert.dev dependency from the Pipedrive CRM integration. Here's what was done: Review Feedback Addressed
Key Changes
Verification
The PR is ready for review. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 7 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/pipedrive-crm/api/callback.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/api/callback.ts:48">
P2: Return a 400 error when the OAuth provider does not supply a `code` instead of sending an empty string to the token endpoint.</violation>
<violation number="2" location="packages/app-store/pipedrive-crm/api/callback.ts:53">
P1: Validate the token endpoint response before storing it—surface non-200 errors instead of treating them as valid Pipedrive tokens.</violation>
</file>
<file name="packages/app-store/pipedrive-crm/api/add.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/api/add.ts:36">
P1: The new OAuth redirect no longer includes a populated state payload (encodeOAuthState(req) yields undefined for fresh GETs), so the callback loses tenant context. Build and encode a state object (tenant/user identifiers) before redirecting.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
…tials Co-Authored-By: unknown <>
What does this PR do?
Removes the dependency on Revert.dev from the Pipedrive CRM integration and implements direct OAuth2 authentication with Pipedrive's native API. The changes follow the same patterns used by other CRM integrations in the codebase (particularly HubSpot and Zoom).
Summary
Key Changes:
OAuthManagerpattern (same as Zoom, Google Calendar, etc.)REVERT_*variables, now requiresPIPEDRIVE_CLIENT_IDandPIPEDRIVE_CLIENT_SECRETBreaking Change: Existing Pipedrive integrations will need to be reconfigured with new OAuth credentials.
Updates since last revision
CrmService.tsto use Cal.com'sOAuthManagerfor proper token managementgetPipedriveAppKeyshelper function for client credentials extraction with Zod validationrequestPipedrivehelper method that usesOAuthManager.request()for all API callsisTokenObjectUnusable,isAccessTokenUnusable) to detect Pipedrive error responsesDiagram
%%{ init : { "theme" : "default" }}%% graph TB subgraph "OAuth Flow" A["api/add.ts<br/>OAuth Authorization"]:::major-edit B["api/callback.ts<br/>Token Exchange"]:::major-edit end subgraph "CRM Integration" C["lib/CrmService.ts<br/>Pipedrive API Client"]:::major-edit D["lib/getPipedriveAppKeys.ts<br/>Credentials Helper"]:::major-edit E["config.json<br/>App Configuration"]:::minor-edit end subgraph "External APIs" F["Pipedrive OAuth API<br/>oauth.pipedrive.com"]:::context G["Pipedrive REST API<br/>api.pipedrive.com"]:::context end A --> B B --> C C --> G A --> F B --> F C --> D classDef major-edit fill:#90EE90 classDef minor-edit fill:#ADD8E6 classDef context fill:#FFFFFFMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Environment Variables Required:
PIPEDRIVE_CLIENT_ID- OAuth client ID from Pipedrive developer portalPIPEDRIVE_CLIENT_SECRET- OAuth client secret from Pipedrive developer portalTest Plan:
Review Checklist for Human:
isTokenObjectUnusablecallback detects "invalid_grant" errorsisAccessTokenUnusablecallback detects 401 responsesapi_domainis correctly preserved and updated in token objectNotes
deals:read,deals:write,persons:read,persons:write,activities:read,activities:writeexpiry_datein millisecondsSession Info: