Skip to content

Commit

Permalink
Fix unit "Not Sure" option (#145)
Browse files Browse the repository at this point in the history
* Fix unit "Not Sure" option

* Update OpenOversight/tests/conftest.py

Co-authored-by: Madison Swain-Bowden <bowdenm@spu.edu>

Co-authored-by: Madison Swain-Bowden <bowdenm@spu.edu>
  • Loading branch information
sea-kelp and AetherUnbound authored Jun 17, 2022
1 parent 043ac9d commit 1831bd8
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 16 deletions.
4 changes: 2 additions & 2 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ def list_officer(
):
form_data["gender"] = genders

unit_choices = [
unit_choices = ["Not Sure"] + [
uc[0]
for uc in db.session.query(Unit.descrip)
.filter_by(department_id=department_id)
Expand Down Expand Up @@ -717,7 +717,7 @@ def list_officer(
"race": RACE_CHOICES,
"gender": GENDER_CHOICES,
"rank": [(rc, rc) for rc in rank_choices],
"unit": [("Not Sure", "Not Sure")] + [(uc, uc) for uc in unit_choices],
"unit": [(uc, uc) for uc in unit_choices],
}

next_url = url_for(
Expand Down
1 change: 1 addition & 0 deletions OpenOversight/app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ class Currency(db.TypeDecorator):
"""

impl = db.Numeric
cache_ok = True

def load_dialect_impl(self, dialect):
typ = db.Numeric()
Expand Down
18 changes: 13 additions & 5 deletions OpenOversight/app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ def filter_by_form(form_data, officer_query, department_id=None):
form_data["rank"].append(None)

unit_ids = []
include_null_unit = False
if form_data.get("unit"):
unit_ids = [
unit.id
Expand All @@ -361,17 +362,24 @@ def filter_by_form(form_data, officer_query, department_id=None):
]

if "Not Sure" in form_data["unit"]:
# Convert "Not Sure" to None, so the resulting SQL query allows NULL values
form_data["unit"].append(None)
include_null_unit = True

if form_data.get("badge") or unit_ids or job_ids:
if form_data.get("badge") or unit_ids or include_null_unit or job_ids:
officer_query = officer_query.join(Officer.assignments)
if form_data.get("badge"):
officer_query = officer_query.filter(
Assignment.star_no.like("%%{}%%".format(form_data["badge"]))
)
if unit_ids:
officer_query = officer_query.filter(Assignment.unit_id.in_(unit_ids))

if unit_ids or include_null_unit:
# Split into 2 expressions because the SQL IN keyword does not match NULLs
unit_filters = []
if unit_ids:
unit_filters.append(Assignment.unit_id.in_(unit_ids))
if include_null_unit:
unit_filters.append(Assignment.unit_id.is_(None))
officer_query = officer_query.filter(or_(*unit_filters))

if job_ids:
officer_query = officer_query.filter(Assignment.job_id.in_(job_ids))
officer_query = officer_query.options(selectinload(Officer.assignments_lazy))
Expand Down
9 changes: 6 additions & 3 deletions OpenOversight/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from decimal import Decimal
from io import BytesIO
from pathlib import Path
from typing import List
from typing import List, Optional

import pytest
from faker import Faker
Expand Down Expand Up @@ -131,12 +131,14 @@ def generate_officer():
)


def build_assignment(officer: Officer, units: List[Unit], jobs: Job):
def build_assignment(officer: Officer, units: List[Optional[Unit]], jobs: Job):
unit = random.choice(units)
unit_id = unit.id if unit else None
return models.Assignment(
star_no=pick_star(),
job_id=random.choice(jobs).id,
officer=officer,
unit_id=random.choice(units).id,
unit_id=unit_id,
star_date=pick_date(officer.full_name().encode("utf-8")),
resign_date=pick_date(officer.full_name().encode("utf-8")),
)
Expand Down Expand Up @@ -308,6 +310,7 @@ def add_mockdata(session):
]
session.add_all(test_units)
session.commit()
test_units.append(None)

test_images = [
models.Image(
Expand Down
21 changes: 17 additions & 4 deletions OpenOversight/tests/routes/test_officer_and_department.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from OpenOversight.app.models import (
Assignment,
Department,
Face,
Image,
Incident,
Job,
Expand Down Expand Up @@ -1728,8 +1729,14 @@ def test_admin_can_upload_photos_of_dept_officers(
officer = department.officers[3]
officer_face_count = len(officer.face)

crop_mock = MagicMock(return_value=Image.query.first())
upload_mock = MagicMock(return_value=Image.query.first())
# Filter out images that the officer is already tagged in
officer_faces = Face.query.filter_by(officer_id=officer.id).all()
image = Image.query.filter(
Image.id.not_in([face.img_id for face in officer_faces])
).first()

crop_mock = MagicMock(return_value=image)
upload_mock = MagicMock(return_value=image)
with patch(
"OpenOversight.app.main.views.upload_image_to_s3_and_store_in_db",
upload_mock,
Expand Down Expand Up @@ -1830,8 +1837,14 @@ def test_ac_can_upload_photos_of_dept_officers(
officer = department.officers[4]
officer_face_count = len(officer.face)

crop_mock = MagicMock(return_value=Image.query.first())
upload_mock = MagicMock(return_value=Image.query.first())
# Filter out images that the officer is already tagged in
officer_faces = Face.query.filter_by(officer_id=officer.id).all()
image = Image.query.filter(
Image.id.not_in([face.img_id for face in officer_faces])
).first()

crop_mock = MagicMock(return_value=image)
upload_mock = MagicMock(return_value=image)
with patch(
"OpenOversight.app.main.views.upload_image_to_s3_and_store_in_db",
upload_mock,
Expand Down
34 changes: 32 additions & 2 deletions OpenOversight/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
from mock import MagicMock, Mock, patch

import OpenOversight
from OpenOversight.app.models import Image
from OpenOversight.app.utils import crop_image, upload_image_to_s3_and_store_in_db
from OpenOversight.app.models import Department, Image, Officer, Unit
from OpenOversight.app.utils import (
crop_image,
filter_by_form,
upload_image_to_s3_and_store_in_db,
)
from OpenOversight.tests.routes.route_helpers import login_user


Expand Down Expand Up @@ -305,3 +309,29 @@ def test_crop_image_calls_upload_image_to_s3_and_store_in_db_with_user_id(
assert (
current_user.get_id() in upload_image_to_s3_and_store_in_db.call_args[0]
)


@pytest.mark.parametrize(
"units, has_officers_with_unit, has_officers_with_no_unit",
[
(["Not Sure"], False, True),
(["Donut Devourers"], True, False),
(["Not Sure", "Donut Devourers"], True, True),
],
)
def test_filter_by_form_filter_unit(
mockdata, units, has_officers_with_unit, has_officers_with_no_unit
):
form_data = dict(unit=units)
unit_id = Unit.query.filter_by(descrip="Donut Devourers").one().id
department_id = Department.query.first().id

officers = filter_by_form(form_data, Officer.query, department_id).all()

for officer in officers:
found = False
if has_officers_with_unit:
found = found or any([a.unit_id == unit_id for a in officer.assignments])
if has_officers_with_no_unit:
found = found or any([a.unit_id is None for a in officer.assignments])
assert found

0 comments on commit 1831bd8

Please sign in to comment.