Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cm 499 delete account and data #501

Merged
merged 14 commits into from
Oct 8, 2023
Merged

Conversation

rodriguesk
Copy link
Member

Detailed information:

Simply deletes user account from user account table in database.

Closing issues:

List all issues the pull request solve:

Test plan (required)

  • [ x] The PR contains new PyTest unit/integration tests for any function or functional added.
  • [ x] The PR changes existing PyTest unit/integration tests to keep all tests up to date.
  • [ x] The PR does not lead to degradation in unit test coverage.
  • [ x] Project parts affected by changes in this PR was tested manually on your local (using Postman or in any other way). List everything you've tested below:
    • Deletes user account from account table in database
    • User can no longer log in after their account is deleted

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (cd1cb16) 73.44% compared to head (b1e5564) 73.64%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #501      +/-   ##
===========================================
+ Coverage    73.44%   73.64%   +0.20%     
===========================================
  Files           82       82              
  Lines         2598     2618      +20     
  Branches       314      315       +1     
===========================================
+ Hits          1908     1928      +20     
  Misses         657      657              
  Partials        33       33              
Files Coverage Δ
app/account/schemas.py 100.00% <100.00%> (ø)
app/models.py 100.00% <100.00%> (ø)
app/account/routes.py 95.31% <87.50%> (-1.12%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

danmash
danmash previously approved these changes Sep 30, 2023
Copy link
Member

@danmash danmash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rodriguesk , nicely done 👍 the PR seems valid, the only difference with the requirement I see is that you are using POST instead of DELETE, but I think this is fine.
According to docs conversations will be stored in the db after user deletion with null value
https://docs.sqlalchemy.org/en/20/orm/cascades.html#delete

@rodriguesk
Copy link
Member Author

rodriguesk commented Sep 30, 2023 via email

@danmash
Copy link
Member

danmash commented Sep 30, 2023

@rodriguesk I am unable to test my change on local due to API errors, I think the solution here is ondelete="SET NULL", because looks like the MSSQL default action is "NO ACTION"

@danmash
Copy link
Member

danmash commented Oct 1, 2023

@Svenstar74 you could check this branch again, let me know if you still have issues

@rodriguesk
Copy link
Member Author

rodriguesk commented Oct 2, 2023

@danmash I did the flask db upgrade and removed the database volume and rebuilt the dockers and then tested your code changes using postman and still am getting the error shown below.
Could you help solve this please? It's a bit too hard for me to figure out. Here's how you can replicate manual testing to get the error I did:

  1. run in the terminal:
    docker-compose -p climatemind-backend --profile webapp -f docker/docker-compose.yml up -d --build
  2. Open a browser and turn on dev tools to see the developer console and access the network API calls and responses.
  3. Go to: http://localhost:3000/
  4. Use the browser to make a new fake account with email test@test.net and password testing123
    and write down the session id and the JWT token
  5. Replace the JWT token and Session id in the following cURL code and run the cURL code in the terminal

curl --location 'http://localhost:5000/user-account'
--header 'Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmcmVzaCI6dHJ1ZSwiaWF0IjoxNjk2Mjg3MzcyLCJqdGkiOiI0NWE2MTJjMS1lNjQyLTQyMTEtOGRlNC01OTFkMWVhMTUyYmUiLCJuYmYiOjE2OTYyODczNzIsInR5cGUiOiJhY2Nlc3MiLCJzdWIiOiJENzYzMDFGMy00MTk3LTQwMkItQjlBRS04MkYzOEYzMkFBODIiLCJleHAiOjE2OTYyOTQ1NzJ9.595EpNTEkONJJEOQCuAqP_jDASHq8S2aR-5N9lr26Lk'
--header 'X-Session-Id: 6df9b0ad-e427-460f-b654-c680d22e9208'
--header 'Content-Type: text/plain'
--data '{
"currentPassword": "testing123"
}'

