lucyparsons / OpenOversight

Police oversight and accountability through public data 👮
https://openoversight.com
GNU General Public License v3.0
237 stars 79 forks source link

Address missing UUIDs in pre-existing users #1042

Closed michplunkett closed 1 year ago

michplunkett commented 1 year ago

Description of Changes

The previous build ran into a problem where we added a middleware creation for the _uuid column in the users table, but did not deal with the problem of the rows that lacked UUIDs in the users table. This was dealt with through the use of a loop in the migration that adds a UUID to all pre-existing rows via Python 🐍.

Tests and linting

% git checkout address_missing_uuids
% make clean_all                          
rm -rf ./OpenOversight/app/static/dist/
docker-compose stop
...
% git checkout 872a01ab  # Checks out a previous version of the branch before we merged in the UUIDs
...
HEAD is now at 872a01ab Remove `HiddenField`s and add `created_by` to `LicensePlate` and `Location` (#1034)
% make start    
docker-compose build
...
% make dev # Create tables and the application at the point in time for the above commit
... # Verify that all tables and data are in place, etc.
% git checkout address_missing_uuids      
Previous HEAD position was 872a01ab Remove `HiddenField`s and add `created_by` to `LicensePlate` and `Location` (#1034)
Switched to branch 'address_missing_uuids'
Your branch is up to date with 'origin/address_missing_uuids'.
% (env) michaelp@MacBook-Air-5 OpenOversight % make start
docker-compose build
...
docker-compose up -d
WARN[0000] The "APPROVE_REGISTRATIONS" variable is not set. Defaulting to a blank string. 
[+] Running 2/0
 ✔ Container openoversight-postgres-1  Running                                                                                                                                                             0.0s 
 ✔ Container openoversight-web-1       Running                                                                                                                                                             0.0s 
% docker exec -it openoversight-web-1 bash
I have no name!@0e2d34544358:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade b38c133bed3c -> a35aa1a114fa, Add create and last_update columns
INFO  [alembic.runtime.migration] Running upgrade a35aa1a114fa -> 52d3f6a21dd9, add _uuid column to users
I have no name!@0e2d34544358:/usr/src/app$ exit
%
sea-kelp commented 1 year ago

Would doing this as a separate migration cause issues for us when we try to apply changes to our instance?

michplunkett commented 1 year ago

The thing you'd have to make sure of is that the _uuid field isn't ever null when it's marked as non-null. Other than that, you could do this any which way you wanted, imo. That's the one limitation.