Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: delegation credential error webhooks + refactor repeated code#25232

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ThyMinimalDev merged 3 commits intomainfromfix-delegation-credential-error-webhook
Nov 17, 2025

Conversation

@ThyMinimalDev
Copy link
Contributor

@ThyMinimalDevThyMinimalDev commentedNov 17, 2025
edited by devin-ai-integrationbot
Loading

What does this PR do?

This PR contains two sets of changes:

1. Original Changes (Delegation Credential Error Webhook Fixes)

  • Fixes the delegation credential error webhook system to properly resolve organizations from delegation credentials
  • Changes webhook trigger parameter fromorgId todelegationCredentialId for more accurate organization resolution
  • Adds newWebhookRepository.findByOrgIdAndTrigger() method for fetching webhooks by organization and trigger event
  • Enriches webhook payload withdelegationCredentialId andorganizationId fields
  • Fixes GoogleCalendar to includedelegatedToId check before triggering webhooks

2. Refactoring Changes (Code DRY Improvements)

  • Extracts repeated delegation credential error webhook triggering logic into helper methods
  • AddstriggerDelegationCredentialError() private method inOffice365CalendarService class
  • AddstriggerDelegationCredentialError() helper function inTeamsVideoApiAdapter
  • Replaces 4 instances of repeated code in each file with calls to the helper methods

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code
  • N/A - No documentation changes required (internal refactoring and bug fix)
  • I confirm automated tests are in place that prove my fix is effective or that my feature works

How should this be tested?

Testing the webhook fixes:

  1. Set up a delegation credential for Office365 or Google Calendar
  2. Configure a webhook subscriber forDELEGATION_CREDENTIAL_ERROR trigger at the organization level
  3. Trigger a delegation credential error (e.g., invalid tenant ID, missing client credentials, or non-existent Azure user)
  4. Verify the webhook is fired with the correct payload includingdelegationCredentialId andorganizationId

Testing the refactoring:

  1. Run existing tests for Office365Calendar and Office365Video integrations
  2. Verify delegation credential error scenarios still trigger webhooks correctly
  3. Check that all 4 error scenarios in each file still work as expected

Human Review Checklist

Critical items to review:

  1. Webhook Logic Change: Verify the change from passingorgId directly to looking up the delegation credential first is correct. Does this properly handle all edge cases?

  2. New Repository Method: ReviewWebhookRepository.findByOrgIdAndTrigger() - is the query correct? Does it properly filter by organization and trigger event?

  3. Payload Changes: The webhook payload now includesdelegationCredentialId andorganizationId. Is this a breaking change for existing webhook consumers? Should this be documented?

  4. GoogleCalendar Fix: Verify the addition ofcredential.delegatedToId check in GoogleCalendar is correct and consistent with other integrations.

  5. Refactoring Correctness: Verify the extracted helper methods maintain exactly the same behavior as the original repeated code. Check that:

    • All conditional checks are preserved
    • The webhook payload structure is identical
    • Error handling remains the same
  6. Edge Cases: What happens ifdelegationCredential is not found in the database? The code returns early, but is this the correct behavior?


Session Info:

@ThyMinimalDevThyMinimalDev requested a review froma team as acode ownerNovember 17, 2025 20:59
@graphite-appgraphite-appbot requested a review froma teamNovember 17, 2025 21:00
@keithwillcodekeithwillcode added corearea: core, team members only foundation platformAnything related to our platform plan labelsNov 17, 2025
…into helper methods- Added private triggerDelegationCredentialError method in Office365CalendarService class- Added triggerDelegationCredentialError helper function in TeamsVideoApiAdapter- Replaced all 4 instances in Office365CalendarService with helper method call- Replaced all 4 instances in TeamsVideoApiAdapter with helper function call- Keeps code DRY by eliminating repeated if statement and webhook trigger logicCo-Authored-By: morgan@cal.com <morgan@cal.com>
@vercel
Copy link

vercelbot commentedNov 17, 2025
edited
Loading

The latest updates on your projects. Learn more aboutVercel for GitHub.

2 Skipped Deployments
ProjectDeploymentPreviewCommentsUpdated (UTC)
calIgnoredIgnoredNov 17, 2025 9:15pm
cal-euIgnoredIgnoredNov 17, 2025 9:15pm

Copy link
Contributor

@cubic-dev-aicubic-dev-aibot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

@devin-ai-integrationdevin-ai-integrationbot changed the titlefix: delegation credential error webhooksfix: delegation credential error webhooks + refactor repeated codeNov 17, 2025
Copy link
Contributor

@emrysalemrysal left a comment

Choose a reason for hiding this comment

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

Happy with this. Better error handling; more centralized - and the query looks sane (search for index by teamId first, some filters after)

@ThyMinimalDevThyMinimalDev merged commitb8530e2 intomainNov 17, 2025
39 of 40 checks passed
@ThyMinimalDevThyMinimalDev deleted the fix-delegation-credential-error-webhook branchNovember 17, 2025 22:00
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@cubic-dev-aicubic-dev-ai[bot]cubic-dev-ai[bot] left review comments

@emrysalemrysalemrysal approved these changes

Assignees

No one assigned

Labels

corearea: core, team members onlyfoundationplatformAnything related to our platform planready-for-e2esize/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ThyMinimalDev@emrysal@keithwillcode

[8]ページ先頭

©2009-2025 Movatter.jp