-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
feat: Refactor to Use credentialRepository for credential create. #17336
base: main
Are you sure you want to change the base?
feat: Refactor to Use credentialRepository for credential create. #17336
Conversation
…dateBookingRefs_credCreate
…edentials_repository
@vijayraghav-io is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (10/25/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add foundation team as reviewer" took an action on this PR • (10/25/24)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (10/25/24)1 label was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (10/28/24)1 label was added to this PR based on Keith Williams's automation. |
type: appMetadata.type, | ||
}, | ||
select: credentialForCalendarServiceSelect, | ||
const createdcredential = await CredentialRepository.create({ |
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.
The Prisma.JsonNull handling for key is also a solid choice for cleaner code. Nicely done!
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.
I did notice, In the second test case you are using the newGoogleCalCredentialId
, This might be involuntary as google_meet_video
should ideally connect to the newGoogleMeetCredentialId
instead of newGoogleCalCredentialId
.
Good notice!. This is intentional, google_video being a special case and that's why this test case was added.
here the credential of googleCal is mapped to |
ahh got it, I need a final check, I'll let you know.. |
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.
tested the changes locally, looks good to me.
E2E results are ready! |
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.
Just blocking this from merge until @joeauyeung can take a look
What does this PR do?
Refer PR: #16878
To Do :
Remove explicit callings of
BookingReferenceRepository.reconnectWithNewCredential()
everywhere, introduced from the PR #16878, if that Issue fix PR is merged before.Mandatory Tasks (DO NOT REMOVE)