feat: remove Revert.dev dependency from Pipedrive integration #22492
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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: anik@cal.com <adhabal2002@gmail.com>
Contributor
🤖 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:
|
Contributor
|
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: anik@cal.com <adhabal2002@gmail.com>
- 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: anik@cal.com <adhabal2002@gmail.com>
dakshgup
added a commit
to dakshgup/cal.com
that referenced
this pull request
Jul 14, 2025
…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
Contributor
|
This PR is being marked as stale due to inactivity. |
3 tasks
github-actions
bot
added
$50
community
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/app-store/pipedrive-crm/lib/CrmService.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/lib/CrmService.ts:90">
After refreshing the token we still keep using the old refresh token in memory, so any rotated refresh_token will make the next refresh attempt fail. Please update the instance field when you persist the new token.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| const refreshedToken = await tokenResponse.json(); | ||
|
|
||
| this.accessToken = refreshedToken.access_token; |
Contributor
There was a problem hiding this comment.
After refreshing the token we still keep using the old refresh token in memory, so any rotated refresh_token will make the next refresh attempt fail. Please update the instance field when you persist the new token.
Prompt for AI agents
Address the following comment on packages/app-store/pipedrive-crm/lib/CrmService.ts at line 90:
<comment>After refreshing the token we still keep using the old refresh token in memory, so any rotated refresh_token will make the next refresh attempt fail. Please update the instance field when you persist the new token.</comment>
<file context>
@@ -9,187 +9,287 @@ import type {
+
+ const refreshedToken = await tokenResponse.json();
+
+ this.accessToken = refreshedToken.access_token;
+ this.expiryDate = Math.round(Date.now() + refreshedToken.expires_in * 1000);
+
</file context>
✅ Addressed in c27c033
Member
|
@anikdhabal Do we have a loom showing the e2e working after this change ? |
Comment on lines
35
to
41
| let clientId = ""; | ||
| let clientSecret = ""; | ||
| const appKeys = await getAppKeysFromSlug(appConfig.slug); | ||
| if (typeof appKeys.client_id === "string") clientId = appKeys.client_id; | ||
| if (typeof appKeys.client_secret === "string") clientSecret = appKeys.client_secret; | ||
| if (!clientId) return res.status(400).json({ message: "Pipedrive client id missing." }); | ||
| if (!clientSecret) return res.status(400).json({ message: "Pipedrive client secret missing." }); |
Member
There was a problem hiding this comment.
Better to abstract it out in a separate fn
Comment on lines
60
to
68
| const appKeys = await getAppKeysFromSlug(appConfig.slug); | ||
| let clientId = ""; | ||
| let clientSecret = ""; | ||
| if (typeof appKeys.client_id === "string") clientId = appKeys.client_id; | ||
| if (typeof appKeys.client_secret === "string") clientSecret = appKeys.client_secret; | ||
|
|
||
| if (!clientId || !clientSecret) { | ||
| throw new Error("Pipedrive client credentials missing for token refresh"); | ||
| } |
Member
There was a problem hiding this comment.
Above abstracted fn could be reused here.
hariombalhara
requested changes
Oct 28, 2025
Member
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.
Contributor
|
❌ 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: anik@cal.com <adhabal2002@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Refactored
CrmService.tsto use Cal.com'sOAuthManagerfor proper token management:getPipedriveAppKeyshelper function for client credentials extractionrequestPipedrivehelper 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: