Added as much tests as possible - #50
Conversation
- Wire up Like/Nope/SuperLike buttons to dismiss the top card with directional fly-off animations (right/left/up) - Add pointer event handlers to the top card for swipe detection: swipe right = like, swipe left = nope, swipe up = super like - Show LIKE/NOPE/SUPER overlay labels that fade in while dragging - Double-tap cycles through a profile's 3 photos with dot indicators - Each profile now generates an imgs[] array (3 unique Unsplash seeds) - Add onerror fallback to picsum.photos for stale Unsplash photo IDs - Show empty-state message when all cards in the deck are exhausted - Add touch-action: none to prevent browser hijacking pointer events Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace innerHTML on titleRow with explicit createElement/textContent to eliminate a potential injection vector if profile data ever comes from an external source (CodeRabbit nitpick) - Replace sort-based shuffle with Fisher-Yates in generateProfiles so image seed selection is uniformly random (CodeRabbit nitpick) - Track the dismissal setTimeout id in a module-level dismissTimerId and call clearTimeout in renderDeck so a Shuffle click during a card's fly-off animation cannot fire a stale callback that attaches duplicate pointer-event listeners to the new deck (CodeRabbit inline comment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements a complete full-stack dating app with multi-photo profile cards, gesture-based interactions, backend swipe recording with simulated matching, web push notifications, and comprehensive test coverage across frontend and backend layers. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Client as App.js
participant Server as Express Server
participant DB as SQLite
participant PushSvc as Push Service
participant Worker as Service Worker
User->>Client: Swipe Card (left/right)
Client->>Client: dismissTop(direction)
Client->>Server: POST /api/swipes<br/>(profileId, action, userId)
Server->>DB: INSERT swipe + check match
DB-->>Server: swipe recorded, match result
alt Match Detected
Server->>Server: fetch user subscriptions
Server->>PushSvc: webpush.sendNotification()
PushSvc-->>Worker: push event
Worker->>User: showNotification("Match!")
User->>Worker: click notification
Worker->>User: focus/open match page
end
Server-->>Client: 201 + swipe id, matched status
Client->>Client: animate card out, show next card
sequenceDiagram
participant User as User (Browser)
participant Client as App.js
participant Server as Express Server
participant DB as SQLite
participant PushMgr as PushManager
User->>Client: Page Load
Client->>Client: setupPushNotifications()
Client->>Client: register service worker (/sw.js)
Client->>User: request notification permission
User-->>Client: permission granted
Client->>Server: GET /api/push/vapid-public-key
Server-->>Client: publicKey
Client->>PushMgr: subscribe(vapidPublicKey)
PushMgr-->>Client: subscription object
Client->>Server: POST /api/push/subscribe<br/>(userId, subscription)
Server->>DB: UPSERT push_subscriptions
DB-->>Server: stored
Server-->>Client: 201 success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
JamezM546
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app.js`:
- Line 226: The API_BASE constant is hard-coded to "http://localhost:3000",
which breaks non-local and HTTPS deployments; update the definition of API_BASE
in app.js (the API_BASE constant) to use a relative base or derive it from the
current origin (e.g., use window.location.origin or an empty string/relative
path) so API calls are same-origin and avoid mixed-content/cors issues; ensure
any code that concatenates endpoints with API_BASE still forms valid URLs after
this change.
- Around line 242-253: The POST fetch in recordSwipe currently only catches
network errors; update recordSwipe to await the fetch response, check
response.ok, and handle non-2xx responses (e.g., log an error with
response.status and response text or throw) instead of silently succeeding;
likewise, add the same response.ok check for the fetch to
`${API_BASE}/api/push/subscribe` (the call that currently logs "subscribed
successfully") so that you only log success when response.ok is true and
otherwise log/handle the server error. Ensure you reference the same call sites
(recordSwipe, getOrCreateUserId usage, and the fetch to `/api/push/subscribe`)
and preserve existing catch handling for network errors.
In `@backend/package.json`:
- Around line 7-9: package.json currently runs Node with --experimental-sqlite
in scripts "start", "dev", and "test" but lacks an engines constraint, which can
allow installs on Node versions that lack the built-in node:sqlite module; add
an "engines" field to package.json specifying a minimum Node version that
supports node:sqlite (e.g. "engines": { "node": ">=22.5.0" }) so package
managers and deployers enforce the runtime requirement and prevent runtime
"Cannot find module 'node:sqlite'" errors referenced by the require of
'node:sqlite' in db.js.
In `@backend/routes/swipes.js`:
- Around line 41-61: The insert currently ignores the client-provided userId
(variables userId and cleanUserId) so swipes aren't owned; update the DB write
and reads: validate and require userId (trim and reject empty), alter the INSERT
prepared via db.prepare/stmt to include an owner column (e.g. user_id) and pass
cleanUserId into stmt.run, and ensure the swipes table schema contains that
column; also update the read endpoints that query the swipes table to filter by
the authenticated/validated userId so history/stats are per-user rather than
global.
- Around line 40-82: The route handler is marked async but has no awaits,
causing thrown synchronous errors (e.g., from db.prepare or stmt.run) to become
unhandled rejections; remove the async keyword from the router.post handler
declaration so synchronous exceptions from db.prepare/stmt.run propagate to
Express error middleware (or alternatively wrap the handler body in try/catch
and call next(err)); keep the fire-and-forget pattern for sendMatchNotification
(it already has .catch) so it remains non-blocking.
In `@backend/server.js`:
- Around line 16-17: The app currently serves the repository root via
app.use(express.static(path.join(__dirname, '..'))), exposing backend files and
secrets (e.g., backend/vapid_keys.json); change static hosting to a dedicated
frontend/public directory and update the express.static call to point only to
that folder (locate the express.static usage and the path.join call in server.js
and replace the root path with the new public/frontend directory), and ensure
any references to __dirname or relative paths align with the new folder so
backend files (like vapid_keys.json and source/test files) are no longer
web-accessible.
In `@backend/vapid.js`:
- Around line 7-18: The module currently generates and writes VAPID keys as a
side effect at import time via KEYS_PATH, loadOrGenerateKeys, and the vapidKeys
top-level call; change this so the module never writes or creates keys on
require: remove the automatic call that sets vapidKeys at module load, make
loadOrGenerateKeys (or better, getVapidKeys) only read keys from environment
(e.g., process.env.VAPID_PUBLIC_KEY / VAPID_PRIVATE_KEY) or an existing file and
throw/return null if missing, and export a separate explicit function (e.g.,
initializeVapidKeysForDev or generateVapidKeysToFile) that a developer-run
bootstrap script can call to generate/write keys using
webpush.generateVAPIDKeys; ensure server code uses the exported getter and fails
loudly if keys are not provided rather than creating them automatically.
In `@sw.js`:
- Around line 31-35: The notification handler is comparing
event.notification.data.url (often a relative path like "/") against absolute
client.url, so existing tabs aren't matched; normalize the notification URL to
an absolute URL using the service worker origin (e.g., via new
URL(event.notification.data.url, self.location.origin)) and use that normalized
URL when iterating windowClients and when calling client.focus() or
clients.openWindow; update references to event.notification.data.url,
windowClients, client.url, client.focus(), and clients.openWindow to use the
normalized absolute URL for matching and opening.
In `@test/frontend.test.js`:
- Around line 2-33: The test imports generateProfiles from data.js but the app
now uses a different generator that produces imgs (plural) in app.js, so update
the test to exercise the same generator the UI uses: either import the shared
generator used by app.js (or extract it from app.js into a common module) and
replace references to profile.img with profile.imgs (and assert imgs is an array
of image URLs with each entry starting with 'https://images.unsplash'), or
change the import to pull the generator from app.js (the function producing
imgs) and adjust assertions to check profile.imgs array shape and contents
instead of profile.img.
In `@vitest.config.js`:
- Line 17: The config currently uses resolve(__dirname) (alias mapping '@':
resolve(__dirname)) but __dirname is unavailable in ESM; fix by importing
dirname and fileURLToPath from 'path'/'url' (or node builtins) and create a
local __dirname via dirname(fileURLToPath(import.meta.url)), then replace
resolve(__dirname) use with resolve(__dirname) (the newly defined constant) so
the alias mapping in vitest.config.js works at runtime; ensure this
initialization appears before the alias object is constructed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 221f451a-788c-422e-bde1-d9edbb9f575f
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
app.jsbackend/.gitignorebackend/db.jsbackend/package.jsonbackend/routes/push.jsbackend/routes/swipes.jsbackend/server.jsbackend/tests/push.test.jsbackend/tests/swipes.test.jsbackend/vapid.jsindex.htmlpackage.jsonstyles.csssw.jstest/frontend.test.jsvitest.config.js
| // ------------------- | ||
| // Backend API | ||
| // ------------------- | ||
| const API_BASE = "http://localhost:3000"; |
There was a problem hiding this comment.
Don't pin the client to localhost.
backend/server.js now serves the frontend and API from the same origin, so hard-coding http://localhost:3000 breaks any non-local deployment and HTTPS because these requests become cross-origin or mixed-content. Use relative URLs or derive the base from window.location.
Suggested fix
-const API_BASE = "http://localhost:3000";
+const API_BASE = window.location.origin;📝 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.
| const API_BASE = "http://localhost:3000"; | |
| const API_BASE = window.location.origin; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app.js` at line 226, The API_BASE constant is hard-coded to
"http://localhost:3000", which breaks non-local and HTTPS deployments; update
the definition of API_BASE in app.js (the API_BASE constant) to use a relative
base or derive it from the current origin (e.g., use window.location.origin or
an empty string/relative path) so API calls are same-origin and avoid
mixed-content/cors issues; ensure any code that concatenates endpoints with
API_BASE still forms valid URLs after this change.
| function recordSwipe(profile, action) { | ||
| fetch(`${API_BASE}/api/swipes`, { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ | ||
| profileId: profile.id, | ||
| profileName: profile.name, | ||
| action, | ||
| userId: getOrCreateUserId(), | ||
| }), | ||
| }).catch(err => console.warn("Swipe not recorded:", err.message)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l app.jsRepository: kelloggm/ai-tinder-fork
Length of output: 77
🏁 Script executed:
cat -n app.js | sed -n '235,315p'Repository: kelloggm/ai-tinder-fork
Length of output: 3186
Add response.ok checks to handle HTTP error responses.
fetch only rejects on network failures. A 400/500 from /api/swipes currently falls through without error handling, and /api/push/subscribe will still log "subscribed successfully" even if the backend rejects the request with an error status code. Both calls need to check response.ok before proceeding.
Suggested fix
function recordSwipe(profile, action) {
- fetch(`${API_BASE}/api/swipes`, {
+ return fetch(`${API_BASE}/api/swipes`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({
profileId: profile.id,
profileName: profile.name,
action,
userId: getOrCreateUserId(),
}),
- }).catch(err => console.warn("Swipe not recorded:", err.message));
+ })
+ .then(resp => {
+ if (!resp.ok) throw new Error(`Swipe API returned ${resp.status}`);
+ })
+ .catch(err => console.warn("Swipe not recorded:", err.message));
}
@@
- await fetch(`${API_BASE}/api/push/subscribe`, {
+ const subscribeResp = await fetch(`${API_BASE}/api/push/subscribe`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ userId, subscription }),
});
+ if (!subscribeResp.ok) {
+ throw new Error(`Push subscribe failed: ${subscribeResp.status}`);
+ }Also applies to: 300-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app.js` around lines 242 - 253, The POST fetch in recordSwipe currently only
catches network errors; update recordSwipe to await the fetch response, check
response.ok, and handle non-2xx responses (e.g., log an error with
response.status and response text or throw) instead of silently succeeding;
likewise, add the same response.ok check for the fetch to
`${API_BASE}/api/push/subscribe` (the call that currently logs "subscribed
successfully") so that you only log success when response.ok is true and
otherwise log/handle the server error. Ensure you reference the same call sites
(recordSwipe, getOrCreateUserId usage, and the fetch to `/api/push/subscribe`)
and preserve existing catch handling for network errors.
| "start": "node --experimental-sqlite server.js", | ||
| "dev": "node --experimental-sqlite --watch server.js", | ||
| "test": "node --experimental-sqlite --test tests/*.test.js" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "backend/package.json engines field:"
jq '.engines // "MISSING"' backend/package.json
echo
echo "Repository references requiring sqlite experimental runtime:"
rg -n --type=json --type=js 'node:sqlite|--experimental-sqlite'Repository: kelloggm/ai-tinder-fork
Length of output: 484
🌐 Web query:
Node.js node:sqlite minimum version requirement
💡 Result:
The node:sqlite module requires Node.js version 22.5.0 or later. It was added in v22.5.0 as an experimental feature (initially requiring --experimental-sqlite flag), with stability improvements in later versions (no longer requiring the flag starting v23.4.0/v22.13.0, release candidate in v25.7.0). As of 2026-03-23, it is available in Node.js v22+ branches.
Citations:
- 1: https://nodejs.org/api/sqlite.html
- 2: https://nodejs.org/docs/latest/api/sqlite.html
- 3: https://nodejs.org/download/release/v22.12.0/docs/api/sqlite.html
- 4: lib,src,test,doc: add node:sqlite module nodejs/node#53752
- 5: https://nodejs.org/download/release/v23.8.0/docs/api/sqlite.html
- 6: https://nodejs.org/download/release/v22.9.0/docs/api/sqlite.html
🏁 Script executed:
#!/bin/bash
# Check for other experimental features or Node version constraints
rg -n 'experimental-|--loader|--no-warnings' backend/Repository: kelloggm/ai-tinder-fork
Length of output: 311
Declare an explicit Node runtime requirement for SQLite support.
Lines 7-9 depend on --experimental-sqlite flag and require the node:sqlite module (line 3 in backend/db.js). Without an engines.node contract, installs can succeed on incompatible Node versions (< 22.5.0) and then fail at runtime with Cannot find module 'node:sqlite'.
💡 Suggested manifest guard
{
"name": "snow-day-backend",
"version": "1.0.0",
"description": "Swipe action API for the Tinder clone snow-day activity",
"main": "server.js",
+ "engines": {
+ "node": ">=22"
+ },
"scripts": {📝 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.
| "start": "node --experimental-sqlite server.js", | |
| "dev": "node --experimental-sqlite --watch server.js", | |
| "test": "node --experimental-sqlite --test tests/*.test.js" | |
| { | |
| "name": "snow-day-backend", | |
| "version": "1.0.0", | |
| "description": "Swipe action API for the Tinder clone snow-day activity", | |
| "main": "server.js", | |
| "engines": { | |
| "node": ">=22" | |
| }, | |
| "scripts": { | |
| "start": "node --experimental-sqlite server.js", | |
| "dev": "node --experimental-sqlite --watch server.js", | |
| "test": "node --experimental-sqlite --test tests/*.test.js" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/package.json` around lines 7 - 9, package.json currently runs Node
with --experimental-sqlite in scripts "start", "dev", and "test" but lacks an
engines constraint, which can allow installs on Node versions that lack the
built-in node:sqlite module; add an "engines" field to package.json specifying a
minimum Node version that supports node:sqlite (e.g. "engines": { "node":
">=22.5.0" }) so package managers and deployers enforce the runtime requirement
and prevent runtime "Cannot find module 'node:sqlite'" errors referenced by the
require of 'node:sqlite' in db.js.
| router.post('/', async (req, res) => { | ||
| const { profileId, profileName, action, userId } = req.body; | ||
|
|
||
| if (!profileId || typeof profileId !== 'string' || profileId.trim() === '') { | ||
| return res.status(400).json({ error: 'profileId is required and must be a non-empty string.' }); | ||
| } | ||
| if (!profileName || typeof profileName !== 'string' || profileName.trim() === '') { | ||
| return res.status(400).json({ error: 'profileName is required and must be a non-empty string.' }); | ||
| } | ||
| if (!action || !VALID_ACTIONS.has(action)) { | ||
| return res.status(400).json({ error: `action must be one of: ${[...VALID_ACTIONS].join(', ')}.` }); | ||
| } | ||
|
|
||
| const cleanId = profileId.trim(); | ||
| const cleanName = profileName.trim(); | ||
| const cleanUserId = userId && typeof userId === 'string' ? userId.trim() : null; | ||
| const swipedAt = new Date().toISOString(); | ||
|
|
||
| const stmt = db.prepare( | ||
| 'INSERT INTO swipes (profile_id, profile_name, action, swiped_at) VALUES (?, ?, ?, ?)' | ||
| ); | ||
| const result = stmt.run(cleanId, cleanName, action, swipedAt); | ||
|
|
||
| // Simulate match for like/superlike and send push notification if subscribed | ||
| const chance = MATCH_CHANCE[action]; | ||
| const isMatch = chance !== undefined && Math.random() < chance; | ||
|
|
||
| if (isMatch && cleanUserId) { | ||
| // Fire-and-forget — don't block the response | ||
| sendMatchNotification(cleanUserId, cleanName).catch(err => | ||
| console.error('Match notification failed:', err.message) | ||
| ); | ||
| } | ||
|
|
||
| return res.status(201).json({ | ||
| id: result.lastInsertRowid, | ||
| profileId: cleanId, | ||
| profileName: cleanName, | ||
| action, | ||
| swipedAt, | ||
| matched: isMatch, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check Express version and look for sendMatchNotification implementation
cd backend || cd .
# Look for package.json to confirm Express version
if [ -f "package.json" ]; then
grep -A 5 '"express"' package.json | head -10
fi
# Find sendMatchNotification function
rg -n "sendMatchNotification" --type js -B 2 -A 10Repository: kelloggm/ai-tinder-fork
Length of output: 1388
🌐 Web query:
Express 4.x async route handler error handling unhandled rejection
💡 Result:
In Express 4.x, async route handlers do not automatically forward errors or rejected promises to Express error handling middleware. If an async function throws an error or a promise rejects without being caught, it results in an unhandled promise rejection, which Node.js warns about (and crashes the process in Node.js 15+ by default). Express 4.x does not await async handlers or catch their rejections natively. To properly handle errors in Express 4.x async route handlers and prevent unhandled rejections: 1. Use an async wrapper middleware that catches promise rejections and calls next(err): const asyncHandler = (fn) => (req, res, next) => { Promise.resolve(fn(req, res, next)).catch(next); }; // Usage: app.get('/users/:id', asyncHandler(async (req, res) => { const user = await User.findById(req.params.id); res.json(user); })); 2. Add a central error handling middleware at the end of your middleware stack (must have 4 parameters: (err, req, res, next)): app.use((err, req, res, next) => { console.error(err.stack); res.status(500).json({ error: 'Something broke!' }); }); 3. As a safety net, handle process-level unhandled rejections (logs but does not forward to specific request): process.on('unhandledRejection', (reason, promise) => { console.error('Unhandled Rejection at:', promise, 'reason:', reason); }); Alternative: Use the express-async-errors package, which patches Express routers globally to handle async errors: require('express-async-errors'); // Now async handlers work without wrappers app.get('/users', async (req, res) => { // errors auto-forwarded to error middleware }); Note: This applies specifically to Express 4.x (latest 4.22.1 as of 2026). Express 5.x (latest 5.2.1) natively supports async error handling without wrappers, automatically calling next(err) on rejections. Example of broken Express 4.x code causing unhandled rejection: app.get('/', async (req, res) => { throw new Error('broken'); // Unhandled rejection! }); With wrapper, the error reaches the error middleware instead.
Citations:
- 1: https://oneuptime.com/blog/post/2026-02-02-express-error-handling/view
- 2: https://expressjs.com/en/guide/error-handling
- 3: https://stackoverflow.com/questions/33410101/unhandled-rejections-in-express-applications
- 4: https://medium.com/@Adekola_Olawale/how-to-fix-unhandled-rejection-errors-in-node-js-applications-1665c87bbdfc
- 5: https://expressjs.com/en/guide/error-handling.html
- 6: https://oneuptime.com/blog/post/2026-01-25-handle-async-await-errors-properly/view
- 7: https://expressjs.com/en/advanced/best-practice-performance.html
- 8: https://www.npmjs.com/package/express?activeTab=versions
- 9: https://dev.to/siddharth_g/express-5-brings-built-in-promise-support-for-error-handling-5bjf
- 10: https://medium.com/@priyanshu0dubey/express-js-version-5-a-detailed-comparison-with-version-4-71c46c269082
- 11: https://dev.to/mahmud007/goodbye-asynchandler-native-async-support-in-express-5-2o9p
- 12: https://www.npmjs.com/package/express-async-errors
Remove async from this route handler or wrap it with proper error handling.
In Express 4.19.x, async route handlers without await statements create a trap: if db.prepare() or stmt.run() throw an error, the error becomes an unhandled promise rejection that bypasses the error middleware entirely. This turns database failures into crashes or unhandled rejections instead of proper 500 responses.
Suggested fix
-router.post('/', async (req, res) => {
+router.post('/', (req, res, next) => {
+ try {
const { profileId, profileName, action, userId } = req.body;
@@
if (isMatch && cleanUserId) {
- sendMatchNotification(cleanUserId, cleanName).catch(err =>
+ void sendMatchNotification(cleanUserId, cleanName).catch(err =>
console.error('Match notification failed:', err.message)
);
}
@@
return res.status(201).json({
id: result.lastInsertRowid,
profileId: cleanId,
profileName: cleanName,
action,
swipedAt,
matched: isMatch,
});
+ } catch (err) {
+ return next(err);
+ }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routes/swipes.js` around lines 40 - 82, The route handler is marked
async but has no awaits, causing thrown synchronous errors (e.g., from
db.prepare or stmt.run) to become unhandled rejections; remove the async keyword
from the router.post handler declaration so synchronous exceptions from
db.prepare/stmt.run propagate to Express error middleware (or alternatively wrap
the handler body in try/catch and call next(err)); keep the fire-and-forget
pattern for sendMatchNotification (it already has .catch) so it remains
non-blocking.
| const { profileId, profileName, action, userId } = req.body; | ||
|
|
||
| if (!profileId || typeof profileId !== 'string' || profileId.trim() === '') { | ||
| return res.status(400).json({ error: 'profileId is required and must be a non-empty string.' }); | ||
| } | ||
| if (!profileName || typeof profileName !== 'string' || profileName.trim() === '') { | ||
| return res.status(400).json({ error: 'profileName is required and must be a non-empty string.' }); | ||
| } | ||
| if (!action || !VALID_ACTIONS.has(action)) { | ||
| return res.status(400).json({ error: `action must be one of: ${[...VALID_ACTIONS].join(', ')}.` }); | ||
| } | ||
|
|
||
| const cleanId = profileId.trim(); | ||
| const cleanName = profileName.trim(); | ||
| const cleanUserId = userId && typeof userId === 'string' ? userId.trim() : null; | ||
| const swipedAt = new Date().toISOString(); | ||
|
|
||
| const stmt = db.prepare( | ||
| 'INSERT INTO swipes (profile_id, profile_name, action, swiped_at) VALUES (?, ?, ?, ?)' | ||
| ); | ||
| const result = stmt.run(cleanId, cleanName, action, swipedAt); |
There was a problem hiding this comment.
Persist the swipe owner on write.
userId is accepted from the client but ignored in the insert. The read endpoints below therefore only have enough information to return global history/stats across every browser, which leaks other users' activity and makes per-user stats impossible. Store the owner and filter reads by it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routes/swipes.js` around lines 41 - 61, The insert currently ignores
the client-provided userId (variables userId and cleanUserId) so swipes aren't
owned; update the DB write and reads: validate and require userId (trim and
reject empty), alter the INSERT prepared via db.prepare/stmt to include an owner
column (e.g. user_id) and pass cleanUserId into stmt.run, and ensure the swipes
table schema contains that column; also update the read endpoints that query the
swipes table to filter by the authenticated/validated userId so history/stats
are per-user rather than global.
| // Serve frontend static files from the project root | ||
| app.use(express.static(path.join(__dirname, '..'))); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the exact code in backend/server.js at lines 16-17
sed -n '10,25p' backend/server.jsRepository: kelloggm/ai-tinder-fork
Length of output: 404
🏁 Script executed:
# Check the vapid.js file for default KEYS_PATH
cat backend/vapid.jsRepository: kelloggm/ai-tinder-fork
Length of output: 706
🏁 Script executed:
# Search for vapid_keys.json references in the codebase
rg "vapid_keys" --type jsRepository: kelloggm/ai-tinder-fork
Length of output: 173
🏁 Script executed:
# Check if vapid_keys.json exists
find . -name "vapid_keys.json" -o -name "*vapid*keys*"Repository: kelloggm/ai-tinder-fork
Length of output: 49
🏁 Script executed:
# Check .gitignore to see if vapid_keys.json is protected
cat .gitignore 2>/dev/null || echo "No .gitignore found"Repository: kelloggm/ai-tinder-fork
Length of output: 86
🏁 Script executed:
# Check the overall directory structure to understand the app layout
tree -L 2 -I 'node_modules' 2>/dev/null || find . -maxdepth 2 -type d | head -20Repository: kelloggm/ai-tinder-fork
Length of output: 428
Serving the repository root exposes backend files and secrets.
express.static(path.join(__dirname, '..')) makes the entire repository web-accessible. Since backend/vapid.js writes the VAPID private key to backend/vapid_keys.json by default, it becomes fetchable at /backend/vapid_keys.json. Additionally, backend source code and test files are exposed. The repository has no .gitignore to provide any protection. Create a dedicated public/ or frontend/ directory for static assets and configure express.static to serve only that directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/server.js` around lines 16 - 17, The app currently serves the
repository root via app.use(express.static(path.join(__dirname, '..'))),
exposing backend files and secrets (e.g., backend/vapid_keys.json); change
static hosting to a dedicated frontend/public directory and update the
express.static call to point only to that folder (locate the express.static
usage and the path.join call in server.js and replace the root path with the new
public/frontend directory), and ensure any references to __dirname or relative
paths align with the new folder so backend files (like vapid_keys.json and
source/test files) are no longer web-accessible.
| const KEYS_PATH = process.env.VAPID_KEYS_PATH || path.join(__dirname, 'vapid_keys.json'); | ||
|
|
||
| function loadOrGenerateKeys() { | ||
| if (fs.existsSync(KEYS_PATH)) { | ||
| return JSON.parse(fs.readFileSync(KEYS_PATH, 'utf8')); | ||
| } | ||
| const keys = webpush.generateVAPIDKeys(); | ||
| fs.writeFileSync(KEYS_PATH, JSON.stringify(keys, null, 2)); | ||
| return keys; | ||
| } | ||
|
|
||
| const vapidKeys = loadOrGenerateKeys(); |
There was a problem hiding this comment.
Don't generate VAPID keys as a module-import side effect.
Requiring backend/server.js pulls this module in and can create backend/vapid_keys.json during test or server startup. That makes the suite depend on a writable checkout, and stateless deployments can silently rotate keys and invalidate existing subscriptions. Prefer stable keys from environment/secret storage and keep generation as an explicit dev bootstrap step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/vapid.js` around lines 7 - 18, The module currently generates and
writes VAPID keys as a side effect at import time via KEYS_PATH,
loadOrGenerateKeys, and the vapidKeys top-level call; change this so the module
never writes or creates keys on require: remove the automatic call that sets
vapidKeys at module load, make loadOrGenerateKeys (or better, getVapidKeys) only
read keys from environment (e.g., process.env.VAPID_PUBLIC_KEY /
VAPID_PRIVATE_KEY) or an existing file and throw/return null if missing, and
export a separate explicit function (e.g., initializeVapidKeysForDev or
generateVapidKeysToFile) that a developer-run bootstrap script can call to
generate/write keys using webpush.generateVAPIDKeys; ensure server code uses the
exported getter and fails loudly if keys are not provided rather than creating
them automatically.
| const url = event.notification.data.url; | ||
| // Focus an existing window if one is open | ||
| for (const client of windowClients) { | ||
| if (client.url === url && 'focus' in client) { | ||
| return client.focus(); |
There was a problem hiding this comment.
Normalize notification target URL before matching clients.
Line 31 uses payload URLs like '/' (see backend/routes/swipes.js), but Line 34 compares against absolute client.url, so existing tabs are often not focused and a new tab is opened on Line 40.
💡 Proposed fix
self.addEventListener('notificationclick', event => {
event.notification.close();
event.waitUntil(
clients.matchAll({ type: 'window', includeUncontrolled: true }).then(windowClients => {
- const url = event.notification.data.url;
+ const targetUrl = new URL(
+ event.notification?.data?.url || '/',
+ self.location.origin
+ ).href;
// Focus an existing window if one is open
for (const client of windowClients) {
- if (client.url === url && 'focus' in client) {
+ if (client.url === targetUrl && 'focus' in client) {
return client.focus();
}
}
// Otherwise open a new window
if (clients.openWindow) {
- return clients.openWindow(url);
+ return clients.openWindow(targetUrl);
}
})
);
});Also applies to: 40-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sw.js` around lines 31 - 35, The notification handler is comparing
event.notification.data.url (often a relative path like "/") against absolute
client.url, so existing tabs aren't matched; normalize the notification URL to
an absolute URL using the service worker origin (e.g., via new
URL(event.notification.data.url, self.location.origin)) and use that normalized
URL when iterating windowClients and when calling client.focus() or
clients.openWindow; update references to event.notification.data.url,
windowClients, client.url, client.focus(), and clients.openWindow to use the
normalized absolute URL for matching and opening.
| import { generateProfiles } from '../data.js'; | ||
|
|
||
| describe('generateProfiles', () => { | ||
| it('returns 12 profiles by default', () => { | ||
| const profiles = generateProfiles(); | ||
| expect(profiles.length).toBe(12); | ||
| }); | ||
|
|
||
| it('accepts custom count', () => { | ||
| const profiles = generateProfiles(3); | ||
| expect(profiles.length).toBe(3); | ||
| }); | ||
|
|
||
| it('profiles have required properties', () => { | ||
| const profiles = generateProfiles(2); | ||
|
|
||
| profiles.forEach(profile => { | ||
| expect(profile.id).toBeDefined(); | ||
| expect(typeof profile.name).toBe('string'); | ||
| expect(profile.name.length).toBeGreaterThan(0); | ||
| expect(typeof profile.age).toBe('number'); | ||
| expect(profile.age).toBeGreaterThanOrEqual(18); | ||
| expect(profile.age).toBeLessThan(40); | ||
| expect(typeof profile.city).toBe('string'); | ||
| expect(typeof profile.title).toBe('string'); | ||
| expect(typeof profile.bio).toBe('string'); | ||
| expect(Array.isArray(profile.tags)).toBe(true); | ||
| expect(profile.tags.length).toBeGreaterThanOrEqual(3); | ||
| expect(profile.tags.every(tag => typeof tag === 'string')).toBe(true); | ||
|
|
||
| expect(typeof profile.img).toBe('string'); | ||
| expect(profile.img.startsWith('https://images.unsplash')).toBe(true); |
There was a problem hiding this comment.
This suite is exercising the wrong profile generator.
It imports generateProfiles from data.js, but the changed UI path in app.js now creates and consumes imgs. Because data.js still returns img, this suite can stay green while the actual card rendering path is broken. Please retarget the test to the app.js path, or extract one shared generator and assert imgs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/frontend.test.js` around lines 2 - 33, The test imports generateProfiles
from data.js but the app now uses a different generator that produces imgs
(plural) in app.js, so update the test to exercise the same generator the UI
uses: either import the shared generator used by app.js (or extract it from
app.js into a common module) and replace references to profile.img with
profile.imgs (and assert imgs is an array of image URLs with each entry starting
with 'https://images.unsplash'), or change the import to pull the generator from
app.js (the function producing imgs) and adjust assertions to check profile.imgs
array shape and contents instead of profile.img.
| }, | ||
| resolve: { | ||
| alias: { | ||
| '@': resolve(__dirname) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Root package module type:"
jq -r '.type' package.json
echo
echo "vitest config usage of __dirname:"
rg -n --type=js '\b__dirname\b' vitest.config.jsRepository: kelloggm/ai-tinder-fork
Length of output: 167
🏁 Script executed:
cat vitest.config.jsRepository: kelloggm/ai-tinder-fork
Length of output: 491
__dirname is undefined in ESM and will break Vitest startup.
Line 17 uses resolve(__dirname), but __dirname doesn't exist in ESM modules. Since package.json declares "type": "module", this config file is ESM and the reference will cause a ReferenceError when Vitest loads the configuration.
Fix by importing dirname and fileURLToPath from Node.js builtins and defining __dirname:
Proposed fix
import { defineConfig } from 'vitest/config';
-import { resolve } from 'path';
+import { dirname, resolve } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+const __dirname = dirname(fileURLToPath(import.meta.url));
export default defineConfig({📝 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.
| '@': resolve(__dirname) | |
| import { defineConfig } from 'vitest/config'; | |
| import { dirname, resolve } from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; | |
| const __dirname = dirname(fileURLToPath(import.meta.url)); | |
| export default defineConfig({ | |
| resolve: { | |
| alias: { | |
| '@': resolve(__dirname) | |
| } | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vitest.config.js` at line 17, The config currently uses resolve(__dirname)
(alias mapping '@': resolve(__dirname)) but __dirname is unavailable in ESM; fix
by importing dirname and fileURLToPath from 'path'/'url' (or node builtins) and
create a local __dirname via dirname(fileURLToPath(import.meta.url)), then
replace resolve(__dirname) use with resolve(__dirname) (the newly defined
constant) so the alias mapping in vitest.config.js works at runtime; ensure this
initialization appears before the alias object is constructed.
2 participants
Based on Pull Request #9, added frontend and backend tests as much as possible
Summary by CodeRabbit
Release Notes
New Features
Tests