Improve unit & rank search filters (#91)

* Make default arguments immutable

* Make unit a multiple-select option

* Add unit description to officer list, re-arrange items

* Convert currently_on_force to property, add to officer list

* Make really long form filters scrollable

* Move uncertain currently_on_force logic out of template

* Add comment describing behavior

* Allow unit to be missing

* Only scroll if the box overflows, set max height

Author: AetherUnbound

OpenOversight/app/__init__.py

@@ -107,13 +107,6 @@ class which will render the field accordion open.
         """
         return " in " if form_data.get(field) else ""
 
-    @app.template_filter("currently_on_force")
-    def officer_currently_on_force(assignments):
-        if not assignments:
-            return "Uncertain"
-        most_recent = max(assignments, key=lambda x: x.star_date or datetime.date.min)
-        return "Yes" if most_recent.resign_date is None else "No"
-
     # Add commands
     Migrate(
         app, db, os.path.join(os.path.dirname(__file__), "..", "migrations")

OpenOversight/app/main/forms.py

@@ -563,6 +563,12 @@ class BrowseForm(Form):
         get_label="job_title",
         get_pk=lambda job: job.job_title,
     )  # query set in view function
+    unit = QuerySelectField(
+        "unit",
+        validators=[Optional()],
+        get_label="descrip",
+        get_pk=lambda unit: unit.descrip,
+    )  # query set in view function
     name = StringField("Last name")
     badge = StringField("Badge number")
     unique_internal_identifier = StringField("Unique ID")

OpenOversight/app/main/views.py

@@ -613,9 +613,9 @@ def edit_department(department_id):
 def list_officer(
     department_id,
     page=1,
-    race=[],
-    gender=[],
-    rank=[],
+    race=None,
+    gender=None,
+    rank=None,
     min_age="16",
     max_age="100",
     name=None,
@@ -630,14 +630,14 @@ def list_officer(
         .all()
     )
     form_data = form.data
-    form_data["race"] = race
-    form_data["gender"] = gender
-    form_data["rank"] = rank
+    form_data["race"] = race or []
+    form_data["gender"] = gender or []
+    form_data["rank"] = rank or []
     form_data["min_age"] = min_age
     form_data["max_age"] = max_age
     form_data["name"] = name
     form_data["badge"] = badge
-    form_data["unit"] = unit
+    form_data["unit"] = unit or []
     form_data["unique_internal_identifier"] = unique_internal_identifier
 
     OFFICERS_PER_PAGE = int(current_app.config["OFFICERS_PER_PAGE"])
@@ -658,8 +658,6 @@ def list_officer(
         form_data["name"] = name_arg
     if badge_arg := request.args.get("badge"):
         form_data["badge"] = badge_arg
-    if (unit_arg := request.args.get("unit")) and unit_arg != "Not Sure":
-        form_data["unit"] = int(unit_arg)
     if uid := request.args.get("unique_internal_identifier"):
         form_data["unique_internal_identifier"] = uid
     if (races := request.args.getlist("race")) and all(
@@ -674,18 +672,23 @@ def list_officer(
         form_data["gender"] = genders
 
     unit_choices = [
-        (unit.id, unit.descrip)
-        for unit in Unit.query.filter_by(department_id=department_id)
+        uc[0]
+        for uc in db.session.query(Unit.descrip)
+        .filter_by(department_id=department_id)
         .order_by(Unit.descrip.asc())
         .all()
     ]
     rank_choices = [
         jc[0]
         for jc in db.session.query(Job.job_title, Job.order)
         .filter_by(department_id=department_id)
-        .order_by(Job.order)
+        .order_by(Job.job_title)
         .all()
     ]
+    if (units := request.args.getlist("unit")) and all(
+        unit in unit_choices for unit in units
+    ):
+        form_data["unit"] = units
     if (ranks := request.args.getlist("rank")) and all(
         rank in rank_choices for rank in ranks
     ):
@@ -710,7 +713,7 @@ def list_officer(
         "race": RACE_CHOICES,
         "gender": GENDER_CHOICES,
         "rank": [(rc, rc) for rc in rank_choices],
-        "unit": [("Not Sure", "Not Sure")] + unit_choices,
+        "unit": [("Not Sure", "Not Sure")] + [(uc, uc) for uc in unit_choices],
     }
 
     next_url = url_for(

OpenOversight/app/models.py

@@ -188,12 +188,27 @@ def job_title(self):
                 self.assignments_lazy, key=lambda x: x.star_date or date.min
             ).job.job_title
 
+    def unit_descrip(self):
+        if self.assignments_lazy:
+            unit = max(
+                self.assignments_lazy, key=lambda x: x.star_date or date.min
+            ).unit
+            return unit.descrip if unit else None
+
     def badge_number(self):
         if self.assignments_lazy:
             return max(
                 self.assignments_lazy, key=lambda x: x.star_date or date.min
             ).star_no
 
+    def currently_on_force(self):
+        if self.assignments_lazy:
+            most_recent = max(
+                self.assignments_lazy, key=lambda x: x.star_date or date.min
+            )
+            return "Yes" if most_recent.resign_date is None else "No"
+        return "Uncertain"
+
     def __repr__(self):
         if self.unique_internal_identifier:
             return "<Officer ID {}: {} {} {} {} ({})>".format(

OpenOversight/app/static/css/openoversight.css

@@ -484,6 +484,11 @@ tr:hover .row-actions {
     padding: 5px 0;
 }
 
+.filter-sidebar .panel-body-long {
+    max-height: 20vh;
+    overflow-y: auto;
+}
+
 .filter-sidebar .panel-heading .accordion-toggle:after {
     font-family: 'Glyphicons Halflings';
     content: "\e252";

OpenOversight/app/templates/list_officer.html

@@ -70,7 +70,7 @@ <h3 class="panel-title accordion-toggle">Gender</h3>
             <h3 class="panel-title accordion-toggle">Rank</h3>
           </div>
           <div class="collapse {{ form_data | field_in_query("rank") }}" id="filter-rank">
-            <div class="panel-body">
+            <div class="panel-body panel-body-long">
               <div class="form-group checkbox">
                 {% for choice in choices['rank'] %}
                   <label class="form-check">
@@ -86,13 +86,15 @@ <h3 class="panel-title accordion-toggle">Rank</h3>
             <h3 class="panel-title accordion-toggle">Unit</h3>
           </div>
           <div class="collapse {{ form_data | field_in_query("unit") }}" id="filter-unit">
-            <div class="panel-body">
+            <div class="panel-body panel-body-long">
               <div class="form-group">
-                <select class="form-control" name="unit">
-                  {% for choice in choices['unit'] %}
-                  <option id="unit-{{ choice[0] }}" value="{{choice[0]}}" {% if choice[0] == form_data['unit'] %}selected='true'{% endif %}>{{choice[1]}}</option>
-                  {% endfor %}
-                </select>
+                <div class="form-group checkbox">
+                    {% for choice in choices['unit'] %}
+                        <label class="form-check">
+                            <input type="checkbox" class="form-check-input" id="unit-{{ choice[0] }}" name="unit" value="{{ choice[0] }}" {% if choice[0] in form_data['unit'] %}checked="checked" {% endif %}/>{{choice[1]}}
+                        </label>
+                    {% endfor %}
+                </div>
               </div>
             </div>
           </div>
@@ -152,14 +154,18 @@ <h2>
                 <div class="row">
                   <div class="col-md-6 col-xs-6">
                     <dl>
-                      <dt>Job Title</dt>
+                      <dt>Rank</dt>
                       <dd>{{ officer.job_title()|default('Unknown') }}</dd>
-                      <dt>Race</dt>
-                      <dd>{{ officer.race_label()|default('Unknown')|lower|title }}</dd>
+                      <dt>Unit</dt>
+                      <dd>{{ officer.unit_descrip()|default('Unknown') }}</dd>
+                      <dt>Currently on the Force</dt>
+                      <dd>{{ officer.currently_on_force() }}</dd>
                     </dl>
                   </div>
                   <div class="col-md-6 col-xs-6">
                     <dl>
+                      <dt>Race</dt>
+                      <dd>{{ officer.race_label()|default('Unknown')|lower|title }}</dd>
                       <dt>Gender</dt>
                       <dd>{{ officer.gender_label()|default('Unknown') }}</dd>
                       <dt>Number of Photos</dt>

OpenOversight/app/templates/partials/officer_general_information.html

@@ -50,7 +50,7 @@ <h3>
                 </tr>
                 <tr>
                     <td><b>Currently on the force</b></td>
-                    <td>{{ assignments | currently_on_force }}</td>
+                    <td>{{ officer.currently_on_force() }}</td>
                 </tr>
             </tbody>
         </table>

OpenOversight/app/utils.py

@@ -348,20 +348,30 @@ def filter_by_form(form_data, officer_query, department_id=None):
             .all()
         ]
 
-        print(form_data)
         if "Not Sure" in form_data["rank"]:
             form_data["rank"].append(None)
 
-    if form_data.get("badge") or form_data.get("unit") or job_ids:
+    unit_ids = []
+    if form_data.get("unit"):
+        unit_ids = [
+            unit.id
+            for unit in Unit.query.filter_by(department_id=department_id)
+            .filter(Unit.descrip.in_(form_data.get("unit")))
+            .all()
+        ]
+
+        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)
+
+    if form_data.get("badge") or unit_ids 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 form_data.get("unit"):
-            officer_query = officer_query.filter(
-                Assignment.unit_id == form_data["unit"]
-            )
+        if unit_ids:
+            officer_query = officer_query.filter(Assignment.unit_id.in_(unit_ids))
         if job_ids:
             officer_query = officer_query.filter(Assignment.job_id.in_(job_ids))
     officer_query = officer_query.options(selectinload(Officer.assignments_lazy))

OpenOversight/tests/routes/test_officer_and_department.py

@@ -1654,6 +1654,7 @@ def test_browse_filtering_allows_good(client, mockdata, session):
         ]
         officer = Officer.query.filter_by(department_id=AC_DEPT).first()
         job = Job.query.filter_by(department_id=officer.department_id).first()
+        unit = Unit.query.filter_by(department_id=officer.department_id).first()
         form = AddOfficerForm(
             first_name="A",
             last_name="A",
@@ -1662,6 +1663,7 @@ def test_browse_filtering_allows_good(client, mockdata, session):
             gender="M",
             star_no=666,
             job_id=job.id,
+            unit=unit.id,
             department=department_id,
             birth_year=1990,
             links=links,
@@ -1695,14 +1697,18 @@ def test_browse_filtering_allows_good(client, mockdata, session):
             data=data,
             follow_redirects=True,
         )
-        filter_list = rv.data.decode("utf-8").split("<dt>Race</dt>")[1:]
-        assert any("<dd>White</dd>" in token for token in filter_list)
 
-        filter_list = rv.data.decode("utf-8").split("<dt>Job Title</dt>")[1:]
+        filter_list = rv.data.decode("utf-8").split("<dt>Rank</dt>")[1:]
         assert any(
             "<dd>{}</dd>".format(job.job_title) in token for token in filter_list
         )
 
+        filter_list = rv.data.decode("utf-8").split("<dt>Unit</dt>")[1:]
+        assert any("<dd>{}</dd>".format(unit.descrip) in token for token in filter_list)
+
+        filter_list = rv.data.decode("utf-8").split("<dt>Race</dt>")[1:]
+        assert any("<dd>White</dd>" in token for token in filter_list)
+
         filter_list = rv.data.decode("utf-8").split("<dt>Gender</dt>")[1:]
         assert any("<dd>Male</dd>" in token for token in filter_list)