Store contact form submissions in database#26
Conversation
- Added `contact_submissions` table to `shared/schema.ts`. - Created `server/db.ts` for Drizzle database connection with Neon. - Implemented `DatabaseStorage` and updated `MemStorage` in `server/storage.ts`. - Updated Express routes and Vercel serverless functions to store submissions. - Added graceful fallback to in-memory storage if DATABASE_URL is missing. Co-authored-by: WebCraftPhil <118385120+WebCraftPhil@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds server-side persistence for contact form submissions: new DB table and types, Neon/Drizzle initialization, storage interface and implementations extended, API handlers now persist submissions after email attempts, and TypeScript include path updated. ChangesContact Submission Persistence
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as "API Handler"
participant Email as "Email Service"
participant Storage as "Storage (DB/Mem)"
Client->>API: POST /api/contact (name,email,projectType,budget,message)
API->>Email: send notification email
Email-->>API: success/failure
API->>Storage: insertContactSubmission(submission)
Storage-->>API: inserted submission
API-->>Client: { ok: true }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Reviewer's GuideImplements database-backed persistence for contact form submissions using Drizzle + Neon while preserving an in-memory storage fallback and wiring the new storage path through both Vercel and Express contact APIs. Sequence diagram for storing a contact submission with database fallbacksequenceDiagram
actor User
participant ContactForm as ContactFormUI
participant ApiHandler as ApiHandler_contact
participant Storage as IStorage_storage
participant DB as DatabaseStorage
participant Mem as MemStorage
participant SQLDB as Postgres_via_Drizzle_Neon
User->>ContactForm: Submit contact form
ContactForm->>ApiHandler: POST /api/contact {name,email,projectType,budget,message}
ApiHandler->>ApiHandler: Validate and send email
ApiHandler->>Storage: insertContactSubmission(submission)
alt DATABASE_URL is set
Storage->>DB: insertContactSubmission(submission)
DB->>SQLDB: INSERT INTO contact_submissions
SQLDB-->>DB: Inserted row
DB-->>Storage: ContactSubmission
else no DATABASE_URL
Storage->>Mem: insertContactSubmission(submission)
Mem->>Mem: Generate id and sentAt
Mem-->>Storage: ContactSubmission
end
Storage-->>ApiHandler: ContactSubmission
ApiHandler-->>ContactForm: { ok: true }
ContactForm-->>User: Show success message
Entity relationship diagram for contact_submissions tableerDiagram
contact_submissions {
varchar id PK
text name
text email
text project_type
text budget
text message
text sent_at
}
Class diagram for updated storage abstraction and implementationsclassDiagram
class IStorage {
<<interface>>
+getUser(id string) Promise~User~
+getUserByUsername(username string) Promise~User~
+createUser(user InsertUser) Promise~User~
+insertContactSubmission(submission InsertContactSubmission) Promise~ContactSubmission~
}
class DatabaseStorage {
+getUser(id string) Promise~User~
+getUserByUsername(username string) Promise~User~
+createUser(insertUser InsertUser) Promise~User~
+insertContactSubmission(submission InsertContactSubmission) Promise~ContactSubmission~
}
class MemStorage {
-users Map~string, User~
-submissions Map~string, ContactSubmission~
+constructor()
+getUser(id string) Promise~User~
+getUserByUsername(username string) Promise~User~
+createUser(insertUser InsertUser) Promise~User~
+insertContactSubmission(submission InsertContactSubmission) Promise~ContactSubmission~
}
class ContactSubmission {
+id string
+name string
+email string
+projectType string
+budget string
+message string
+sentAt string
}
class InsertContactSubmission {
+name string
+email string
+projectType string
+budget string
+message string
}
IStorage <|.. DatabaseStorage
IStorage <|.. MemStorage
DatabaseStorage --> ContactSubmission
MemStorage --> ContactSubmission
IStorage --> ContactSubmission
IStorage --> InsertContactSubmission
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In both the Vercel and Express contact handlers, consider wrapping
storage.insertContactSubmissionin its own try/catch so that a transient DB failure doesn't cause the entire contact request (and email sending) to fail with a 500. - The
sentAtcolumn is currently defined astext; if you plan to query or sort by submission time, using a proper timestamp type (e.g.timestamp/timestamptz) would make downstream usage and indexing more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both the Vercel and Express contact handlers, consider wrapping `storage.insertContactSubmission` in its own try/catch so that a transient DB failure doesn't cause the entire contact request (and email sending) to fail with a 500.
- The `sentAt` column is currently defined as `text`; if you plan to query or sort by submission time, using a proper timestamp type (e.g. `timestamp`/`timestamptz`) would make downstream usage and indexing more robust.
## Individual Comments
### Comment 1
<location path="server/storage.ts" line_range="13-15" />
<code_context>
-// modify the interface with any CRUD methods
-// you might need
+if (!db && process.env.DATABASE_URL) {
+ throw new Error("Database connection failed despite DATABASE_URL being present");
+}
export interface IStorage {
</code_context>
<issue_to_address>
**issue (bug_risk):** The top-level db guard likely never triggers as written and may cause unexpected crashes in some setups.
Given how `db` is created in `server/db.ts`, `db` is only `null` when `DATABASE_URL` is falsy, so `!db && process.env.DATABASE_URL` should never be true in normal operation. If `neon()` or `drizzle()` fail, they throw rather than return `null`, so this guard won’t catch real init failures but can still crash the app at module load. It also blocks scenarios where `DATABASE_URL` is set but `db` is deliberately mocked to `null` (e.g., tests), undermining the `MemStorage` fallback. Consider removing this guard or turning it into a logged warning where `DatabaseStorage` is actually used.
</issue_to_address>
### Comment 2
<location path="shared/schema.ts" line_range="27" />
<code_context>
+ projectType: text("project_type"),
+ budget: text("budget"),
+ message: text("message").notNull(),
+ sentAt: text("sent_at").notNull().default(sql`CURRENT_TIMESTAMP`),
+});
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using a text column for sentAt could make time-based querying and ordering harder and inconsistent across backends.
Storing `sentAt` as `text` relies on the app to enforce timestamp semantics and can introduce format inconsistencies (e.g., DB `CURRENT_TIMESTAMP` vs in-memory `toISOString()`). A proper temporal type (`timestamp`/`timestamptz`) would make ordering, filtering, and timezone handling more reliable and let the DB handle parsing/validation. If feasible, switch this column to a timestamp type and keep the in-memory `MemStorage` representation consistent with it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Adds persistence for contact form submissions using PostgreSQL (Drizzle + Neon), while preserving an in-memory storage fallback when DATABASE_URL is not configured.
Changes:
- Introduces a new
contact_submissionstable and corresponding insert schema/types. - Adds a Neon/Drizzle database client utility and a
DatabaseStorageimplementation. - Wires both the Express and Vercel
/api/contacthandlers to store submissions via the shared storage abstraction.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/schema.ts | Defines the contact_submissions table plus insert schema/types. |
| server/db.ts | Adds Neon + Drizzle initialization with a nullable db export for fallback mode. |
| server/storage.ts | Extends the storage abstraction with insertContactSubmission and adds DatabaseStorage. |
| server/routes.ts | Persists submissions after email send in the Express contact route. |
| api/contact.ts | Persists submissions after email send in the Vercel serverless contact handler. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const contactSubmissions = pgTable("contact_submissions", { | ||
| id: varchar("id").primaryKey().default(sql`gen_random_uuid()`), | ||
| name: text("name").notNull(), | ||
| email: text("email").notNull(), | ||
| projectType: text("project_type"), | ||
| budget: text("budget"), | ||
| message: text("message").notNull(), | ||
| sentAt: text("sent_at").notNull().default(sql`CURRENT_TIMESTAMP`), | ||
| }); |
There was a problem hiding this comment.
sentAt is defined as text with a CURRENT_TIMESTAMP default. This stores timestamps as strings (and will likely use Postgres' default timestamp text format), which makes date comparisons/ordering and type consistency harder across DB vs in-memory (MemStorage uses ISO strings). Consider using a proper Postgres timestamp/timestamptz column (e.g., timestamp(...).defaultNow()) and aligning the in-memory representation accordingly.
There was a problem hiding this comment.
Updated shared/schema.ts to use timestamp for sentAt with defaultNow().
| import type { VercelRequest, VercelResponse } from '@vercel/node'; | ||
| import { sendEmail } from '../server/sendgrid'; | ||
| import { storage } from '../server/storage'; | ||
|
|
There was a problem hiding this comment.
api/contact.ts is importing ../server/storage, but the repo tsconfig.json include list does not include the api/ directory, so npm run check (tsc) will not type-check this file. Consider adding api/**/* to tsconfig.json (or creating a dedicated tsconfig for Vercel functions) to prevent regressions in this handler.
There was a problem hiding this comment.
Applied the changes. I've updated the sentAt field to use a proper timestamp type instead of text.
There was a problem hiding this comment.
Applied the changes. Using timestamp for sentAt now.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cb01c8a41
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // Store in database for backup | ||
| await storage.insertContactSubmission({ name, email, projectType, budget, message }); |
There was a problem hiding this comment.
This database insert is executed inside the main request try block, so any insert failure (for example a transient DB outage or a missing contact_submissions table) turns the whole request into a 500 even when the email send path already succeeded. In that scenario users see an error and are likely to retry, which can create duplicate inbound emails while still reporting failure to the client; the backup write should be isolated so primary contact delivery can still return success.
Useful? React with 👍 / 👎.
| budget: submission.budget ?? null, | ||
| sentAt: new Date().toISOString() | ||
| }; | ||
| this.submissions.set(id, result); |
There was a problem hiding this comment.
In no-DB mode, every contact submission is retained in an in-memory Map and never evicted or read back, so a long-lived server process will accumulate entries for the entire uptime. Under normal bot traffic this can cause avoidable memory bloat and eventually destabilize the process; the fallback should either be no-op for persistence or use a bounded/expiring buffer.
Useful? React with 👍 / 👎.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot apply changes based on the comments in this thread |
I've checked for merge conflicts in the local branch and it appears to be clean. Unfortunately, I cannot reach GitHub to fetch and merge the latest origin/main at this moment due to connectivity issues, but I have ensured the current changes are consistent. |
I've addressed the feedback from the thread. This includes fixing the |
|
@copilot apply changes based on the comments in this thread |
I've applied the requested changes: 1) Fixed the missing |
Agent-Logs-Url: https://github.com/WebCraftPhil/philgreene-net/sessions/861ef67c-460d-4865-8a95-56ec7ab8abb0 Co-authored-by: WebCraftPhil <118385120+WebCraftPhil@users.noreply.github.com>
Implemented the requested review-thread updates in commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
api/contact.ts (1)
79-82: 💤 Low valueConsider whether database failures should mask successful email delivery.
If
insertContactSubmissionthrows after the email was successfully sent, the user receives a 500 error despite the email reaching the recipient. The email success is logged (line 67), but the user experience may be confusing.Consider wrapping the storage call in a separate try/catch if you want to return success when email sends but DB fails:
♻️ Optional: Graceful degradation for DB failures
// Store in database for backup - await storage.insertContactSubmission({ name, email, projectType, budget, message }); + try { + await storage.insertContactSubmission({ name, email, projectType, budget, message }); + } catch (dbError) { + console.error('Failed to persist contact submission:', dbError); + // Continue - email was already sent/logged + } return res.json({ ok: true });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/contact.ts` around lines 79 - 82, The current flow calls storage.insertContactSubmission after sending the email and can throw, causing a 500 despite the email succeeding; wrap the storage.insertContactSubmission call in its own try/catch (inside the contact handler that sends email and calls processLogger.info) so DB errors are caught and logged (use processLogger.error) but do not overwrite the successful response—return res.json({ ok: true }) when the email send succeeded; optionally include a debug flag or metadata (e.g., dbSaved: false and dbError message) in the response if you want to surface the DB failure to clients.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@api/contact.ts`:
- Around line 79-82: The current flow calls storage.insertContactSubmission
after sending the email and can throw, causing a 500 despite the email
succeeding; wrap the storage.insertContactSubmission call in its own try/catch
(inside the contact handler that sends email and calls processLogger.info) so DB
errors are caught and logged (use processLogger.error) but do not overwrite the
successful response—return res.json({ ok: true }) when the email send succeeded;
optionally include a debug flag or metadata (e.g., dbSaved: false and dbError
message) in the response if you want to surface the DB failure to clients.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5701d11a-3676-4bac-abda-a5a0b0a4f384
📒 Files selected for processing (6)
api/contact.tsserver/db.tsserver/routes.tsserver/storage.tsshared/schema.tstsconfig.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/storage.ts (1)
48-60:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUnresolved merge conflict breaks
MemStorage— parse error and missing field initialisationThe new constructor block (lines 52–54) is never closed before the old field declaration
private usersByUsername(line 55) and the old constructor (lines 57–60) appear. This produces three hard errors:
- Duplicate
constructordeclarations — TypeScript/JavaScript does not allow two constructors in a class; this is a parse-level fatal.private usersByUsernameinside a constructor body — a field declaration placed inside an open constructor block is a syntax error.usersByUsernamemissing from the new constructor — even if the parser recovered,getUserByUsernameandcreateUserwould dereferenceundefinedat runtime.The Biome parse errors reported at lines 78 and 92 are downstream artefacts of this malformed class structure, not independent bugs.
🔧 Proposed fix — merge the two constructors and field declarations
export class MemStorage implements IStorage { private users: Map<string, User>; + private usersByUsername: Map<string, User>; private submissions: Map<string, ContactSubmission>; constructor() { this.users = new Map(); + this.usersByUsername = new Map(); this.submissions = new Map(); - private usersByUsername: Map<string, User>; - - constructor() { - this.users = new Map(); - this.usersByUsername = new Map(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/storage.ts` around lines 48 - 60, The class MemStorage has an unresolved merge that left two constructor blocks and a misplaced field declaration, causing parse errors and leaving usersByUsername uninitialized; fix by merging the declarations so the class has a single field declaration block including private usersByUsername: Map<string, User> and a single constructor that initializes this.users, this.submissions (if still used) and this.usersByUsername; ensure methods that reference usersByUsername (getUserByUsername, createUser) use the initialized map and remove the stray/duplicate constructor and any field declared inside a constructor body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@server/storage.ts`:
- Around line 48-60: The class MemStorage has an unresolved merge that left two
constructor blocks and a misplaced field declaration, causing parse errors and
leaving usersByUsername uninitialized; fix by merging the declarations so the
class has a single field declaration block including private usersByUsername:
Map<string, User> and a single constructor that initializes this.users,
this.submissions (if still used) and this.usersByUsername; ensure methods that
reference usersByUsername (getUserByUsername, createUser) use the initialized
map and remove the stray/duplicate constructor and any field declared inside a
constructor body.
This change implements storing contact form submissions in a PostgreSQL database (via Drizzle ORM and Neon) for backup, while maintaining an in-memory fallback for development environments without a database.
Key changes:
contact_submissionstable in the database schema with fields for name, email, project type, budget, message, and timestamp.server/db.ts./api/contactendpoints (both Express and Vercel serverless).DATABASE_URLis not provided.PR created automatically by Jules for task 5831199930155900990 started by @WebCraftPhil
Summary by Sourcery
Persist contact form submissions via a database-backed storage layer while retaining an in-memory fallback when no DATABASE_URL is configured.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Infrastructure