[#382] Update User API Endpoints #471
Conversation
seanpreston
changed the title
seanpreston
changed the title
| @@ -234,6 +234,10 @@ def delete_all(cls, db: Session) -> int: # pylint: disable=W0622 | |||
| deleted_count = db.query(cls).delete() | |||
| return deleted_count | |||
|
|
|||
| def refresh_from_db(self, db: Session) -> FidesopsBase: | |||
There was a problem hiding this comment.
I like this! I've recently queried for records that I just updated
There was a problem hiding this comment.
There's an intricacy here in the way DB sessions work that I'll explain.
The data in a DB session isn't guaranteed to be the same as in the database. In the event that another session mutates that same data that's currently held in another session, the data in that session becomes out of date. This manifests in our API tests because we have one session scoped to the test, and another that exists on the server within the API endpoint itself. What's happening is the test-scoped session creates data that the endpoint scoped session then mutates, and the test-scoped session no longer has accurate data. Without an effective refresh our assertions past this time would fail. This is why we also call db.expunge(...) before we call this function, to manually purge this object from the session so it's refreshed from the database.
There's a rabbit hole to go down here, that if we call db.expunge(self) within refresh_from_db(...) the following error is raised:
sqlalchemy.exc.InvalidRequestError: Instance <FidesopsUser at 0x7f5d4c70a640> is not present in this Session
which would imply the instance isn't part of the session at all, when if you call this same method within a pdb.set_trace(), the method works without issue.
There was a problem hiding this comment.
That's pretty interesting! So if db.expunge(self) isn't called refresh_from_db(...) won't get the most up to date data from the DB? I guess I assumed that the new query would pull in data directly from the DB instead of the session.
* adds user update scope * adds a get_current_user from token dependency * adds user update schema * adds update user endpoint * expose generate_auth_header as a util method * adds test for user update * adds password reset endpoint and tests for unauthorized user update * changes error code to 401, adds refresh_from_db method to the base class * add tests for update password endpoint * add type annotations * remove same user requirement for user update, add password reset scope * remove boolean return type * remove auto import
* adds user update scope * adds a get_current_user from token dependency * adds user update schema * adds update user endpoint * expose generate_auth_header as a util method * adds test for user update * adds password reset endpoint and tests for unauthorized user update * changes error code to 401, adds refresh_from_db method to the base class * add tests for update password endpoint * add type annotations * remove same user requirement for user update, add password reset scope * remove boolean return type * remove auto import
Purpose
This PR adds an API endpoints to:
Changes
/api/v1/user/{user_id}— updates the currently logged-in user's first and last names./api/v1/user/{user_id}/reset-password— updates the currently logged-in user's password ifold_passwordmatches the user's current password at the time of the update.Checklist
Run Unsafe PR Checkslabel has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #