feat: add check-in progress photos (front, back, side)#1533
Conversation
PR Validation ResultsChange Detection
✅ All checks passed. Thank you! |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a progress photos feature to the check-in flow, allowing users to upload, replace, and delete front, back, and side photos. The changes span both the frontend (React components, React Query hooks, and translation updates) and the backend (Express routes, multer middleware with magic-byte image validation, Zod schemas, database migrations, and RLS policies). The review feedback highlights a critical timezone-related bug in checkInPhotoService.ts where formatting database DATE values using toISOString().split('T')[0] can cause day shifts, and recommends using a shared date utility library instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
merge the three migration sql into a single file. and also attach some screenshots in the AFTER section of the PR desc |
zeusal
force-pushed
the
feature/issue-229-progress-photos
branch
from
1eca85c to
19ac745
Compare
I have tried to do it, can you verify again please. |
Resolves CodeWithCJ#229. Add the ability to upload and track progress photos from the check-in screen, with three fixed angle slots (front, back, side) per day, plus the backend storage, retrieval, and security hardening. Frontend: - New CheckInPhotos UI on the check-in screen with front/back/side slots (upload / replace / remove) and English (en) translation strings. Backend: - New check_in_photos table, routes, service, schemas, and upload middleware. - Photos stored under uploads/check-in/<userId>/<date>/<type>.<ext>; only the relative path is persisted, so records stay portable across deployments. - Uploads validated by real image magic bytes (not the spoofable mime type or filename), buffered in memory, and written to a temp file that is atomically promoted only after the DB commit, so a failed insert never orphans a file and a same-name replace never clobbers a valid photo. - Photos served through an authenticated, ownership-checked route (RLS-scoped) with X-Content-Type-Options: nosniff; direct static access to /uploads/check-in is blocked. - RLS: enable row-level security and add the per-user diary policy for check_in_photos (previously RLS was enabled with no policy, so every insert was denied). - DB: point check_in_photos.user_id at public."user" (Better Auth), matching check_in_measurements; uploads failed with a foreign key violation for users absent from the legacy auth.users table. Add the prerequisite PRIMARY KEY on check_in_measurements so the photos foreign key target is valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zeusal
force-pushed
the
feature/issue-229-progress-photos
branch
from
19ac745 to
563d4b9
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a progress photos feature (front, back, and side) for check-ins, including frontend components, a custom hook, backend routes, services, database migrations, and RLS policies. The code review identifies a high-severity security vulnerability (IDOR/Authorization Bypass) where Row-Level Security could be bypassed, allowing unauthorized access to private photos. Additionally, the feedback highlights performance concerns regarding synchronous file system operations blocking the Node.js event loop, and suggests frontend optimizations using useMemo to prevent unnecessary map re-creations on every render.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -0,0 +1,183 @@ | |||
| import { useRef, useCallback } from 'react'; | |||
There was a problem hiding this comment.
Import useMemo from 'react' to support the memoization of the photo map.
| import { useRef, useCallback } from 'react'; | |
| import { useRef, useCallback, useMemo } from 'react'; |
|
Take a look at gemini comments. some times it could also be false positive. |
|
@apedley could you review this. |
- checkInPhotoService: convert synchronous fs calls (existsSync/unlinkSync/ mkdirSync/writeFileSync/renameSync) to async fs.promises equivalents so file I/O no longer blocks the event loop; handle ENOENT instead of pre-checking existence. Preserves the temp-file -> commit -> rename atomic semantics. - CheckInPhotos: memoize photoMap with useMemo([photos]) so the map is not rebuilt on every render, and import useMemo. The flagged IDOR in getPhotoFileById is a false positive: the query runs under getClient(userId), which sets the RLS context, and check_in_photos has a per-user diary RLS policy, so ownership is enforced at the database layer. https://claude.ai/code/session_01LEQ8rd34Jgy2LA24F96aP3
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a progress photo upload feature for check-ins, including frontend UI components, React Query hooks, backend API routes, database migrations, and magic-byte image validation. Feedback on the changes suggests using the standard apiCall wrapper instead of direct fetch in the frontend, deriving file extensions from validated magic bytes rather than trusting client-provided filenames, and removing the redundant check_in_measurement_id column to avoid data inconsistency. Additionally, it is recommended to remove capture="environment" to allow gallery uploads, add WebP support to the allowed image signatures, increase the photo_type column length for future-proofing, and replace any types with proper Express types in the route handler.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| async (req: any, res: any) => { | ||
| if (!req.file) { |
There was a problem hiding this comment.
Using any for req and res disables TypeScript's type safety and requires disabling the eslint rule. Type them properly using express.Request and express.Response to maintain type safety.
| async (req: any, res: any) => { | |
| if (!req.file) { | |
| async (req: express.Request & { file?: Express.Multer.File; userId?: string }, res: express.Response) => { |
- upload (frontend): route uploadCheckInPhoto through apiCall with isFormData instead of a raw fetch, so it shares the app's API base URL, cookie auth, and error handling (Gemini #1). - stored extension: derive the file extension from the validated image magic bytes (getImageExtension) instead of the spoofable client filename, so the served Content-Type can't be mismatched under nosniff (Gemini #2). - file input: drop capture="environment" so mobile users can pick the camera or an existing gallery photo, not just the camera (Gemini #4). - uploads: accept WebP, validating the full RIFF....WEBP signature (not just the RIFF prefix, which also matches WAV/AVI) (Gemini #5). - routes: type the upload handler as (req, res) with a narrow req.file cast instead of (req: any, res: any) (Gemini #7). - tests: assert the derived extension and add WebP accept / non-WebP RIFF reject cases. https://claude.ai/code/session_01LEQ8rd34Jgy2LA24F96aP3
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a progress photos feature for check-ins, allowing users to upload, view, and delete front, back, and side photos. It includes frontend UI components, React Query hooks, and backend support with a database migration, RLS policies, secure image validation, and file system persistence. The review feedback suggests several improvements: expanding the database photo_type column length for future extensibility, removing a redundant index, logging instead of throwing file deletion errors to avoid unnecessary 500 responses, adding client-side file size validation, and disabling all photo slots during an active upload to prevent concurrent upload race conditions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- migration: drop the redundant (user_id, entry_date) index; the unique constraint on (user_id, entry_date, photo_type) already builds a composite index whose leading columns cover those lookups (Gemini #2). - deletePhoto: the DB row is the source of truth, so a non-ENOENT unlink failure is now logged instead of thrown, avoiding a 500 (and a confusing orphaned-file state) after a successful delete (Gemini #3). - upload (frontend): reject files over 10 MB client-side with a toast before the request, mirroring the server multer limit (Gemini #4). - CheckInPhotos: add a global 'disabled' prop so every slot is blocked while any upload is in flight, while the spinner stays on the active slot only, preventing a second upload from racing the first (Gemini #5). https://claude.ai/code/session_01LEQ8rd34Jgy2LA24F96aP3
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a progress photo upload feature (front, back, and side views) to track physique over time. On the frontend, it adds translation keys, an API service, a custom React Query hook, and a CheckInPhotos UI component. On the backend, it implements a database migration with RLS policies, a secure multer upload middleware with magic-byte validation, and routes/services to handle photo management securely. The review feedback highlights several improvement opportunities: handling potential transmission errors in res.sendFile with a callback, verifying file existence on disk before serving, and exposing/utilizing the deletion loading state (isDeleting) in the frontend hook and components to prevent concurrent conflicting operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- serve route: pass an error callback to res.sendFile so an async transmission failure is logged and answered with 500 only when headers haven't been sent yet, instead of escaping the try/catch (Gemini #1). - getPhotoFileById: verify the file exists on disk (fs.promises.access) before returning its path, so a missing file yields a clean 404 rather than a 500 from sendFile (Gemini #2). - useCheckInPhotos: expose isDeleting (deleteMutation.isPending) (Gemini #3). - CheckInPhotos: consume isDeleting and disable every slot while a delete is in flight (disabled={isUploading || isDeleting}), matching the existing upload guard so concurrent operations can't race (Gemini #4, #5). https://claude.ai/code/session_01LEQ8rd34Jgy2LA24F96aP3
|
Hi @CodeWithCJ I'm trying to put all of Gemini's suggestions into practice, but if you think the code is fine, I'll stop asking the bot. |
|
@apedley will review and provide his feedback today or tomorrow. |
|
…kup, compact empty UI Address review feedback on the progress photos feature: - checkInPhotoService: resolve the uploads root from SPARKY_FITNESS_CUSTOM_UPLOADS_DIRECTORY (matching the rest of the server) instead of hardcoding SparkyFitnessServer/uploads. The stored 'uploads/...'-rooted file_path is resolved against the configured root, keeping the path-traversal guard in getPhotoFileById correct. - db_schema_backup.sql: add the missing check_in_photos primary key and (user_id, entry_date, photo_type) unique constraint, plus the check_in_measurements primary key the migration adds. - CheckInPhotos: collapse the section to a compact single-row header when no photos exist so users who don't use the feature have less to scroll. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WqrJhyCWBwLYvHGXmWVizX
Hi, as far as the user interface is concerned, I have two option: one of them is a drop-down menu 
And second one: Which one do you prefer? |
|
First one looks good, thanks! |
The first one is already push to branch |
4 participants
Resolves #229. Add the ability to upload and track progress photos from the check-in screen, with three fixed angle slots (front, back, side) per day, plus the backend storage, retrieval, and security hardening.
Frontend:
Backend:
uploads/check-in/<userId>/<date>/<type>.<ext>; only the relative path is persisted, so records stay portable across deployments.Tip
Help us review and merge your PR faster!
Please ensure you have completed the Checklist below.
For Frontend changes, please run
pnpm run validateto check for any errors.PRs that include tests and clear screenshots are highly preferred!
Note: AI-generated descriptions must be manually edited for conciseness. Do not paste raw AI summaries.
Description
What problem does this PR solve?
(Keep it concise. 1–2 sentences.)
How did you implement the solution?
(Brief technical approach.)
Linked Issue
Linked Issue: Closes #229
How to Test
pnpm startinSparkyFitnessServer/).The new migrations apply automatically on boot (
check_in_measurementsPK →check_in_photostable → FK repoint topublic."user"), and RLS policies arere-applied, so an existing DB is upgraded in place.
pnpm devinSparkyFitnessFrontend/) and log in.can be replaced, and persists after reloading the page / reopening the date.
SparkyFitnessServer/uploads/check-in/<userId>/<date>/..txt→.jpg) is rejected on upload.each user only sees their own — photos are served by id through an
ownership/RLS-checked route, and
/uploads/check-inis not publicly served.PR Type
Checklist
All PRs:
New features only:
Frontend changes (SparkyFitnessFrontend/):
pnpm run validateand it passes.Backend changes (SparkyFitnessServer/):
rls_policies.sqlfor any new user-specific tables.UI changes (components, screens, pages):
Mobile changes (SparkyFitnessMobile/):
Screenshots
main)Notes for Reviewers