-
Notifications
You must be signed in to change notification settings - Fork 11.5k
Copy of PR #22492: feat: remove Revert.dev dependency from Pipedrive integration #22500
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
Copy of PR #22492: feat: remove Revert.dev dependency from Pipedrive integration #22500
Conversation
…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
|
@dakshgup is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update removes all references to the "REVERT" integration and its environment variables, transitioning the Pipedrive CRM integration to use direct OAuth authentication and API calls. The Pipedrive service logic, configuration, and API handlers are refactored for OAuth, and publisher metadata is updated. Unused variables and legacy comments are eliminated. Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/app-store/pipedrive-crm/api/callback.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/pipedrive-crm/api/add.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/app-store/pipedrive-crm/lib/CrmService.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/14/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/14/25)1 label was added to this PR based on Keith Williams's automation. |
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
🧹 Nitpick comments (2)
packages/app-store/pipedrive-crm/lib/CrmService.ts (2)
121-121: Remove redundant double negation.The double negation is unnecessary as the value will be coerced to boolean in the ternary operator.
-const [firstName, lastName] = !!attendee.name ? attendee.name.split(" ") : [attendee.email, ""]; +const [firstName, lastName] = attendee.name ? attendee.name.split(" ") : [attendee.email, ""];
337-343: Consider implementing placeholder methods.The unimplemented methods
getAppOptionsandhandleAttendeeNoShowuse console.log statements. Consider throwing a NotImplementedError or returning appropriate default values to better indicate these are intentionally not implemented.getAppOptions() { - console.log("No options implemented"); + // Intentionally not implemented for Pipedrive integration + return {}; } async handleAttendeeNoShow() { - console.log("Not implemented"); + // Intentionally not implemented for Pipedrive integration + return Promise.resolve(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.env.appStore.example(0 hunks)packages/app-store/pipedrive-crm/api/add.ts(2 hunks)packages/app-store/pipedrive-crm/api/callback.ts(1 hunks)packages/app-store/pipedrive-crm/config.json(1 hunks)packages/app-store/pipedrive-crm/lib/CrmService.ts(2 hunks)turbo.json(0 hunks)
💤 Files with no reviewable changes (2)
- turbo.json
- .env.appStore.example
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/app-store/pipedrive-crm/api/callback.ts (2)
packages/app-store/pipedrive-crm/api/add.ts (1)
handler(12-44)packages/lib/constants.ts (1)
WEBAPP_URL_FOR_OAUTH(22-22)
🪛 Biome (1.9.4)
packages/app-store/pipedrive-crm/lib/CrmService.ts
[error] 121-121: Avoid redundant double-negation.
It is not necessary to use double-negation when a value will already be coerced to a boolean.
Unsafe fix: Remove redundant double-negation
(lint/complexity/noExtraBooleanCast)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Security Check
🔇 Additional comments (4)
packages/app-store/pipedrive-crm/config.json (1)
7-12: LGTM! Metadata updates align with the migration.The configuration changes correctly update the publisher information and URLs to reflect the direct Pipedrive integration.
packages/app-store/pipedrive-crm/api/add.ts (1)
35-41: OAuth implementation looks secure and well-structured.The OAuth authorization URL is properly constructed with:
- Dynamic redirect URI using
WEBAPP_URL_FOR_OAUTH- Encoded state parameter for CSRF protection
- Comprehensive scopes for CRM operations
- Proper URL encoding of all parameters
packages/app-store/pipedrive-crm/lib/CrmService.ts (2)
54-115: Token refresh implementation is well-structured.The
getValidAccessTokenmethod correctly:
- Checks token expiry before refreshing
- Uses Basic Auth for refresh requests
- Updates the token in the database
- Has proper error handling and logging
209-222: Activity creation properly formats Pipedrive data.The implementation correctly:
- Calculates duration in minutes
- Formats date and time according to Pipedrive's API requirements
- Uses plain text for notes instead of HTML
- Associates the activity with the first contact
| const tokenResponse = await fetch("https://oauth.pipedrive.com/oauth/token", { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: authHeader, | ||
| "Content-Type": "application/x-www-form-urlencoded", | ||
| }, | ||
| body: new URLSearchParams({ | ||
| grant_type: "authorization_code", | ||
| code: code || "", | ||
| redirect_uri: redirectUri, | ||
| }), | ||
| }); | ||
|
|
||
| const pipedriveToken: PipedriveToken = await tokenResponse.json(); | ||
|
|
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.
Add error handling for token exchange response.
The token exchange request should check the response status before parsing JSON to handle failures gracefully.
Apply this fix:
const tokenResponse = await fetch("https://oauth.pipedrive.com/oauth/token", {
method: "POST",
headers: {
Authorization: authHeader,
"Content-Type": "application/x-www-form-urlencoded",
},
body: new URLSearchParams({
grant_type: "authorization_code",
code: code || "",
redirect_uri: redirectUri,
}),
});
+if (!tokenResponse.ok) {
+ const errorText = await tokenResponse.text();
+ return res.status(400).json({
+ message: `Failed to exchange authorization code: ${tokenResponse.status} ${errorText}`
+ });
+}
const pipedriveToken: PipedriveToken = await tokenResponse.json();🤖 Prompt for AI Agents
In packages/app-store/pipedrive-crm/api/callback.ts between lines 46 and 60, the
code fetches the token without checking if the response was successful. Add a
check for tokenResponse.ok after the fetch call; if false, handle the error by
throwing or returning an appropriate error message before parsing the JSON. This
ensures graceful handling of failed token exchange requests.
This is a copy of #22492
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:
REVERT_*variables, now requiresPIPEDRIVE_CLIENT_IDandPIPEDRIVE_CLIENT_SECRETBreaking Change: Existing Pipedrive integrations will need to be reconfigured with new OAuth credentials.
Review & Testing Checklist for Human
Recommended Test Plan:
PIPEDRIVE_CLIENT_IDandPIPEDRIVE_CLIENT_SECRETin environmentDiagram
%%{ 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:#FFFFFFNotes
getValidAccessToken()implementation is incomplete - it doesn't actually refresh expired tokens, just returns the existing one. This should be addressed before production use.deals:read,deals:write,persons:read,persons:write,activities:read,activities:writeSession Info:
Summary by CodeRabbit
New Features
Refactor
Chores