Upstream batch 5 (#408)

* Make url_for work with all endpoints  (lucyparsons#1070)

Some of the `ModelView` based endpoints were missing a distinct name
that could be used for `url_for`, and instead string concatenation was
used (mostly in templates). I also moved some logic out of templates
(e.g. calculation of total pay), added helper methods and made some
other changed to reduce the code needed in the templates and improve
readability a little bit.

I also added an anonymous user class that is useful to call things like
`.is_admin_or_coordinator(department)` on any `current_user` object,
without first making sure the user is not anonymous.

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

* Replace inline styles with external css (lucyparsons#1073)

<!-- New Contributor? Welcome!

We recommend you check your privacy settings, so the name and email
associated with
the commits are what you want them to be. See the contribution guide at

https://github.com/lucyparsons/OpenOversight/blob/develop/CONTRIB.md#recommended-privacy-settings
for more infos.

Also make sure you have read and abide by the code of conduct:

https://github.com/lucyparsons/OpenOversight/blob/develop/CODE_OF_CONDUCT.md

If this pull request is not ready for review yet, please submit it as a
draft.
-->
lucyparsons#969

Removes the H021 tag and does the relevant clean-up. In this case,
inline styles are refactored into external CSS via pre-existing
stylesheets.

Unsure of the CSS class names, very open to changes. One in particular I
really disliked, but left a note-to-self. Historically more of a
back-end dev, so apologies if I missed something obvious with these
changes!

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

(`pre-commit` technically fails on some .py files and migrations, but
none of the new changes it seems)

* Add missing <img> alt tags (lucyparsons#1074)

## Fixes issue
lucyparsons#970

## Description of Changes
Removes H013 from pre-commit ignore list and adds alt tag + text to
<img>s where needed. Adds some tips and resources in the CONTRIB.md for
folks who encounter this in the future.

## Tests and linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [ ] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

* Update font-awesome icons (lucyparsons#1076)

* Fix `Departments` pagination automatic filter application (lucyparsons#1078)

## Fixes issue
Fixes lucyparsons#1077 

## Description of Changes
Changes check if there are "filters" being applied to the list given in
the `departments` view. If there are no filters applied then we generate
a "next" link that does not automatically apply filters.

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

---------

Co-authored-by: sea-kelp <[email protected]>

* Add default Officer Overtime value (lucyparsons#1083)

## Fixes issue
lucyparsons#856 

## Description of Changes
Adds default of $0 in officer "Overtime Pay" field. Users don't have to
fill this in if they don't have any overtime pay information and use the
form faster!

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

---------

Co-authored-by: abandoned-prototype <[email protected]>
Co-authored-by: benj <[email protected]>
Co-authored-by: Michael Plunkett <[email protected]>
Co-authored-by: Spraynard <[email protected]>

.pre-commit-config.yaml

@@ -129,4 +129,4 @@ repos:
           - OpenOversight/app/templates
           - --profile=jinja
           - --use-gitignore
-          - --ignore=H006,T028,H031,H021,H013,H011
+          - --ignore=H006,T028,H031,H011

CONTRIB.md

@@ -23,6 +23,14 @@ We use [pre-commit](https://pre-commit.com/) for automated linting and style che
 
 You can run `pre-commit run --all-files` or `make lint` to run pre-commit over your local codebase, or `pre-commit run` to run it only over the currently stages files.
 
+### Accessibility
+Keep in mind when adding images that `alt` tags are required for screen readers. If text outside of the image explains what the image is or is referring to, the tag can be an empty string (`alt=""`). The tag can also be empty if the image is decoration and does not add information or context. If the image has text or important information, use the present tense to describe what is happening in the image.
+
+For further reading:
+- https://www.a11yproject.com/
+- https://www.w3.org/WAI/tutorials/images/decision-tree/
+- https://accessibility.huit.harvard.edu/describe-content-images
+
 ## Development Environment
 You can use our Docker-compose environment to stand up a development OpenOversight.
 

OpenOversight/app/__init__.py

@@ -17,6 +17,7 @@
 from OpenOversight.app.filters import instantiate_filters
 from OpenOversight.app.models.config import config
 from OpenOversight.app.models.database import db
+from OpenOversight.app.models.users import AnonymousUser
 from OpenOversight.app.utils.constants import MEGABYTE
 
 
@@ -25,6 +26,7 @@
 
 login_manager = LoginManager()
 login_manager.session_protection = "strong"
+login_manager.anonymous_user = AnonymousUser
 login_manager.login_view = "auth.login"
 
 limiter = Limiter(

OpenOversight/app/filters.py

@@ -82,6 +82,10 @@ def thousands_separator(value: int) -> str:
     return f"{value:,}"
 
 
+def display_currency(value: float) -> str:
+    return f"${value:,.2f}"
+
+
 def instantiate_filters(app: Flask):
     """Instantiate all template filters"""
     app.template_filter("capfirst")(capfirst_filter)
@@ -93,3 +97,4 @@ def instantiate_filters(app: Flask):
     app.template_filter("display_time")(display_time)
     app.template_filter("local_time")(local_time)
     app.template_filter("thousands_separator")(thousands_separator)
+    app.template_filter("display_currency")(display_currency)

OpenOversight/app/main/forms.py

@@ -173,7 +173,9 @@ class SalaryForm(Form):
         "Salary", validators=[NumberRange(min=0, max=1000000), validate_money]
     )
     overtime_pay = DecimalField(
-        "Overtime Pay", validators=[NumberRange(min=0, max=1000000), validate_money]
+        "Overtime Pay",
+        default=0,
+        validators=[NumberRange(min=0, max=1000000), validate_money],
     )
     year = IntegerField(
         "Year",

OpenOversight/app/main/views.py

@@ -4,6 +4,7 @@
 from datetime import datetime
 from http import HTTPMethod, HTTPStatus
 from traceback import format_exc
+from typing import Optional
 
 from flask import (
     Response,
@@ -327,13 +328,16 @@ def officer_profile(officer_id: int):
             .all()
         )
         assignments = Assignment.query.filter_by(officer_id=officer_id).all()
-        face_paths = []
-        for face in faces:
-            face_paths.append(serve_image(face.image.filepath))
+        face_paths = [(face, serve_image(face.image.filepath)) for face in faces]
         if not face_paths:
             # Add in the placeholder image if no faces are found
             face_paths = [
-                url_for("static", filename="images/placeholder.png", _external=True)
+                (
+                    None,
+                    url_for(
+                        "static", filename="images/placeholder.png", _external=True
+                    ),
+                )
             ]
     except Exception:
         current_app.logger.exception("Error loading officer profile")
@@ -351,8 +355,7 @@ def officer_profile(officer_id: int):
     return render_template(
         "officer.html",
         officer=officer,
-        paths=face_paths,
-        faces=faces,
+        face_paths=face_paths,
         assignments=assignments,
         form=form,
     )
@@ -856,7 +859,7 @@ def redirect_list_officer(
     unique_internal_identifier=None,
     unit=None,
     current_job=None,
-    require_photo: bool = False,
+    require_photo: Optional[bool] = None,
 ):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
@@ -896,7 +899,7 @@ def list_officer(
     unique_internal_identifier=None,
     unit=None,
     current_job=None,
-    require_photo: bool = False,
+    require_photo: Optional[bool] = None,
 ):
     form = BrowseForm()
     form.rank.query = (
@@ -2088,24 +2091,31 @@ def populate_obj(self, form: FlaskForm, obj: Incident):
 main.add_url_rule(
     "/incidents/",
     defaults={"obj_id": None},
+    endpoint="incident_api",
     view_func=incident_view,
     methods=[HTTPMethod.GET],
 )
 main.add_url_rule(
     "/incidents/new",
+    endpoint="incident_api_new",
     view_func=incident_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
 main.add_url_rule(
-    "/incidents/<int:obj_id>", view_func=incident_view, methods=[HTTPMethod.GET]
+    "/incidents/<int:obj_id>",
+    endpoint="incident_api",
+    view_func=incident_view,
+    methods=[HTTPMethod.GET],
 )
 main.add_url_rule(
     "/incidents/<int:obj_id>/edit",
+    endpoint="incident_api_edit",
     view_func=incident_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
 main.add_url_rule(
     "/incidents/<int:obj_id>/delete",
+    endpoint="incident_api_delete",
     view_func=incident_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2192,7 +2202,7 @@ def redirect_get_notes(officer_id: int, obj_id=None):
 def redirect_edit_note(officer_id: int, obj_id=None):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
-        f"{url_for('main.note_api', officer_id=officer_id, obj_id=obj_id)}/edit",
+        url_for("main.note_api_edit", officer_id=officer_id, obj_id=obj_id),
         code=HTTPStatus.PERMANENT_REDIRECT,
     )
 
@@ -2202,14 +2212,15 @@ def redirect_edit_note(officer_id: int, obj_id=None):
 def redirect_delete_note(officer_id: int, obj_id=None):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
-        f"{url_for('main.note_api', officer_id=officer_id, obj_id=obj_id)}/delete",
+        url_for("main.note_api_delete", officer_id=officer_id, obj_id=obj_id),
         code=HTTPStatus.PERMANENT_REDIRECT,
     )
 
 
 note_view = NoteApi.as_view("note_api")
 main.add_url_rule(
     "/officers/<int:officer_id>/notes/new",
+    endpoint="note_api",
     view_func=note_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2220,6 +2231,7 @@ def redirect_delete_note(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/notes/<int:obj_id>",
+    endpoint="note_api",
     view_func=note_view,
     methods=[HTTPMethod.GET],
 )
@@ -2230,6 +2242,7 @@ def redirect_delete_note(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/notes/<int:obj_id>/edit",
+    endpoint="note_api_edit",
     view_func=note_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2240,6 +2253,7 @@ def redirect_delete_note(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/notes/<int:obj_id>/delete",
+    endpoint="note_api_delete",
     view_func=note_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2255,7 +2269,7 @@ def redirect_delete_note(officer_id: int, obj_id=None):
 def redirect_new_description(officer_id: int):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
-        url_for("main.description_api", officer_id=officer_id),
+        url_for("main.description_api_new", officer_id=officer_id),
         code=HTTPStatus.PERMANENT_REDIRECT,
     )
 
@@ -2273,7 +2287,7 @@ def redirect_get_description(officer_id: int, obj_id=None):
 def redirect_edit_description(officer_id: int, obj_id=None):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
-        f"{url_for('main.description_api', officer_id=officer_id, obj_id=obj_id)}/edit",
+        url_for("main.description_api_edit", officer_id=officer_id, obj_id=obj_id),
         code=HTTPStatus.PERMANENT_REDIRECT,
     )
 
@@ -2283,14 +2297,15 @@ def redirect_edit_description(officer_id: int, obj_id=None):
 def redirect_delete_description(officer_id: int, obj_id=None):
     flash(FLASH_MSG_PERMANENT_REDIRECT)
     return redirect(
-        f"{url_for('main.description_api', officer_id=officer_id, obj_id=obj_id)}/delete",
+        url_for("main.description_api_delete", officer_id=officer_id, obj_id=obj_id),
         code=HTTPStatus.PERMANENT_REDIRECT,
     )
 
 
 description_view = DescriptionApi.as_view("description_api")
 main.add_url_rule(
     "/officers/<int:officer_id>/descriptions/new",
+    endpoint="description_api_new",
     view_func=description_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2301,6 +2316,7 @@ def redirect_delete_description(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/descriptions/<int:obj_id>",
+    endpoint="description_api",
     view_func=description_view,
     methods=[HTTPMethod.GET],
 )
@@ -2311,6 +2327,7 @@ def redirect_delete_description(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/descriptions/<int:obj_id>/edit",
+    endpoint="description_api_edit",
     view_func=description_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )
@@ -2321,6 +2338,7 @@ def redirect_delete_description(officer_id: int, obj_id=None):
 )
 main.add_url_rule(
     "/officers/<int:officer_id>/descriptions/<int:obj_id>/delete",
+    endpoint="description_api_delete",
     view_func=description_view,
     methods=[HTTPMethod.GET, HTTPMethod.POST],
 )

OpenOversight/app/models/database.py

@@ -1,9 +1,10 @@
+import operator
 import re
 import time
 import uuid
 from datetime import date, datetime
 from decimal import Decimal
-from typing import List
+from typing import List, Optional
 
 from authlib.jose import JoseError, JsonWebToken
 from cachetools import cached
@@ -304,21 +305,27 @@ def gender_label(self):
     def job_title(self):
         if self.assignments:
             return max(
-                self.assignments, key=lambda x: x.start_date or date.min
+                self.assignments, key=operator.attrgetter("start_date_or_min")
             ).job.job_title
 
     def unit_description(self):
         if self.assignments:
-            unit = max(self.assignments, key=lambda x: x.start_date or date.min).unit
+            unit = max(
+                self.assignments, key=operator.attrgetter("start_date_or_min")
+            ).unit
             return unit.description if unit else None
 
     def badge_number(self):
         if self.assignments:
-            return max(self.assignments, key=lambda x: x.start_date or date.min).star_no
+            return max(
+                self.assignments, key=operator.attrgetter("start_date_or_min")
+            ).star_no
 
     def currently_on_force(self):
         if self.assignments:
-            most_recent = max(self.assignments, key=lambda x: x.start_date or date.min)
+            most_recent = max(
+                self.assignments, key=operator.attrgetter("start_date_or_min")
+            )
             return "Yes" if most_recent.resign_date is None else "No"
         return "Uncertain"
 
@@ -376,6 +383,16 @@ class Salary(BaseModel, TrackUpdates):
     def __repr__(self):
         return f"<Salary: ID {self.officer_id} : {self.salary}"
 
+    @property
+    def total_pay(self) -> float:
+        return self.salary + self.overtime_pay
+
+    @property
+    def year_repr(self) -> str:
+        if self.is_fiscal_year:
+            return f"FY{self.year}"
+        return str(self.year)
+
 
 class Assignment(BaseModel, TrackUpdates):
     __tablename__ = "assignments"
@@ -407,6 +424,14 @@ class Assignment(BaseModel, TrackUpdates):
     def __repr__(self):
         return f"<Assignment: ID {self.officer_id} : {self.star_no}>"
 
+    @property
+    def start_date_or_min(self):
+        return self.start_date or date.min
+
+    @property
+    def start_date_or_max(self):
+        return self.start_date or date.max
+
 
 class Unit(BaseModel, TrackUpdates):
     __tablename__ = "unit_types"
@@ -738,6 +763,12 @@ class User(UserMixin, BaseModel):
         unique=False,
     )
 
+    def is_admin_or_coordinator(self, department: Optional[Department]) -> bool:
+        return self.is_administrator or (
+            department is not None
+            and (self.is_area_coordinator and self.ac_department_id == department.id)
+        )
+
     def _jwt_encode(self, payload, expiration):
         secret = current_app.config["SECRET_KEY"]
         header = {"alg": SIGNATURE_ALGORITHM}

OpenOversight/app/models/users.py

@@ -0,0 +1,8 @@
+from flask_login import AnonymousUserMixin
+
+from OpenOversight.app.models.database import Department
+
+
+class AnonymousUser(AnonymousUserMixin):
+    def is_admin_or_coordinator(self, department: Department) -> bool:
+        return False

OpenOversight/app/static/css/openoversight.css

@@ -659,3 +659,28 @@ tr:hover .row-actions {
     visibility: hidden;
     width: auto
 }
+
+.red {
+  color: #a00;
+}
+
+.no-box-shadow {
+  box-shadow: none;
+}
+
+.no-border-top {
+  border-top: none;
+}
+
+.no-display {
+  display: none;
+}
+
+.slightly-above {
+  position: relative;
+  top: -0.8em;
+}
+
+.bottom-margin {
+  margin-bottom: 2rem;
+}