PR with the new data & account deletion feature, privacy policy, and terms & conditions.#39
Conversation
📝 WalkthroughWalkthroughThis PR implements full account deletion for DearDiary: a typed deletion lifecycle backed by a Zustand store drives a client-side service that calls a new Supabase Edge Function, which deletes storage objects, executes a DB RPC to remove user data, and deletes the Clerk user. Mutation guards are added to the journal store, sync store, and auto-sync hook. A profile-screen confirmation card, in-app legal document screens, and a migration from ChangesAccount Deletion, Legal Screens & Auth Dialog Migration
Sequence Diagram(s)sequenceDiagram
participant User
participant ProfileScreen
participant deleteCurrentAccount
participant EdgeFunction as delete-account Edge Function
participant SupabaseDB as Supabase DB (RPC)
participant ClerkAPI
participant clearLocalUserData
User->>ProfileScreen: Taps "Delete My Data and Account", types "DELETE", confirms
ProfileScreen->>deleteCurrentAccount: { confirmationPhrase: "DELETE", getToken, signOut, userId }
deleteCurrentAccount->>EdgeFunction: POST /functions/v1/delete-account (Bearer JWT)
EdgeFunction->>EdgeFunction: Validate JWT sub + confirmationPhrase
EdgeFunction->>SupabaseDB: rpc delete_deardiary_user_data(userId)
SupabaseDB-->>EdgeFunction: JSONB deletion counts
EdgeFunction->>ClerkAPI: DELETE /users/{clerkUserId}
ClerkAPI-->>EdgeFunction: 200/204
EdgeFunction-->>deleteCurrentAccount: { success: true, requestId }
deleteCurrentAccount->>clearLocalUserData: userId (stores, app-lock, export files)
clearLocalUserData-->>deleteCurrentAccount: void
deleteCurrentAccount-->>ProfileScreen: success result
ProfileScreen->>User: Redirect to /login + success dialog
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/profile/profile-screen.tsx (1)
403-437:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock account deletion while sync is already running.
handleDeleteAccountPressonly checks deletion state. If manual or auto-sync has already setisSyncing, deletion can run while sync continues writing journal or achievement data after the remote cleanup RPC.🔒 Suggested guard
function handleDeleteAccountPress() { if (deletionInProgress) { return; } + + if (isSyncing) { + showDialog({ + confirmText: "OK", + message: + "Please wait for your current sync to finish before deleting your account.", + title: "Sync in progress", + }); + return; + } const userId = user?.id;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/profile/profile-screen.tsx` around lines 403 - 437, The handleDeleteAccountPress function only checks if deletionInProgress is true but does not check if isSyncing is already in progress, allowing account deletion to proceed while sync operations are still writing journal or achievement data. Add an additional guard at the beginning of the handleDeleteAccountPress function to also return early if isSyncing is true, similar to the existing deletionInProgress check, to prevent deletion from running concurrently with sync operations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/settings/privacy.tsx`:
- Around line 112-120: The SettingsRow component with label "External Deletion
Page" and icon "external-link" has an onPress handler that incorrectly routes to
privacyPolicyHref instead of accountDeletionUrl. Fix this by changing the
router.push call in the onPress callback to use accountDeletionUrl so that users
can actually access the configured external deletion page.
In `@components/auth/auth-screen.tsx`:
- Around line 339-341: The error message in the handleSocialPress function is
hardcoded to display "Google signup" but this function handles multiple
providers (Google, Apple, etc.) and auth modes (signup and login). Replace the
hardcoded string "Google signup needs one more step before continuing." with a
dynamic message that uses the current provider and authMode variables to
accurately reflect which provider and operation type the user attempted, such as
"Apple login" or "Google signup" based on the actual selected provider and auth
mode values.
In `@components/legal/legal-document-screen.tsx`:
- Around line 37-113: The legal-document-screen component contains multiple
hardcoded hex color values and spacing measurements scattered throughout the
JSX, such as "`#FFF7FB`", "`#27272A`", "`#51515B`", "`#71717B`" for colors and padding
values like 24, 36, 28, 56 for spacing. Extract all these repeated visual tokens
into named constants defined at the top of the file (or in a shared constants
file), then replace every hardcoded occurrence with references to these
constants. This includes colors used in LinearGradient colors array, Text
className color values, View style padding/margin values, and any other repeated
design tokens to ensure consistency and ease future design updates.
- Around line 89-98: The map functions for sections and paragraphs are using
text content as keys (section.title and paragraph respectively), which will
cause collisions when content repeats. Replace the key={section.title} in the
sections.map call and key={paragraph} in the section.body.map call with unique,
stable identifiers. Consider adding index-based keys or unique ID properties to
the data structures if available, ensuring each rendered item has a distinct key
that won't collide with other items in the list.
In `@components/profile/profile-screen.tsx`:
- Around line 440-475: The handleConfirmDeleteAccount function uses
deletionInProgress as a guard that blocks all actions, but when the deletion
fails with stage "failed", the backend keeps deletionInProgress true to prevent
new mutations while the UI still shows disabled buttons and a spinner. Separate
the logic by creating a flag that represents an active request in flight
(deletionInProgress && deletionStage !== "failed"), then use this flag for both
the initial guard check in handleConfirmDeleteAccount and for the UI state
controls that disable the Confirm/Cancel buttons and display the loading
spinner. This allows users to retry deletion even when a previous attempt has
failed, while still preventing concurrent requests.
- Around line 1046-1053: The TextInput component with placeholder="DELETE" lacks
an accessible label for screen readers, making it unclear to accessibility users
what the input's purpose is. Add an accessibilityLabel prop to this TextInput
component that clearly describes its purpose, such as indicating that it's a
confirmation input requiring the user to type "DELETE" to confirm the deletion
action.
In `@docs/privacy.md`:
- Around line 17-20: The deletion architecture steps 1-3 in the privacy.md file
use repetitive "User" as the subject, creating monotonous sentence structure.
Rewrite steps 1-3 to use imperative voice (command form) to match the style of
step 4 and improve readability. Change "User starts deletion..." to "Start
deletion...", "User reviews consequences" to "Review the consequences", and
"User types DELETE" to "Type DELETE" to create consistency and variety in
sentence structure throughout the numbered list.
In `@lib/account/accountDeletionService.ts`:
- Around line 70-104: The clearLocalUserData function can throw after remote
deletion succeeds, but the outer catch block returns a generic error without
indicating remoteDataDeleted was true, causing the UI to incorrectly clear
guards while local data remains. Wrap both clearLocalUserData(userId) calls (one
in the failure branch after remoteDataDeleted check and one in the success
branch) in try-catch blocks, and when clearLocalUserData throws in either
location, call deletionStore.failDeletion with code "local_cleanup_failed" and
keepGuardActive: true along with the requestId before returning, ensuring the UI
understands that remote data was deleted even though local cleanup failed.
- Around line 140-149: The fetch request in the account deletion Edge Function
call lacks a timeout mechanism, which can leave the deletion state hanging if
the network request doesn't complete. Implement a timeout using AbortController
by creating an abort controller with a timeout duration, passing it to the fetch
signal option, and catching the resulting AbortError. Additionally, update the
getDeletionErrorResult function to recognize and handle AbortError exceptions by
mapping them to a retryable failure state, ensuring that timeout scenarios are
properly communicated as recoverable errors rather than fatal failures.
In `@lib/account/clearLocalUserData.ts`:
- Around line 49-62: The clearNotificationState function currently only swallows
expected notification cleanup errors via isExpectedNotificationCleanupError, but
for unexpected errors it just logs a warning in development mode and then
returns without failing. This allows account deletion to proceed even when
journal reminders remain scheduled due to an unexpected disableJournalReminders
failure. After the __DEV__ conditional logging block within the outer catch
handler, throw the error to ensure the account deletion process fails when an
unexpected notification cleanup error occurs, rather than silently falling
through.
In `@supabase/functions/delete-account/deno.json`:
- Line 3: The delete-account function lacks JWT signature verification, creating
a security vulnerability where forged bearer tokens could delete arbitrary user
data. To fix this, either enable verify_jwt = true in the function's
configuration settings, or implement explicit JWT signature verification (such
as using jwks_uri or a public key) before the function trusts and uses the
claims.sub value to delete user data from the database, storage, and Clerk.
Ensure that signature verification happens before any delete operations that
depend on claims.sub.
In `@supabase/functions/delete-account/index.ts`:
- Line 30: The userOwnedStorageBuckets array is initialized as empty, which
causes the deleteUserStorageObjects() function to skip all storage deletion
operations during account deletion, leaving user data orphaned in Supabase
Storage. Identify all Supabase Storage bucket names where user data is actually
stored in the application (such as journals, attachments, or other
user-generated content buckets) and populate the userOwnedStorageBuckets array
with these bucket name strings so that deleteUserStorageObjects() can properly
iterate over and delete user objects from the correct buckets.
- Around line 333-334: The `list()` method call in the `deleteStoragePrefix`
function uses the default limit of 100 items per request, which causes items
beyond that limit to be silently skipped. Implement pagination by wrapping the
list operation in a while loop that uses `limit` and `offset` parameters to
fetch all pages from the storage bucket. For each iteration, set the offset to
track your position, fetch the items with the limit parameter, process and
delete those items, and continue looping until the returned results contain
fewer items than the limit (indicating the final page has been reached).
- Around line 380-388: The fetch call to the Clerk API endpoint
(https://api.clerk.com/v1/users/${encodeURIComponent(userId)}) lacks a timeout
mechanism, which can cause the function to hang indefinitely if the upstream
service becomes unresponsive. Add an AbortController with a timeout (following
the pattern used in reflect-on-entry, journal-ai-chat, and
generate-insight-report functions) by creating an AbortController instance,
setting a timeout that calls controller.abort() after a reasonable duration
(typically 5000-10000ms), and passing the controller.signal to the fetch
request's options object.
---
Outside diff comments:
In `@components/profile/profile-screen.tsx`:
- Around line 403-437: The handleDeleteAccountPress function only checks if
deletionInProgress is true but does not check if isSyncing is already in
progress, allowing account deletion to proceed while sync operations are still
writing journal or achievement data. Add an additional guard at the beginning of
the handleDeleteAccountPress function to also return early if isSyncing is true,
similar to the existing deletionInProgress check, to prevent deletion from
running concurrently with sync operations.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bb366ef5-57ad-483f-90af-987eca547a1e
📒 Files selected for processing (32)
.env.exampleREADME.mdapp/_layout.tsxapp/legal/privacy-policy.tsxapp/legal/terms.tsxapp/settings/privacy.tsxcomponents/auth/auth-screen.tsxcomponents/auth/auth-utils.tscomponents/auth/reset-password-screen.tsxcomponents/legal/legal-document-screen.tsxcomponents/profile/profile-screen.tsxcontent/legal/legalVersions.tscontent/legal/privacyPolicy.tscontent/legal/termsAndConditions.tsdata/profile.tsdocs/database.mddocs/play-store-account-deletion.mddocs/privacy.mddocs/user-data-inventory.mdhooks/useAutoSync.tslib/account/accountDeletionErrors.tslib/account/accountDeletionService.tslib/account/clearLocalUserData.tsstore/journal-store.tsstore/useAIInsightReportStore.tsstore/useAccountDeletionStore.tsstore/useSyncStore.tssupabase/config.tomlsupabase/functions/delete-account/deno.jsonsupabase/functions/delete-account/index.tssupabase/migrations/20260621104500_add_delete_deardiary_user_data.sqltypes/accountDeletion.ts
[+] Implemented account & data deletion feature for users.
[+] Added privacy policy, and terms & conditions pages.
[+] Bug fixes and improvements to existing features.
Summary by CodeRabbit