Pass db_session to user delete method by guptapratykshh · Pull Request #3509 · augurlabs/augur

guptapratykshh · 2026-01-06T09:15:43Z

Description
This PR fixes a critical bug where users attempting to delete their accounts would encounter a 500 Internal Server Error. The root cause was that the user_delete() route handler was calling current_user.delete() without passing the required session parameter.

Fixed user_delete() function in augur/api/view/routes.py to pass db_session parameter to User.delete() method
Added try-except error handling to prevent unhandled exceptions
Added error logging for better debugging and issue tracking
Stored username before deletion to ensure success message displays correctly

This PR fixes #2672

Notes for Reviewers

Signed commits

Yes, I signed my commits.

augur/api/view/routes.py

augur/api/gunicorn_conf.py

guptapratykshh · 2026-01-08T06:50:54Z

Reverted all the imports and else block that were removed , please review the PR @MoralCode

MoralCode · 2026-01-09T19:55:00Z

Code looks good! Just make sure to rebase your changes.

Id also be curious how this was tested/get a couple maintainers to give it the thumbs up. Overall looks like this is very close to being able to ship!

guptapratykshh · 2026-01-09T20:54:15Z

I tested this by checking the code (the User.delete() method needs a session parameter that wasn't being passed) and writing a mock test to verify the fix works correctly.

Also the docker build failure is not related to this PR. It is CI issue with the geckodriver download because gitHub API returned null. Rerunning the build should fix it.

MoralCode · 2026-02-04T18:18:52Z

augur/api/view/routes.py

+        else:
+            logger.error(f"Failed to delete account {username}: {result[1]}")
+            flash("An error occurred removing the account")
+    except Exception as e:


should we roll back the transaction here if an error happens?

Looking at User.delete() - it calls session.commit() internally at the end. If an exception is raised before that commit, the changes aren't persisted anyway.

But I guess explicitly calling session.rollback() would be cleaner to reset the session state? That said, since db_session is a scoped session in, it should handle cleanup on the next request. So it's not strictly necessary.

MoralCode · 2026-02-04T18:22:15Z

I verified this by .... writing a mock test to verify the fix works correctly.

Is that test included?

shlokgilda

Looks good overall. Just a nitpick comment about logger.

shlokgilda · 2026-02-05T07:43:45Z

augur/api/view/routes.py

+        else:
+            logger.error(f"Failed to delete account {username}: {result[1]}")
+            flash("An error occurred removing the account")
+    except Exception as e:


But I guess explicitly calling session.rollback() would be cleaner to reset the session state? That said, since db_session is a scoped session in, it should handle cleanup on the next request. So it's not strictly necessary.

shlokgilda · 2026-02-05T07:44:40Z

augur/api/view/routes.py

+            logger.error(f"Failed to delete account {username}: {result[1]}")
+            flash("An error occurred removing the account")
+    except Exception as e:
+        logger.error(f"Exception occurred while deleting account: {e}")


nit: including the username in the exception log would help correlate errors with specific users during debugging

logger.error(f"Exception occurred while deleting account {username}: {e}")

MoralCode · 2026-02-09T22:36:56Z

Hello! Just wanted to check in to see if you were still interested in helping the maintainers merge this PR. We noticed it has been a little while since this last had activity, and are considering closing it or taking it over if it remains in its current state.

Please react to or reply to this to confirm your interest in the next 7 days or let us know if you are no longer interested in this so we can best prioritize everyone's contributions.

Thanks!

- Pass required db_session parameter to current_user.delete() - Add try-except block for proper exception handling - Store username before deletion for accurate flash message - Add error logging for debugging failed deletions Signed-off-by: Pratyksh Gupta <pratykshgupta9999@gmail.com> Signed-off-by: guptapratykshh <pratykshgupta9999@gmail.com> Signed-off-by: Adrian Edwards <adredwar@redhat.com>

Signed-off-by: Adrian Edwards <adredwar@redhat.com>

MoralCode · 2026-03-16T19:39:31Z

Taking this over as it is a relatively straightforward fix to a known, reproducible bug and the original submitter has not responded

shlokgilda · 2026-03-17T14:33:44Z

augur/api/view/routes.py

@@ -198,10 +198,11 @@ def authorize_user():
 @app.route('/account/delete')


Not from this PR, but user_delete is on a GET route. Might be worth a follow-up to change this to POST.

shlokgilda · 2026-03-17T14:35:31Z

augur/api/view/routes.py

        flash(f"Account {current_user.login_name} successfully removed")
        logout_user()
    else:
+        logger.error(f"Exception occurred while deleting account {current_user.login_name}: {e}")


There's no try/except (e will be undefined). Either wrap the delete in a try/except or just drop the logger line.

shlokgilda · 2026-03-17T14:36:07Z

augur/api/view/routes.py

        flash(f"Account {current_user.login_name} successfully removed")
        logout_user()
    else:
+        logger.error(f"Exception occurred while deleting account {current_user.login_name}: {e}")


current_user.login_name might not be valid after delete() since it commits the row deletion internally. Capture the username before calling delete.

guptapratykshh force-pushed the fix/delete-account-2672 branch 3 times, most recently from 2c82017 to a08dd38 Compare January 6, 2026 13:18

MoralCode reviewed Jan 7, 2026

View reviewed changes

augur/api/view/routes.py Show resolved Hide resolved

augur/api/gunicorn_conf.py Show resolved Hide resolved

guptapratykshh force-pushed the fix/delete-account-2672 branch from ba588f4 to 1e7fa95 Compare January 7, 2026 16:36

guptapratykshh requested a review from MoralCode January 7, 2026 16:37

guptapratykshh force-pushed the fix/delete-account-2672 branch 2 times, most recently from 8df7eb0 to 0cf49f2 Compare January 8, 2026 06:49

guptapratykshh force-pushed the fix/delete-account-2672 branch from 0cf49f2 to 51d3688 Compare January 9, 2026 20:39

guptapratykshh force-pushed the fix/delete-account-2672 branch from 0a52048 to 4793eaf Compare January 12, 2026 04:33

MoralCode added this to the v0.93.0 milestone Jan 21, 2026

MoralCode reviewed Feb 4, 2026

View reviewed changes

MoralCode added the discussion Seeking active feedback, usually for items under active development label Feb 4, 2026

MoralCode requested a review from shlokgilda February 4, 2026 18:21

shlokgilda reviewed Feb 5, 2026

View reviewed changes

MoralCode added the stale Stuff that's abandoned or not making forward progress and may need taking over/reassignment/closing label Feb 9, 2026

MoralCode modified the milestones: v0.93.0, v0.94.0 Mar 6, 2026

MoralCode changed the title ~~Fix #2672: Pass db_session to user delete method~~ Pass db_session to user delete method Mar 16, 2026

MoralCode force-pushed the fix/delete-account-2672 branch from 9728436 to 4aefc7a Compare March 16, 2026 19:35

MoralCode requested a review from Ulincsys as a code owner March 16, 2026 19:35

MoralCode force-pushed the fix/delete-account-2672 branch from 4aefc7a to 7e90eaa Compare March 16, 2026 19:37

MoralCode added 2 commits March 16, 2026 15:38

Revert original fix in 18b31ac

c97f765

Signed-off-by: Adrian Edwards <adredwar@redhat.com>

start simple

0da33d7

Signed-off-by: Adrian Edwards <adredwar@redhat.com>

MoralCode force-pushed the fix/delete-account-2672 branch from 7e90eaa to 0da33d7 Compare March 16, 2026 19:38

MoralCode removed the stale Stuff that's abandoned or not making forward progress and may need taking over/reassignment/closing label Mar 16, 2026

MoralCode self-assigned this Mar 16, 2026

shlokgilda requested changes Mar 17, 2026

View reviewed changes

		@@ -198,10 +198,11 @@ def authorize_user():
		@app.route('/account/delete')

Conversation

guptapratykshh commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

guptapratykshh commented Jan 8, 2026

Uh oh!

MoralCode commented Jan 9, 2026

Uh oh!

guptapratykshh commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoralCode commented Feb 4, 2026

Uh oh!

shlokgilda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MoralCode commented Feb 9, 2026

Uh oh!

MoralCode commented Mar 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

guptapratykshh commented Jan 6, 2026 •

edited

Loading