Address database warnings and deprecated syntax (#1115)

## Fixes issue
#1054

## Description of Changes
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>

## 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] 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$
```

OpenOversight/app/commands.py

@@ -502,7 +502,7 @@ def bulk_add_officers(filename, no_create, update_by_name, update_static_fields)
         for row in csvfile:
             department_id = row["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:
@@ -639,7 +639,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,

OpenOversight/app/csv_imports.py

@@ -48,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()
@@ -246,7 +246,7 @@ def _handle_assignments_csv(
                 )
             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"):

OpenOversight/app/main/downloads.py

@@ -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
@@ -14,6 +15,7 @@
     Link,
     Officer,
     Salary,
+    db,
 )
 
 
@@ -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)

OpenOversight/app/main/forms.py

@@ -168,7 +168,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",

OpenOversight/app/main/model_view.py

@@ -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

OpenOversight/app/main/views.py

@@ -291,11 +291,9 @@ def profile(username: str):
         user = User.by_username(username).one()
     else:
         abort(HTTPStatus.NOT_FOUND)
-    try:
-        pref = User.query.filter_by(id=current_user.id).one().dept_pref
-        department = Department.query.filter_by(id=pref).one().name
-    except NoResultFound:
-        department = None
+
+    pref = db.session.get(User, current_user.id).dept_pref
+    department = db.session.get(Department, pref).name if pref else None
     return render_template("profile.html", user=user, department=department)
 
 
@@ -313,7 +311,6 @@ def officer_profile(officer_id: int):
     form = AssignmentForm()
     try:
         officer = Officer.query.filter_by(id=officer_id).one()
-        officer.incidents.query.order_by(Incident.date.desc(), Incident.time.desc())
     except NoResultFound:
         abort(HTTPStatus.NOT_FOUND)
     except:  # noqa: E722
@@ -379,7 +376,7 @@ def redirect_add_assignment(officer_id: int):
 @ac_or_admin_required
 def add_assignment(officer_id: int):
     form = AssignmentForm()
-    officer = Officer.query.filter_by(id=officer_id).first()
+    officer = db.session.get(Officer, officer_id)
     form.job_title.query = (
         Job.query.filter_by(department_id=officer.department_id)
         .order_by(Job.order.asc())
@@ -441,23 +438,23 @@ def redirect_edit_assignment(officer_id: int, assignment_id: int):
 @login_required
 @ac_or_admin_required
 def edit_assignment(officer_id: int, assignment_id: int):
-    officer = Officer.query.filter_by(id=officer_id).one()
+    officer = db.session.get(Officer, officer_id)
 
     if current_user.is_area_coordinator and not current_user.is_administrator:
         if not ac_can_edit_officer(officer, current_user):
             abort(HTTPStatus.FORBIDDEN)
 
-    assignment = Assignment.query.filter_by(id=assignment_id).one()
+    assignment = db.session.get(Assignment, assignment_id)
     form = AssignmentForm(obj=assignment)
     form.job_title.query = (
         Job.query.filter_by(department_id=officer.department_id)
         .order_by(Job.order.asc())
         .all()
     )
-    form.job_title.data = Job.query.filter_by(id=assignment.job_id).one()
+    form.job_title.data = db.session.get(Job, assignment.job_id)
     form.unit.query = unit_choices(officer.department_id)
     if form.unit.data and isinstance(form.unit.data, int):
-        form.unit.data = Unit.query.filter_by(id=form.unit.data).one()
+        form.unit.data = db.session.get(Unit, form.unit.data)
     if form.validate_on_submit():
         form.job_title.data = Job.query.filter_by(
             id=int(form.job_title.raw_data[0])
@@ -491,7 +488,7 @@ def redirect_add_salary(officer_id: int):
 @ac_or_admin_required
 def add_salary(officer_id: int):
     form = SalaryForm()
-    officer = Officer.query.filter_by(id=officer_id).first()
+    officer = db.session.get(Officer, officer_id)
     if not officer:
         flash("Officer not found")
         abort(HTTPStatus.NOT_FOUND)
@@ -556,12 +553,12 @@ def redirect_edit_salary(officer_id: int, salary_id: int):
 @login_required
 @ac_or_admin_required
 def edit_salary(officer_id: int, salary_id: int):
-    officer = Officer.query.filter_by(id=officer_id).one()
+    officer = db.session.get(Officer, officer_id)
     if current_user.is_area_coordinator and not current_user.is_administrator:
         if not ac_can_edit_officer(officer, current_user):
             abort(HTTPStatus.FORBIDDEN)
 
-    salary = Salary.query.filter_by(id=salary_id).one()
+    salary = db.session.get(Salary, salary_id)
     form = SalaryForm(obj=salary)
     if form.validate_on_submit():
         form.populate_obj(salary)
@@ -591,7 +588,7 @@ def redirect_display_submission(image_id: int):
 @login_required
 def display_submission(image_id: int):
     try:
-        image = Image.query.filter_by(id=image_id).one()
+        image = db.session.get(Image, image_id)
         proper_path = serve_image(image.filepath)
     except NoResultFound:
         abort(HTTPStatus.NOT_FOUND)
@@ -610,7 +607,7 @@ def redirect_display_tag(tag_id: int):
 @main.route("/tags/<int:tag_id>")
 def display_tag(tag_id: int):
     try:
-        tag = Face.query.filter_by(id=tag_id).one()
+        tag = db.session.get(Face, tag_id)
         proper_path = serve_image(tag.original_image.filepath)
     except NoResultFound:
         abort(HTTPStatus.NOT_FOUND)
@@ -638,7 +635,7 @@ def redirect_classify_submission(image_id: int, contains_cops: int):
 @login_required
 def classify_submission(image_id: int, contains_cops: int):
     try:
-        image = Image.query.filter_by(id=image_id).one()
+        image = db.session.get(Image, image_id)
         if image.contains_cops is not None and not current_user.is_administrator:
             flash("Only administrator can re-classify image")
             return redirect(redirect_url())
@@ -916,7 +913,7 @@ def list_officer(
     form_data["unique_internal_identifier"] = unique_internal_identifier
     form_data["require_photo"] = require_photo
 
-    department = Department.query.filter_by(id=department_id).first()
+    department = db.session.get(Department, department_id)
     if not department:
         abort(HTTPStatus.NOT_FOUND)
 
@@ -1189,7 +1186,7 @@ def redirect_edit_officer(officer_id: int):
 @login_required
 @ac_or_admin_required
 def edit_officer(officer_id: int):
-    officer = Officer.query.filter_by(id=officer_id).one()
+    officer = db.session.get(Officer, officer_id)
     form = EditOfficerForm(obj=officer)
 
     if request.method == HTTPMethod.GET:
@@ -1265,7 +1262,7 @@ def redirect_delete_tag(tag_id: int):
 @login_required
 @ac_or_admin_required
 def delete_tag(tag_id: int):
-    tag = Face.query.filter_by(id=tag_id).first()
+    tag = db.session.get(Face, tag_id)
 
     if not tag:
         flash("Tag not found")
@@ -1301,7 +1298,7 @@ def redirect_set_featured_tag(tag_id: int):
 @login_required
 @ac_or_admin_required
 def set_featured_tag(tag_id: int):
-    tag = Face.query.filter_by(id=tag_id).first()
+    tag = db.session.get(Face, tag_id)
 
     if not tag:
         flash("Tag not found")
@@ -1373,7 +1370,7 @@ def redirect_label_data(
 @login_required
 def label_data(department_id: Optional[int] = None, image_id: Optional[int] = None):
     if department_id:
-        department = Department.query.filter_by(id=department_id).one()
+        department = db.session.get(Department, department_id)
         if image_id:
             image = (
                 Image.query.filter_by(id=image_id)
@@ -1390,7 +1387,7 @@ def label_data(department_id: Optional[int] = None, image_id: Optional[int] = No
     else:
         department = None
         if image_id:
-            image = Image.query.filter_by(id=image_id).one()
+            image = db.session.get(Image, image_id)
         else:
             # Select a random untagged image from the entire database
             image_query = Image.query.filter_by(contains_cops=True).filter_by(
@@ -1408,7 +1405,7 @@ def label_data(department_id: Optional[int] = None, image_id: Optional[int] = No
 
     form = FaceTag()
     if form.validate_on_submit():
-        officer_exists = Officer.query.filter_by(id=form.officer_id.data).first()
+        officer_exists = db.session.get(Officer, form.officer_id.data)
         existing_tag = (
             db.session.query(Face)
             .filter(Face.officer_id == form.officer_id.data)
@@ -1482,7 +1479,7 @@ def redirect_complete_tagging(image_id: int):
 @login_required
 def complete_tagging(image_id: int):
     # Select a random untagged image from the database
-    image = Image.query.filter_by(id=image_id).first()
+    image = db.session.get(Image, image_id)
     if not image:
         abort(HTTPStatus.NOT_FOUND)
     image.is_tagged = True
@@ -1523,8 +1520,8 @@ def submit_data():
     preferred_dept_id = Department.query.first().id
     # try to use preferred department if available
     try:
-        if User.query.filter_by(id=current_user.id).one().dept_pref:
-            preferred_dept_id = User.query.filter_by(id=current_user.id).one().dept_pref
+        if current_user.dept_pref:
+            preferred_dept_id = current_user.dept_pref
             form = AddImageForm()
         else:
             form = AddImageForm()
@@ -1855,7 +1852,7 @@ def redirect_upload(department_id: int, officer_id: Optional[int] = None):
 @limiter.limit("250/minute")
 def upload(department_id: int, officer_id: Optional[int] = None):
     if officer_id:
-        officer = Officer.query.filter_by(id=officer_id).first()
+        officer = db.session.get(Officer, officer_id)
         if not officer:
             return jsonify(error="This officer does not exist."), HTTPStatus.NOT_FOUND
         if not (
@@ -2054,11 +2051,11 @@ def populate_obj(self, form: FlaskForm, obj: Incident):
             for officer in officers:
                 if officer["oo_id"]:
                     try:
-                        of = Officer.query.filter_by(id=int(officer["oo_id"])).first()
+                        of = db.session.get(Officer, int(officer["oo_id"]))
                     # Sometimes we get a string in officer["oo_id"], this parses it
                     except ValueError:
                         our_id = officer["oo_id"].split('value="')[1][:-2]
-                        of = Officer.query.filter_by(id=int(our_id)).first()
+                        of = db.session.get(Officer, int(our_id))
                     if of and of not in obj.officers:
                         obj.officers.append(of)
 

OpenOversight/app/models/database.py

@@ -359,8 +359,8 @@ class Salary(BaseModel, TrackUpdates):
         ),
     )
     officer = db.relationship("Officer", back_populates="salaries")
-    salary = db.Column(db.Numeric, index=True, unique=False, nullable=False)
-    overtime_pay = db.Column(db.Numeric, index=True, unique=False, nullable=True)
+    salary = db.Column(db.Float, index=True, unique=False, nullable=False)
+    overtime_pay = db.Column(db.Float, index=True, unique=False, nullable=True)
     year = db.Column(db.Integer, index=True, unique=False, nullable=False)
     is_fiscal_year = db.Column(db.Boolean, index=False, unique=False, nullable=False)
 
@@ -691,7 +691,11 @@ class Incident(BaseModel, TrackUpdates):
         "Officer",
         secondary=officer_incidents,
         lazy="subquery",
-        backref=db.backref("incidents", cascade_backrefs=False),
+        backref=db.backref(
+            "incidents",
+            cascade_backrefs=False,
+            order_by="Incident.date.desc(), Incident.time.desc()",
+        ),
     )
     department_id = db.Column(
         db.Integer, db.ForeignKey("departments.id", name="incidents_department_id_fkey")

OpenOversight/app/utils/forms.py

@@ -173,7 +173,7 @@ def create_incident(self, form: IncidentForm, user: User) -> Incident:
     if "officers" in form.data:
         form_officers = [o for o in form.data["officers"] if o["oo_id"]]
         for officer in form_officers:
-            of = Officer.query.filter_by(id=int(officer["oo_id"])).one()
+            of = db.session.get(Officer, int(officer["oo_id"]))
             if of:
                 officers.append(of)
 

OpenOversight/migrations/versions/2024-07-22-0538_5865f488470c_change_salary_column_types.py

@@ -0,0 +1,46 @@
+"""change salary column types
+
+Revision ID: 5865f488470c
+Revises: 939ea0f2b26d
+Create Date: 2024-07-22 05:38:36.531798
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+
+revision = "5865f488470c"
+down_revision = "939ea0f2b26d"
+
+
+def upgrade():
+    with op.batch_alter_table("salaries", schema=None) as batch_op:
+        batch_op.alter_column(
+            "salary",
+            existing_type=sa.NUMERIC(),
+            type_=sa.Float(),
+            existing_nullable=False,
+        )
+        batch_op.alter_column(
+            "overtime_pay",
+            existing_type=sa.NUMERIC(),
+            type_=sa.Float(),
+            existing_nullable=True,
+        )
+
+
+def downgrade():
+    with op.batch_alter_table("salaries", schema=None) as batch_op:
+        batch_op.alter_column(
+            "overtime_pay",
+            existing_type=sa.Float(),
+            type_=sa.NUMERIC(),
+            existing_nullable=True,
+        )
+        batch_op.alter_column(
+            "salary",
+            existing_type=sa.Float(),
+            type_=sa.NUMERIC(),
+            existing_nullable=False,
+        )

OpenOversight/tests/conftest.py

@@ -676,7 +676,7 @@ def add_mockdata(session):
 
     test_incidents = [
         Incident(
-            date=date(2016, 3, 16),
+            date=date(2017, 12, 11),
             time=time(4, 20),
             report_number="42",
             description="### A thing happened\n **Markup** description",
@@ -690,7 +690,7 @@ def add_mockdata(session):
         ),
         Incident(
             date=date(2017, 12, 11),
-            time=time(2, 40),
+            time=time(4, 40),
             report_number="38",
             description="A thing happened",
             department_id=2,