172.23.0.1 - - [02/Oct/2023 22:57:19] "POST /user-account HTTP/1.1" 500 -
Traceback (most recent call last):
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context
self.dialect.do_execute(
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
cursor.execute(statement, parameters)
pyodbc.IntegrityError: ('23000', '[23000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The DELETE statement conflicted with the REFERENCE constraint "fk_sessions__user_uuid__users". The conflict occurred in database "sqldb-web-prod-001", table "dbo.sessions", column 'user_uuid'. (547) (SQLExecDirectW)')

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2464, in call
return self.wsgi_app(environ, start_response)
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2450, in wsgi_app
response = self.handle_exception(e)
File "/usr/local/lib/python3.8/site-packages/flask_cors/extension.py", line 165, in wrapped_function
return cors_after_request(app.make_response(f(*args, **kwargs)))
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1867, in handle_exception
reraise(exc_type, exc_value, tb)
File "/usr/local/lib/python3.8/site-packages/flask/compat.py", line 39, in reraise
raise value
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app
response = self.full_dispatch_request()
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request
rv = self.handle_user_exception(e)
File "/usr/local/lib/python3.8/site-packages/flask_cors/extension.py", line 165, in wrapped_function
return cors_after_request(app.make_response(f(*args, **kwargs)))
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1821, in handle_user_exception
reraise(exc_type, exc_value, tb)
File "/usr/local/lib/python3.8/site-packages/flask/compat.py", line 39, in reraise
raise value
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1950, in full_dispatch_request
rv = self.dispatch_request()
File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1936, in dispatch_request
return self.view_functionsrule.endpoint
File "/usr/local/lib/python3.8/site-packages/flask_cors/decorator.py", line 128, in wrapped_function
resp = make_response(f(*args, **kwargs))
File "/usr/local/lib/python3.8/site-packages/flask_jwt_extended/view_decorators.py", line 115, in decorator
return fn(*args, **kwargs)
File "/app/app/account/routes.py", line 157, in delete_user_account
current_user.delete_user()
File "/app/app/models.py", line 45, in delete_user
self.query.filter_by(user_uuid=self.user_uuid).delete()
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/query.py", line 3899, in delete
delete_op.exec
()
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1697, in exec

self._do_exec()
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1928, in _do_exec
self._execute_stmt(delete_stmt)
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1702, in _execute_stmt
self.result = self.query._execute_crud(stmt, self.mapper)
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/query.py", line 3541, in _execute_crud
return conn.execute(stmt, self._params)
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
return meth(self, multiparams, params)
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/sql/elements.py", line 298, in _execute_on_connection
return connection._execute_clauseelement(self, multiparams, params)
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1124, in _execute_clauseelement
ret = self._execute_context(
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1316, in _execute_context
self.handle_dbapi_exception(
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1510, in handle_dbapi_exception
util.raise
(
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 182, in raise

raise exception
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context
self.dialect.do_execute(
File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.IntegrityError: (pyodbc.IntegrityError) ('23000', '[23000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The DELETE statement conflicted with the REFERENCE constraint "fk_sessions__user_uuid__users". The conflict occurred in database "sqldb-web-prod-001", table "dbo.sessions", column 'user_uuid'. (547) (SQLExecDirectW)')
[SQL: DELETE FROM users WHERE users.user_uuid = ?]
[parameters: ('D76301F3-4197-402B-B9AE-82F38F32AA82',)]
(Background on this error at: http://sqlalche.me/e/13/gkpj)

@rodriguesk
Copy link
Member Author

@danmash I don't know why pytest doesn't get these errors... ideally the unit test would actually test this part too, right?

@danmash
Copy link
Member

danmash commented Oct 3, 2023

I don't have a test account. I am unable to go beyond Q10 to create the new account.
Screenshot 2023-10-03 at 12 38 14

@Svenstar74
Copy link
Contributor

Svenstar74 commented Oct 3, 2023

@danmash It seems like the error still occurs:

2023-10-03 13:42:57 172.18.0.1 - - [03/Oct/2023 11:42:57] "POST /user-account HTTP/1.1" 500 -
2023-10-03 13:42:57 Traceback (most recent call last):
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context
2023-10-03 13:42:57     self.dialect.do_execute(
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
2023-10-03 13:42:57     cursor.execute(statement, parameters)
2023-10-03 13:42:57 pyodbc.IntegrityError: ('23000', '[23000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The DELETE statement conflicted with the REFERENCE constraint "fk_sessions__user_uuid__users". The conflict occurred in database "sqldb-web-prod-001", table "dbo.sessions", column \'user_uuid\'. (547) (SQLExecDirectW)')
2023-10-03 13:42:57 
2023-10-03 13:42:57 The above exception was the direct cause of the following exception:
2023-10-03 13:42:57 
2023-10-03 13:42:57 Traceback (most recent call last):
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2464, in __call__
2023-10-03 13:42:57     return self.wsgi_app(environ, start_response)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2450, in wsgi_app
2023-10-03 13:42:57     response = self.handle_exception(e)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask_cors/extension.py", line 165, in wrapped_function
2023-10-03 13:42:57     return cors_after_request(app.make_response(f(*args, **kwargs)))
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1867, in handle_exception
2023-10-03 13:42:57     reraise(exc_type, exc_value, tb)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/_compat.py", line 39, in reraise
2023-10-03 13:42:57     raise value
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 2447, in wsgi_app
2023-10-03 13:42:57     response = self.full_dispatch_request()
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1952, in full_dispatch_request
2023-10-03 13:42:57     rv = self.handle_user_exception(e)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask_cors/extension.py", line 165, in wrapped_function
2023-10-03 13:42:57     return cors_after_request(app.make_response(f(*args, **kwargs)))
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1821, in handle_user_exception
2023-10-03 13:42:57     reraise(exc_type, exc_value, tb)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/_compat.py", line 39, in reraise
2023-10-03 13:42:57     raise value
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1950, in full_dispatch_request
2023-10-03 13:42:57     rv = self.dispatch_request()
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask/app.py", line 1936, in dispatch_request
2023-10-03 13:42:57     return self.view_functions[rule.endpoint](**req.view_args)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask_cors/decorator.py", line 128, in wrapped_function
2023-10-03 13:42:57     resp = make_response(f(*args, **kwargs))
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/flask_jwt_extended/view_decorators.py", line 115, in decorator
2023-10-03 13:42:57     return fn(*args, **kwargs)
2023-10-03 13:42:57   File "/app/app/account/routes.py", line 157, in delete_user_account
2023-10-03 13:42:57     current_user.delete_user()
2023-10-03 13:42:57   File "/app/app/models.py", line 45, in delete_user
2023-10-03 13:42:57     self.query.filter_by(user_uuid=self.user_uuid).delete()
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/query.py", line 3899, in delete
2023-10-03 13:42:57     delete_op.exec_()
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1697, in exec_
2023-10-03 13:42:57     self._do_exec()
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1928, in _do_exec
2023-10-03 13:42:57     self._execute_stmt(delete_stmt)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/persistence.py", line 1702, in _execute_stmt
2023-10-03 13:42:57     self.result = self.query._execute_crud(stmt, self.mapper)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/orm/query.py", line 3541, in _execute_crud
2023-10-03 13:42:57     return conn.execute(stmt, self._params)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
2023-10-03 13:42:57     return meth(self, multiparams, params)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/sql/elements.py", line 298, in _execute_on_connection
2023-10-03 13:42:57     return connection._execute_clauseelement(self, multiparams, params)
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1124, in _execute_clauseelement
2023-10-03 13:42:57     ret = self._execute_context(
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1316, in _execute_context
2023-10-03 13:42:57     self._handle_dbapi_exception(
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1510, in _handle_dbapi_exception
2023-10-03 13:42:57     util.raise_(
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/util/compat.py", line 182, in raise_
2023-10-03 13:42:57     raise exception
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/base.py", line 1276, in _execute_context
2023-10-03 13:42:57     self.dialect.do_execute(
2023-10-03 13:42:57   File "/usr/local/lib/python3.8/site-packages/sqlalchemy/engine/default.py", line 593, in do_execute
2023-10-03 13:42:57     cursor.execute(statement, parameters)
2023-10-03 13:42:57 sqlalchemy.exc.IntegrityError: (pyodbc.IntegrityError) ('23000', '[23000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The DELETE statement conflicted with the REFERENCE constraint "fk_sessions__user_uuid__users". The conflict occurred in database "sqldb-web-prod-001", table "dbo.sessions", column \'user_uuid\'. (547) (SQLExecDirectW)')
2023-10-03 13:42:57 [SQL: DELETE FROM users WHERE users.user_uuid = ?]
2023-10-03 13:42:57 [parameters: ('65510044-9406-4D68-A505-29A84EA7D50B',)]
2023-10-03 13:42:57 (Background on this error at: http://sqlalche.me/e/13/gkpj)

@danmash
Copy link
Member

danmash commented Oct 3, 2023

@Svenstar74 @rodriguesk I am still unable to test it on my local. Try the new migration I've just pushed.
I think the problem here is that mssql db doesn't set ondelete action
for our foreign keys. I tried the new migration and now I see the action was changed in my local db.

Screenshot 2023-10-03 at 13 03 09

Even if there are still some errors left, you could solve them by moving in this direction.

@Svenstar74
Copy link
Contributor

Update: It's looking good.
I pulled the recent changes and also rebuilt the docker backend stuff and the account was deleted successfully.
Only thing worth mentioning: Deleting may take a few seconds, so if I delete the account (and get logged out) and login immediately afterwards, I'm logged in even though the account is deleted.

danmash
danmash previously approved these changes Oct 3, 2023
@danmash
Copy link
Member

danmash commented Oct 3, 2023

note the different behaviour ondelete of the 3 foreign keys. Change if needed.

@danmash
Copy link
Member

danmash commented Oct 6, 2023

@rodriguesk feel free to merge when ready

@rodriguesk
Copy link
Member Author

rodriguesk commented Oct 6, 2023 via email

@rodriguesk rodriguesk merged commit 9efd207 into develop Oct 8, 2023
@delete-merged-branch delete-merged-branch bot deleted the cm-499-delete-account-and-data branch October 8, 2023 02:33
rodriguesk added a commit that referenced this pull request Oct 8, 2023
* Removed flask-selfdoc from project.

* Update score_nodes.py

added relatedPersonalValues to the GET response of the /feed endpoint to include all the personal values associated with each climate change impact for user's feed.

* run linting

* optional parameter to skip recaptcha

* change spelling

* remove timedelta

* Update installation.md

instructions how to free up port 5000 on macs.

* Cm 499 delete account and data (#501)

* Black formatting

* Extra test to ensure deleted user can't login

* added api documentation

* #499 ondelete="SET NULL" or "CASCADE"

* #499 explicit ondelete action for user foreign keys

* Change route method from POST to DELETE

* switch POST documentation to DELETE in app/static/Climate-Mind_bundled.yml

---------

Co-authored-by: Daniil Mashkin <dm@zint.io>
Co-authored-by: Daniil Mashkin <danmash@users.noreply.github.com>

---------

Co-authored-by: Jason Hutson <110316760+HutsonJason@users.noreply.github.com>
Co-authored-by: Svenstar74 <sven.firmbach@gmx.de>
Co-authored-by: Daniil Mashkin <dm@zint.io>
Co-authored-by: Daniil Mashkin <danmash@users.noreply.github.com>
rodriguesk added a commit that referenced this pull request Nov 24, 2023
* Removed flask-selfdoc from project.

* Update score_nodes.py

added relatedPersonalValues to the GET response of the /feed endpoint to include all the personal values associated with each climate change impact for user's feed.

* run linting

* optional parameter to skip recaptcha

* change spelling

* remove timedelta

* Update installation.md

instructions how to free up port 5000 on macs.

* Cm 499 delete account and data (#501)

* Black formatting

* Extra test to ensure deleted user can't login

* added api documentation

* #499 ondelete="SET NULL" or "CASCADE"

* #499 explicit ondelete action for user foreign keys

* Change route method from POST to DELETE

* switch POST documentation to DELETE in app/static/Climate-Mind_bundled.yml

---------

Co-authored-by: Daniil Mashkin <dm@zint.io>
Co-authored-by: Daniil Mashkin <danmash@users.noreply.github.com>

* Analytics endpoint (#504)

* Initial commit for #503

* Modified 2 files

* Fixing some bugs

* Remove init not needed

Remove

* Fixed bugs

* clearer datetime string format documentation.

* Fixed bug with date time

* Lint

* #503 unittests fixed

---------

Co-authored-by: Daniil Mashkin <dm@zint.io>

---------

Co-authored-by: Jason Hutson <110316760+HutsonJason@users.noreply.github.com>
Co-authored-by: Svenstar74 <sven.firmbach@gmx.de>
Co-authored-by: Daniil Mashkin <dm@zint.io>
Co-authored-by: Daniil Mashkin <danmash@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

delete account and associated data
4 participants