Skip to content

fix: Adds mandatory credentiaId in Destination Calendar API endpoint POST#11880

Merged
zomars merged 6 commits intomainfrom
fix/credential-id-check-on-dest-cal-api
Oct 17, 2023
Merged

fix: Adds mandatory credentiaId in Destination Calendar API endpoint POST#11880
zomars merged 6 commits intomainfrom
fix/credential-id-check-on-dest-cal-api

Conversation

@alishaz-polymath
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath commented Oct 13, 2023

What does this PR do?

This PR adds credentialId to be passed as a mandatory input to allow linking with the Credential table. Without this, there's a strong possibility of unexpected issues. If someone goes to add a destination calendar, the relationship with the respective credential link would be missing from the DestinationCalendar table & the Credential table and hence, would probably experience some form of side-effects and potentially impact the main flow of createEvent in EventManager.

Requirement/Documentation

  • NA

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • POST API call to DestinationCalendar endpoint with credentialId

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

Checklist

@vercel
Copy link
Copy Markdown

vercel Bot commented Oct 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ai ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 8:13am
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 8:13am
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 8:13am
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 16, 2023 8:13am
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 8:13am
qa ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 8:13am
ui ⬜️ Ignored (Inspect) Visit Preview Oct 16, 2023 8:13am

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 13, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link to collect XP and win prizes!

@zomars zomars added ai area: AI, cal.ai core area: core, team members only labels Oct 13, 2023
@alishaz-polymath alishaz-polymath added 🐛 bug Something isn't working High priority Created by Linear-GitHub Sync api area: API, enterprise API, access token, OAuth and removed ai area: AI, cal.ai labels Oct 13, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 13, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Oct 13, 2023

Current Playwright Test Results Summary

✅ 149 Passing - ⚠️ 11 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 10/16/2023 08:15:42am UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: 9015b4d

Started: 10/16/2023 08:12:27am UTC

⚠️ Flakes

📄   apps/web/playwright/locale.e2e.ts • 5 Flakes

Top 1 Common Error Messages

null

5 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
unauthorized user sees correct translations (de) should use correct translations and html attributes
Retry 2Retry 1Initial Attempt
-146.51% (-63) -63 / 43 runs
failed over last 7 days
146.51% (63) 63 / 43 runs
flaked over last 7 days
unauthorized user sees correct translations (ar) should use correct translations and html attributes
Retry 2Retry 1Initial Attempt
-146.51% (-63) -63 / 43 runs
failed over last 7 days
146.51% (63) 63 / 43 runs
flaked over last 7 days
authorized user sees correct translations (de) [locale1] should return correct translations and html attributes
Retry 2Retry 1Initial Attempt
0% (0) 0 / 42 runs
failed over last 7 days
73.81% (31) 31 / 42 runs
flaked over last 7 days
authorized user sees correct translations (ar) should return correct translations and html attributes
Retry 2Retry 1Initial Attempt
-72.73% (-8) -8 / 11 runs
failed over last 7 days
127.27% (14) 14 / 11 runs
flaked over last 7 days
authorized user sees changed translations (de->ar) should return correct translations and html attributes
Retry 2Retry 1Initial Attempt
-280% (-14) -14 / 5 runs
failed over last 7 days
280% (14) 14 / 5 runs
flaked over last 7 days

📄   apps/web/playwright/auth/delete-account.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Can delete user account
Retry 2Retry 1Initial Attempt
1.34% (4) 4 / 298 runs
failed over last 7 days
13.76% (41) 41 / 298 runs
flaked over last 7 days

📄   apps/web/playwright/booking-seats.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Reschedule for booking with seats Should reschedule booking with seats
Retry 1Initial Attempt
0% (0) 0 / 283 runs
failed over last 7 days
0.71% (2) 2 / 283 runs
flaked over last 7 days

📄   apps/web/playwright/manage-booking-questions.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Manage Booking Questions For User EventType Do a booking with a user added question and verify a few thing in b/w
Retry 1Initial Attempt
2.85% (8) 8 / 281 runs
failed over last 7 days
0.71% (2) 2 / 281 runs
flaked over last 7 days

📄   apps/web/playwright/change-username.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Change username on settings User can change username
Retry 1Initial Attempt
1.74% (5) 5 / 287 runs
failed over last 7 days
26.83% (77) 77 / 287 runs
flaked over last 7 days

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 2 Flakes

Top 1 Common Error Messages

null

2 Test Cases Affected

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should open embed iframe on click - Configured with light theme
Retry 1Initial Attempt
2.35% (7) 7 / 298 runs
failed over last 7 days
32.21% (96) 96 / 298 runs
flaked over last 7 days
Popup Tests should be able to reschedule
Retry 2Retry 1Initial Attempt
7.07% (21) 21 / 297 runs
failed over last 7 days
91.58% (272) 272 / 297 runs
flaked over last 7 days

View Detailed Build Results


Copy link
Copy Markdown
Member Author

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

Self review added

if (!parsedBody.eventTypeId) {
parsedBody.userId = userId;
}
const assignedUserId = isAdmin ? parsedBody.userId || userId : userId;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

assignedUserId is the one for whom the destination calendar is being created. In case of connection with an eventTypeId, we simply use this to validate their ownership of the eventTypeId passed in the request.

Comment on lines +71 to +74
const credential = await prisma.credential.findFirst({
where: { type: parsedBody.integration, userId: assignedUserId },
select: { id: true, type: true, userId: true },
});
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We check if an entry exists in credential table for the assignedUserId & integration

statusCode: 400,
message: "Bad request, eventTypeId invalid",
});
parsedBody.userId = undefined;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If the body contains an eventTypeId, it means the destination calendar needs to be related to an event type instead, and so we make sure userId inside the parsedBody is not defined.

Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

LGTM

@zomars zomars merged commit 2756dff into main Oct 17, 2023
@zomars zomars deleted the fix/credential-id-check-on-dest-cal-api branch October 17, 2023 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api area: API, enterprise API, access token, OAuth 🐛 bug Something isn't working core area: core, team members only High priority Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants