Skip to content

Commit

Permalink
Merge from upstream (#500)
Browse files Browse the repository at this point in the history
* Replace `flake8` with `ruff` (lucyparsons#1101)

Replacing `flake8` with `ruff`, which has approximate parity and is more
than 10x faster:
https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-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.

* Remove `black` and use `ruff` formatter (lucyparsons#1106)

* Address `SQLAlchemy` deprecation warnings (lucyparsons#1111)

Part of lucyparsons#1054

Address warnings for for `RemovedIn20Warning` and `LegacyAPIWarning`
while using the [`SQLALCHEMY_WARN_20`
flag](https://docs.sqlalchemy.org/en/20/changelog/migration_14.html#sqlalchemy-2-0-deprecations-mode).
If you would like me to do any additional manual checks, etc. to verify
these changes, please don't hesitate to ask.

<details><summary>Unmodified warnings</summary>

```console
=============================================================================================== warnings summary ===============================================================================================
../../local/lib/python3.11/site-packages/flask_wtf/recaptcha/widgets.py:2: 18 warnings
  /usr/local/lib/python3.11/site-packages/flask_wtf/recaptcha/widgets.py:2: DeprecationWarning: 'flask.Markup' is deprecated and will be removed in Flask 2.4. Import 'markupsafe.Markup' instead.
    from flask import Markup

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:678: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:691: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:704: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:725: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    first_officer = Officer.query.get(1)

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:740: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    first_officer = Officer.query.get(1)

OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

OpenOversight/tests/test_commands.py::test_add_job_title__success
OpenOversight/tests/test_commands.py::test_add_job_title__different_departments
  /usr/src/app/OpenOversight/app/commands.py:645: RemovedIn20Warning: "Job" object is being merged into a Session along the backref cascade path for relationship "Department.jobs"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    job = Job(

OpenOversight/tests/test_utils.py::test_filters_do_not_exclude_officers_without_assignments
  /usr/src/app/OpenOversight/tests/test_utils.py:102: RemovedIn20Warning: "Officer" object is being merged into a Session along the backref cascade path for relationship "Department.officers"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    officer = Officer(

OpenOversight/tests/test_models.py::test_salary_repr
  /usr/src/app/OpenOversight/tests/test_models.py:170: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    salary = Salary.query.first()

OpenOversight/tests/test_commands.py::test_csv_import_new
  /usr/src/app/OpenOversight/tests/conftest.py:855: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    if len(list(officer.salaries)) > 0:

OpenOversight/tests/test_database_cache.py::test_documented_assignments
OpenOversight/tests/routes/test_image_tagging.py::test_admin_can_delete_tag
OpenOversight/tests/routes/test_descriptions.py::test_officer_descriptions_markdown
OpenOversight/tests/routes/test_notes.py::test_officer_notes_markdown
  /usr/local/lib/python3.11/site-packages/jinja2/environment.py:487: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    return getattr(obj, attribute)

OpenOversight/tests/test_database_cache.py: 1 warning
OpenOversight/tests/routes/test_incidents.py: 9 warnings
OpenOversight/tests/routes/test_officer_and_department.py: 1 warning
  /usr/src/app/OpenOversight/app/utils/forms.py:203: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    return Incident(

OpenOversight/tests/test_database_cache.py: 2 warnings
OpenOversight/tests/routes/test_descriptions.py: 29 warnings
OpenOversight/tests/routes/test_incidents.py: 34 warnings
OpenOversight/tests/routes/test_notes.py: 29 warnings
OpenOversight/tests/routes/test_singular_redirects.py: 21 warnings
OpenOversight/tests/routes/test_officer_and_department.py: 52 warnings
  /usr/local/lib/python3.11/site-packages/flask_sqlalchemy/query.py:30: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    rv = self.get(ident)

OpenOversight/tests/test_database_cache.py: 1 warning
OpenOversight/tests/routes/test_officer_and_department.py: 11 warnings
  /usr/src/app/OpenOversight/app/utils/forms.py:78: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(

OpenOversight/tests/routes/test_image_tagging.py::test_ac_cannot_delete_tag_not_in_their_dept
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:126: RemovedIn20Warning: The ``aliased`` and ``from_joinpoint`` keyword arguments to Query.join() are deprecated and will be removed in SQLAlchemy 2.0. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Face.query.join(Face.officer, aliased=True)

OpenOversight/tests/routes/test_image_tagging.py::test_user_is_redirected_to_correct_department_after_tagging
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:285: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    department = Department.query.get(department_id)

OpenOversight/tests/test_commands.py::test_csv_new_salary
  /usr/src/app/OpenOversight/tests/test_commands.py:463: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
    df.loc[0, "salary"] = "123456.78"

OpenOversight/tests/test_commands.py::test_bulk_add_officers__success
  /usr/src/app/OpenOversight/tests/test_commands.py:511: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(base_officer=first_officer, job_id=job.id)

OpenOversight/tests/test_commands.py::test_bulk_add_officers__success
  /usr/src/app/OpenOversight/tests/test_commands.py:519: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(base_officer=different_officer, job=job, created_by=user.id)

OpenOversight/tests/routes/test_incidents.py::test_admins_can_edit_incident_date_and_address
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:253: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    updated = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_descriptions.py::test_admins_can_delete_descriptions
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:293: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Description.query.get(description_id)

OpenOversight/tests/routes/test_image_tagging.py::test_ac_cannot_set_featured_tag_not_in_their_dept
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:326: RemovedIn20Warning: The ``aliased`` and ``from_joinpoint`` keyword arguments to Query.join() are deprecated and will be removed in SQLAlchemy 2.0. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Face.query.join(Face.officer, aliased=True)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:889: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Officer.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident.officers = [officer]

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/app/models/database_imports.py:308: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Officer.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident.officers = data.get("officers", [])

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:1012: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident3 = Incident.query.get(123456)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:1035: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    updated_link = Link.query.get(55051)

OpenOversight/tests/routes/test_descriptions.py::test_acs_can_delete_their_descriptions_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:322: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Description.query.get(description_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/app/csv_imports.py:560: RemovedIn20Warning: Using plain strings to indicate SQL statements without using the text() construct is  deprecated and will be removed in version 2.0.  Ensure plain SQL statements are passed using the text() construct. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    db.session.execute(raw_sql)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1160: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop1 = Officer.query.get(99001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1163: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop2 = Officer.query.get(99002)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1165: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop2.assignments[0] == Assignment.query.get(98001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1167: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop3 = Officer.query.get(99003)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1169: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop3.salaries[0] == Salary.query.get(77001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1171: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident = Incident.query.get(66001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1176: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    link = Link.query.get(55001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1279: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop1 = Officer.query.get(cop1_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1283: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop2 = Officer.query.get(cop2_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1285: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop2.assignments[0] == Assignment.query.get(a2_id)

OpenOversight/tests/routes/test_descriptions.py::test_acs_cannot_delete_descriptions_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:354: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Description.query.get(description_id)

OpenOversight/tests/routes/test_incidents.py: 12 warnings
  /usr/src/app/OpenOversight/app/main/forms.py:505: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    officer = Officer.query.get(officer_id)

OpenOversight/tests/routes/test_notes.py::test_admins_can_delete_notes
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:263: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_notes.py::test_acs_can_delete_their_notes_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:288: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_notes.py::test_acs_cannot_delete_notes_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:317: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_incidents.py::test_admins_can_delete_incidents
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:683: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_incidents.py::test_acs_can_delete_incidents_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:697: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_incidents.py::test_acs_cannot_delete_incidents_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:713: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_user_api.py: 21 warnings
  /usr/src/app/OpenOversight/app/auth/views.py:295: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user_id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_delete_other_admin
  /usr/src/app/OpenOversight/app/auth/views.py:342: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user_id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:137: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert not User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_delete_other_admin
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:153: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert User.query.get(user.id) is not None

OpenOversight/tests/routes/test_user_api.py::test_admin_can_disable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:178: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_disable_self
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:201: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_enable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:213: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_enable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:229: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_approve_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:296: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_approve_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:312: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-True-True]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-False-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[True-False-True-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-True-True-False]
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:349: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-True-True]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-False-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[True-False-True-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-True-True-False]
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:366: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:97: RemovedIn20Warning: "Note" object is being merged into a Session along the backref cascade path for relationship "Officer.notes"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_note = Note(

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:108: RemovedIn20Warning: "Description" object is being merged into a Session along the backref cascade path for relationship "Officer.descriptions"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_description = Description(

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:119: RemovedIn20Warning: "Salary" object is being merged into a Session along the backref cascade path for relationship "Officer.salaries"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_salary = Salary(

OpenOversight/tests/routes/test_officer_and_department.py: 109 warnings
  /usr/src/app/OpenOversight/tests/routes/test_officer_and_department.py:1777: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert (

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```
</details>

 - [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.

* Address database warnings and deprecated syntax (lucyparsons#1115)

lucyparsons#1054

Addressed the following error in tests by modifying the `Salary` table
and deprecated syntax throughout the application.

```console
FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
```

<details><summary>Warnings before changes</summary>

```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_models.py::test_salary_repr
  /usr/src/app/OpenOversight/tests/test_models.py:170: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    salary = Salary.query.first()

OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

OpenOversight/tests/test_commands.py::test_csv_import_new
  /usr/src/app/OpenOversight/tests/conftest.py:855: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    if len(list(officer.salaries)) > 0:

OpenOversight/tests/routes/test_image_tagging.py::test_admin_can_delete_tag
OpenOversight/tests/routes/test_descriptions.py::test_officer_descriptions_markdown
OpenOversight/tests/test_database_cache.py::test_documented_assignments
  /usr/local/lib/python3.11/site-packages/jinja2/environment.py:487: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    return getattr(obj, attribute)

OpenOversight/tests/test_commands.py::test_csv_new_salary
  /usr/src/app/OpenOversight/tests/test_commands.py:463: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
    df.loc[0, "salary"] = "123456.78"

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```
</details>

- [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] Validated warning is gone.

```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```

- [x] Validated data migration works.

```console
I have no name!@36326fd7801f:/usr/src/app$ flask db stamp head
...
INFO  [alembic.runtime.migration] Will assume transactional DDL.
I have no name!@36326fd7801f:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
I have no name!@36326fd7801f:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 5865f488470c -> 939ea0f2b26d, change salary column types
I have no name!@36326fd7801f:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
I have no name!@36326fd7801f:/usr/src/app$
```

* Remove `transaction.rollback()` and use `session` fixture (lucyparsons#1117)

lucyparsons#1054
- This is the last PR for this issue. ❗❗❗❗❗❗

Removed the redundant `transaction.rollback()` function call in the
`session` fixture and changed all references to `db.session` to
`session` so that tests are using the correct scoping.

Addressed this warning:
```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```

- [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] Validate that no more warnings show up in the `make test` command.

```console
---------- coverage: platform linux, python 3.11.9-final-0 -----------
Name                                                                                                         Stmts   Miss  Cover
--------------------------------------------------------------------------------------------------------------------------------
OpenOversight/__init__.py                                                                                        0      0   100%
OpenOversight/app/__init__.py                                                                                   74      1    99%
```

* Fix database fixture and password unit test (lucyparsons#1120)

* Add user profile tests (lucyparsons#1119)

## Fixes issue
lucyparsons#436

## Description of Changes
Added tests to validate `/user/` route logic and correct profile logic
to match pre-specified tests.

<img width="497" alt="Screenshot 2024-07-31 at 5 27 37 PM"
src="https://github.com/user-attachments/assets/78703665-1623-4703-8fa9-a1cca59ba319">

There is not a `/users/` route, so I marked it out.

## 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.

* Upgrade pre-commit libraries

---------

Co-authored-by: Michael Plunkett <[email protected]>
  • Loading branch information
sea-kelp and michplunkett authored Sep 2, 2024
1 parent e660929 commit 57167f2
Show file tree
Hide file tree
Showing 105 changed files with 777 additions and 602 deletions.
52 changes: 18 additions & 34 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.0.1
rev: v4.6.0
hooks:
- id: trailing-whitespace
- id: check-docstring-first
Expand All @@ -23,15 +23,15 @@ repos:
- id: requirements-txt-fixer

- repo: https://github.com/pre-commit/pygrep-hooks
rev: v1.7.0
rev: v1.10.0
hooks:
- id: python-check-blanket-noqa
- id: python-check-mock-methods
- id: python-no-eval
- id: python-no-log-warn

- repo: https://github.com/PyCQA/isort
rev: 5.12.0
rev: 5.13.2
hooks:
- id: isort
name: Run isort to sort imports
Expand All @@ -47,20 +47,6 @@ repos:
- --ensure-newline-before-comments
- --line-length=88

- repo: https://github.com/pycqa/pydocstyle
rev: 5.1.1
hooks:
- id: pydocstyle
name: Run pydocstyle
args:
- --convention=pep257
# Do not require docstrings, only check existing ones. (D1)
# Allow for a newline after a docstring. (D202)
# Don't require a summary line. (D205)
# Don't require a period on the first line. (D400)
- --add-ignore=D1,D202,D205,D400
exclude: tests/

- repo: local
hooks:
- id: no-shebang
Expand All @@ -73,37 +59,35 @@ repos:
files: \.py$

- repo: https://github.com/ikamensh/flynt/
rev: '1.0.0'
rev: '1.0.1'
hooks:
- id: flynt
exclude: ^build/.*$|^.tox/.*$|^venv/.*$
files: \.py$
args:
- --quiet

- repo: https://github.com/PyCQA/flake8
rev: 7.0.0
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.6.3
hooks:
- id: flake8
args:
- --ignore=E203,E402,E501,E800,W503,W391,E261,W504
- --select=B,C,E,F,W,T4,B9
- --exclude=./OpenOversight/app/db_repository/versions/

- repo: https://github.com/psf/black
rev: 23.7.0
hooks:
- id: black
args:
- --safe
- id: ruff
types_or: [python,pyi]
args:
- --fix
- --select=B,C,E,F,W
- --ignore=C901,E203,E402,E501,W391,E261
- id: ruff-format
types_or: [python,pyi]
args:
- --target-version=py311

- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.9.0
rev: v1.11.2
hooks:
- id: mypy

- repo: https://github.com/Riverside-Healthcare/djLint
rev: v1.32.1
rev: v1.35.2
hooks:
- id: djlint-reformat
pass_filenames: false
Expand Down
58 changes: 24 additions & 34 deletions OpenOversight/app/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,11 +224,7 @@ def update_officer_field(officer_field_name):
):
ImportLog.log_change(
officer,
"Updated {}: {} --> {}".format(
officer_field_name,
getattr(officer, officer_field_name),
row[officer_field_name],
),
f"Updated {officer_field_name}: {getattr(officer, officer_field_name)} --> {row[officer_field_name]}",
)
setattr(officer, officer_field_name, row[officer_field_name])

Expand Down Expand Up @@ -260,30 +256,23 @@ def update_officer_field(officer_field_name):
new_value = parse(row[field_name])
if isinstance(old_value, date):
new_value = new_value.date()
except Exception as e:
except Exception as err:
msg = (
'Field {} is a date-type, but "{}" was specified for '
"Officer {} {} and cannot be parsed as a date-type.\nError "
"message from dateutil: {}".format(
field_name,
row[field_name],
officer.first_name,
officer.last_name,
e,
)
f'Field {field_name} is a date-type, but "{row[field_name]}"'
f" was specified for Officer {officer.first_name} "
f"{officer.last_name} and cannot be parsed as a "
f"date-type.\nError message from dateutil: {err}"
)
raise Exception(msg)
raise Exception(msg) from err
else:
new_value = row[field_name]
if old_value is None:
update_officer_field(field_name)
elif str(old_value) != str(new_value):
msg = "Officer {} {} has differing {} field. Old: {}, new: {}".format(
officer.first_name,
officer.last_name,
field_name,
old_value,
new_value,
msg = (
f"Officer {officer.first_name} {officer.last_name} has "
f"differing {field_name} field. Old: {old_value}, "
f"new: {new_value}"
)
if update_static_fields:
print(msg)
Expand Down Expand Up @@ -368,23 +357,25 @@ def process_assignment(row, officer, compare=False):
.all()
)
for assignment, job in assignments:
assignment_fieldnames = [
assignment_field_names = [
"star_no",
"unit_id",
"start_date",
"resign_date",
]
i = 0
for fieldname in assignment_fieldnames:
current = getattr(assignment, fieldname)
for field_name in assignment_field_names:
current = getattr(assignment, field_name)
# Test if fields match between row and existing assignment
if (
current
and fieldname in row
and is_equal(row[fieldname], current)
) or (not current and (fieldname not in row or not row[fieldname])):
and field_name in row
and is_equal(row[field_name], current)
) or (
not current and (field_name not in row or not row[field_name])
):
i += 1
if i == len(assignment_fieldnames):
if i == len(assignment_field_names):
job_title = job.job_title
if (
job_title and row.get("job_title", "Not Sure") == job_title
Expand Down Expand Up @@ -549,7 +540,7 @@ def bulk_add_officers(
department_id = row["department_id"]
department = departments.get(department_id)
if row["department_id"] not in departments:
department = Department.query.filter_by(id=department_id).one_or_none()
department = db.session.get(Department, department_id)
if department:
departments[department_id] = department
else:
Expand All @@ -574,9 +565,8 @@ def bulk_add_officers(
)
else:
raise Exception(
"Officer {} {} missing badge number and unique identifier".format(
row["first_name"], row["last_name"]
)
f"Officer {row['first_name']} {row['last_name']} "
"missing badge number and unique identifier"
)
else:
officer = Officer.query.filter_by(
Expand Down Expand Up @@ -689,7 +679,7 @@ def add_department(name, short_name, state, unique_internal_identifier):
@with_appcontext
def add_job_title(department_id, job_title, is_sworn_officer, order):
"""Add a rank to a department."""
department = Department.query.filter_by(id=department_id).one_or_none()
department = db.session.get(Department, department_id)
is_sworn = is_sworn_officer == "true"
job = Job(
job_title=job_title,
Expand Down
33 changes: 13 additions & 20 deletions OpenOversight/app/csv_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from contextlib import contextmanager
from typing import Dict, Optional

from sqlalchemy import sql
from sqlalchemy.exc import SQLAlchemyError

from OpenOversight.app.models.database import (
Expand Down Expand Up @@ -47,7 +48,7 @@ def _create_or_update_model(
return update_method(row, existing_model_lookup[int(row["id"])])
else:
if model is not None:
existing = model.query.filter_by(id=int(row["id"])).first()
existing = db.session.get(model, int(row["id"]))
if existing:
db.session.delete(existing)
db.session.flush()
Expand Down Expand Up @@ -208,21 +209,17 @@ def _handle_assignments_csv(
officer_id = row["officer_id"]
if officer_id != "" and officer_id[0] != "#":
all_rel_officers.add(int(officer_id))
wrong_department = all_rel_officers - set(
[int(oid) for oid in all_officers.keys() if oid[0] != "#"]
)
wrong_department = all_rel_officers - {
int(oid) for oid in all_officers.keys() if oid[0] != "#"
}
if len(wrong_department) > 0:
raise Exception(
"Referenced {} officers in assignment csv that belong to different "
"department. Example ids: {}".format(
len(wrong_department),
", ".join(map(str, list(wrong_department)[:3])),
)
f"Referenced {len(wrong_department)} officers in assignment "
"csv that belong to different department. Example IDs"
f": {', '.join(map(str, list(wrong_department)[:3]))}"
)
print(
"Deleting assignments from {} officers to overwrite.".format(
len(all_rel_officers)
)
f"Deleting assignments from {len(all_rel_officers)} officers to overwrite."
)
(
db.session.query(Assignment)
Expand All @@ -245,13 +242,11 @@ def _handle_assignments_csv(
officer = all_officers.get(row["officer_id"])
if not officer:
raise Exception(
"Officer with id {} does not exist (in this department)".format(
row["officer_id"]
)
f"Officer with id {row['officer_id']} does not exist (in this department)"
)
if row.get("unit_id"):
assert (
Unit.query.filter_by(id=int(row.get("unit_id"))).one().department.id
db.session.get(Unit, int(row.get("unit_id"))).department.id
== department_id
)
elif row.get("unit_name"):
Expand Down Expand Up @@ -331,9 +326,7 @@ def _handle_salaries(
officer = all_officers.get(row["officer_id"])
if not officer:
raise Exception(
"Officer with id {} does not exist (in this department)".format(
row["officer_id"]
)
f"Officer with id {row['officer_id']} does not exist (in this department)"
)
row["officer_id"] = officer.id
_create_or_update_model(
Expand Down Expand Up @@ -565,7 +558,7 @@ def import_csv_files(
select setval('incidents_id_seq', (select max(id) from incidents));
"""
try:
db.session.execute(raw_sql)
db.session.execute(sql.text(raw_sql))
print("Updated sequences.")
except SQLAlchemyError:
print("Failed to update sequences")
1 change: 1 addition & 0 deletions OpenOversight/app/filters.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Contains all templates filters."""

from datetime import datetime
from zoneinfo import ZoneInfo

Expand Down
10 changes: 5 additions & 5 deletions OpenOversight/app/formfields.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def _value(self):
else:
return self.data and self.data.strftime(self.format) or ""

def process_formdata(self, valuelist):
if valuelist and valuelist != [""]:
time_str = " ".join(valuelist)
def process_formdata(self, values):
if values and values != [""]:
time_str = " ".join(values)
try:
components = time_str.split(":")
hour = 0
Expand All @@ -36,6 +36,6 @@ def process_formdata(self, valuelist):
else:
raise ValueError
self.data = datetime.time(hour, minutes, seconds)
except ValueError:
except ValueError as err:
self.data = None
raise ValueError(self.gettext("Not a valid time"))
raise ValueError(self.gettext("Not a valid time")) from err
6 changes: 4 additions & 2 deletions OpenOversight/app/main/downloads.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import csv
import io
from datetime import date
from http import HTTPStatus
from typing import Any, Callable, Dict, List, TypeVar

from flask import Response, abort
Expand All @@ -14,6 +15,7 @@
Link,
Officer,
Salary,
db,
)


Expand Down Expand Up @@ -44,9 +46,9 @@ def make_downloadable_csv(
field_names: List[str],
record_maker: Callable[[T], _Record],
) -> Response:
department = Department.query.filter_by(id=department_id).first()
department = db.session.get(Department, department_id)
if not department:
abort(404)
abort(HTTPStatus.NOT_FOUND)

csv_output = io.StringIO()
csv_writer = csv.DictWriter(csv_output, fieldnames=field_names)
Expand Down
6 changes: 4 additions & 2 deletions OpenOversight/app/main/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ class AssignmentForm(Form):

class SalaryForm(Form):
salary = DecimalField(
"Salary", validators=[NumberRange(min=0, max=1000000), validate_money]
"Salary",
default=0,
validators=[NumberRange(min=0, max=1000000), validate_money],
)
overtime_pay = DecimalField(
"Overtime Pay",
Expand Down Expand Up @@ -441,7 +443,7 @@ class DateFieldForm(Form):
time_field = TimeField("Time", validators=[Optional()])

def validate_time_field(self, field):
if not type(field.data) is time:
if type(field.data) is not time:
raise ValidationError("Not a valid time.")

def validate_date_field(self, field):
Expand Down
4 changes: 2 additions & 2 deletions OpenOversight/app/main/model_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def edit(self, obj_id, form=None):
[KEY_DEPT_ALL_NOTES],
)
case Link.__name__:
officer = Officer.query.filter_by(id=obj.officer_id).first()
officer = db.session.get(Officer, obj.officer_id)
if officer:
Department(
id=officer.department_id
Expand Down Expand Up @@ -217,7 +217,7 @@ def populate_obj(self, form, obj):
form.populate_obj(obj)

# if the object doesn't have a creator id set it to current user
if hasattr(obj, "created_by") and not getattr(obj, "created_by"):
if hasattr(obj, "created_by") and not obj.created_by:
obj.created_by = current_user.id
# if the object keeps track of who updated it last, set to current user
if hasattr(obj, "last_updated_by"):
Expand Down
Loading

0 comments on commit 57167f2

Please sign in to comment.