diff --git a/OpenOversight/app/commands.py b/OpenOversight/app/commands.py index 991d02585..175da28d2 100644 --- a/OpenOversight/app/commands.py +++ b/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, diff --git a/OpenOversight/app/csv_imports.py b/OpenOversight/app/csv_imports.py index 7904b2bd4..ac796fb21 100644 --- a/OpenOversight/app/csv_imports.py +++ b/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"): diff --git a/OpenOversight/app/main/downloads.py b/OpenOversight/app/main/downloads.py index e055618ab..9cabd5f90 100644 --- a/OpenOversight/app/main/downloads.py +++ b/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) diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index ce1dc0145..f48274458 100644 --- a/OpenOversight/app/main/forms.py +++ b/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", diff --git a/OpenOversight/app/main/model_view.py b/OpenOversight/app/main/model_view.py index 023ccfd76..6d3806f48 100644 --- a/OpenOversight/app/main/model_view.py +++ b/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 diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index b09447752..b4e2cb1d9 100644 --- a/OpenOversight/app/main/views.py +++ b/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/") 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) diff --git a/OpenOversight/app/models/database.py b/OpenOversight/app/models/database.py index 1ea4f5c36..c50e0d59d 100644 --- a/OpenOversight/app/models/database.py +++ b/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") diff --git a/OpenOversight/app/utils/forms.py b/OpenOversight/app/utils/forms.py index c99480d23..344899e2a 100644 --- a/OpenOversight/app/utils/forms.py +++ b/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) diff --git a/OpenOversight/migrations/versions/2024-07-22-0538_5865f488470c_change_salary_column_types.py b/OpenOversight/migrations/versions/2024-07-22-0538_5865f488470c_change_salary_column_types.py new file mode 100644 index 000000000..0a578d003 --- /dev/null +++ b/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, + ) diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py index d9904ef6e..0040e0b13 100644 --- a/OpenOversight/tests/conftest.py +++ b/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, diff --git a/OpenOversight/tests/routes/test_image_tagging.py b/OpenOversight/tests/routes/test_image_tagging.py index 2cae18f1b..e247fffe0 100644 --- a/OpenOversight/tests/routes/test_image_tagging.py +++ b/OpenOversight/tests/routes/test_image_tagging.py @@ -114,7 +114,7 @@ def test_ac_can_delete_tag_in_their_dept(mockdata, client, session): assert b"Deleted this tag" in rv.data # test tag was deleted from database - deleted_tag = Face.query.filter_by(id=tag_id).first() + deleted_tag = session.get(Face, tag_id) assert deleted_tag is None @@ -136,7 +136,7 @@ def test_ac_cannot_delete_tag_not_in_their_dept(mockdata, client, session): assert rv.status_code == HTTPStatus.FORBIDDEN # test tag was not deleted from database - deleted_tag = Face.query.filter_by(id=tag_id).first() + deleted_tag = session.get(Face, tag_id) assert deleted_tag is not None @@ -241,7 +241,7 @@ def test_user_cannot_tag_officer_mismatched_with_department(mockdata, client, se follow_redirects=True, ) - department = Department.query.filter_by(id=2).one_or_none() + department = session.get(Department, 2) assert (f"The officer is not in {department.name}, {department.state}.").encode( ENCODING_UTF_8 ) in rv.data @@ -255,7 +255,7 @@ def test_user_can_finish_tagging(mockdata, client, session): rv = client.get( url_for("main.complete_tagging", image_id=image_id), follow_redirects=True ) - image = Image.query.filter_by(id=image_id).one() + image = session.get(Image, image_id) assert b"Marked image as completed." in rv.data assert image.last_updated_by == user.id @@ -352,7 +352,7 @@ def test_featured_tag_replaces_others(mockdata, client, session): _, user = login_admin(client) tag1 = Face.query.first() - officer = Officer.query.filter_by(id=tag1.officer_id).one() + officer = session.get(Officer, tag1.officer_id) # Add second tag for officer second_image = ( diff --git a/OpenOversight/tests/routes/test_incidents.py b/OpenOversight/tests/routes/test_incidents.py index eb3f9e4df..582e20e05 100644 --- a/OpenOversight/tests/routes/test_incidents.py +++ b/OpenOversight/tests/routes/test_incidents.py @@ -860,8 +860,8 @@ def test_admins_cannot_inject_unsafe_html(mockdata, client, session): ({"occurred_before": "2017-12-12"}, ["38", "42"], ["39"]), ( {"occurred_after": "2017-12-10", "occurred_before": "2019-01-01"}, - ["38"], - ["39", "42"], + ["38", "42"], + ["39"], ), ({"report_number": "38"}, ["38"], ["42", "39"]), # Base case ({"report_number": "3"}, ["38", "39"], ["42"]), # Test inclusive match diff --git a/OpenOversight/tests/routes/test_notes.py b/OpenOversight/tests/routes/test_notes.py index 5e50c5059..292072bab 100644 --- a/OpenOversight/tests/routes/test_notes.py +++ b/OpenOversight/tests/routes/test_notes.py @@ -249,8 +249,11 @@ def test_admins_can_delete_notes(mockdata, client, session): with current_app.test_request_context(): login_admin(client) note = Note.query.first() - officer = Officer.query.filter_by(id=note.officer_id).first() - cache_params = (Department(id=officer.department_id), KEY_DEPT_ALL_NOTES) + officer = session.get(Officer, note.officer_id) + cache_params = ( + session.get(Department, officer.department_id), + KEY_DEPT_ALL_NOTES, + ) put_database_cache_entry(*cache_params, 1) assert has_database_cache_entry(*cache_params) is True diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index 7f4953859..f86d27dd3 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -4,7 +4,6 @@ import random import re from datetime import date, datetime -from decimal import Decimal from http import HTTPStatus from io import BytesIO @@ -183,7 +182,7 @@ def test_admin_can_add_assignment(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=3).one() + officer = session.get(Officer, 3) job = Job.query.filter_by( department_id=officer.department_id, job_title="Police Officer" ).one() @@ -211,7 +210,7 @@ def test_admin_can_add_assignment(mockdata, client, session): def test_admin_add_assignment_validation_error(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=3).one() + officer = session.get(Officer, 3) job = Job.query.filter_by( department_id=officer.department_id, job_title="Police Officer" ).one() @@ -292,7 +291,7 @@ def test_admin_can_edit_assignment(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=3).one() + officer = session.get(Officer, 3) job = Job.query.filter_by( department_id=officer.department_id, job_title="Police Officer" ).one() @@ -327,7 +326,7 @@ def test_admin_can_edit_assignment(mockdata, client, session): start_date=date(2019, 2, 1), resign_date=date(2019, 11, 30), ) - officer = Officer.query.filter_by(id=3).one() + officer = session.get(Officer, 3) rv = client.post( url_for( @@ -374,7 +373,7 @@ def test_admin_edit_assignment_validation_error( # Attempt to set resign date to a date before the start date form = AssignmentForm(resign_date=date(2018, 12, 31)) - officer = Officer.query.filter_by(id=officer.id).one() + officer = session.get(Officer, officer.id) rv = client.post( url_for( @@ -424,7 +423,7 @@ def test_ac_can_edit_officer_in_their_dept_assignment(mockdata, client, session) assert officer.assignments[0].start_date == date(2019, 1, 1) assert officer.assignments[0].resign_date == date(2019, 12, 31) - officer = Officer.query.filter_by(id=officer.id).one() + officer = session.get(Officer, officer.id) job = Job.query.filter_by( department_id=officer.department_id, job_title="Commander" ).one() @@ -481,7 +480,7 @@ def test_ac_cannot_edit_assignment_outside_their_dept(mockdata, client, session) login_ac(client) - officer = Officer.query.filter_by(id=officer.id).one() + officer = session.get(Officer, officer.id) job = Job.query.filter_by( department_id=officer.department_id, job_title="Commander" ).one() @@ -1220,8 +1219,8 @@ def test_admin_can_add_new_officer(mockdata, client, session, department, faker) assert officer.descriptions[0].last_updated_by == admin.id assert len(officer.salaries) == 1 - assert officer.salaries[0].salary == Decimal("123.45") - assert officer.salaries[0].overtime_pay == Decimal("543.21") + assert officer.salaries[0].salary == 123.45 + assert officer.salaries[0].overtime_pay == 543.21 assert officer.salaries[0].created_by == admin.id assert officer.descriptions[0].last_updated_by == admin.id @@ -1269,7 +1268,7 @@ def test_admin_can_add_new_officer_with_unit( def test_ac_can_add_new_officer_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) first_name = "Testy" last_name = "OTester" middle_initial = "R" @@ -1311,7 +1310,7 @@ def test_ac_can_add_new_officer_in_their_dept(mockdata, client, session): def test_ac_can_add_new_officer_with_unit_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) unit = random.choice(unit_choices(department_id=department.id)) first_name = "Testy" last_name = "OTester" @@ -1455,7 +1454,7 @@ def test_ac_cannot_edit_officer_not_in_their_dept(mockdata, client, session): assert rv.status_code == HTTPStatus.FORBIDDEN # Ensure changes were not made to database - officer = Officer.query.filter_by(id=officer.id).one() + officer = session.get(Officer, officer.id) assert officer.last_name == old_last_name @@ -1480,7 +1479,7 @@ def test_ac_can_see_officer_not_in_their_dept(mockdata, client, session): def test_ac_can_edit_officer_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) unit = random.choice(unit_choices(department.id)) first_name = "Testier" last_name = "OTester" @@ -1530,7 +1529,7 @@ def test_ac_can_edit_officer_in_their_dept(mockdata, client, session): assert last_name not in rv.data.decode(ENCODING_UTF_8) # Check the changes were added to the database - officer = Officer.query.filter_by(id=officer.id).one() + officer = session.get(Officer, officer.id) assert officer.last_name == new_last_name @@ -1618,7 +1617,7 @@ def test_ac_can_add_new_unit_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) form = AddUnitForm(description="Test", department=department.id) rv = client.post( @@ -2037,7 +2036,7 @@ def test_admin_can_upload_photos_of_dept_officers( data = {"file": (test_jpg_bytes_io, "204Cat.png")} - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) officer = department.officers[3] officer_face_count = len(officer.face) @@ -2077,7 +2076,7 @@ def test_upload_photo_sends_500_on_s3_error( data = {"file": (test_png_bytes_io, "204Cat.png")} - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) mock = MagicMock(return_value=None) officer = department.officers[0] officer_face_count = len(officer.face) @@ -2099,7 +2098,7 @@ def test_upload_photo_sends_415_for_bad_file_type(mockdata, client, session): with current_app.test_request_context(): login_admin(client) data = {"file": (BytesIO(b"my file contents"), "test_cop1.png")} - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) officer = department.officers[0] mock = MagicMock(return_value=False) with patch("OpenOversight.app.main.views.allowed_file", mock): @@ -2118,7 +2117,7 @@ def test_user_cannot_upload_officer_photo(mockdata, client, session): with current_app.test_request_context(): login_user(client) data = {"file": (BytesIO(b"my file contents"), "test_cop1.png")} - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) officer = department.officers[0] rv = client.post( url_for("main.upload", department_id=department.id, officer_id=officer.id), @@ -2137,7 +2136,7 @@ def test_ac_can_upload_photos_of_dept_officers( data = { "file": (test_png_bytes_io, "204Cat.png"), } - department = Department.query.filter_by(id=AC_DEPT).first() + department = session.get(Department, AC_DEPT) officer = department.officers[4] officer_face_count = len(officer.face) @@ -2210,7 +2209,7 @@ def test_edit_officers_with_blank_uids(mockdata, client, session): def test_admin_can_add_salary(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).first() + officer = session.get(Officer, AC_DEPT) cache_params = (Department(id=officer.department_id), KEY_DEPT_ALL_SALARIES) put_database_cache_entry(*cache_params, 1) @@ -2285,7 +2284,7 @@ def test_ac_cannot_add_non_dept_salary(mockdata, client, session): def test_admin_can_edit_salary(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=1).first() + officer = session.get(Officer, 1) cache_params = (Department(id=officer.department_id), KEY_DEPT_ALL_SALARIES) put_database_cache_entry(*cache_params, 1) @@ -2311,7 +2310,7 @@ def test_admin_can_edit_salary(mockdata, client, session): assert "$123,456.78" in rv.data.decode(ENCODING_UTF_8) form = SalaryForm(salary=150000) - officer = Officer.query.filter_by(id=1).one() + officer = session.get(Officer, 1) rv = client.post( url_for( @@ -2327,7 +2326,7 @@ def test_admin_can_edit_salary(mockdata, client, session): assert "Edited officer salary" in rv.data.decode(ENCODING_UTF_8) assert "$150,000.00" in rv.data.decode(ENCODING_UTF_8) - officer = Officer.query.filter_by(id=1).one() + officer = session.get(Officer, 1) assert officer.salaries[0].salary == 150000 assert has_database_cache_entry(*cache_params) is False @@ -2358,7 +2357,7 @@ def test_ac_can_edit_salary_in_their_dept(mockdata, client, session): assert "$123,456.78" in rv.data.decode(ENCODING_UTF_8) form = SalaryForm(salary=150000) - officer = Officer.query.filter_by(id=officer_id).one() + officer = session.get(Officer, officer.id) rv = client.post( url_for( @@ -2374,7 +2373,7 @@ def test_ac_can_edit_salary_in_their_dept(mockdata, client, session): assert "Edited officer salary" in rv.data.decode(ENCODING_UTF_8) assert "$150,000.00" in rv.data.decode(ENCODING_UTF_8) - officer = Officer.query.filter_by(id=officer_id).one() + officer = session.get(Officer, officer.id) assert officer.salaries[0].salary == 150000 @@ -2404,7 +2403,7 @@ def test_ac_cannot_edit_non_dept_salary(mockdata, client, session): login_ac(client) form = SalaryForm(salary=150000) - officer = Officer.query.filter_by(id=officer_id).one() + officer = session.get(Officer, officer.id) rv = client.post( url_for( @@ -2419,7 +2418,7 @@ def test_ac_cannot_edit_non_dept_salary(mockdata, client, session): assert rv.status_code == HTTPStatus.FORBIDDEN - officer = Officer.query.filter_by(id=officer_id).one() + officer = session.get(Officer, officer.id) assert float(officer.salaries[0].salary) == 123456.78 @@ -2580,7 +2579,7 @@ def test_ac_can_add_link_with_content_warning( def test_admin_can_edit_link_on_officer_profile(mockdata, client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=1).one() + officer = session.get(Officer, 1) cache_params = (Department(id=officer.department_id), KEY_DEPT_ALL_LINKS) put_database_cache_entry(*cache_params, 1) diff --git a/OpenOversight/tests/routes/test_singular_redirects.py b/OpenOversight/tests/routes/test_singular_redirects.py index ec8e29c87..ea75828b5 100644 --- a/OpenOversight/tests/routes/test_singular_redirects.py +++ b/OpenOversight/tests/routes/test_singular_redirects.py @@ -104,7 +104,7 @@ def test_redirect_with_department_id(client, session, source_route, target_route def test_redirect_with_officer_id(client, session, source_route, target_route): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) resp_no_redirect = client.get( url_for(source_route, officer_id=officer.id), follow_redirects=False, @@ -128,7 +128,7 @@ def test_redirect_with_officer_id(client, session, source_route, target_route): def test_redirect_add_assignment(client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) resp_no_redirect = client.post( url_for("main.redirect_add_assignment", officer_id=officer.id), follow_redirects=False, @@ -149,7 +149,7 @@ def test_redirect_add_assignment(client, session): def test_redirect_edit_assignment(client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) job = Job.query.filter_by( department_id=officer.department_id, job_title="Police Officer" ).one() @@ -211,7 +211,7 @@ def test_redirect_edit_assignment(client, session): def test_redirect_add_salary(client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) resp_no_redirect = client.post( url_for("main.redirect_add_salary", officer_id=officer.id), follow_redirects=False, @@ -235,7 +235,7 @@ def test_redirect_add_salary(client, session): def test_redirect_edit_salary(client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) salary = Salary.query.filter_by(officer_id=officer.id).one() resp_no_redirect = client.post( url_for( @@ -448,7 +448,7 @@ def test_redirect_complete_tagging(client, session): def test_redirect_submit_complaint(client, session): with current_app.test_request_context(): login_user(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) query_string = { "officer_first_name": officer.first_name, "officer_last_name": officer.last_name, @@ -476,7 +476,7 @@ def test_redirect_submit_complaint(client, session): def test_redirect_upload(client, session): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).one() + officer = session.get(Officer, AC_DEPT) resp_no_redirect = client.post( url_for( "main.redirect_upload", @@ -517,7 +517,7 @@ def test_redirect_upload(client, session): def test_redirect_note(client, session, source_route, target_route): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).first() + officer = session.get(Officer, AC_DEPT) note = Note.query.filter_by(officer_id=officer.id).first() resp_no_redirect = client.get( url_for(source_route, officer_id=officer.id, obj_id=note.id), @@ -550,7 +550,7 @@ def test_redirect_note(client, session, source_route, target_route): def test_redirect_description(client, session, source_route, target_route): with current_app.test_request_context(): login_admin(client) - officer = Officer.query.filter_by(id=AC_DEPT).first() + officer = session.get(Officer, AC_DEPT) description = Description.query.filter_by(officer_id=officer.id).first() resp_no_redirect = client.get( url_for( diff --git a/OpenOversight/tests/test_commands.py b/OpenOversight/tests/test_commands.py index 3bde7a114..4ca3d6696 100644 --- a/OpenOversight/tests/test_commands.py +++ b/OpenOversight/tests/test_commands.py @@ -344,7 +344,7 @@ def test_csv_changed_static_field(csvfile): assert "has differing birth_year field" in str(exc.value) -def test_csv_new_assignment(csvfile, monkeypatch): +def test_csv_new_assignment(csvfile, monkeypatch, session): monkeypatch.setattr("builtins.input", lambda: "y") # Delete all current officers and assignments Assignment.query.delete() @@ -375,9 +375,9 @@ def test_csv_new_assignment(csvfile, monkeypatch): assert n_created == 0 assert n_updated == 1 - officer = Officer.query.filter_by(id=officer.id).one() - assert len(list(officer.assignments)) == 2 - for assignment in officer.assignments: + new_officer = session.get(Officer, officer.id) + assert len(list(new_officer.assignments)) == 2 + for assignment in new_officer.assignments: assert ( assignment.job.job_title == "Commander" or assignment.job.job_title == "CAPTAIN" @@ -451,7 +451,7 @@ def test_csv_new_officer(csvfile, monkeypatch): assert Officer.query.count() == n_officers + 1 -def test_csv_new_salary(csvfile, monkeypatch): +def test_csv_new_salary(csvfile, monkeypatch, session): monkeypatch.setattr("builtins.input", lambda: "y") # Delete all current officers and salaries Salary.query.delete() @@ -459,7 +459,7 @@ def test_csv_new_salary(csvfile, monkeypatch): assert Officer.query.count() == 0 - df = pd.read_csv(csvfile) + df = pd.read_csv(csvfile, dtype={"salary": "str"}) df.loc[0, "salary"] = "123456.78" df.to_csv(csvfile) @@ -485,9 +485,9 @@ def test_csv_new_salary(csvfile, monkeypatch): assert n_updated == 1 assert Officer.query.count() == officer_count - officer = Officer.query.filter_by(id=officer.id).one() - assert len(list(officer.salaries)) == 2 - for salary in officer.salaries: + new_officer = session.get(Officer, officer.id) + assert len(list(new_officer.salaries)) == 2 + for salary in new_officer.salaries: assert float(salary.salary) == 123456.78 or float(salary.salary) == 150000.00 diff --git a/OpenOversight/tests/test_models.py b/OpenOversight/tests/test_models.py index dc5940d90..98d0916ce 100644 --- a/OpenOversight/tests/test_models.py +++ b/OpenOversight/tests/test_models.py @@ -571,3 +571,53 @@ def test_images_added_with_user_id(mockdata, faker): db.session.commit() saved = Image.query.filter_by(created_by=user_id).first() assert saved is not None + + +def test_salary_rounding_and_sorting(mockdata, session): + officer = Officer.query.first() + large_number = 123123456789.01 + small_number = 0.01 + session.add( + Salary( + officer_id=officer.id, + salary=large_number, + overtime_pay=large_number, + year=2000, + is_fiscal_year=False, + ) + ) + + session.add( + Salary( + officer_id=officer.id, + salary=small_number, + overtime_pay=small_number, + year=2001, + is_fiscal_year=False, + ) + ) + + session.commit() + + salaries = ( + Salary.query.filter_by(officer_id=officer.id) + .order_by(Salary.salary.desc()) + .all() + ) + + assert salaries[0].salary == large_number + assert salaries[0].overtime_pay == large_number + assert salaries[-1].salary == small_number + assert salaries[-1].overtime_pay == small_number + + +def test_officer_incident_sort_order(mockdata, session): + officer = session.get(Officer, 1) + + sorted_incidents = sorted( + officer.incidents, + key=lambda i: (i.date, i.time), + reverse=True, + ) + + assert officer.incidents == sorted_incidents