Upstream batch 4 #361
Merged
Upstream batch 4 #361
Conversation
This file contains 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
sea-kelp
force-pushed
the
upstream-batch-4
branch
from
97bf95e
to
cec1e3d
Compare
lucyparsons#928 Add `created_by` and `created_at` columns to tables so that we can better audit things happening in the application. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. <details><summary>DB migration output</summary> ```console $ flask db stamp head [2023-08-01 18:18:34,782] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running stamp_revision -> 18f43ac4622f $ flask db upgrade [2023-08-01 18:18:49,052] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b429700e2dd2, add created_by and created_at columns $ flask db downgrade [2023-08-01 18:18:54,540] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running downgrade b429700e2dd2 -> 18f43ac4622f, add created_by and created_at columns $ flask db upgrade [2023-08-01 18:19:04,407] INFO in __init__: OpenOversight startup INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b429700e2dd2, add created_by and created_at columns $ ``` </details>
## Fixes issue lucyparsons#1005 ## Description of Changes This fixes the issue we were having with trying to drop the `faces_user_id_fkey` in the push to `main`. What happened was the `faces_user_id_fkey` wasn't saved under a name in the previous database model. PosgreSQL will drop any attached constraints when a column is dropped (presuming there are no `CASCADE` effects), so I am removing the `drop_constraint` commands and JUST dropping the column. This will allow PostgreSQL to take care of the constraint problem for us. ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. <details><summary>DB migration output</summary> ```console :/usr/src/app$ flask db upgrade [2023-08-03 21:48:37,989] INFO in __init__: OpenOversight startup ... INFO [sqlalchemy.engine.Engine] COMMIT INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b38c133bed3c, add created_by and created_at columns :/usr/src/app$ flask db downgrade [2023-08-03 21:48:50,167] INFO in __init__: OpenOversight startup ... INFO [sqlalchemy.engine.Engine] COMMIT INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running downgrade b38c133bed3c -> 18f43ac4622f, add created_by and created_at columns :/usr/src/app$ flask db upgrade [2023-08-03 21:48:58,053] INFO in __init__: OpenOversight startup ... INFO [sqlalchemy.engine.Engine] COMMIT INFO [alembic.runtime.migration] Context impl PostgresqlImpl. INFO [alembic.runtime.migration] Will assume transactional DDL. INFO [alembic.runtime.migration] Running upgrade 18f43ac4622f -> b38c133bed3c, add created_by and created_at columns :/usr/src/app$ ``` </details>
…ns#1011) lucyparsons#997 I created a cacheing file that allows us to remove values from the cache when its underlying dataset has changed for all versions of a `SQLAlchemy` `Model`. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
lucyparsons#1010 Edit the `login_user` functions so that they return the user (removing the need for an extra database query) and create constants for the login values. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Fixes issue lucyparsons#973 ## Description of Changes Remove an unnecessary command in the `Makefile` to create an empty `.env` file. ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Fixes issue lucyparsons#1010 ## Description of Changes Added typing to parameters and returns for the `login_` functions. ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Fixes issue <!-- LINK YOUR ISSUE HERE --> ## Description of Changes Addressed DB warnings that we'd see during creation and removed redundant/unnecessary Model properties. ```console SAWarning: relationship 'Face.users' will copy column users.id to column faces.user_id, which conflicts with relationship(s): 'Face.user' (copies users.id to faces.user_id), 'User.faces' (copies users.id to faces.user_id). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="faces,user"' to the 'Face.users' relationship. SAWarning: relationship 'Officer.assignments_lazy' will copy column officers.id to column assignments.officer_id, which conflicts with relationship(s): 'Assignment.officer' (copies officers.id to assignments.officer_id), 'Officer.assignments' (copies officers.id to assignments.officer_id). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="assignments,officer"' to the 'Officer.assignments_lazy' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) department = Department( SAWarning: relationship 'Assignment.base_officer' will copy column officers.id to column assignments.officer_id, which conflicts with relationship(s): 'Assignment.officer' (copies officers.id to assignments.officer_id), 'Officer.assignments' (copies officers.id to assignments.officer_id), 'Officer.assignments_lazy' (copies officers.id to assignments.officer_id). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="assignments,assignments_lazy,officer"' to the 'Assignment.base_officer' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) department = Department( SAWarning: relationship 'Image.users' will copy column users.id to column raw_images.created_by, which conflicts with relationship(s): 'Image.user' (copies users.id to raw_images.created_by), 'User.raw_images' (copies users.id to raw_images.created_by). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="raw_images,user"' to the 'Image.users' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) department = Department( SAWarning: relationship 'User.classifications' will copy column users.id to column raw_images.created_by, which conflicts with relationship(s): 'Image.user' (copies users.id to raw_images.created_by), 'User.raw_images' (copies users.id to raw_images.created_by). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="raw_images,user"' to the 'User.classifications' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) department = Department( SAWarning: relationship 'Face.users' will copy column users.id to column faces.created_by, which conflicts with relationship(s): 'Face.user' (copies users.id to faces.created_by), 'User.faces' (copies users.id to faces.created_by). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="faces,user"' to the 'Face.users' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) department = Department( SAWarning: relationship 'User.tags' will copy column users.id to column faces.created_by, which conflicts with relationship(s): 'Face.user' (copies users.id to faces.created_by), 'User.faces' (copies users.id to faces.created_by). If this is not the intention, consider if these relationships should be linked with back_populates, or if viewonly=True should be applied to one or more if they are read-only. For the less common case that foreign key constraints are partially overlapping, the orm.foreign() annotation can be used to isolate the columns that should be written towards. To silence this warning, add the parameter 'overlaps="faces,user"' to the 'User.tags' relationship. (Background on this error at: https://sqlalche.me/e/14/qzyx) ``` Relevant information: https://stackoverflow.com/a/59920780 ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. The warnings are gone: ```console Postgres is up ## Populate database with test data docker-compose run --rm web python ./test_data.py -p WARN[0000] The "APPROVE_REGISTRATIONS" variable is not set. Defaulting to a blank string. [+] Creating 1/0 ✔ Container openoversight-postgres-1 Running 0.0s /usr/local/lib/python3.11/site-packages/flask_limiter/extension.py:293: UserWarning: Using the in-memory storage for tracking rate limits as no storage was explicitly specified. This is not recommended for production use. See: https://flask-limiter.readthedocs.io#configuring-a-storage-backend for documentation about configuring the storage backend. warnings.warn( [2023-08-08 18:05:22,006] INFO in __init__: OpenOversight startup /usr/local/lib/python3.11/site-packages/flask_limiter/extension.py:293: UserWarning: Using the in-memory storage for tracking rate limits as no storage was explicitly specified. This is not recommended for production use. See: https://flask-limiter.readthedocs.io#configuring-a-storage-backend for documentation about configuring the storage backend. warnings.warn( [2023-08-08 18:05:22,097] INFO in __init__: OpenOversight startup [*] Populating database with test data... 2023-08-08 18:05:27,443 INFO sqlalchemy.engine.Engine select pg_catalog.version() ```
This should be merged after this PR: lucyparsons#1020 lucyparsons#737 Since we have the `mypy.ini` file, I presume that we are intending on using `mypy`. To stay up to date, I am adding `mypy` to the `pre-commit` hook. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
lucyparsons#561 Added markdown file capability to Sphinx builds and updated most `.rst` files to working `.md` files. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. ```shell (env) docs % make html Running Sphinx v7.1.2 loading pickled environment... done building [mo]: targets for 0 po files that are out of date writing output... building [html]: targets for 0 source files that are out of date updating environment: 0 added, 0 changed, 0 removed reading sources... looking for now-outdated files... none found no targets are out of date. build succeeded. The HTML pages are in _build/html. (env) docs % ```
lucyparsons#742 Added cacheing for the CSV layer of the application. Tests were added for adding, editing, and deleting with regard to clearing a model's respective cache key. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. --------- Co-authored-by: abandoned-prototype <[email protected]>
lucyparsons#647 I pluralized nouns in routes for the site, added tests for all pluralized routes, fixed bugs that were not found, and added HTML files that were missing. <img width="909" alt="Screenshot 2023-08-10 at 11 12 25 AM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/3f0e1448-6b46-4f8c-8419-52fa62157dce"> Source: [Link](https://blog.dreamfactory.com/best-practices-for-naming-rest-api-endpoints/#:~:text=Using%20plural%20nouns%20is%20a,only%20represents%20a%20single%20user.) Resource: [Link](https://www.restapitutorial.com/lessons/restfulresourcenaming.html) - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
Added a `display_name` property to the `Department` model which adds the state to all displays of the department name and sorted all department lists by state and department name. <img width="943" alt="Screenshot 2023-08-17 at 3 21 55 PM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/19639e8f-66e3-49ce-ab65-21c60053ad61"> - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
…ation` (lucyparsons#1034) lucyparsons#1033 Remove `created_by` from `Form` models and add `created_by` to `LicensePlate` and `Location` models. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. - [x] Manually created an incident with a link and license plate.
Add creation and last update columns to track when and by whom models were updated. Run alembic migration N/A - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Fixes issue lucyparsons#1023 ## Description of Changes Address the warnings that pop up when populating rows in the DB by giving the foreign key constraints names. The warning no longer shows up when running `make dev`. I named the foreign keys according to the motif that they were given by default: <img width="368" alt="Screenshot 2023-08-22 at 3 31 50 PM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/4682dfaa-7d35-4a9f-9ccb-dac9073a5d99"> ```zsh /usr/src/app/OpenOversight/app/../migrations/env.py:75: SAWarning: Cannot correctly sort tables; there are unresolvable cycles between tables "departments, users", which is usually caused by mutually dependent foreign key constraints. Foreign key constraints involving these tables will not be considered; this warning may raise an error in a future release. context.run_migrations() ``` ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment. - [x] Ran `make dev` to create tables and populate data to validate changes to migrations.
* Make a constant for the JWT signing algorithm * add users._uuid column This column will provide a unique identifier that can be used to lookup users. * Add uuid property to User The _uuid column is prefixed with an underscore to deter developers from modifying it outside of specific circumstances. Providing a decorator to access the value of _uuid reads better, and further discourages interacting with _uuid directly. * Use uuid instead of id in JWT payloads JWTs serve to authenticate requests that confirm or change a user's email address, as well as requests to change a user's password. In some circumstances, it would be good to invalidate a JWT. For actions that use JWTs, the application validates the request by checking that the JWT was signed by the correct key, that it has not expired, and that the claims contained in the JWT are valid. Rotating the signing key would invalidate all JWTs signed by that key, which is undesirable. However, we can render specific JWTs invalid by changing the value of the user ID contained in the payload claims. This, too, is undesirable, because the user ID is referenced in database associations. Since the user's UUID provides the same capability of uniquely identifying a user, we can use it in place of the ID in the JWT payload, which gives us an easily-rotated attribute that won't cause complications when changed. * Invalidate pw reset JWT after pw changed This change ensures a password reset token cannot be reused after the user has successfully changed their password. * Ensure pw reset token invalidated on email change This commit regenerates the user's UUID when their email address changes, so that any JWTs sent to the old email address would no longer be valid. * Add comment explaining UUID concept * Regenerate user UUID when setting password In addition to regenerating the UUID when a password is reset via the forgot password flow, this commit regenerates the UUID any time the password is set. This ensures a password reset JWT is invalidated in a scenario where a user has generated the password reset JWT, but then remembers their password and resets it via the normal password change flow. Prior to this change, a new user would only have their UUID assigned when the changes were committed to the database, but now the UUID gets populated as part of the password hash assignment. A few tests have been updated to reflect this new behavior. --------- Co-authored-by: Michael Plunkett <[email protected]>
## Fixes issue Fixes lucyparsons#1038 ## Description of Changes Update `down_revision` since 2 migrations are using `b38c133bed3c` as their `down_revision`. ## Notes for Deployment None! ## Screenshots (if appropriate) N/A ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
I added to `Officer` and `Email` coverage in another PR and thought it'd be better to add in its own PR. I also addressed an issue with a broken `<form>` tag. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Description of Changes
We are currently seeing an error in the deployment process due to the
lack of the `gen_random_uuid()` function in the PostgreSQL instance.
```zsh
INFO [alembic.runtime.migration] Running upgrade a35aa1a114fa -> 52d3f6a21dd9, add _uuid column to users
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1900, in _execute_context
self.dialect.do_execute(
File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 736, in do_execute
cursor.execute(statement, parameters)
psycopg2.errors.UndefinedFunction: function gen_random_uuid() does not exist
HINT: No function matches the given name and argument types. You might need to add explicit type casts.
```
## Tests and linting
- [x] This branch is up-to-date with the `develop` branch.
- [x] `pytest` passes on my local development environment.
- [x] `pre-commit` passes on my local development environment.
…s#1041) Removed auto-generated Alembic comments from migrations and moved the UUID creation from the database to the server. This strategy, recommended by @sea-kelp, allows us to keep the functionality we want without requiring new instances to add any extensions to their PostgreSQL instance and prevents us from being in a coupled state with a modified PostgreSQL instance. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
Moved configurations from `.flake8` file to `.pre-commit-config.yaml` and deleted `.flake8`. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
lucyparsons#1045 Added image carousel for officers with multiple images and additional test coverage. Multiple images: <img width="1251" alt="Screenshot 2023-08-27 at 1 27 07 AM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/e7858594-557d-4e8f-9c08-bb92c854ec8a"> Single image: <img width="1253" alt="Screenshot 2023-08-27 at 1 27 37 AM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/f46a19aa-326a-4e13-9561-d22b345ee2aa"> No images: <img width="1251" alt="Screenshot 2023-08-27 at 1 27 58 AM" src="https://github.com/lucyparsons/OpenOversight/assets/5885605/b3f3c37a-7542-422d-9e54-b788e01dec4a"> - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
<!-- New Contributor? Welcome! We recommend you check your privacy settings, so the name and email associated with the commits are what you want them to be. See the contribution guide at https://github.com/lucyparsons/OpenOversight/blob/develop/CONTRIB.md#recommended-privacy-settings for more infos. Also make sure you have read and abide by the code of conduct: https://github.com/lucyparsons/OpenOversight/blob/develop/CODE_OF_CONDUCT.md If this pull request is not ready for review yet, please submit it as a draft. --> Several misc timezone-related changes: * Validate input to /timezone * Remove pytz dependency https://blog.ganssle.io/articles/2018/03/pytz-fastest-footgun.html * Include timezone in local_time and local_date_time output * Add tests for jinja filters * Fix typo `thousands_seperator` -> `thousands_separator` * Revert change to make session cookie permanent None! N/A - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
## Fixes issue lucyparsons#1027 ## Description of Changes Reduce lines in the redirect tests using `@pytest.mark.parametrize`. The coverage stays the same, but the amount of lines use goes dowwwwwwn. I also moved the `UX-Docs` files. ## Tests and linting - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
lucyparsons#1061 Instead of manually updating `last_updated_at`, we should let SQL Alchemy take care of it. - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
This invalidates all of a user's sessions when their email or password is updated. This builds upon changes made in lucyparsons@0912f90. See the Flask-Login documentation for an explanation of how this works: https://flask-login.readthedocs.io/en/latest/#alternative-tokens None! - [x] This branch is up-to-date with the `develop` branch. - [x] `pytest` passes on my local development environment. - [x] `pre-commit` passes on my local development environment.
sea-kelp
force-pushed
the
upstream-batch-4
branch
from
e2dfc56
to
bb31f34
Compare
AetherUnbound
approved these changes
Nov 11, 2023
There was a problem hiding this comment.
Tested all the changes locally with a prod backup, looks great! Thanks @sea-kelp!
|
Crap I screwed up the merge, going to reset this and apply the commits individually. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of Changes
Last batch of upstream changes!
Notes for Deployment
A couple of database migrations in this batch
Screenshots (if appropriate)
N/A
Tests and linting
I have rebased my changes on
mainjust lintpassesjust testpasses