Account Deletion & Data Export#32
Conversation
|
👋 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds client-side export and delete-account flows in ProfileScreen, two new db exports ( Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as ProfileScreen
participant AuthClient as Supabase Auth (client)
participant Proxy as Proxy Server (/api/delete-account)
participant DBService as Supabase (service role)
User->>UI: Tap "Delete account"
UI->>User: Confirm (step 1)
User->>UI: Confirm (step 2)
UI->>AuthClient: getActiveSession() -> access_token, userId
AuthClient-->>UI: access_token + userId
UI->>Proxy: POST /api/delete-account { userId, accessToken } with x-proxy-secret
activate Proxy
Proxy->>DBService: Verify accessToken matches userId
DBService-->>Proxy: verification result
Proxy->>DBService: Delete auth user (service role key)
DBService-->>Proxy: deletion result
Proxy->>DBService: Delete logs / profiles (best-effort, retries)
Proxy-->>UI: 200 OK or error
UI->>AuthClient: signOut() (errors tolerated)
AuthClient-->>UI: signOut result
sequenceDiagram
actor User
participant UI as ProfileScreen
participant DBClient as Supabase (client)
participant FS as expo-file-system
participant ShareAPI as Share
User->>UI: Tap "Export my data"
UI->>DBClient: fetch profiles (concurrent)
UI->>DBClient: fetch logs (concurrent)
DBClient-->>UI: profiles + logs
UI->>UI: format CSV (# PROFILE, # LOGS, csvEscape)
UI->>FS: writeFile(tempPath, csv)
FS-->>UI: temp file URI
UI->>ShareAPI: Share.share({ url: temp file URI })
ShareAPI-->>User: system share dialog
UI->>FS: deleteFile(tempPath) (cleanup after delay)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
Review rate limit: 8/10 reviews remaining, refill in 6 minutes and 20 seconds. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/db.ts`:
- Around line 151-175: The CSV builder in profileSection emits many raw cells
(e.g., profile.name, profile.occupation, profile.created_at, log.id,
log.timestamp, log.type, log.category, log.context, log.duration_mins) without
escaping, which breaks CSV when values contain commas/quotes/newlines; add a
small helper (e.g., csvEscape(value)) that converts undefined/null to empty
string, replaces any double-quote with two double-quotes, and wraps the result
in quotes, then apply that helper to every cell in the profileSection array and
inside the logs.map output so all exported cells are properly escaped.
- Around line 104-129: deleteUserAccount currently only signs the user out
(supabase.auth.signOut) and never removes the auth.users record, and it also
ignores signOut errors; replace this client-side-only logic with a server-side
account-deletion flow. Create or call a server endpoint/function (e.g.,
deleteUserAccountAdmin or a serverless route) which uses the Supabase service
role to call auth.admin.deleteUser(userId) and then deletes related rows (logs,
profiles) or relies on DB cascades so the operation is atomic; update the client
deleteUserAccount to call that endpoint and handle/report any errors from the
network call and from signOut (if you still sign out after deletion). Ensure you
reference the current function name deleteUserAccount, the supabase.auth.signOut
usage, and add a server-side method that invokes auth.admin.deleteUser(userId)
with proper error handling and authorization.
In `@src/screens/ProfileScreen.tsx`:
- Around line 43-47: The current share flow in ProfileScreen calls Share.share({
message: csv }) with the CSV string from exportUserData; instead, write the CSV
string to a temporary file and share that file URI via the url param to provide
a downloadable attachment. Replace the Share.share usage so that after calling
exportUserData(userId) you create a temp file (e.g., using Expo FileSystem or
react-native-fs), write the CSV into it, obtain the file:// URI, and call
Share.share({ url: tempFileUri, title: 'Firsthand data export' }) (clean up the
temp file after sharing); update references around exportUserData and
Share.share accordingly.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3926148b-eb51-4dad-9201-e224efeb5814
⛔ Files ignored due to path filters (1)
Build_plan.mdis excluded by!**/*.md
📒 Files selected for processing (2)
src/lib/db.tssrc/screens/ProfileScreen.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 3 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
Fixed 4 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
…ing profile imports
📝 CodeRabbit Chat: Implement requested code changes
fix: CodeRabbit auto-fixes for PR #32
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 70-78: The code destructures userId and accessToken directly from
req.body which will throw if req.body is null, an array, or a primitive; before
destructuring, validate that req.body is a non-null plain object (e.g., typeof
req.body === 'object' && req.body !== null && !Array.isArray(req.body')), and if
that check fails return res.status(400).json({ error: 'invalid request body' });
then proceed to access userId and accessToken and keep the existing string/trim
checks for userId and accessToken.
- Around line 80-142: The four outbound fetch calls (verifyResponse,
deleteLogsResponse, deleteProfileResponse, deleteUserResponse) can hang
indefinitely; wrap each fetch in a timeout using an AbortController (or a shared
timeout helper) so the request is aborted after a reasonable deadline (e.g.,
5–10s), handle the aborted error path to return a 504 or appropriate error to
the client, and ensure you clean up the controller (clear timers) and log
timeout-specific messages; update the handler's fetch calls to use this
timeout-aware wrapper or inline AbortController logic for the calls to
/auth/v1/user, /rest/v1/logs, /rest/v1/profiles, and
/auth/v1/admin/users/:userId.
In `@src/lib/db.ts`:
- Around line 113-126: Remove use of the client-exposed
EXPO_PUBLIC_PROXY_SECRET: stop reading EXPO_PUBLIC_PROXY_SECRET/proxySecret and
remove the 'x-proxy-secret' header when calling the delete-account endpoint from
deleteAccount (or the function wrapping the fetch call); rely only on the
session access token already sent. Add an AbortController around the fetch to
enforce a 30-second timeout (create controller, pass signal to fetch, and abort
after 30000ms). Finally, change the csvEscape function signature so its input
parameter is typed as unknown instead of any and narrow/check the type inside
the function before operating on it.
- Around line 120-131: The fetch call that posts to
`${proxyUrl}/api/delete-account` can hang indefinitely; add an AbortController
with a suitable timeout (e.g., 5-10s) and pass controller.signal into the fetch
options, set a timer via setTimeout to call controller.abort(), and clear the
timer after fetch settles (in finally) to avoid leaks; ensure you handle
AbortError where this request is made (the code referencing proxyUrl,
proxySecret, userId, session.access_token and the response variable) so the
delete flow fails fast instead of hanging.
In `@src/screens/ProfileScreen.tsx`:
- Around line 41-93: Declare the async handler with an explicit Promise<void>
return type (change "const handleExport = async () => {" to "const handleExport
= async (): Promise<void> =>") and change all catch bindings to typed unknown
(e.g., "catch (error: unknown)" and the two cleanup catches to "catch
(cleanupErr: unknown)"); after that, narrow the unknowns before using them in
logs/alerts (e.g., if (error instanceof Error) console.error('Export error:',
error.message) else console.error('Export error:', String(error))) so the code
remains type-safe while preserving the existing error handling in handleExport,
the setTimeout cleanup callback, and the immediate cleanup block in the main
catch.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d4d2b81a-0c50-4edf-9d51-3094890db012
⛔ Files ignored due to path filters (2)
Build_plan.mdis excluded by!**/*.mdpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
.env.examplepackage.jsonserver/index.jssrc/lib/db.tssrc/screens/ProfileScreen.tsx
| const proxyUrl = process.env.EXPO_PUBLIC_PROXY_URL; | ||
| const proxySecret = process.env.EXPO_PUBLIC_PROXY_SECRET; | ||
|
|
||
| if (!proxyUrl || !proxySecret) { | ||
| throw new Error('Missing proxy configuration'); | ||
| } | ||
|
|
||
| // Call server endpoint to delete user account using service role | ||
| const response = await fetch(`${proxyUrl}/api/delete-account`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'x-proxy-secret': proxySecret, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /dev/shm/repo && head -150 src/lib/db.ts | tail -50Repository: FLAiRistaken/firsthand
Length of output: 130
🏁 Script executed:
cd /dev/shm/repo && sed -n '104,146p' src/lib/db.tsRepository: FLAiRistaken/firsthand
Length of output: 130
🏁 Script executed:
cd /dev/shm/repo && sed -n '1,30p' src/lib/db.tsRepository: FLAiRistaken/firsthand
Length of output: 130
🏁 Script executed:
cat -n src/lib/db.ts | sed -n '100,150p'Repository: FLAiRistaken/firsthand
Length of output: 2012
🏁 Script executed:
wc -l src/lib/db.tsRepository: FLAiRistaken/firsthand
Length of output: 84
Remove EXPO_PUBLIC_PROXY_SECRET from client-side authentication.
Anything under EXPO_PUBLIC_* is bundled into the client app, so the header value on lines 123–126 is extractable from the app bundle. The shared-secret check provides no real security here. Rely solely on the session access token (already being sent on line 129) for authentication; move any server-side secrets to a server-only environment.
Also: the fetch call (lines 121–131) lacks an AbortController timeout. Use 30-second timeout per guidelines for src/lib fetch calls. Additionally, csvEscape on line 149 uses value: any — use unknown instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/db.ts` around lines 113 - 126, Remove use of the client-exposed
EXPO_PUBLIC_PROXY_SECRET: stop reading EXPO_PUBLIC_PROXY_SECRET/proxySecret and
remove the 'x-proxy-secret' header when calling the delete-account endpoint from
deleteAccount (or the function wrapping the fetch call); rely only on the
session access token already sent. Add an AbortController around the fetch to
enforce a 30-second timeout (create controller, pass signal to fetch, and abort
after 30000ms). Finally, change the csvEscape function signature so its input
parameter is typed as unknown instead of any and narrow/check the type inside
the function before operating on it.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 5 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 5 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 119-207: Current flow deletes logs and profiles before calling the
auth admin delete, risking irreversible partial deletion; move the auth deletion
to be performed first using the existing deleteUserController/fetch to
/auth/v1/admin/users/${userId} and only on success proceed to delete logs and
profiles (deleteLogsController fetch to /rest/v1/logs?user_id=eq.${userId} and
deleteProfileController fetch to /rest/v1/profiles?id=eq.${userId}). Keep the
same timeout/abort handling and error responses for deleteUserResponse, and for
the subsequent DB deletes implement best-effort behavior (log failures,
optionally retry a few times) rather than returning 500 so the handler does not
leave an active auth account without data.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f0aff63-4279-4859-9270-b674d1f37a27
📒 Files selected for processing (3)
server/index.jssrc/lib/db.tssrc/screens/ProfileScreen.tsx
…nchronous cleanup for database records
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/index.js`:
- Around line 109-110: The code treats any non-2xx from the Supabase
/auth/v1/user call (verifyResponse) as a 401; change the logic so only explicit
auth errors map to 401/403: inspect verifyResponse.status — if 401 return
res.status(401).json(...) and if 403 return res.status(403).json(...); for
rate-limit and server errors (e.g., 429, 5xx) return a 5xx upstream error (e.g.,
res.status(502).json({ error: 'Upstream authentication service error' })) and/or
forward the original status, optionally logging the verifyResponse.status and
body for debugging; update the code around verifyResponse.ok and the
res.status(...) calls to implement this branching.
- Around line 189-193: Return the HTTP success response before running the long
best-effort cleanup loop: call res.json({ success: true }) immediately after
confirming auth deletion, then trigger the two bestEffortDelete calls (logs and
profiles) asynchronously without awaiting them so the client is not blocked and
retries won't run against an expired token; locate the current awaited calls to
bestEffortDelete and the res.json in server/index.js and change the control flow
so res.json is sent first and bestEffortDelete runs in the background (or is
enqueued to a worker).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 31210eab-2124-4bf3-9aff-eb627041e570
📒 Files selected for processing (1)
server/index.js
…on in the background
Implemented account deletion and data export. Added
deleteUserAccountandexportUserDatatosrc/lib/db.ts. Connected these functions insrc/screens/ProfileScreen.tsx. UpdatedBuild_plan.mdtasks.PR created automatically by Jules for task 14860465044460491026 started by @FLAiRistaken
Summary by CodeRabbit
New Features
Bug Fixes / UX