[UXIT-3791][filecoin.io V3] Hook up NewsletterForm [skip percy]#2181
[UXIT-3791][filecoin.io V3] Hook up NewsletterForm [skip percy]#2181barbaraperic wants to merge 7 commits intomainfrom
Conversation
…g and add API subscription route
…olledFormInput for improved form handling; update success message in useNewsletterForm hook; enhance Mailchimp subscription API error handling and response parsing.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
There was a problem hiding this comment.
Pull request overview
Adds a functional newsletter subscription flow to the Filecoin site by wiring the footer newsletter form to a new /api/subscribe endpoint that proxies Mailchimp’s embedded JSONP subscription endpoint, and centralizing client-side form handling in a custom hook.
Changes:
- Added a Next.js route handler to submit newsletter signups to Mailchimp and normalize responses to JSON.
- Introduced
useNewsletterFormto own form validation + submission + notification dialog behavior. - Refactored
NewsletterFormto use shared controlled form components and the new hook.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/filecoin-site/src/app/api/subscribe/route.ts | New API route that calls Mailchimp subscribe endpoint and returns normalized JSON responses. |
| apps/filecoin-site/src/app/_hooks/useNewsletterForm.ts | New hook for form schema, submission, and notification dialog interactions. |
| apps/filecoin-site/src/app/_components/Footer/NewsletterForm.tsx | Refactor footer newsletter UI to use ControlledForm + useNewsletterForm + NotificationDialog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| let data: { result?: string; msg?: string } | ||
| try { | ||
| const json = text.replace(/^[^(]+\(/, '').replace(/\)$/, '') |
There was a problem hiding this comment.
The JSONP stripping logic only removes the final ) and will fail to parse common JSONP formats that end with ); or have trailing whitespace/newlines. Make the extraction more robust (e.g., strip an optional trailing semicolon/whitespace and/or match the expected callback wrapper) so successful Mailchimp responses don't get treated as parse failures.
| const json = text.replace(/^[^(]+\(/, '').replace(/\)$/, '') | |
| const callbackRegex = new RegExp( | |
| `^\\s*${MAILCHIMP_JSONP_CALLBACK}\\s*\\(([\\s\\S]*?)\\)\\s*;?\\s*$`, | |
| ) | |
| const match = text.match(callbackRegex) | |
| if (!match) { | |
| throw new Error('Invalid Mailchimp JSONP format') | |
| } | |
| const json = match[1] |
There was a problem hiding this comment.
I'm not sure what this is doing
…ironment variables, improving error handling and user feedback in the subscription process, and updating translations for better user experience.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (data.result === 'error') { | ||
| return Response.json( | ||
| { ok: false, ...(data.msg && { message: data.msg }) }, | ||
| { status: 400 }, | ||
| ) | ||
| } | ||
|
|
||
| const isAlreadySubscribed = data.msg | ||
| ?.toLowerCase() | ||
| .includes('already subscribed') |
There was a problem hiding this comment.
The "already subscribed" check occurs after the error check, but "already subscribed" responses from Mailchimp likely have result='error'. This means already-subscribed emails will be returned as errors (status 400) instead of successes with isAlreadySubscribed=true. The logic should check for "already subscribed" within the error handling block, treating it as a special case that returns ok=true.
There was a problem hiding this comment.
It actually has result="success"
Mailchimp response: {
result: 'success',
msg: "You're already subscribed, your profile has been updated. Thank you!",
type: 'custom',
webEngagementCookieValue: null
}
| export async function POST(request: NextRequest) { | ||
| let body: { email?: unknown } | ||
|
|
||
| try { | ||
| body = await request.json() | ||
| } catch { | ||
| return Response.json({ ok: false }, { status: 400 }) | ||
| } | ||
|
|
||
| const { email } = body | ||
|
|
||
| if (!email || typeof email !== 'string') { | ||
| return Response.json({ ok: false }, { status: 400 }) | ||
| } | ||
|
|
||
| const baseUrl = getMailchimpSubscribeUrl() | ||
| if (!baseUrl) { | ||
| return Response.json({ ok: false }, { status: 503 }) | ||
| } | ||
|
|
||
| const response = await fetch( | ||
| `${baseUrl}&EMAIL=${encodeURIComponent(email)}&c=${MAILCHIMP_JSONP_CALLBACK}`, | ||
| { method: 'GET' }, | ||
| ) | ||
|
|
||
| if (!response.ok) { | ||
| return Response.json({ ok: false }, { status: 502 }) | ||
| } | ||
|
|
||
| const text = await response.text() | ||
|
|
||
| let data: { result?: string; msg?: string } | ||
| try { | ||
| const json = text.replace(/^[^(]+\(/, '').replace(/\)$/, '') | ||
| data = JSON.parse(json) | ||
| } catch { | ||
| console.error( | ||
| 'Failed to parse Mailchimp response as JSON from Mailchimp API', | ||
| ) | ||
| return Response.json({ ok: false }, { status: 502 }) | ||
| } | ||
|
|
||
| if (data.result === 'error') { | ||
| return Response.json( | ||
| { ok: false, ...(data.msg && { message: data.msg }) }, | ||
| { status: 400 }, | ||
| ) | ||
| } | ||
|
|
||
| const isAlreadySubscribed = data.msg | ||
| ?.toLowerCase() | ||
| .includes('already subscribed') | ||
|
|
||
| return Response.json({ ok: true, isAlreadySubscribed }) | ||
| } |
There was a problem hiding this comment.
The API route lacks rate limiting, which could allow abuse of the newsletter subscription endpoint. Consider implementing rate limiting middleware or using a service like Vercel's edge config to prevent spam submissions and protect the Mailchimp API quota.
There was a problem hiding this comment.
Mailchimp already has a limit rate
Mailchimp raw response: handle_response({"result":"error","msg":"Too many subscribe attempts for this email address. Please try again in about 5 minutes. (#6592)"})handle_response({"result":"error","msg":"There are errors below","errors":"handleRecordAndEmailValidation","type":"validation_error"})| const response = await fetch( | ||
| `${baseUrl}&EMAIL=${encodeURIComponent(email)}&c=${MAILCHIMP_JSONP_CALLBACK}`, | ||
| { method: 'GET' }, | ||
| ) | ||
|
|
There was a problem hiding this comment.
The fetch call to Mailchimp lacks a timeout configuration, which could cause the API route to hang indefinitely if Mailchimp's servers are unresponsive. Consider adding an AbortSignal with a reasonable timeout (e.g., 10 seconds) to prevent long-running requests.
| const response = await fetch( | |
| `${baseUrl}&EMAIL=${encodeURIComponent(email)}&c=${MAILCHIMP_JSONP_CALLBACK}`, | |
| { method: 'GET' }, | |
| ) | |
| const controller = new AbortController() | |
| const timeoutId = setTimeout(() => { | |
| controller.abort() | |
| }, 10_000) | |
| let response: Response | |
| try { | |
| response = await fetch( | |
| `${baseUrl}&EMAIL=${encodeURIComponent(email)}&c=${MAILCHIMP_JSONP_CALLBACK}`, | |
| { method: 'GET', signal: controller.signal }, | |
| ) | |
| } catch (error) { | |
| if ((error as Error).name === 'AbortError') { | |
| return Response.json({ ok: false }, { status: 504 }) | |
| } | |
| throw error | |
| } finally { | |
| clearTimeout(timeoutId) | |
| } |
| if (data.result === 'error') { | ||
| return Response.json( | ||
| { ok: false, ...(data.msg && { message: data.msg }) }, | ||
| { status: 400 }, |
There was a problem hiding this comment.
The Mailchimp error message is passed directly to the client without sanitization. While Mailchimp is likely to return safe content, consider sanitizing or limiting the message content to prevent potential XSS if Mailchimp's response were to be compromised or contain unexpected HTML/script content.
…uccessful subscription and enhancing error logging for Mailchimp API response parsing.
There was a problem hiding this comment.
I'm pasting here a suggestion for the API endpoint. Not sure if it works, let me know :)
import { type NextRequest } from 'next/server'
import { z } from 'zod'
const MAILCHIMP_JSONP_CALLBACK = 'handle_response'
const RequestSchema = z.object({
email: z.email(),
})
export async function POST(request: NextRequest) {
try {
const body = await request.json()
const { email } = RequestSchema.parse(body)
const baseUrl = getMailchimpSubscribeUrl()
// This might be encapsulated in getMailchimpSubscribeUrl too
const url = new URL(baseUrl)
url.searchParams.set('EMAIL', email)
url.searchParams.set('c', MAILCHIMP_JSONP_CALLBACK)
const response = await fetch(url, { method: 'GET' })
if (!response.ok) {
return Response.json({ ok: false }, { status: 502 })
}
const text = await response.text()
// handle response here.
} catch (error) {
return Response.json(
{ ok: false },
{ status: 400, statusText: String(error) },
)
}
}
function getMailchimpSubscribeUrl() {
const u = process.env.MAILCHIMP_U
const id = process.env.MAILCHIMP_LIST_ID
return `https://protocol.us16.list-manage.com/subscribe/post-json?u=${u}&id=${id}`
}| body: JSON.stringify({ email: values.email }), | ||
| }) | ||
|
|
||
| const body = await response.json().catch(() => ({})) |
There was a problem hiding this comment.
This catch clause is not doing anything, right?
| const body = await response.json().catch(() => ({})) | |
| const body = await response.json() |
| const MAILCHIMP_JSONP_CALLBACK = 'handle_response' | ||
|
|
||
| export async function POST(request: NextRequest) { | ||
| let body: { email?: unknown } |
There was a problem hiding this comment.
Why is this initialized here? What's the purpose of it?
| try { | ||
| body = await request.json() | ||
| } catch { | ||
| return Response.json({ ok: false }, { status: 400 }) | ||
| } | ||
|
|
||
| const { email } = body | ||
|
|
||
| if (!email || typeof email !== 'string') { | ||
| return Response.json({ ok: false }, { status: 400 }) | ||
| } |
There was a problem hiding this comment.
Could we use Zod here?
| const response = await fetch( | ||
| `${baseUrl}&EMAIL=${encodeURIComponent(email)}&c=${MAILCHIMP_JSONP_CALLBACK}`, | ||
| { method: 'GET' }, | ||
| ) |
There was a problem hiding this comment.
Would something like this work? Just for readability.
const url = new URL(baseUrl)
url.searchParams.set('EMAIL', email)
url.searchParams.set('c', MAILCHIMP_JSONP_CALLBACK)
const response = await fetch(url, { method: 'GET' })
| return Response.json({ ok: false }, { status: 502 }) | ||
| } | ||
|
|
||
| const text = await response.text() |
There was a problem hiding this comment.
This should probably be wrapped in a try/catch as well.
|
|
||
| let data: { result?: string; msg?: string } | ||
| try { | ||
| const json = text.replace(/^[^(]+\(/, '').replace(/\)$/, '') |
There was a problem hiding this comment.
I'm not sure what this is doing
| "description": "Enterprise-grade hot storage for AI & data lakes.", | ||
| "labels": ["Drag and drop", "S3-compatible"], | ||
| "keyFeatures": ["S3-compatible API", "Client-side encryption", "Access control"] | ||
| "keyFeatures": [ |
There was a problem hiding this comment.
This doesn't belong to this PR
📝 Description
This PR adds a functional newsletter subscription form that integrates with Mailchimp for the Filecoin site.
🛠️ Key Changes
NewsletterForm.tsx- Refactored to useControlledFormandControlledFormInputfor form handlinguseNewsletterForm.ts- New custom hook that manages form state, validation, and submission logic with notification dialogs for success/error feedbackapi/subscribe/route.ts- New API route that handles Mailchimp subscription requests with robust error handling for inconsistent API responsesNotes
MAILCHIMP_UandMAILCHIMP_LIST_IDenvironment variables to be set.🧪 How to Test
📸 Screenshots