Skip to content

Conversation

@mfts
Copy link
Owner

@mfts mfts commented Nov 5, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved notification settings handling to ensure team preset defaults are properly applied when creating documents and links.

@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
papermark Ready Ready Preview Comment Nov 5, 2025 0:06am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Walkthrough

The change modifies webhook event handlers to implement a fallback mechanism for the enableNotification property. When link.enableNotification is undefined, the code now defaults to preset?.enableNotification, and if that is also undefined, defaults to false. This applies across three handlers: handleDocumentCreate, handleLinkCreate, and handleDataroomCreate.

Changes

Cohort / File(s) Summary
Webhook enableNotification fallback
pages/api/webhooks/services/[...path]/index.ts
Modified three webhook handlers to use nullish coalescing operator (??) for enableNotification assignment, allowing fallback to preset values when link-level values are undefined

Possibly related PRs

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for respecting the enableNotification preset setting when it's not explicitly provided on the link.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
pages/api/webhooks/services/[...path]/index.ts (1)

504-507: Consider using ?? for boolean preset fallbacks to match the pattern.

While not introduced by this PR, there's an inconsistency: emailProtected, emailAuthenticated, and allowDownload use the logical OR operator (||), which doesn't respect explicit false values. If a user sets emailProtected: false, the current code would incorrectly fall through to check the preset.

Consider aligning these fields with the pattern now used for enableNotification and showBanner:

-        emailProtected: link.emailProtected || preset?.emailProtected || false,
+        emailProtected: link.emailProtected ?? preset?.emailProtected ?? false,
-        emailAuthenticated:
-          link.emailAuthenticated || preset?.emailAuthenticated || false,
+        emailAuthenticated:
+          link.emailAuthenticated ?? preset?.emailAuthenticated ?? false,
-        allowDownload: link.allowDownload || preset?.allowDownload,
+        allowDownload: link.allowDownload ?? preset?.allowDownload,

Apply similar changes at lines 734-737 and 948-952.

Also applies to: 734-737, 948-952

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f870457 and c22ceb6.

📒 Files selected for processing (1)
  • pages/api/webhooks/services/[...path]/index.ts (3 hunks)
⏰ 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). (1)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
pages/api/webhooks/services/[...path]/index.ts (1)

508-508: LGTM! Correct implementation of preset fallback.

The use of nullish coalescing (??) correctly handles the fallback logic. When link.enableNotification is explicitly set to false, it will be respected. Only when it's undefined will the code fall back to the preset value, and finally to false as a safe default.

Also applies to: 738-738, 953-953

@mfts mfts merged commit 8b42352 into main Nov 5, 2025
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants