Conversation
… reply-notificaties Web-kant van Epic PolderLabs/ribba.app#35: - Schema-migratie: inquiries, inquiry_recipients, conversations, messages, user_profiles + RLS + realtime (ribba.app#36, met gedocumenteerde afwijkingen) - POST /api/inquiry-submit met CORS, rate limit, honeypot en outreach-mails naar rijscholen via Resend (ribba.app#33) - Web-chat gateway /chat/[token]: OTP-gate via Supabase Auth, claim-flow met server-side e-mailmatch, geanonimiseerde WhatsApp-stijl realtime chat (ribba.app#42) - Smart app banners alleen op chat-pagina's: apple-itunes-app meta + chat-manifest.webmanifest (ribba.app#43) - Reply-notificatie e-mails via 5-min cron met settle-delay, bundeling en opt-out (ribba.app#44)
|
Deployment failed with the following error: Learn More: https://vercel.link/3Fpeeb1 |
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a Marketplace inquiry intake API, Supabase-backed authenticated chat, email notifications and opt-out flows, app deep-link fallbacks, universal-link configuration, shared types/helpers, and database migrations. ChangesMarketplace platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Learner
participant InquirySubmit
participant Supabase
participant ChatGateway
participant ChatThread
participant NotificationCron
Learner->>InquirySubmit: Submit inquiry
InquirySubmit->>Supabase: Store inquiry and recipients
InquirySubmit-->>Learner: Return inquiry id
ChatGateway->>Supabase: Resolve token and claim participant
ChatThread->>Supabase: Load and send messages
NotificationCron->>Supabase: Find unread replies
NotificationCron-->>Learner: Send reply notification
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
app/api/cron/chat-notifications/route.ts (2)
127-168: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRun the cheap opt-out / push / email-preference filters before the per-side
messagesquery.For every candidate conversation the handler issues a separate
messagesquery per side (up to 2×N round-trips) at lines 128-139, and only afterwards evaluates the opt-out (148-154) and push/email-preference (155-168) skips. Since those skip conditions rely only on already-loaded data (recipientRow,profiles), moving them ahead of the query avoids DB work for recipients that will never be mailed. This reduces load on the messages table and shortens the run within the 60s budget.♻️ Reorder cheap checks ahead of the messages query
const counterpartRole: ChatRole = side === 'leerling' ? 'rijschool' : 'leerling'; + + // Notify-regel per kant (goedkope checks eerst, vóór de messages-query). + const sideUserId = side === 'leerling' ? conv.leerling_user_id : conv.rijschool_user_id; + const optedOut = side === 'leerling' + ? recipientRow.leerling_email_optout_at !== null + : recipientRow.rijschool_email_optout_at !== null; + if (optedOut) { + skipped++; + continue; + } + if (sideUserId) { + const profile = profiles.get(sideUserId); + if (profile) { + if (profile.expo_push_token) { skipped++; continue; } + if (!profile.email_notifications) { skipped++; continue; } + } + } + let unreadQuery = supabase .from('messages') .select('body, created_at') .eq('conversation_id', conv.id) .eq('sender_role', counterpartRole) .is('read_at', null) .lte('created_at', settleCutoff) .order('created_at', { ascending: false }); if (lastNotified) { unreadQuery = unreadQuery.gt('created_at', lastNotified); } const { data: unread } = await unreadQuery; const unreadMessages = (unread ?? []) as Pick<MessageRow, 'body' | 'created_at'>[]; if (unreadMessages.length === 0) { skipped++; continue; } - - // Notify-regel per kant. - const sideUserId = side === 'leerling' ? conv.leerling_user_id : conv.rijschool_user_id; - const optedOut = side === 'leerling' - ? recipientRow.leerling_email_optout_at !== null - : recipientRow.rijschool_email_optout_at !== null; - if (optedOut) { - skipped++; - continue; - } - if (sideUserId) { - const profile = profiles.get(sideUserId); - if (profile) { - // Actieve push in de app → geen dubbele e-mail (ribbaPro#144 dekt push). - if (profile.expo_push_token) { - skipped++; - continue; - } - if (!profile.email_notifications) { - skipped++; - continue; - } - } - }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/cron/chat-notifications/route.ts` around lines 127 - 168, Move the opt-out, push-token, and email-preference checks in the per-side candidate loop before constructing and executing unreadQuery. Keep the existing sideUserId, optedOut, profiles, skipped++, and continue behavior unchanged, then run the messages query only for recipients who pass all cheap filters.
57-70: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd pagination to the candidate query. Without an explicit limit/range loop, this request can hit the server row cap and silently skip later conversations in a large backlog. A batched or keyset-paginated fetch would avoid missed notifications.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/cron/chat-notifications/route.ts` around lines 57 - 70, Update the candidate query in the chat notification handler to fetch all matching conversations through explicit batching or keyset pagination instead of a single unbounded request. Use a stable ordering and repeatedly apply range/cursor boundaries until no rows remain, then process the combined candidates while preserving the existing filters and nested selection.lib/marketplace-emails.ts (3)
10-17: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider replacing hand-rolled HTML escaping with a vetted library.
The current
escapeHtmlcovers the standard five characters in the correct order and is adequate for the current text-content escaping use case. However, static analysis flags this as CWE-79. A vetted library (e.g.,escape-htmlordompurify) would provide stronger guarantees, especially if future templates place user input in attribute or JavaScript contexts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/marketplace-emails.ts` around lines 10 - 17, Replace the hand-rolled escaping in escapeHtml with a vetted HTML-escaping library such as escape-html, adding the dependency and using it consistently for all callers; preserve the existing function interface and behavior for text content.Source: Linters/SAST tools
63-72: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueEscape
ctaHrefandctaColorin thewraptemplate for defense-in-depth.Both values are inserted unescaped into
hrefandstyleattributes. All current callers pass hardcoded values (URLs from env vars + UUID tokens, fixed color strings), so this is not exploitable today. Escaping them would prevent a future regression if a caller ever passes user-controlled input.🛡️ Proposed fix: escape attribute values
const cta = opts.ctaLabel && opts.ctaHref ? ` <tr><td style="padding:24px 32px 8px 32px"> - <a href="${opts.ctaHref}" style="display:inline-block;background:${opts.ctaColor || '`#2563EB`'};color:`#FFFFFF`;padding:13px 24px;border-radius:12px;text-decoration:none;font-weight:700;font-size:14px"> + <a href="${escapeHtml(opts.ctaHref)}" style="display:inline-block;background:${escapeHtml(opts.ctaColor || '`#2563EB`')};color:`#FFFFFF`;padding:13px 24px;border-radius:12px;text-decoration:none;font-weight:700;font-size:14px"> ${escapeHtml(opts.ctaLabel)} </a> </td></tr> ` : '';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/marketplace-emails.ts` around lines 63 - 72, In wrap, escape the dynamic ctaHref and ctaColor values before interpolating them into the href and style attributes, using the existing HTML attribute escaping helper (such as escapeHtml). Preserve the current fallback color and CTA rendering behavior while ensuring both attribute values are safely encoded.
25-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the Resend API
fetchcall.Without a timeout, a hanging Resend response can consume the entire
maxDurationof the serverless function on a single email, leaving remaining outreach emails unsent with no error logged. The cron retry mitigates this, but a timeout would allow the loop to continue to the next recipient and log the failure.⏱️ Proposed fix: add AbortController timeout
async function sendMail(to: string, subject: string, html: string, emailType: string): Promise<boolean> { if (!RESEND_API_KEY) { console.warn('marketplace-emails: RESEND_API_KEY not set, skipping', emailType, to); return false; } + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 10_000); const res = await fetch('https://api.resend.com/emails', { method: 'POST', headers: { Authorization: `Bearer ${RESEND_API_KEY}`, 'Content-Type': 'application/json', }, body: JSON.stringify({ from: 'Ribba <noreply@ribba.app>', to, subject, html, }), + signal: controller.signal, }); + clearTimeout(timeout); if (!res.ok) { const errText = await res.text().catch(() => ''); console.error('marketplace-emails: send failed', emailType, res.status, errText.slice(0, 500)); return false; } return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/marketplace-emails.ts` around lines 25 - 49, Add an AbortController-based timeout to the fetch call in sendMail, passing its signal and aborting after a reasonable duration; catch timeout or network errors, log the emailType and failure details, and return false so callers can continue processing subsequent recipients.components/chat/OtpGate.tsx (1)
78-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding an explicit "resend code" button.
Currently, users who don't receive the OTP can click "Ander e-mailadres gebruiken" and re-enter the same address, but this isn't obvious. A dedicated resend action would improve UX, especially given email delivery delays.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/chat/OtpGate.tsx` around lines 78 - 110, Add a dedicated resend-code action to the OTP form returned by OtpGate, allowing users to request a new code without changing their email. Implement a resend handler that invokes the existing code-sending flow, manages loading/error state consistently with handleVerify, and disables the action while busy; render a clearly labeled Dutch “Verificatiecode opnieuw versturen” button near the existing alternate-email action.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/chat/claim/route.ts`:
- Around line 169-173: Handle the error returned by the inquiry_recipients query
in the conversation backfill logic, rather than silently treating a failed
request as an empty recipient list. Update the query near the recipientIds check
to destructure error, log or propagate it using the route’s existing
error-handling pattern, and only continue to the recipientIds processing when
the query succeeds.
In `@app/api/chat/resolve/route.ts`:
- Around line 38-42: Handle the error returned by the Supabase query in the
inquiry lookup within the resolve route: destructure both data and error, return
an appropriate 500 response when the query fails, and only use the existing 404
response when no inquiry is found. Use the `inquiry` lookup and its surrounding
route handler to implement this distinction.
In `@app/api/notifications/opt-out/route.ts`:
- Around line 66-71: Handle and validate the Supabase update within the
sideUserId branch of the opt-out route: destructure the returned error from the
user_profiles update and, when present, log or return an appropriate failure
response instead of continuing to the success page. Ensure the inquiry_recipient
opt-out stamp and user_profiles preference remain consistent.
In `@components/chat/ChatGateway.tsx`:
- Around line 148-152: Always render the support email link in the error state
instead of guarding it with !errorMsg. Update the JSX in ChatGateway’s error
message block so the mailto:hallo@ribba.app link appears whether or not errorMsg
is set, while preserving the existing fallback text behavior.
- Around line 102-125: Wrap the entire async workflow inside the useEffect IIFE
in a try-catch, including res.json(), getSupabase(), and
supabase.auth.getSession(). Preserve the existing invalid response handling, and
setPhase('invalid') in the catch so any unexpected failure exits the resolving
state instead of leaving the spinner indefinitely.
In `@components/chat/ChatThread.tsx`:
- Around line 64-67: Handle the missing-session branch in the ChatThread
initialization logic before the early return: when getSession() returns no
session and the operation is not cancelled, call setLoading(false) and
setLoadError(true) (or the component’s equivalent recovery state), then return.
Keep cancellation behavior unchanged and use the existing session-loading
effect/function to apply the fix.
- Around line 63-105: Reorder the initialization in the async setup block so the
realtime channel is created and subscribed before fetching messages, preferably
waiting for the SUBSCRIBED confirmation before issuing the initial query.
Preserve the existing INSERT id-based deduplication and UPDATE merging, and
ensure cleanup/cancellation still handles the channel correctly.
In `@lib/marketplace-db.ts`:
- Around line 65-79: Update lookupRecipientByToken to destructure and check the
error from both maybeSingle() queries for rijschool_chat_token and
leerling_chat_token; throw each database error before evaluating the returned
data, allowing the caller’s existing catch block to return a 500 instead of
treating failures as not found.
---
Nitpick comments:
In `@app/api/cron/chat-notifications/route.ts`:
- Around line 127-168: Move the opt-out, push-token, and email-preference checks
in the per-side candidate loop before constructing and executing unreadQuery.
Keep the existing sideUserId, optedOut, profiles, skipped++, and continue
behavior unchanged, then run the messages query only for recipients who pass all
cheap filters.
- Around line 57-70: Update the candidate query in the chat notification handler
to fetch all matching conversations through explicit batching or keyset
pagination instead of a single unbounded request. Use a stable ordering and
repeatedly apply range/cursor boundaries until no rows remain, then process the
combined candidates while preserving the existing filters and nested selection.
In `@components/chat/OtpGate.tsx`:
- Around line 78-110: Add a dedicated resend-code action to the OTP form
returned by OtpGate, allowing users to request a new code without changing their
email. Implement a resend handler that invokes the existing code-sending flow,
manages loading/error state consistently with handleVerify, and disables the
action while busy; render a clearly labeled Dutch “Verificatiecode opnieuw
versturen” button near the existing alternate-email action.
In `@lib/marketplace-emails.ts`:
- Around line 10-17: Replace the hand-rolled escaping in escapeHtml with a
vetted HTML-escaping library such as escape-html, adding the dependency and
using it consistently for all callers; preserve the existing function interface
and behavior for text content.
- Around line 63-72: In wrap, escape the dynamic ctaHref and ctaColor values
before interpolating them into the href and style attributes, using the existing
HTML attribute escaping helper (such as escapeHtml). Preserve the current
fallback color and CTA rendering behavior while ensuring both attribute values
are safely encoded.
- Around line 25-49: Add an AbortController-based timeout to the fetch call in
sendMail, passing its signal and aborting after a reasonable duration; catch
timeout or network errors, log the emailType and failure details, and return
false so callers can continue processing subsequent recipients.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5bcf8bc-de98-4240-8276-7a6e42e3ec43
📒 Files selected for processing (22)
app/api/chat/claim/route.tsapp/api/chat/resolve/route.tsapp/api/cron/chat-notifications/route.tsapp/api/inquiry-submit/route.tsapp/api/notifications/opt-out/route.tsapp/chat/[token]/layout.tsxapp/chat/[token]/page.tsxapp/globals.csscomponents/chat/ChatGateway.tsxcomponents/chat/ChatThread.tsxcomponents/chat/MessageComposer.tsxcomponents/chat/OtpGate.tsxdocs/ARCHITECTUUR.mdlib/cors.tslib/marketplace-db.tslib/marketplace-emails.tslib/marketplace-types.tsmiddleware.tspublic/chat-manifest.webmanifestsupabase/migrations/20260711000000_marketplace_mvp.sqlsupabase/migrations/README.mdvercel.json
Comment on lines
+148
to
+152
| <h1>Deze link werkt niet</h1> | ||
| <p className="chat-muted"> | ||
| {errorMsg ?? 'De chat-link is ongeldig of verlopen. Gebruik de meest recente link uit je e-mail, of mail ons op '} | ||
| {!errorMsg && <a href="mailto:hallo@ribba.app">hallo@ribba.app</a>} | ||
| </p> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Error state hides the support email link.
When errorMsg is set (e.g., "Er ging iets mis."), the !errorMsg condition suppresses the mailto:hallo@ribba.app link, leaving users with a generic error and no contact path. Consider always showing the support link.
✨ Proposed fix
<p className="chat-muted">
- {errorMsg ?? 'De chat-link is ongeldig of verlopen. Gebruik de meest recente link uit je e-mail, of mail ons op '}
- {!errorMsg && <a href="mailto:hallo@ribba.app">hallo@ribba.app</a>}
+ {errorMsg && <>{errorMsg}{' '}</>}
+ {!errorMsg && 'De chat-link is ongeldig of verlopen. Gebruik de meest recente link uit je e-mail, of mail ons op '}
+ <a href="mailto:hallo@ribba.app">hallo@ribba.app</a>
</p>📝 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.
Suggested change
| <h1>Deze link werkt niet</h1> | |
| <p className="chat-muted"> | |
| {errorMsg ?? 'De chat-link is ongeldig of verlopen. Gebruik de meest recente link uit je e-mail, of mail ons op '} | |
| {!errorMsg && <a href="mailto:hallo@ribba.app">hallo@ribba.app</a>} | |
| </p> | |
| <h1>Deze link werkt niet</h1> | |
| <p className="chat-muted"> | |
| {errorMsg && <>{errorMsg}{' '}</>} | |
| {!errorMsg && 'De chat-link is ongeldig of verlopen. Gebruik de meest recente link uit je e-mail, of mail ons op '} | |
| <a href="mailto:hallo@ribba.app">hallo@ribba.app</a> | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/chat/ChatGateway.tsx` around lines 148 - 152, Always render the
support email link in the error state instead of guarding it with !errorMsg.
Update the JSX in ChatGateway’s error message block so the
mailto:hallo@ribba.app link appears whether or not errorMsg is set, while
preserving the existing fallback text behavior.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
melvinmt
changed the title
Sync met ribba.app#36-contract (issuecomment-4933108256) en de reviews op ribba.app#45 / ribbaPro#218: - marketplace_profiles i.p.v. user_profiles: minimaal rol-register, geen expo_push_token (push_tokens-tabel van de app is SSoT) - inquiry_recipients: + opened_at, decline_reason, expires_at, school_id; conversations: + school_id, last_message_preview (trigger onderhoudt preview) - Gedeelde SECURITY DEFINER RPC's i.p.v. Next-only claim-routes: get_chat_context, claim_inquiry, claim_inquiry_recipient (e-mail-match tegen actueel cbr_rijscholen.email), get_inquiry_for_recipient, mark_messages_read — web-chat en ribbaPro-app delen exact dezelfde semantiek; /api/chat/resolve en /api/chat/claim vervallen - messages: géén client-UPDATE meer; read_at alleen via mark_messages_read (NULL → now(), alleen counterpart-berichten) - Realtime publication: + inquiry_recipients (app-inboxen #140/#142) - Cron: push/e-mail-dedupe via push_tokens-tabel - inquiry-submit: 24u (e-mail × rijschool) dedupe, marketing_optin, bevestigingsmail naar de leerling, startdatum-synthese gedocumenteerd - AASA: /chat/* toegevoegd — één URL-schema voor mail-links; met de app geïnstalleerd opent de universal link de app (assetlinks dekt Android al via handle_all_urls)
melvinmt
changed the title
Sync met de definitieve ribbaPro#218-stand (commits 1a55c037 + 0070eb99):
- AASA: /chat/* vervangen door /i/* + /r/* — de app registreert alleen die
paden (op link.ribba.app); /chat-parsing is daar ge-revert
- Browser-fallbacks /i/[id] en /r/[id] terug (app-download CTA, noindex)
- Mails bevatten nu twee links: web-chat /chat/{token} (primaire CTA) én de
universal link /r/{recipient_id} resp. /i/{inquiry_id} ('open in de app');
kale ids zijn geen bearer-tokens — claimen vereist e-mail-match (RPC's)
- Token-expiry (review ribba.app): chat_tokens_expire_at (30 dagen, rolling —
cron verlengt bij elke notificatiemail); get_chat_context weigert verlopen
tokens voor niet-geclaimde bezoekers, geclaimde deelnemers behouden toegang
- Gateway toont een duidelijke melding bij een verlopen link
…esend, mail-timeout - lookupRecipientByToken en opt-out: DB-fouten niet meer stil als 'niet gevonden'/succes behandelen (throw → bestaande 500/foutpagina-afhandeling) - ChatGateway: try/catch om de resolve-flow (geen eeuwige spinner) + support-mailto altijd zichtbaar in de foutstaat - ChatThread: ontbrekende sessie → foutstaat i.p.v. spinner; realtime-kanaal subscribet vóór de initial fetch (geen verloren berichten in de race), historie laadt ook bij een falend kanaal - OtpGate: 'code opnieuw versturen'-actie - Cron: goedkope filters vóór de messages-query; kandidaten-query begrensd (oudste eerst, limit 200 — rest pakt de volgende run op) - sendMail: 10s AbortSignal-timeout + catch → false, volgende ontvanger gaat door; ctaHref/ctaColor ge-escaped in het mail-template
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/api/cron/chat-notifications/route.ts (1)
69-75: 🚀 Performance & Scalability | 🔵 TrivialAscending order +
limit(200)can starve active conversations and burn the cap on already-notified rows.The candidate query can't exclude conversations whose sides were already notified (the comment on Line 55-57 notes PostgREST can't compare two columns). Combined with
order('last_message_at', ascending: true), each run refetches the oldest rows in the 7-day window first — which are the ones most likely already handled (and repeatedly skipped by the throttle) — while conversations with the most recent activity sort last and may never enter the top 200 once the backlog exceeds the cap. That inverts the intended prioritization: the newest replies are the ones a notification is most useful for.Consider a DB-side predicate that filters already-settled rows (e.g. a
SECURITY DEFINERRPC or a generatedneeds_notificationboolean/column that compareslast_message_atagainst the two*_last_notified_atcolumns), so the 200-row budget is spent only on conversations that actually need a mail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/cron/chat-notifications/route.ts` around lines 69 - 75, Update the chat-notifications candidate query to exclude conversations whose latest activity is already covered by either side’s notification timestamp, using a database-side predicate such as an RPC or generated needs-notification field. Then order remaining candidates by last_message_at descending before applying limit(200), so the cap prioritizes conversations that actually require notification and have the newest activity.lib/marketplace-types.ts (1)
93-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making
ChatContexta discriminated union for thefoundflag.When
get_chat_contextreturns{ found: false }or{ found: false, expired: true }, the remaining fields (role,inquiry_id,recipient_id, etc.) are absent from the JSON response. The current interface declares them as required, so TypeScript won't flag callers that access these fields without first checkingfound.♻️ Proposed discriminated union
-export interface ChatContext { - found: boolean; - expired?: boolean; // token verlopen (alleen relevant bij found: false) - role: ChatRole; - inquiry_id: string; - recipient_id: string; - conversation_id: string | null; - status: InquiryRecipientStatus; - claimed: boolean; - expected_email_masked: string | null; - counterpart_name: string; - inquiry_preview: { - voornaam: string; - rijbewijs_categorie: RijbewijsCategorie; - schakeling: Schakeling | null; - gewenste_startdatum: string | null; - bericht: string | null; - created_at: string; - }; - contact: { name: string; email: string; phone: string | null } | null; -} +export type ChatContext = + | { found: false; expired?: boolean } + | { + found: true; + role: ChatRole; + inquiry_id: string; + recipient_id: string; + conversation_id: string | null; + status: InquiryRecipientStatus; + claimed: boolean; + expected_email_masked: string | null; + counterpart_name: string; + inquiry_preview: { + voornaam: string; + rijbewijs_categorie: RijbewijsCategorie; + schakeling: Schakeling | null; + gewenste_startdatum: string | null; + bericht: string | null; + created_at: string; + }; + contact: { name: string; email: string; phone: string | null } | null; + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/marketplace-types.ts` around lines 93 - 113, Change ChatContext into a discriminated union keyed by found: define a found:false variant containing only the error-state fields, including optional expired, and a found:true variant containing the existing role, inquiry_id, recipient_id, conversation_id, status, claimed, expected_email_masked, counterpart_name, inquiry_preview, and contact fields. Preserve the response shape and ensure callers must check found before accessing successful-context fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/notifications/opt-out/route.ts`:
- Around line 25-48: Change the opt-out flow so GET only validates the token and
renders a confirmation page without updating the recipient; move the Supabase
mutation in GET’s try block to a POST handler that performs the update after
explicit confirmation, reusing lookupRecipientByToken, getServiceClient, and the
existing role-specific fields. Preserve the current rate limiting, invalid-token
response, and error handling for both methods.
In `@app/globals.css`:
- Around line 862-865: Update the .chat-bubble p rule by replacing the
deprecated word-break: break-word declaration with overflow-wrap: anywhere,
while preserving the existing white-space behavior.
In `@components/chat/ChatGateway.tsx`:
- Around line 145-148: Update handleSwitchAccount and the OtpGate rendering path
so switching accounts remounts OtpGate, resetting its step, email, and code
state. Use a state-driven remount key or equivalent tied to handleSwitchAccount,
while preserving the existing sign-out and mismatchEmail reset behavior.
In `@supabase/migrations/20260711000000_marketplace_mvp.sql`:
- Around line 315-319: Update the v_claimed_by_caller assignment so its boolean
expression is normalized with COALESCE(..., false), ensuring anonymous callers
produce FALSE rather than NULL. Keep the expiry check in the IF block unchanged
so expired tokens are rejected unless the caller is explicitly claimed.
---
Nitpick comments:
In `@app/api/cron/chat-notifications/route.ts`:
- Around line 69-75: Update the chat-notifications candidate query to exclude
conversations whose latest activity is already covered by either side’s
notification timestamp, using a database-side predicate such as an RPC or
generated needs-notification field. Then order remaining candidates by
last_message_at descending before applying limit(200), so the cap prioritizes
conversations that actually require notification and have the newest activity.
In `@lib/marketplace-types.ts`:
- Around line 93-113: Change ChatContext into a discriminated union keyed by
found: define a found:false variant containing only the error-state fields,
including optional expired, and a found:true variant containing the existing
role, inquiry_id, recipient_id, conversation_id, status, claimed,
expected_email_masked, counterpart_name, inquiry_preview, and contact fields.
Preserve the response shape and ensure callers must check found before accessing
successful-context fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e430a35-0020-42bf-94a6-ca602035f33b
📒 Files selected for processing (24)
app/api/cron/chat-notifications/route.tsapp/api/inquiry-submit/route.tsapp/api/notifications/opt-out/route.tsapp/chat/[token]/layout.tsxapp/chat/[token]/page.tsxapp/globals.cssapp/i/[id]/page.tsxapp/r/[id]/page.tsxcomponents/MarketplaceAppFallback.tsxcomponents/chat/ChatGateway.tsxcomponents/chat/ChatThread.tsxcomponents/chat/MessageComposer.tsxcomponents/chat/OtpGate.tsxdocs/ARCHITECTUUR.mdlib/cors.tslib/marketplace-db.tslib/marketplace-emails.tslib/marketplace-types.tsmiddleware.tspublic/.well-known/apple-app-site-associationpublic/chat-manifest.webmanifestsupabase/migrations/20260711000000_marketplace_mvp.sqlsupabase/migrations/README.mdvercel.json
✅ Files skipped from review due to trivial changes (5)
- app/r/[id]/page.tsx
- public/chat-manifest.webmanifest
- supabase/migrations/README.md
- vercel.json
- docs/ARCHITECTUUR.md
🚧 Files skipped from review as they are similar to previous changes (3)
- components/MarketplaceAppFallback.tsx
- public/.well-known/apple-app-site-association
- app/i/[id]/page.tsx
…OtpGate-remount - Opt-out: GET valideert alleen en toont een bevestigingsknop; de mutatie zit in POST — mail-scanners/prefetchers GET'en links en mogen niemand afmelden - Migratie: v_claimed_by_caller ge-coalesced naar false — bij een anonieme caller was de expressie NULL en werd de token-expiry-check stil overgeslagen - ChatGateway: OtpGate remount (key) bij account-wissel zodat step/email/code resetten; ChatContext als discriminated union op found (succes-velden pas na de found-check toegankelijk) - CSS: word-break: break-word (deprecated) → overflow-wrap: anywhere
…e pg_cron
Vercel weigerde de deploy ('Deployment failed', link naar cron usage-and-pricing):
het plan staat maar 2 crons + dagelijkse cadence toe, dus een 3e cron op
*/5 breekt élke deploy (ook de chat-gateway kwam zo nooit live).
- chat-notifications uit vercel.json gehaald → deploy weer groen
- Supabase pg_cron job 'chat-notifications-5min' (elke 5 min) doet via pg_net
een GET naar link.ribba.app/api/cron/chat-notifications met de CRON_SECRET
bearer (secret in Supabase Vault, niet in cron.job.command). Echte 5-min
cadence, geen Vercel-plan-upgrade nodig.
- Route-auth ongewijzigd (zelfde CRON_SECRET-check)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/api/notifications/opt-out/route.ts (1)
14-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider returning an HTML page instead of JSON for rate-limit errors.
Since this endpoint is visited directly in the browser by clicking an email link, returning a raw JSON response (as currently done in the rate-limit checks) provides a suboptimal user experience.
You could extend the
pagehelper to accept a custom HTTP status code and return a standard HTML error page when the user is rate-limited.♻️ Proposed refactor
-function page(title: string, body: string, extraHtml = ''): NextResponse { +function page(title: string, body: string, extraHtml = '', status = 200): NextResponse { return new NextResponse( `<!DOCTYPE html> <html lang="nl"><head><meta charset="utf-8"><meta name="viewport" content="width=device-width, initial-scale=1"> <meta name="robots" content="noindex"><title>${title} — Ribba</title> <style>body{font-family:Inter,-apple-system,sans-serif;background:`#F5F5F4`;display:flex;min-height:100vh;align-items:center;justify-content:center;margin:0;padding:16px} .card{background:`#fff`;border-radius:20px;padding:40px 28px;max-width:440px;text-align:center;box-shadow:0 1px 3px rgba(15,23,42,.06)} h1{font-size:22px;color:`#1C1917`;margin:0 0 12px}p{color:`#57534E`;font-size:15px;line-height:1.6;margin:0} button{background:`#2563EB`;color:`#fff`;font-weight:600;font-size:15px;padding:13px 26px;border-radius:12px;border:none;cursor:pointer;margin-top:24px}</style> </head><body><div class="card"><h1>${title}</h1><p>${body}</p>${extraHtml}</div></body></html>`, - { status: 200, headers: { 'Content-Type': 'text/html; charset=utf-8' } }, + { status, headers: { 'Content-Type': 'text/html; charset=utf-8' } }, ); }Then update the handlers to use it:
if (rateLimited(request)) { - return NextResponse.json({ error: 'Te veel verzoeken.' }, { status: 429 }); + return page('Te veel verzoeken', 'Je hebt dit te vaak geprobeerd. Probeer het later opnieuw.', '', 429); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/notifications/opt-out/route.ts` around lines 14 - 26, Update the rate-limit responses in the route handlers to return the existing page HTML helper instead of raw JSON, using a clear user-facing error message. Extend page to accept an optional HTTP status while preserving status 200 by default, and pass the rate-limit status when generating these error pages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/api/notifications/opt-out/route.ts`:
- Around line 14-26: Update the rate-limit responses in the route handlers to
return the existing page HTML helper instead of raw JSON, using a clear
user-facing error message. Extend page to accept an optional HTTP status while
preserving status 200 by default, and pass the rate-limit status when generating
these error pages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1ab7288-665f-4da0-b017-f6b8b15225d6
📒 Files selected for processing (8)
.gitignoreapp/api/cron/chat-notifications/route.tsapp/api/notifications/opt-out/route.tsapp/globals.csscomponents/chat/ChatGateway.tsxcomponents/chat/ChatThread.tsxlib/marketplace-types.tssupabase/migrations/20260711000000_marketplace_mvp.sql
🚧 Files skipped from review as they are similar to previous changes (6)
- components/chat/ChatThread.tsx
- lib/marketplace-types.ts
- components/chat/ChatGateway.tsx
- app/globals.css
- app/api/cron/chat-notifications/route.ts
- supabase/migrations/20260711000000_marketplace_mvp.sql
…e.a. Verwerkt de high-effort review-bevindingen op ribba-web#25 (migratie 20260718000000 + live toegepast op het gedeelde project): #1 inquiry_recipients: column-level SELECT — geclaimde deelnemers konden de bearer chat-tokens van de tegenpartij lezen; token/notified_email-kolommen ge-REVOKE't voor authenticated. #2 cron: throttle-stamp VÓÓR de send (+ revert bij send-fail) i.p.v. onbewaakt erna — voorkomt een duplicate-mailstorm als de stamp-UPDATE faalt. #3 inquiry-submit: no-email-scholen niet meer in de leerling-bevestigingsmail (ze worden nooit gecontacteerd en kunnen structureel niet reageren). #4 cron: retry-sweep voor recipients die na een mislukte outreach op 'pending' bleven (had geen enkel retry-pad). #5 opt-out: honoreert chat_tokens_expire_at — een verlopen/gelekt token kan niet langer account-breed de e-mailnotificaties uitzetten. #6 inquiry-submit: transactionele submit_inquiry-RPC (advisory lock op e-mail) tegen de dedupe-TOCTOU; vervangt read-then-insert + compenserende delete. #7 cron: list_notifiable_conversation_ids-RPC doet de kolom-vergelijking server-side, zodat de 200-cap niet vol loopt met al-genotificeerde conversaties en nieuwere niet verhongeren. #8 messages-INSERT-policy blokkeert nu status 'declined'/'expired'. #9 smart-banner appArgument: token → /r/{recipient_id} of /i/{inquiry_id} server-side geresolved, i.p.v. het door de app niet-geroute /chat/{token}. #10 (los commit) chat-notifications cron → Supabase pg_cron; deploy-blocker weg. tsc + build + lint schoon; submit_inquiry/dedupe/notifiable live geverifieerd op het gedeelde project, RPC-route end-to-end getest (RESEND uit, testrijen opgeruimd).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wat
Web-kant van Epic PolderLabs/ribba.app#35: schema + gedeelde RPC's (#36), inquiry-intake (#33), web-chat gateway (#42), smart app banners (#43) en reply-notificatie-mails (#44).
Architectuur
supabase/migrations/20260711000000_marketplace_mvp.sql(idempotent, gedeeld project):inquiries,inquiry_recipients,conversations,messages,marketplace_profiles+ RLS + realtime (incl.inquiry_recipients) + gedeelde SECURITY DEFINER RPC's — web-chat en ribbaPro-app gebruiken exact dezelfde semantiek:get_chat_context(p_token)— token → geanonimiseerde context; weigert verlopen tokens (30 dagen rolling, cron verlengt bij elke mail) voor niet-geclaimde bezoekersclaim_inquiry(p_inquiry_id)/claim_inquiry_recipient(p_recipient_id)— kale /i- en /r-ids zijn claimbaar (e-mail-match verplicht; rijschool tegen actueelcbr_rijscholen.email), conform het feat(registreren): oprichter krijgt school_role='owner' (eigenaar-SSOT) #36-contract en ribbaPro's MagicLinkLandingScreenget_inquiry_for_recipient— server-side masking (contact pas na accept)mark_messages_read— enige schrijfpad voorread_at; géén client-UPDATE opmessagesPOST /api/inquiry-submit— CORS, rate limit, 24u (e-mail × rijschool) dedupe, honeypotwebsite,marketing_optin, outreach-mails + leerling-bevestiging viaafter()/chat/[token]— OTP-gate → RPC-claim → realtime chat; banners alleen hierpush_tokens; verlengt token-expiry per verzonden mail/chat/{token}(primaire CTA) én universal link/r/{recipient_id}//i/{inquiry_id}("open in de app"). AASA dekt/i/*+/r/*; zonder app tonen die routes een download-fallback. Alle mail-links gebruiken uitsluitend link.ribba.app.Vereist vóór livegang
supabase/migrations/README.md) — er bestaat nu nergens een schema{{ .Token }}) + custom SMTP (Resend) in het Auth-dashboard — gedeeld met de apps, één configuratie dekt beidepush_tokens-kolomnaam verifiëren (aanname:user_id)cbr_rijscholen vullen✅ gedaan (vergelijker-sync, 6.916 rijen)Verificatie
tscschoon, build slaagt (routes/chat/[token],/i/[id],/r/[id], cron), lint = 9 pre-existing problemen; smoke-tests op validatie/honeypot/CORS/banners. E2E chat-flow na stap 1–2.Summary by CodeRabbit
New Features
Improvements
Documentation