feat(pipedrive-crm): remove Revert.dev and add native Pipedrive OAuth2 (fixes #16797, #22492)#26450
Closed
Karrrthik7 wants to merge 1 commit intocalcom:mainfrom
Closed
feat(pipedrive-crm): remove Revert.dev and add native Pipedrive OAuth2 (fixes #16797, #22492)#26450Karrrthik7 wants to merge 1 commit intocalcom:mainfrom
Karrrthik7 wants to merge 1 commit intocalcom:mainfrom
Conversation
|
@Karrrthik7 is attempting to deploy a commit to the cal-staging Team on Vercel. A member of the Team first needs to authorize it. |
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.
5 issues found across 5 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/add.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/api/add.ts:37">
P1: Missing `response_type: "code"` parameter in OAuth authorization URL. This is a required parameter for the OAuth2 authorization code flow and without it, Pipedrive's OAuth authorization will likely fail. See `zoho-bigin/api/add.ts` for the correct pattern.</violation>
</file>
<file name="packages/app-store/pipedrive-crm/components/EventTypeAppCardInterface.tsx">
<violation number="1" location="packages/app-store/pipedrive-crm/components/EventTypeAppCardInterface.tsx:22">
P2: Missing `res.ok` check before parsing JSON. If the API returns an error status (4xx/5xx), the response may not contain `url` and the error won't be surfaced to the user.</violation>
</file>
<file name="packages/app-store/pipedrive-crm/lib/CrmService.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/lib/CrmService.ts:78">
P1: Token refresh failure is silently swallowed. If `refreshAccessToken` fails, `getToken()` returns without a valid token, causing subsequent API calls to fail with expired credentials. Consider re-throwing the error after logging, or validating the token is valid after refresh attempt.</violation>
<violation number="2" location="packages/app-store/pipedrive-crm/lib/CrmService.ts:189">
P2: Unlike other API methods (`createPipedriveEvent`, `updateMeeting`), `deleteMeeting` doesn't check `res.ok` or throw on failure. Delete failures will be silently ignored.</violation>
</file>
<file name="packages/app-store/pipedrive-crm/api/callback.ts">
<violation number="1" location="packages/app-store/pipedrive-crm/api/callback.ts:24">
P1: Missing validation for absent `code` parameter. The current check `code && typeof code !== "string"` passes when `code` is `undefined`, allowing the flow to continue and cast `undefined as string` on line 48. This will cause a confusing 500 error from Pipedrive instead of a clear 400 from your handler. Per early-return preference, validate that code exists.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+37
to
40
| const params = new URLSearchParams({ | ||
| client_id: String(appKeys.client_id), | ||
| redirect_uri: redirectUri, | ||
| }); |
Contributor
There was a problem hiding this comment.
P1: Missing response_type: "code" parameter in OAuth authorization URL. This is a required parameter for the OAuth2 authorization code flow and without it, Pipedrive's OAuth authorization will likely fail. See zoho-bigin/api/add.ts for the correct pattern.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/pipedrive-crm/api/add.ts, line 37:
<comment>Missing `response_type: "code"` parameter in OAuth authorization URL. This is a required parameter for the OAuth2 authorization code flow and without it, Pipedrive's OAuth authorization will likely fail. See `zoho-bigin/api/add.ts` for the correct pattern.</comment>
<file context>
@@ -30,8 +32,13 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse)
- newTab: true,
+ const redirectUri = `${WEBAPP_URL_FOR_OAUTH}/api/integrations/pipedrive/callback`;
+ const state = encodeOAuthState(req);
+ const params = new URLSearchParams({
+ client_id: String(appKeys.client_id),
+ redirect_uri: redirectUri,
</file context>
Suggested change
| const params = new URLSearchParams({ | |
| client_id: String(appKeys.client_id), | |
| redirect_uri: redirectUri, | |
| }); | |
| const params = new URLSearchParams({ | |
| client_id: String(appKeys.client_id), | |
| redirect_uri: redirectUri, | |
| response_type: "code", | |
| }); |
| setLoading(true); | ||
| const teamId = eventType.team?.id; | ||
| const q = teamId ? `?teamId=${teamId}` : ""; | ||
| const res = await fetch(`/api/integrations/pipedrive/add${q}`); |
Contributor
There was a problem hiding this comment.
P2: Missing res.ok check before parsing JSON. If the API returns an error status (4xx/5xx), the response may not contain url and the error won't be surfaced to the user.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/pipedrive-crm/components/EventTypeAppCardInterface.tsx, line 22:
<comment>Missing `res.ok` check before parsing JSON. If the API returns an error status (4xx/5xx), the response may not contain `url` and the error won't be surfaced to the user.</comment>
<file context>
@@ -4,24 +4,55 @@ import AppCard from "@calcom/app-store/_components/AppCard";
+ setLoading(true);
+ const teamId = eventType.team?.id;
+ const q = teamId ? `?teamId=${teamId}` : "";
+ const res = await fetch(`/api/integrations/pipedrive/add${q}`);
+ const json = await res.json();
+ if (json?.url) {
</file context>
Suggested change
| const res = await fetch(`/api/integrations/pipedrive/add${q}`); | |
| const res = await fetch(`/api/integrations/pipedrive/add${q}`); | |
| if (!res.ok) { | |
| throw new Error(`Failed to initiate Pipedrive connection: ${res.status}`); | |
| } |
Comment on lines
+189
to
+192
| await fetch(`https://api.pipedrive.com/v1/activities/${uid}`, { | ||
| method: "DELETE", | ||
| headers: headers, | ||
| }; | ||
|
|
||
| return await fetch(`${this.revertApiUrl}crm/events/${uid}`, requestOptions); | ||
| headers: { Authorization: `Bearer ${currentToken.access_token}` }, | ||
| }); |
Contributor
There was a problem hiding this comment.
P2: Unlike other API methods (createPipedriveEvent, updateMeeting), deleteMeeting doesn't check res.ok or throw on failure. Delete failures will be silently ignored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/pipedrive-crm/lib/CrmService.ts, line 189:
<comment>Unlike other API methods (`createPipedriveEvent`, `updateMeeting`), `deleteMeeting` doesn't check `res.ok` or throw on failure. Delete failures will be silently ignored.</comment>
<file context>
@@ -10,180 +10,195 @@ import type { CredentialPayload } from "@calcom/types/Credential";
- const requestOptions = {
+ await (await this.auth).getToken();
+ const currentToken = this.credential.key as unknown as PipedriveToken;
+ await fetch(`https://api.pipedrive.com/v1/activities/${uid}`, {
method: "DELETE",
- headers: headers,
</file context>
Suggested change
| await fetch(`https://api.pipedrive.com/v1/activities/${uid}`, { | |
| method: "DELETE", | |
| headers: headers, | |
| }; | |
| return await fetch(`${this.revertApiUrl}crm/events/${uid}`, requestOptions); | |
| headers: { Authorization: `Bearer ${currentToken.access_token}` }, | |
| }); | |
| const res = await fetch(`https://api.pipedrive.com/v1/activities/${uid}`, { | |
| method: "DELETE", | |
| headers: { Authorization: `Bearer ${currentToken.access_token}` }, | |
| }); | |
| if (!res.ok) throw new Error("Failed to delete activity in Pipedrive"); |
Comment on lines
+78
to
+79
| } catch (e: unknown) { | ||
| this.log.error(e); |
Contributor
There was a problem hiding this comment.
P1: Token refresh failure is silently swallowed. If refreshAccessToken fails, getToken() returns without a valid token, causing subsequent API calls to fail with expired credentials. Consider re-throwing the error after logging, or validating the token is valid after refresh attempt.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/pipedrive-crm/lib/CrmService.ts, line 78:
<comment>Token refresh failure is silently swallowed. If `refreshAccessToken` fails, `getToken()` returns without a valid token, causing subsequent API calls to fail with expired credentials. Consider re-throwing the error after logging, or validating the token is valid after refresh attempt.</comment>
<file context>
@@ -10,180 +10,195 @@ import type { CredentialPayload } from "@calcom/types/Credential";
+ // persist updated token in this instance for subsequent API calls
+ this.credential.key = pipedriveRefresh as any;
+ currentToken = { ...currentToken, ...pipedriveRefresh };
+ } catch (e: unknown) {
+ this.log.error(e);
}
</file context>
Suggested change
| } catch (e: unknown) { | |
| this.log.error(e); | |
| } catch (e: unknown) { | |
| this.log.error(e); | |
| throw e; |
| const { code } = req.query; | ||
| const state = decodeOAuthState(req); | ||
|
|
||
| if (code && typeof code !== "string") { |
Contributor
There was a problem hiding this comment.
P1: Missing validation for absent code parameter. The current check code && typeof code !== "string" passes when code is undefined, allowing the flow to continue and cast undefined as string on line 48. This will cause a confusing 500 error from Pipedrive instead of a clear 400 from your handler. Per early-return preference, validate that code exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/pipedrive-crm/api/callback.ts, line 24:
<comment>Missing validation for absent `code` parameter. The current check `code && typeof code !== "string"` passes when `code` is `undefined`, allowing the flow to continue and cast `undefined as string` on line 48. This will cause a confusing 500 error from Pipedrive instead of a clear 400 from your handler. Per early-return preference, validate that code exists.</comment>
<file context>
@@ -1,19 +1,73 @@
+ const { code } = req.query;
+ const state = decodeOAuthState(req);
+
+ if (code && typeof code !== "string") {
+ res.status(400).json({ message: "`code` must be a string" });
+ return;
</file context>
Suggested change
| if (code && typeof code !== "string") { | |
| if (!code || typeof code !== "string") { |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
/claim #16797
What does this PR do?
Fixes:
Files changed (key)
Short description of OAuth flow (in-code comments included)
https://oauth.pipedrive.com/oauth/authorize?client_id=<app_key>&redirect_uri=<WEBAPP_URL>/api/integrations/pipedrive/callback&response_type=code&state=
GET /api/integrations/pipedrive/callback?code=...&state=...
Error handling
How to test locally
Environment setup
WEBAPP_URL_FOR_OAUTH=http://localhost:3000
Register Pipedrive OAuth app with redirect URI:
Run
Test steps
Expected behavior
Mandatory Tasks (DO NOT REMOVE)
Checklist
yarn type-check:ci --forceon changed filesNotes / Follow-ups