fix: improve account deletion and session handling#770
Open
Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Open
fix: improve account deletion and session handling#770Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Arunodoy18 wants to merge 1 commit intoAOSSIE-Org:masterfrom
Conversation
- Added session revocation during account deletion - Implemented cascading deletes for related records (followers, friend requests, subscriptions) - Enhanced login checks to prevent deleted users from authenticating - Added proper error handling and logging throughout deletion flow
Contributor
|
🎉 Welcome @Arunodoy18!
We appreciate your contribution! 🚀 |
📝 WalkthroughWalkthroughEnhanced authentication state controller with defensive user verification and error handling across login and initialization flows. Introduced comprehensive account deletion controller that orchestrates multi-step deletion of user sessions, related records, authentication account, and profile data. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/Controller
participant Auth as AppwriteService<br/>(Account)
participant API as Appwrite API
participant DB as Database
App->>App: deleteAccount() called
App->>Auth: Verify authenticated user
Auth-->>App: User confirmed
App->>Auth: revokeAllSessions()
Auth->>API: Fetch all sessions
API-->>Auth: Session list
loop For each session
Auth->>API: Delete session
API-->>Auth: Confirmed
end
Auth-->>App: Sessions revoked
App->>App: deleteRelatedRecords()
App->>DB: Delete from followers
DB-->>App: Confirmed
App->>DB: Delete from following
DB-->>App: Confirmed
App->>DB: Delete from requests
DB-->>App: Confirmed
App->>DB: Delete from subscribed_rooms
DB-->>App: Confirmed
App->>Auth: deleteAuthAccount()
Auth->>API: Update account status
API-->>Auth: Confirmed
Auth-->>App: Auth account deleted
App->>DB: Delete profile image file
DB-->>App: Confirmed
App->>DB: Delete username document
DB-->>App: Confirmed
App->>DB: Delete user document
DB-->>App: Confirmed
App->>App: Navigate to welcome screen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
lib/controllers/auth_state_controller.dart (3)
186-214: Good defensive check ingetLoginState.The addition of verifying the user document exists for authenticated users with complete profiles is a solid defensive measure. This correctly handles the scenario where an account's auth session lingers but the database records have been removed.
One minor note: the
catch (e)on line 199 will catch any Appwrite error (network issues, server errors), not just document-not-found. This could cause legitimate users to be logged out during transient failures. Consider narrowing the catch to check for 404/document-not-found specifically.♻️ Suggested: only log out on document-not-found
} catch (e) { - // User document not found - account may have been deleted - log('User document not found for authenticated user: ${appwriteUser.$id}'); - // Logout the user since their account data is missing - try { - await account.deleteSession(sessionId: 'current'); - } catch (_) {} - return false; + if (e is AppwriteException && e.code == 404) { + log('User document not found for authenticated user: ${appwriteUser.$id}'); + try { + await account.deleteSession(sessionId: 'current'); + } catch (_) {} + return false; + } + // For transient errors, don't log out the user + log('Error verifying user document: $e'); + rethrow; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/controllers/auth_state_controller.dart` around lines 186 - 214, The catch around databases.getDocument in getLoginState is too broad and may log out users on transient errors; change the inner catch to detect Appwrite's "document not found" error (e.g., check for AppwriteException or HTTP status/code 404) and only perform account.deleteSession(sessionId: 'current') and return false when the error is a true not-found; for other errors rethrow or return true/propagate the error so transient/network/server errors do not force a logout; update the catch to reference databases.getDocument, AppwriteException (or the exception's status/code), appwriteUser.$id, and account.deleteSession to implement this conditional handling.
441-444: Pre-existing:logoutdeletes the session before removing registration tokens.This is the same pattern as the deletion flow issue —
deleteSessionon line 442 invalidates authentication, soremoveRegistrationTokenFromSubscribedUpcomingRooms()on line 443 will fail because the database calls require a valid session. Not introduced by this PR, but worth noting since the PR focuses on session handling correctness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/controllers/auth_state_controller.dart` around lines 441 - 444, In the onConfirm handler in AuthStateController (the async block that calls deleteSession and removeRegistrationTokenFromSubscribedUpcomingRooms), reorder the operations so that removeRegistrationTokenFromSubscribedUpcomingRooms() runs before account.deleteSession(sessionId: 'current'); i.e., remove registration tokens while the session is still valid, then delete the session and finally navigate (Get.offAllNamed(AppRoutes.welcomeScreen)); update the onConfirm flow to reflect this ordering using the existing functions deleteSession and removeRegistrationTokenFromSubscribedUpcomingRooms.
297-314:setUserProfileData()is called twice during login — once here and again insideisUserLoggedIn().Line 302 calls
setUserProfileData(), and then line 312 callsisUserLoggedIn()which immediately callssetUserProfileData()again (line 269). This results in two redundantaccount.get()anddatabases.getDocument()round-trips.Consider either skipping the redundant call or passing a flag to
isUserLoggedInto reuse already-loaded data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/controllers/auth_state_controller.dart` around lines 297 - 314, The login method calls setUserProfileData() and then isUserLoggedIn(), which calls setUserProfileData() again; remove the redundant round-trip by letting isUserLoggedIn accept a parameter (e.g., bool profileLoaded) or an optional User object so it can skip reloading when login already loaded the profile. Update the isUserLoggedIn signature and its internal logic to bypass calling setUserProfileData() when profileLoaded is true (or when a non-null User/profile is provided), and call isUserLoggedIn(profileLoaded: true) (or pass the loaded profile) from login after the initial setUserProfileData() call; ensure any callers of isUserLoggedIn are adjusted accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/controllers/auth_state_controller.dart`:
- Around line 227-252: The code is calling .toDouble() directly on
userDataDoc.data["ratingTotal"], which can be null and will throw before the ??
fallback; update the block in auth_state_controller.dart where you set
ratingTotal and ratingCount to first read the raw values (e.g., rawRating =
userDataDoc.data["ratingTotal"]) and only call .toDouble() if rawRating != null,
otherwise assign the default (5); similarly ensure ratingCount is safely read
with a null-aware fallback and proper type conversion if needed (e.g., parse or
cast when non-null) so null values no longer trigger the outer catch that logs
out the user.
- Around line 245-252: The current broad catch around the user document fetch
treats any error (network/500) as "account deleted" and deletes session; change
it to only handle a document-not-found (404) error: detect the specific error
type/code from the exception used in this project (e.g., AppwriteException or
the thrown error that exposes status/code) in the catch block that wraps the
user fetch, and only call account.deleteSession(sessionId: 'current') and throw
Exception("User account data not found") when the error indicates a 404; for all
other errors rethrow (or let them bubble) so transient/server errors don’t log
out the user. Ensure you update the catch around the same function where
account.deleteSession is called.
In `@lib/controllers/delete_account_controller.dart`:
- Around line 94-191: The deleteRelatedRecords method currently uses
databases.listDocuments which returns only up to 25 items; update each block
that lists documents (followers/following using followersCollectionID, friend
requests using friendRequestCollectionId, and subscribed rooms using
subscribedUserCollectionId with upcomingRoomsDatabaseId/userDatabaseID) to
paginate and delete until no documents remain—either by extracting the
listing+deletion into a reusable helper (e.g., _deleteAllMatchingDocuments) that
calls listDocuments with Query.limit(100) (and Query.offset or Query.cursorAfter
as appropriate) in a while loop, or by adding a loop in each try-block that
requests pages and deletes each page’s documents until an empty page is
returned. Ensure the helper accepts databaseId, collectionId, and the base
queries so all four deletion sections call it.
- Around line 209-260: The deleteAccount() flow is wrong: it never calls
deleteAuthAccount() and revokes all sessions too early which causes subsequent
authenticated deletes to 401; fix by reordering and invoking the missing
call—keep the initial authentication check (account.get()), then perform
deleteRelatedRecords(), deleteUserProfilePicture(),
deleteUsernamesCollectionDocument(), deleteUsersCollectionDocument() (all
awaited), then await deleteAuthAccount() (replace the no-op try/catch block with
this call and handle errors), and only after that call revokeAllSessions() as
the final step (or ensure revokeAllSessions() skips the current session until
absolutely last); ensure all calls are awaited and proper try/catch/logging is
kept around deleteAuthAccount(), revokeAllSessions(), and data-deletion
functions.
- Around line 193-205: The deleteAuthAccount() method currently calls
account.updateStatus(), which only blocks the user; replace that call with a
server-side permanent delete flow—invoke the Appwrite server SDK
users.delete(userId) (or call your backend endpoint that performs users.delete)
instead of account.updateStatus(); keep the existing try/catch behavior (log
errors and rethrow) and ensure you pass the correct userId to the server-side
delete routine so the account is removed permanently.
- Around line 71-91: The revokeAllSessions function currently deletes sessions
including the current one which causes the Appwrite client to lose auth
mid-loop; update revokeAllSessions to determine the current session id (e.g.,
via account.get() or client.session info), iterate sessions from
account.listSessions() while skipping the current session during the main loop
(use session.$id to compare), delete all other sessions first with
account.deleteSession(sessionId: ...), then finally delete the current session
last; keep existing try/catch logging behavior and ensure you still proceed with
account deletion even if session revocation errors occur.
---
Nitpick comments:
In `@lib/controllers/auth_state_controller.dart`:
- Around line 186-214: The catch around databases.getDocument in getLoginState
is too broad and may log out users on transient errors; change the inner catch
to detect Appwrite's "document not found" error (e.g., check for
AppwriteException or HTTP status/code 404) and only perform
account.deleteSession(sessionId: 'current') and return false when the error is a
true not-found; for other errors rethrow or return true/propagate the error so
transient/network/server errors do not force a logout; update the catch to
reference databases.getDocument, AppwriteException (or the exception's
status/code), appwriteUser.$id, and account.deleteSession to implement this
conditional handling.
- Around line 441-444: In the onConfirm handler in AuthStateController (the
async block that calls deleteSession and
removeRegistrationTokenFromSubscribedUpcomingRooms), reorder the operations so
that removeRegistrationTokenFromSubscribedUpcomingRooms() runs before
account.deleteSession(sessionId: 'current'); i.e., remove registration tokens
while the session is still valid, then delete the session and finally navigate
(Get.offAllNamed(AppRoutes.welcomeScreen)); update the onConfirm flow to reflect
this ordering using the existing functions deleteSession and
removeRegistrationTokenFromSubscribedUpcomingRooms.
- Around line 297-314: The login method calls setUserProfileData() and then
isUserLoggedIn(), which calls setUserProfileData() again; remove the redundant
round-trip by letting isUserLoggedIn accept a parameter (e.g., bool
profileLoaded) or an optional User object so it can skip reloading when login
already loaded the profile. Update the isUserLoggedIn signature and its internal
logic to bypass calling setUserProfileData() when profileLoaded is true (or when
a non-null User/profile is provided), and call isUserLoggedIn(profileLoaded:
true) (or pass the loaded profile) from login after the initial
setUserProfileData() call; ensure any callers of isUserLoggedIn are adjusted
accordingly.
Comment on lines
+227
to
+252
| try { | ||
| Document userDataDoc = await databases.getDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: usersCollectionID, | ||
| documentId: appwriteUser.$id, | ||
| ); | ||
| profileImageUrl = userDataDoc.data["profileImageUrl"]; | ||
| profileImageID = userDataDoc.data["profileImageID"]; | ||
| userName = userDataDoc.data["username"] ?? "unavailable"; | ||
| ratingTotal = userDataDoc.data["ratingTotal"].toDouble() ?? 5; | ||
| ratingCount = userDataDoc.data["ratingCount"] ?? 1; | ||
| followerDocuments = | ||
| (userDataDoc.data["followers"] as List<dynamic>?)?.map((e) { | ||
| return FollowerUserModel.fromJson(e); | ||
| }).toList() ?? | ||
| []; | ||
| reportsCount = | ||
| (userDataDoc.data['userReports'] as List<dynamic>?)?.length ?? 0; | ||
| } catch (e) { | ||
| // User document not found - account may have been deleted | ||
| log("User document not found, logging out user: $e"); | ||
| try { | ||
| await account.deleteSession(sessionId: 'current'); | ||
| } catch (_) {} | ||
| throw Exception("User account data not found"); | ||
| } |
There was a problem hiding this comment.
Null-safety issue: .toDouble() on a potentially null value will throw before ?? can apply.
On line 236, if userDataDoc.data["ratingTotal"] is null, calling .toDouble() on it throws a NoSuchMethodError. The ?? 5 fallback never gets a chance to execute. With the new try-catch wrapping this block, the exception will be caught and misinterpreted as a "deleted account", causing the user to be logged out and their session deleted — even though their account exists.
🐛 Proposed fix: null-check before calling toDouble
- ratingTotal = userDataDoc.data["ratingTotal"].toDouble() ?? 5;
- ratingCount = userDataDoc.data["ratingCount"] ?? 1;
+ ratingTotal = (userDataDoc.data["ratingTotal"] as num?)?.toDouble() ?? 5.0;
+ ratingCount = (userDataDoc.data["ratingCount"] as int?) ?? 1;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/auth_state_controller.dart` around lines 227 - 252, The code
is calling .toDouble() directly on userDataDoc.data["ratingTotal"], which can be
null and will throw before the ?? fallback; update the block in
auth_state_controller.dart where you set ratingTotal and ratingCount to first
read the raw values (e.g., rawRating = userDataDoc.data["ratingTotal"]) and only
call .toDouble() if rawRating != null, otherwise assign the default (5);
similarly ensure ratingCount is safely read with a null-aware fallback and
proper type conversion if needed (e.g., parse or cast when non-null) so null
values no longer trigger the outer catch that logs out the user.
Comment on lines
+245
to
+252
| } catch (e) { | ||
| // User document not found - account may have been deleted | ||
| log("User document not found, logging out user: $e"); | ||
| try { | ||
| await account.deleteSession(sessionId: 'current'); | ||
| } catch (_) {} | ||
| throw Exception("User account data not found"); | ||
| } |
There was a problem hiding this comment.
Same concern: broad catch treats transient errors as "account deleted".
Similar to the getLoginState issue — a network timeout or server 500 here will trigger session deletion and throw "User account data not found", logging the user out. Narrow this to document-not-found (404) only.
🐛 Proposed fix: check for 404 specifically
} catch (e) {
- // User document not found - account may have been deleted
- log("User document not found, logging out user: $e");
- try {
- await account.deleteSession(sessionId: 'current');
- } catch (_) {}
- throw Exception("User account data not found");
+ if (e is AppwriteException && e.code == 404) {
+ log("User document not found, logging out user: $e");
+ try {
+ await account.deleteSession(sessionId: 'current');
+ } catch (_) {}
+ throw Exception("User account data not found");
+ }
+ rethrow; // Propagate transient errors without destroying session
}📝 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.
Suggested change
| } catch (e) { | |
| // User document not found - account may have been deleted | |
| log("User document not found, logging out user: $e"); | |
| try { | |
| await account.deleteSession(sessionId: 'current'); | |
| } catch (_) {} | |
| throw Exception("User account data not found"); | |
| } | |
| } catch (e) { | |
| if (e is AppwriteException && e.code == 404) { | |
| log("User document not found, logging out user: $e"); | |
| try { | |
| await account.deleteSession(sessionId: 'current'); | |
| } catch (_) {} | |
| throw Exception("User account data not found"); | |
| } | |
| rethrow; // Propagate transient errors without destroying session | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/auth_state_controller.dart` around lines 245 - 252, The
current broad catch around the user document fetch treats any error
(network/500) as "account deleted" and deletes session; change it to only handle
a document-not-found (404) error: detect the specific error type/code from the
exception used in this project (e.g., AppwriteException or the thrown error that
exposes status/code) in the catch block that wraps the user fetch, and only call
account.deleteSession(sessionId: 'current') and throw Exception("User account
data not found") when the error indicates a 404; for all other errors rethrow
(or let them bubble) so transient/server errors don’t log out the user. Ensure
you update the catch around the same function where account.deleteSession is
called.
Comment on lines
+71
to
+91
| Future<void> revokeAllSessions() async { | ||
| try { | ||
| // Get all sessions | ||
| final SessionList sessions = await account.listSessions(); | ||
|
|
||
| // Delete each session | ||
| for (Session session in sessions.sessions) { | ||
| try { | ||
| await account.deleteSession(sessionId: session.$id); | ||
| log('Deleted session: ${session.$id}'); | ||
| } catch (e) { | ||
| log('Failed to delete session ${session.$id}: $e'); | ||
| } | ||
| } | ||
|
|
||
| log('All sessions revoked successfully'); | ||
| } catch (e) { | ||
| log('Error revoking sessions: $e'); | ||
| // Continue with deletion even if session revocation fails | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Appwrite Flutter SDK Session model current property
💡 Result:
In the Appwrite Flutter (Dart) SDK, the Session model has a bool current property:
current: “Returns true if this [is] the current user session.” (i.e., the session being used by the device/client making the request). [1]
To explicitly fetch the current session from the Account API, pass the literal string 'current' as the session ID:
await account.getSession(sessionId: 'current');[2]
Sources: [1] (Session model docs), [2] (Appwrite Flutter Account API docs)
🏁 Script executed:
# Let's examine the file to understand the context better
cat -n lib/controllers/delete_account_controller.dart | head -100Repository: AOSSIE-Org/Resonate
Length of output: 3669
🏁 Script executed:
# Search for Session imports and usages in the codebase
rg -i "session" lib/ --type dart -B 2 -A 2Repository: AOSSIE-Org/Resonate
Length of output: 8644
revokeAllSessions will invalidate the current session, breaking subsequent API calls.
When iterating all sessions, the current session is included. Once deleted, the Appwrite client loses authentication and cannot delete remaining sessions.
Filter out the current session and delete it last instead:
🐛 Proposed fix: delete current session last
Future<void> revokeAllSessions() async {
try {
final SessionList sessions = await account.listSessions();
+ // Delete non-current sessions first
for (Session session in sessions.sessions) {
+ if (session.current) continue;
try {
await account.deleteSession(sessionId: session.$id);
log('Deleted session: ${session.$id}');
} catch (e) {
log('Failed to delete session ${session.$id}: $e');
}
}
+ // Delete current session last
+ try {
+ await account.deleteSession(sessionId: 'current');
+ log('Deleted current session');
+ } catch (e) {
+ log('Failed to delete current session: $e');
+ }
+
log('All sessions revoked successfully');
} catch (e) {
log('Error revoking sessions: $e');
}
}📝 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.
Suggested change
| Future<void> revokeAllSessions() async { | |
| try { | |
| // Get all sessions | |
| final SessionList sessions = await account.listSessions(); | |
| // Delete each session | |
| for (Session session in sessions.sessions) { | |
| try { | |
| await account.deleteSession(sessionId: session.$id); | |
| log('Deleted session: ${session.$id}'); | |
| } catch (e) { | |
| log('Failed to delete session ${session.$id}: $e'); | |
| } | |
| } | |
| log('All sessions revoked successfully'); | |
| } catch (e) { | |
| log('Error revoking sessions: $e'); | |
| // Continue with deletion even if session revocation fails | |
| } | |
| } | |
| Future<void> revokeAllSessions() async { | |
| try { | |
| // Get all sessions | |
| final SessionList sessions = await account.listSessions(); | |
| // Delete non-current sessions first | |
| for (Session session in sessions.sessions) { | |
| if (session.current) continue; | |
| try { | |
| await account.deleteSession(sessionId: session.$id); | |
| log('Deleted session: ${session.$id}'); | |
| } catch (e) { | |
| log('Failed to delete session ${session.$id}: $e'); | |
| } | |
| } | |
| // Delete current session last | |
| try { | |
| await account.deleteSession(sessionId: 'current'); | |
| log('Deleted current session'); | |
| } catch (e) { | |
| log('Failed to delete current session: $e'); | |
| } | |
| log('All sessions revoked successfully'); | |
| } catch (e) { | |
| log('Error revoking sessions: $e'); | |
| // Continue with deletion even if session revocation fails | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/delete_account_controller.dart` around lines 71 - 91, The
revokeAllSessions function currently deletes sessions including the current one
which causes the Appwrite client to lose auth mid-loop; update revokeAllSessions
to determine the current session id (e.g., via account.get() or client.session
info), iterate sessions from account.listSessions() while skipping the current
session during the main loop (use session.$id to compare), delete all other
sessions first with account.deleteSession(sessionId: ...), then finally delete
the current session last; keep existing try/catch logging behavior and ensure
you still proceed with account deletion even if session revocation errors occur.
Comment on lines
+94
to
+191
| Future<void> deleteRelatedRecords() async { | ||
| try { | ||
| final String userId = authStateController.uid!; | ||
|
|
||
| // Delete follower documents where user is the follower | ||
| try { | ||
| final followerDocs = await databases.listDocuments( | ||
| databaseId: userDatabaseID, | ||
| collectionId: followersCollectionID, | ||
| queries: [Query.equal('followerUserId', [userId])], | ||
| ); | ||
| for (var doc in followerDocs.documents) { | ||
| await databases.deleteDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: followersCollectionID, | ||
| documentId: doc.$id, | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| log('Error deleting follower records: $e'); | ||
| } | ||
|
|
||
| // Delete follower documents where user is being followed | ||
| try { | ||
| final followingDocs = await databases.listDocuments( | ||
| databaseId: userDatabaseID, | ||
| collectionId: followersCollectionID, | ||
| queries: [Query.equal('followingUserId', [userId])], | ||
| ); | ||
| for (var doc in followingDocs.documents) { | ||
| await databases.deleteDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: followersCollectionID, | ||
| documentId: doc.$id, | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| log('Error deleting following records: $e'); | ||
| } | ||
|
|
||
| // Delete friend requests (sender or receiver) | ||
| try { | ||
| final sentRequests = await databases.listDocuments( | ||
| databaseId: userDatabaseID, | ||
| collectionId: friendRequestCollectionId, | ||
| queries: [Query.equal('senderId', [userId])], | ||
| ); | ||
| for (var doc in sentRequests.documents) { | ||
| await databases.deleteDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: friendRequestCollectionId, | ||
| documentId: doc.$id, | ||
| ); | ||
| } | ||
|
|
||
| final receivedRequests = await databases.listDocuments( | ||
| databaseId: userDatabaseID, | ||
| collectionId: friendRequestCollectionId, | ||
| queries: [Query.equal('receiverId', [userId])], | ||
| ); | ||
| for (var doc in receivedRequests.documents) { | ||
| await databases.deleteDocument( | ||
| databaseId: userDatabaseID, | ||
| collectionId: friendRequestCollectionId, | ||
| documentId: doc.$id, | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| log('Error deleting friend requests: $e'); | ||
| } | ||
|
|
||
| // Delete subscribed upcoming rooms | ||
| try { | ||
| final subscribedRooms = await databases.listDocuments( | ||
| databaseId: upcomingRoomsDatabaseId, | ||
| collectionId: subscribedUserCollectionId, | ||
| queries: [Query.equal('userID', [userId])], | ||
| ); | ||
| for (var doc in subscribedRooms.documents) { | ||
| await databases.deleteDocument( | ||
| databaseId: upcomingRoomsDatabaseId, | ||
| collectionId: subscribedUserCollectionId, | ||
| documentId: doc.$id, | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| log('Error deleting subscribed rooms: $e'); | ||
| } | ||
|
|
||
| // Note: Stories created by user may need to be handled differently | ||
| // depending on business logic (transfer ownership vs delete) | ||
|
|
||
| log('Related records deleted successfully'); | ||
| } catch (e) { | ||
| log('Error deleting related records: $e'); | ||
| // Continue with deletion even if some related records fail | ||
| } | ||
| } |
There was a problem hiding this comment.
Pagination: listDocuments returns at most 25 documents by default — records beyond the limit are silently left behind.
Appwrite's listDocuments has a default limit of 25. Users with more followers, friend requests, or subscriptions will have orphaned records after deletion. Each query loop should paginate (e.g., using Query.limit(100) with Query.offset(...) or Query.cursorAfter(...)) or loop until all documents are deleted.
🛠️ Example: paginated deletion helper
Future<void> _deleteAllMatchingDocuments({
required String databaseId,
required String collectionId,
required List<String> queries,
}) async {
while (true) {
final docs = await databases.listDocuments(
databaseId: databaseId,
collectionId: collectionId,
queries: [...queries, Query.limit(100)],
);
if (docs.documents.isEmpty) break;
for (var doc in docs.documents) {
await databases.deleteDocument(
databaseId: databaseId,
collectionId: collectionId,
documentId: doc.$id,
);
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/delete_account_controller.dart` around lines 94 - 191, The
deleteRelatedRecords method currently uses databases.listDocuments which returns
only up to 25 items; update each block that lists documents (followers/following
using followersCollectionID, friend requests using friendRequestCollectionId,
and subscribed rooms using subscribedUserCollectionId with
upcomingRoomsDatabaseId/userDatabaseID) to paginate and delete until no
documents remain—either by extracting the listing+deletion into a reusable
helper (e.g., _deleteAllMatchingDocuments) that calls listDocuments with
Query.limit(100) (and Query.offset or Query.cursorAfter as appropriate) in a
while loop, or by adding a loop in each try-block that requests pages and
deletes each page’s documents until an empty page is returned. Ensure the helper
accepts databaseId, collectionId, and the base queries so all four deletion
sections call it.
Comment on lines
+193
to
+205
| /// Deletes the Appwrite authentication account | ||
| Future<void> deleteAuthAccount() async { | ||
| try { | ||
| // Delete the user's authentication account from Appwrite | ||
| // This is the final step and will permanently remove the user | ||
| await account.updateStatus(); | ||
| log('Auth account marked for deletion'); | ||
| } catch (e) { | ||
| log('Error deleting auth account: $e'); | ||
| // This is critical - rethrow to ensure caller knows deletion failed | ||
| rethrow; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Appwrite account updateStatus vs users delete permanent deletion
💡 Result:
-
account.updateStatus()/users.updateStatus(userId, false): Does not delete the user. It blocks/unblocks the user (status boolean) and is explicitly recommended as an alternative to deletion when you want to keep the user ID reserved. User-generated data (documents/files) remains. [1][2] -
users.delete(userId): Deletes the user record and releases the user ID (it can be reused). Appwrite warns you should delete user-related resources (documents, storage files, etc.) before deleting the user to avoid leftover/orphaned data and permission issues. [1][2]
Also note: Appwrite renamed the old client account.delete() behavior to account.updateStatus() to clarify that it blocks rather than deletes; full deletion is done via Dashboard or Server SDK users.delete(). [2][3]
Sources: [1] Appwrite Users API docs (updateStatus + delete) (appwrite.io), [2] Appwrite 0.14 announcement clarifying rename + full deletion via users.delete (dev.to), [3] dart_appwrite changelog noting account.delete() -> account.updateStatus() (pub.dev)
Citations:
- 1: https://appwrite.io/docs/references/cloud/server-dart/users?utm_source=openai
- 2: https://dev.to/appwrite/announcing-appwrite-014-with-11-cloud-function-runtimes-36f5?utm_source=openai
- 3: https://pub.dev/packages/dart_appwrite/changelog?utm_source=openai
🏁 Script executed:
# First, let's look at the code file and its context
cat -n lib/controllers/delete_account_controller.dart | sed -n '185,215p'Repository: AOSSIE-Org/Resonate
Length of output: 1311
account.updateStatus() blocks the account—it does not permanently delete it.
On the Appwrite client SDK, account.updateStatus() sets the user's status to blocked/disabled, reserving the user ID. Permanent deletion requires the server-side users.delete(userId) endpoint. If permanent removal is the goal, replace account.updateStatus() with a call to the Appwrite server SDK's users.delete() method or invoke a backend endpoint that handles it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/delete_account_controller.dart` around lines 193 - 205, The
deleteAuthAccount() method currently calls account.updateStatus(), which only
blocks the user; replace that call with a server-side permanent delete
flow—invoke the Appwrite server SDK users.delete(userId) (or call your backend
endpoint that performs users.delete) instead of account.updateStatus(); keep the
existing try/catch behavior (log errors and rethrow) and ensure you pass the
correct userId to the server-side delete routine so the account is removed
permanently.
Comment on lines
+209
to
+260
| Future<bool> deleteAccount() async { | ||
| try { | ||
| log('Starting account deletion for user: ${authStateController.uid}'); | ||
|
|
||
| // Step 1: Verify authentication (user must be logged in) | ||
| try { | ||
| await account.get(); | ||
| } catch (e) { | ||
| log('User not authenticated: $e'); | ||
| return false; | ||
| } | ||
|
|
||
| // Step 2: Revoke all active sessions/tokens | ||
| await revokeAllSessions(); | ||
|
|
||
| // Step 3: Delete related records | ||
| await deleteRelatedRecords(); | ||
|
|
||
| // Step 4: Delete user profile picture | ||
| if (authStateController.profileImageID != null && | ||
| authStateController.profileImageID!.isNotEmpty) { | ||
| await deleteUserProfilePicture(); | ||
| } | ||
|
|
||
| // Step 5: Delete username document | ||
| if (authStateController.userName != null && | ||
| authStateController.userName!.isNotEmpty) { | ||
| await deleteUsernamesCollectionDocument(); | ||
| } | ||
|
|
||
| // Step 6: Delete user document from users collection | ||
| await deleteUsersCollectionDocument(); | ||
|
|
||
| // Step 7: Delete authentication provider account (final step) | ||
| // Note: Appwrite handles session invalidation when account is deleted | ||
| try { | ||
| // The current session will be automatically invalidated | ||
| // when we navigate away, so we just mark completion | ||
| log('Account deletion completed successfully'); | ||
| } catch (e) { | ||
| log('Note: Session cleanup handled by navigation: $e'); | ||
| } | ||
|
|
||
| // Step 8: Redirect to welcome screen | ||
| Get.offAllNamed(AppRoutes.welcomeScreen); | ||
|
|
||
| return true; | ||
| } catch (e) { | ||
| log('Critical error during account deletion: $e'); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: deleteAuthAccount() is never called, and Step 7 is dead code.
The method deleteAuthAccount() (line 194) is defined but never invoked anywhere in deleteAccount(). Step 7 (lines 244–250) is a no-op try-catch wrapping a log() statement that cannot throw — the auth account is never actually disabled or deleted. This defeats the core purpose of the PR.
Additionally, the ordering is broken: revokeAllSessions() at Step 2 deletes all sessions including the current one, which invalidates the client's authentication. All subsequent authenticated API calls in Steps 3–6 (deleteRelatedRecords, deleteUserProfilePicture, etc.) will fail with a 401 Unauthorized error.
The correct approach:
- Perform all data cleanup first (Steps 3–6).
- Call
deleteAuthAccount()to disable the auth account. - Revoke all sessions last (or at minimum, skip the current session until the very end).
🐛 Proposed fix: reorder steps and invoke deleteAuthAccount
Future<bool> deleteAccount() async {
try {
log('Starting account deletion for user: ${authStateController.uid}');
// Step 1: Verify authentication (user must be logged in)
try {
await account.get();
} catch (e) {
log('User not authenticated: $e');
return false;
}
- // Step 2: Revoke all active sessions/tokens
- await revokeAllSessions();
-
- // Step 3: Delete related records
+ // Step 2: Delete related records
await deleteRelatedRecords();
- // Step 4: Delete user profile picture
+ // Step 3: Delete user profile picture
if (authStateController.profileImageID != null &&
authStateController.profileImageID!.isNotEmpty) {
await deleteUserProfilePicture();
}
- // Step 5: Delete username document
+ // Step 4: Delete username document
if (authStateController.userName != null &&
authStateController.userName!.isNotEmpty) {
await deleteUsernamesCollectionDocument();
}
- // Step 6: Delete user document from users collection
+ // Step 5: Delete user document from users collection
await deleteUsersCollectionDocument();
- // Step 7: Delete authentication provider account (final step)
- // Note: Appwrite handles session invalidation when account is deleted
- try {
- // The current session will be automatically invalidated
- // when we navigate away, so we just mark completion
- log('Account deletion completed successfully');
- } catch (e) {
- log('Note: Session cleanup handled by navigation: $e');
- }
+ // Step 6: Delete authentication account
+ await deleteAuthAccount();
+
+ // Step 7: Revoke all remaining sessions (current session last)
+ await revokeAllSessions();
- // Step 8: Redirect to welcome screen
+ log('Account deletion completed successfully');
+
+ // Step 8: Navigate to welcome screen
Get.offAllNamed(AppRoutes.welcomeScreen);
return true;
} catch (e) {
log('Critical error during account deletion: $e');
return false;
}
}📝 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.
Suggested change
| Future<bool> deleteAccount() async { | |
| try { | |
| log('Starting account deletion for user: ${authStateController.uid}'); | |
| // Step 1: Verify authentication (user must be logged in) | |
| try { | |
| await account.get(); | |
| } catch (e) { | |
| log('User not authenticated: $e'); | |
| return false; | |
| } | |
| // Step 2: Revoke all active sessions/tokens | |
| await revokeAllSessions(); | |
| // Step 3: Delete related records | |
| await deleteRelatedRecords(); | |
| // Step 4: Delete user profile picture | |
| if (authStateController.profileImageID != null && | |
| authStateController.profileImageID!.isNotEmpty) { | |
| await deleteUserProfilePicture(); | |
| } | |
| // Step 5: Delete username document | |
| if (authStateController.userName != null && | |
| authStateController.userName!.isNotEmpty) { | |
| await deleteUsernamesCollectionDocument(); | |
| } | |
| // Step 6: Delete user document from users collection | |
| await deleteUsersCollectionDocument(); | |
| // Step 7: Delete authentication provider account (final step) | |
| // Note: Appwrite handles session invalidation when account is deleted | |
| try { | |
| // The current session will be automatically invalidated | |
| // when we navigate away, so we just mark completion | |
| log('Account deletion completed successfully'); | |
| } catch (e) { | |
| log('Note: Session cleanup handled by navigation: $e'); | |
| } | |
| // Step 8: Redirect to welcome screen | |
| Get.offAllNamed(AppRoutes.welcomeScreen); | |
| return true; | |
| } catch (e) { | |
| log('Critical error during account deletion: $e'); | |
| return false; | |
| } | |
| } | |
| Future<bool> deleteAccount() async { | |
| try { | |
| log('Starting account deletion for user: ${authStateController.uid}'); | |
| // Step 1: Verify authentication (user must be logged in) | |
| try { | |
| await account.get(); | |
| } catch (e) { | |
| log('User not authenticated: $e'); | |
| return false; | |
| } | |
| // Step 2: Delete related records | |
| await deleteRelatedRecords(); | |
| // Step 3: Delete user profile picture | |
| if (authStateController.profileImageID != null && | |
| authStateController.profileImageID!.isNotEmpty) { | |
| await deleteUserProfilePicture(); | |
| } | |
| // Step 4: Delete username document | |
| if (authStateController.userName != null && | |
| authStateController.userName!.isNotEmpty) { | |
| await deleteUsernamesCollectionDocument(); | |
| } | |
| // Step 5: Delete user document from users collection | |
| await deleteUsersCollectionDocument(); | |
| // Step 6: Delete authentication account | |
| await deleteAuthAccount(); | |
| // Step 7: Revoke all remaining sessions (current session last) | |
| await revokeAllSessions(); | |
| log('Account deletion completed successfully'); | |
| // Step 8: Navigate to welcome screen | |
| Get.offAllNamed(AppRoutes.welcomeScreen); | |
| return true; | |
| } catch (e) { | |
| log('Critical error during account deletion: $e'); | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/controllers/delete_account_controller.dart` around lines 209 - 260, The
deleteAccount() flow is wrong: it never calls deleteAuthAccount() and revokes
all sessions too early which causes subsequent authenticated deletes to 401; fix
by reordering and invoking the missing call—keep the initial authentication
check (account.get()), then perform deleteRelatedRecords(),
deleteUserProfilePicture(), deleteUsernamesCollectionDocument(),
deleteUsersCollectionDocument() (all awaited), then await deleteAuthAccount()
(replace the no-op try/catch block with this call and handle errors), and only
after that call revokeAllSessions() as the final step (or ensure
revokeAllSessions() skips the current session until absolutely last); ensure all
calls are awaited and proper try/catch/logging is kept around
deleteAuthAccount(), revokeAllSessions(), and data-deletion functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes issue where account deletion did not permanently remove user records
or invalidate authentication.
Root Cause
The deletion endpoint did not fully remove:
As a result, deleted accounts remained accessible.
Changes
Testing
Summary by CodeRabbit
Release Notes
Bug Fixes
New Features