Sanitize partner websites #2599
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces website URL sanitization for partner records. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OnlinePresenceForm
participant sanitizeWebsite
participant Database
User->>OnlinePresenceForm: Paste website URL
OnlinePresenceForm->>sanitizeWebsite: Sanitize pasted URL
sanitizeWebsite-->>OnlinePresenceForm: Sanitized URL
OnlinePresenceForm->>Database: Save sanitized website URL
sequenceDiagram
participant Script
participant Database
participant sanitizeWebsite
Script->>Database: Query partners with unsanitized websites
loop For each partner
Script->>sanitizeWebsite: Sanitize website URL
sanitizeWebsite-->>Script: Sanitized URL
Script->>Database: Update partner record if changed
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/scripts/partners/sanitize-websites.ts (1)
6-58: Consider adding error handling for database operations.The script performs database updates in a loop without error handling. Consider wrapping the update operation in try-catch to handle potential database errors gracefully:
for (const { id, website } of partners) { let updatedWebsite = sanitizeWebsite(website); const needsUpdate = updatedWebsite !== website; if (needsUpdate) { updatedPartners.push({ id, website: updatedWebsite, }); + try { await prisma.partner.update({ where: { id }, data: { website: updatedWebsite }, }); + } catch (error) { + console.error(`Failed to update partner ${id}:`, error); + // Remove from updatedPartners array on failure + updatedPartners.pop(); + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/lib/actions/partners/update-online-presence.ts(2 hunks)apps/web/lib/social-utils.ts(1 hunks)apps/web/scripts/partners/sanitize-websites.ts(1 hunks)apps/web/ui/partners/online-presence-form.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/web/scripts/partners/sanitize-websites.ts (1)
Learnt from: devkiran
PR: dubinc/dub#2177
File: apps/web/lib/api/links/bulk-create-links.ts:66-84
Timestamp: 2025-06-06T07:59:03.120Z
Learning: In apps/web/lib/api/links/bulk-create-links.ts, the team accepts the risk of potential undefined results from links.find() operations when building invalidLinks arrays, because existing links are fetched from the database based on the input links, so matches are expected to always exist.
🧬 Code Graph Analysis (2)
apps/web/scripts/partners/sanitize-websites.ts (1)
apps/web/lib/social-utils.ts (1)
sanitizeWebsite(48-64)
apps/web/lib/actions/partners/update-online-presence.ts (1)
apps/web/lib/social-utils.ts (1)
sanitizeWebsite(48-64)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
apps/web/lib/social-utils.ts (1)
48-64: LGTM! Well-implemented URL sanitization function.The function correctly handles edge cases with proper null/undefined checks, adds HTTPS protocol when missing, removes query parameters, and gracefully handles invalid URLs. The implementation is robust and follows good practices.
apps/web/lib/actions/partners/update-online-presence.ts (2)
7-7: LGTM! Proper import of sanitization function.
18-18: LGTM! Correct integration of website sanitization in schema validation.The sanitizeWebsite transform is properly applied to ensure consistent URL normalization during validation.
apps/web/ui/partners/online-presence-form.tsx (3)
5-9: LGTM! Proper import of sanitization utilities.
131-142: LGTM! Well-implemented paste handler for website sanitization.The callback correctly sanitizes pasted URLs and only prevents default behavior when sanitization succeeds. The useCallback hook is properly used with the correct dependency.
188-188: LGTM! Proper integration of paste handler.The onPasteWebsite handler is correctly applied to provide immediate URL sanitization on paste events.
| for (const { id, website } of partners) { | ||
| let updatedWebsite = sanitizeWebsite(website); | ||
|
|
||
| const needsUpdate = updatedWebsite !== website; | ||
|
|
||
| if (needsUpdate) { | ||
| updatedPartners.push({ | ||
| id, | ||
| website: updatedWebsite, | ||
| }); | ||
|
|
||
| await prisma.partner.update({ | ||
| where: { | ||
| id, | ||
| }, | ||
| data: { | ||
| website: updatedWebsite, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle the case where sanitization returns null for existing websites.
If sanitizeWebsite returns null for an existing website (e.g., invalid URL), the script will update the database to set the website field to null. This might not be the intended behavior.
Consider skipping the update or logging these cases separately:
for (const { id, website } of partners) {
let updatedWebsite = sanitizeWebsite(website);
+ // Skip if sanitization failed (returned null for existing website)
+ if (website && !updatedWebsite) {
+ console.log(`Skipping invalid website for partner ${id}: ${website}`);
+ continue;
+ }
+
const needsUpdate = updatedWebsite !== website;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const { id, website } of partners) { | |
| let updatedWebsite = sanitizeWebsite(website); | |
| const needsUpdate = updatedWebsite !== website; | |
| if (needsUpdate) { | |
| updatedPartners.push({ | |
| id, | |
| website: updatedWebsite, | |
| }); | |
| await prisma.partner.update({ | |
| where: { | |
| id, | |
| }, | |
| data: { | |
| website: updatedWebsite, | |
| }, | |
| }); | |
| } | |
| for (const { id, website } of partners) { | |
| let updatedWebsite = sanitizeWebsite(website); | |
| // Skip if sanitization failed (returned null for existing website) | |
| if (website && !updatedWebsite) { | |
| console.log(`Skipping invalid website for partner ${id}: ${website}`); | |
| continue; | |
| } | |
| const needsUpdate = updatedWebsite !== website; | |
| if (needsUpdate) { | |
| updatedPartners.push({ | |
| id, | |
| website: updatedWebsite, | |
| }); | |
| await prisma.partner.update({ | |
| where: { | |
| id, | |
| }, | |
| data: { | |
| website: updatedWebsite, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/scripts/partners/sanitize-websites.ts around lines 31 to 50, the
code updates the partner's website even if sanitizeWebsite returns null, which
may unintentionally set the website field to null in the database. Modify the
logic to check if updatedWebsite is null before proceeding with the update; if
it is null, skip the update or log the case separately to avoid overwriting
valid data with null.
Builds on the logic + script of #2585 to sanitize websites as well
Summary by CodeRabbit
New Features
Bug Fixes