From 731ff5a08c0d2a4a222e87d89629cc791c8e725f Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 17:39:02 -0400 Subject: [PATCH 01/32] Add meta block to base template --- OpenOversight/app/templates/base.html | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/OpenOversight/app/templates/base.html b/OpenOversight/app/templates/base.html index c9a238c8b..e718a69f5 100644 --- a/OpenOversight/app/templates/base.html +++ b/OpenOversight/app/templates/base.html @@ -5,10 +5,12 @@ - - + {% block meta %} + + + {% endblock %} - OpenOversight - a Lucy Parsons Labs project + {% block title %}OpenOversight - a Lucy Parsons Labs project{% endblock %} From 68fb05c3b72a41272d1bd6b15208a5b8dcd4b599 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 17:40:43 -0400 Subject: [PATCH 02/32] Add titles and meta descriptions to public templates --- OpenOversight/app/templates/about.html | 6 ++++++ OpenOversight/app/templates/auth/login.html | 3 ++- OpenOversight/app/templates/auth/register.html | 3 ++- OpenOversight/app/templates/auth/reset_password.html | 3 ++- OpenOversight/app/templates/browse.html | 2 ++ OpenOversight/app/templates/incident_detail.html | 11 +++++++++-- OpenOversight/app/templates/incident_list.html | 3 ++- OpenOversight/app/templates/input_find_officer.html | 3 ++- OpenOversight/app/templates/input_find_ooid.html | 2 ++ OpenOversight/app/templates/label_data.html | 4 ++++ OpenOversight/app/templates/list_officer.html | 3 ++- OpenOversight/app/templates/privacy.html | 2 ++ OpenOversight/app/templates/submit_image.html | 5 ++++- OpenOversight/app/templates/tagger_gallery.html | 2 ++ OpenOversight/app/templates/tutorial.html | 3 ++- 15 files changed, 45 insertions(+), 10 deletions(-) diff --git a/OpenOversight/app/templates/about.html b/OpenOversight/app/templates/about.html index 0aa43d107..dba9eb9f1 100644 --- a/OpenOversight/app/templates/about.html +++ b/OpenOversight/app/templates/about.html @@ -1,4 +1,10 @@ {% extends "base.html" %} +{% block title %}About OpenOversight{% endblock %} +{% block meta %} + +{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/auth/login.html b/OpenOversight/app/templates/auth/login.html index 97a7c0b89..911ce523a 100644 --- a/OpenOversight/app/templates/auth/login.html +++ b/OpenOversight/app/templates/auth/login.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} -{% block title %}OpenOversight - Volunteer Login{% endblock %} +{% block title %}Volunteer Login - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/auth/register.html b/OpenOversight/app/templates/auth/register.html index 249832f9f..572facc8a 100644 --- a/OpenOversight/app/templates/auth/register.html +++ b/OpenOversight/app/templates/auth/register.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} -{% block title %}OpenOversight - Volunteer Registration{% endblock %} +{% block title %}Volunteer Registration - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/auth/reset_password.html b/OpenOversight/app/templates/auth/reset_password.html index 42f15de58..6b325665d 100644 --- a/OpenOversight/app/templates/auth/reset_password.html +++ b/OpenOversight/app/templates/auth/reset_password.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} -{% block title %}OpenOversight - Volunteer Password Reset{% endblock %} +{% block title %}Volunteer Password Reset - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/browse.html b/OpenOversight/app/templates/browse.html index c655304d2..2a51d75b3 100644 --- a/OpenOversight/app/templates/browse.html +++ b/OpenOversight/app/templates/browse.html @@ -1,5 +1,7 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} +{% block title %}Browse OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/incident_detail.html b/OpenOversight/app/templates/incident_detail.html index c353d04c0..2a6dd46b1 100644 --- a/OpenOversight/app/templates/incident_detail.html +++ b/OpenOversight/app/templates/incident_detail.html @@ -1,7 +1,14 @@ {% extends 'base.html' %} - +{% set incident = obj %} +{% block title %}{{ incident.department.name }} incident{% if incident.report_number %} {{incident.report_number}}{% endif %} - OpenOversight{% endblock %} +{% block meta %} +{% if incident.description is not None %} + +{% else %} + +{% endif %} +{% endblock %} {% block content %} - {% set incident = obj %}
All Incidents {% if incident.department %} diff --git a/OpenOversight/app/templates/incident_list.html b/OpenOversight/app/templates/incident_list.html index 518e48dbb..3d0225456 100644 --- a/OpenOversight/app/templates/incident_list.html +++ b/OpenOversight/app/templates/incident_list.html @@ -1,5 +1,6 @@ {% extends "list.html" %} - +{% block title %}View all incidents for {{ incident.department.name }} - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block list %}

Incidents

{% if department %} diff --git a/OpenOversight/app/templates/input_find_officer.html b/OpenOversight/app/templates/input_find_officer.html index 5793aae0c..c466ba0c3 100644 --- a/OpenOversight/app/templates/input_find_officer.html +++ b/OpenOversight/app/templates/input_find_officer.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% block content %} - +{% block title %}Find an officer - OpenOversight{% endblock %} +{% block meta %}{% endblock %}
Find an Officer
diff --git a/OpenOversight/app/templates/input_find_ooid.html b/OpenOversight/app/templates/input_find_ooid.html index 43e491209..4548e30f4 100644 --- a/OpenOversight/app/templates/input_find_ooid.html +++ b/OpenOversight/app/templates/input_find_ooid.html @@ -1,4 +1,6 @@ {% extends "base.html" %} +{% block title %}Lookup an Officer's OpenOversight ID{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/label_data.html b/OpenOversight/app/templates/label_data.html index d35f902dd..f66b965fc 100644 --- a/OpenOversight/app/templates/label_data.html +++ b/OpenOversight/app/templates/label_data.html @@ -1,5 +1,9 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} +{% block title %}Volunteer with OpenOversight{% endblock %} +{% block meta %} + +{% endblock %} {% block content %} {% if current_user and current_user.is_authenticated %} diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index 2a4bef593..233cd03d0 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -1,6 +1,7 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %} - +{% block title %}Browse {{ department.name|title }} officers - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}

{{ department.name|title }} Officers

diff --git a/OpenOversight/app/templates/privacy.html b/OpenOversight/app/templates/privacy.html index 7914789d1..cdea90936 100644 --- a/OpenOversight/app/templates/privacy.html +++ b/OpenOversight/app/templates/privacy.html @@ -1,4 +1,6 @@ {% extends "base.html" %} +{% block title %}Privacy Policy - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/submit_image.html b/OpenOversight/app/templates/submit_image.html index 31faafcc7..91e23955f 100644 --- a/OpenOversight/app/templates/submit_image.html +++ b/OpenOversight/app/templates/submit_image.html @@ -1,6 +1,9 @@ {% extends "base.html" %} {% import "bootstrap/wtf.html" as wtf %}¬ - +{% block title %}Submit images to OpenOversight{% endblock %} +{% block meta %} + +{% endblock %} {% block head %} diff --git a/OpenOversight/app/templates/tagger_gallery.html b/OpenOversight/app/templates/tagger_gallery.html index 580350695..931ed522b 100644 --- a/OpenOversight/app/templates/tagger_gallery.html +++ b/OpenOversight/app/templates/tagger_gallery.html @@ -1,4 +1,6 @@ {% extends "base.html" %} +{% block title %}Search results - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/tutorial.html b/OpenOversight/app/templates/tutorial.html index da0c9d8de..94b1e0012 100644 --- a/OpenOversight/app/templates/tutorial.html +++ b/OpenOversight/app/templates/tutorial.html @@ -1,5 +1,6 @@ {% extends "base.html" %} - +{% block title %}Tutorial - OpenOversight{% endblock %} +{% block meta %}{% endblock %} {% block content %}
From 79d746d8f705c84c6afdbbc6c1645b6e12a415cb Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 18:49:56 -0400 Subject: [PATCH 03/32] Add structured data to officer profiles: schema.org, Open Graph, Twitter Card, Google Breadcrumbs --- OpenOversight/app/main/views.py | 6 +- OpenOversight/app/templates/officer.html | 78 ++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index c67244e09..a89fc3a1c 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -188,7 +188,11 @@ def officer_profile(officer_id): ' '.join([str(exception_type), str(value), format_exc()]) )) - + if faces: + officer.image_url = url_for('static', filename=faces[0].image.filepath.replace('/static/',''), _external=True) + if faces[0].face_width and faces[0].face_height: + officer.image_width = faces[0].face_width + officer.image_height = faces[0].face_height return render_template('officer.html', officer=officer, paths=face_paths, faces=faces, assignments=assignments, form=form) diff --git a/OpenOversight/app/templates/officer.html b/OpenOversight/app/templates/officer.html index 2ec81b83c..8ff1ab386 100644 --- a/OpenOversight/app/templates/officer.html +++ b/OpenOversight/app/templates/officer.html @@ -1,4 +1,82 @@ {% extends "base.html" %} +{% block title %}{{ officer.full_name() }} - OpenOversight{% endblock %} +{% block meta %} + {% set job_title = officer.assignments[0].job.job_title if officer.assignments[0] and officer.assignments[0].job.job_title != 'Not Sure' else 'Employee' %} + {% set description = 'See detailed information about ' ~ officer.full_name() ~ ', ' ~ job_title ~ ' of the ' ~ officer.department.name ~ '.' %} + {% set image_url = officer.image_url | default(url_for('static', filename='images/placeholder.png', _external=True)) %} + + + + + + + + + + + {% if officer.image_width and officer.image_height %} + + + {% elif not officer.image_url %} + + + {% endif %} + + + + + + + + + + +{% endblock %} {% block content %}
From 120ebfac968d76b6ffd9e2d00b0b4c3a96faed45 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 18:50:24 -0400 Subject: [PATCH 04/32] Update structured data for incident templates --- .../app/templates/incident_detail.html | 22 +++++++++++++++++++ .../app/templates/incident_list.html | 16 ++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/OpenOversight/app/templates/incident_detail.html b/OpenOversight/app/templates/incident_detail.html index 2a6dd46b1..bafbc9332 100644 --- a/OpenOversight/app/templates/incident_detail.html +++ b/OpenOversight/app/templates/incident_detail.html @@ -7,6 +7,28 @@ {% else %} {% endif %} + + {% endblock %} {% block content %}
diff --git a/OpenOversight/app/templates/incident_list.html b/OpenOversight/app/templates/incident_list.html index 3d0225456..fb7f0776d 100644 --- a/OpenOversight/app/templates/incident_list.html +++ b/OpenOversight/app/templates/incident_list.html @@ -1,6 +1,18 @@ {% extends "list.html" %} -{% block title %}View all incidents for {{ incident.department.name }} - OpenOversight{% endblock %} -{% block meta %}{% endblock %} +{% block title -%} + {%- if objects.items|length > 0 -%} + View all incidents for {{ objects.items[0].department.name }} - OpenOversight + {%- else -%} + No Incidents Found + {%- endif -%} +{% endblock %} +{% block meta %} + {% if objects.items|length > 0 %} + + {% else %} + + {% endif %} +{% endblock %} {% block list %}

Incidents

{% if department %} From eb88463db085a9410bc669cb6cd0938d24d5f7c1 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 18:53:13 -0400 Subject: [PATCH 05/32] flake8 fix --- OpenOversight/app/main/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index a89fc3a1c..86be519ae 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -189,7 +189,7 @@ def officer_profile(officer_id): format_exc()]) )) if faces: - officer.image_url = url_for('static', filename=faces[0].image.filepath.replace('/static/',''), _external=True) + officer.image_url = url_for('static', filename=faces[0].image.filepath.replace('/static/', ''), _external=True) if faces[0].face_width and faces[0].face_height: officer.image_width = faces[0].face_width officer.image_height = faces[0].face_height From 42f0a64e4b8dba5f053c3585485ca64c6f993cb5 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 19:11:30 -0400 Subject: [PATCH 06/32] jinjaaaaaaaaaaaaaa fix --- OpenOversight/app/templates/incident_detail.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenOversight/app/templates/incident_detail.html b/OpenOversight/app/templates/incident_detail.html index bafbc9332..d4bcc40ee 100644 --- a/OpenOversight/app/templates/incident_detail.html +++ b/OpenOversight/app/templates/incident_detail.html @@ -2,7 +2,7 @@ {% set incident = obj %} {% block title %}{{ incident.department.name }} incident{% if incident.report_number %} {{incident.report_number}}{% endif %} - OpenOversight{% endblock %} {% block meta %} -{% if incident.description is not None %} +{% if incident.description != None %} {% else %} From 5c3284986ca22e77c4cd09ca720138fc82b4d6f6 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 21:59:10 -0400 Subject: [PATCH 07/32] Differentiate between local and S3-hosted images for officer structured data --- OpenOversight/app/main/views.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 86be519ae..cfd55caa7 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -189,7 +189,9 @@ def officer_profile(officer_id): format_exc()]) )) if faces: - officer.image_url = url_for('static', filename=faces[0].image.filepath.replace('/static/', ''), _external=True) + officer.image_url = faces[0].image.filepath + if not officer.image_url.startswith('http'): + officer.image_url = url_for('static', filename=faces[0].image.filepath.replace('/static/', ''), _external=True) if faces[0].face_width and faces[0].face_height: officer.image_width = faces[0].face_width officer.image_height = faces[0].face_height From 1db45bb62237e963e633a960b2f86cd044bee424 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 22:06:12 -0400 Subject: [PATCH 08/32] Remove stray comma in officer JSON-LD --- OpenOversight/app/templates/officer.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OpenOversight/app/templates/officer.html b/OpenOversight/app/templates/officer.html index 8ff1ab386..adb6ad304 100644 --- a/OpenOversight/app/templates/officer.html +++ b/OpenOversight/app/templates/officer.html @@ -51,7 +51,7 @@ "@type": "URL", "url": "{{ url_for(request.endpoint, officer_id=officer.id, _external=True) }}" }, - "description": "{{ description }}", + "description": "{{ description }}" } From 7abac1c195519a8369fcfdaf626e083a448f1eaf Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 14 Aug 2020 22:35:18 -0400 Subject: [PATCH 09/32] Fix officer full name display when middle_initial longer than 1 char --- OpenOversight/app/models.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index 39ff84217..643b19084 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -117,10 +117,11 @@ class Officer(db.Model): def full_name(self): if self.middle_initial: + middle_initial = self.middle_initial + '.' if len(self.middle_initial) == 1 else self.middle_initial if self.suffix: - return '{} {}. {} {}'.format(self.first_name, self.middle_initial, self.last_name, self.suffix) + return '{} {}. {} {}'.format(self.first_name, middle_initial, self.last_name, self.suffix) else: - return '{} {}. {}'.format(self.first_name, self.middle_initial, self.last_name) + return '{} {}. {}'.format(self.first_name, middle_initial, self.last_name) if self.suffix: return '{} {} {}'.format(self.first_name, self.last_name, self.suffix) return '{} {}'.format(self.first_name, self.last_name) From d5b4d0ea8d1ee9b6c9bef15f1352ec79c0e005da Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 18 Aug 2020 20:31:37 -0400 Subject: [PATCH 10/32] Fix double periods in middle initials --- OpenOversight/app/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index 643b19084..f7cfe664b 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -119,9 +119,9 @@ def full_name(self): if self.middle_initial: middle_initial = self.middle_initial + '.' if len(self.middle_initial) == 1 else self.middle_initial if self.suffix: - return '{} {}. {} {}'.format(self.first_name, middle_initial, self.last_name, self.suffix) + return '{} {} {} {}'.format(self.first_name, middle_initial, self.last_name, self.suffix) else: - return '{} {}. {}'.format(self.first_name, middle_initial, self.last_name) + return '{} {} {}'.format(self.first_name, middle_initial, self.last_name) if self.suffix: return '{} {} {}'.format(self.first_name, self.last_name, self.suffix) return '{} {}'.format(self.first_name, self.last_name) From 2cec57990eb6e8d9e56019009082a2389b9df2fe Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 25 Sep 2020 17:19:49 -0400 Subject: [PATCH 11/32] Allow multiple comma-separated values for UII and badge number form fields in browse view --- OpenOversight/app/utils.py | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 6809df242..64ea2b61a 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -281,18 +281,32 @@ def filter_by_form(form, officer_query, department_id=None): Officer.department_id == department_id ) if form.get('badge'): - officer_query = officer_query.filter( - subq.c.assignments_star_no.like('%%{}%%'.format(form['badge'])) - ) + if ',' in form['badge']: + or_clauses = [ + subq.c.assignments_star_no.ilike('%%{}%%'.format(star_no.strip())) + for star_no in form['badge'].split(',') + ] + officer_query = officer_query.filter(or_(*or_clauses)) + else: + officer_query = officer_query.filter( + subq.c.assignments_star_no.like('%%{}%%'.format(form['badge'])) + ) if form.get('unit'): officer_query = officer_query.filter( subq.c.assignments_unit_id == form['unit'] ) if form.get('unique_internal_identifier'): - officer_query = officer_query.filter( - Officer.unique_internal_identifier.ilike('%%{}%%'.format(form['unique_internal_identifier'])) - ) + if ',' in form['unique_internal_identifier']: + or_clauses = [ + Officer.unique_internal_identifier.ilike('%%{}%%'.format(uii.strip())) + for uii in form['unique_internal_identifier'].split(',') + ] + officer_query = officer_query.filter(or_(*or_clauses)) + else: + officer_query = officer_query.filter( + Officer.unique_internal_identifier.ilike('%%{}%%'.format(form['unique_internal_identifier'])) + ) race_values = [x for x, _ in RACE_CHOICES] if form.get('race') and all(race in race_values for race in form['race']): if 'Not Sure' in form['race']: From 67adfd90b4ae8669e5a83faeac42c5e737cf6d72 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 25 Sep 2020 17:48:51 -0400 Subject: [PATCH 12/32] Move logic from list_officers.html template to view; Remove officer_name.html partial; Allow filtering by both first name and last name --- OpenOversight/app/main/forms.py | 8 ++- OpenOversight/app/main/views.py | 31 +++++++--- OpenOversight/app/models.py | 19 +++++- OpenOversight/app/templates/list_officer.html | 59 +++++++------------ OpenOversight/app/templates/officer.html | 6 +- .../app/templates/partials/officer_name.html | 4 -- .../app/templates/tagger_gallery.html | 2 +- OpenOversight/app/utils.py | 8 ++- 8 files changed, 78 insertions(+), 59 deletions(-) delete mode 100644 OpenOversight/app/templates/partials/officer_name.html diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index a017b2367..3f964843c 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -37,8 +37,12 @@ class HumintContribution(Form): class FindOfficerForm(Form): - name = StringField( - 'name', default='', validators=[Regexp(r'\w*'), Length(max=50), + last_name = StringField( + 'last_name', default='', validators=[Regexp(r'\w*'), Length(max=50), + Optional()] + ) + first_name = StringField( + 'first_name', default='', validators=[Regexp(r'\w*'), Length(max=50), Optional()] ) badge = StringField('badge', default='', validators=[Regexp(r'\w*'), diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index f4637b65f..38479698d 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -94,7 +94,8 @@ def get_officer(): unit=form.data['unit'] if form.data['unit'] != 'Not Sure' else None, min_age=form.data['min_age'], max_age=form.data['max_age'], - name=form.data['name'], + last_name=form.data['last_name'], + first_name=form.data['first_name'], badge=form.data['badge'], unique_internal_identifier=form.data['unique_internal_identifier']), code=302) @@ -492,8 +493,8 @@ def edit_department(department_id): @main.route('/department/') -def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16', max_age='100', name=None, - badge=None, unique_internal_identifier=None, unit=None): +def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16', max_age='100', last_name=None, + first_name=None, badge=None, unique_internal_identifier=None, unit=None): form = BrowseForm() form.rank.query = Job.query.filter_by(department_id=department_id, is_sworn_officer=True).order_by(Job.order.asc()).all() form_data = form.data @@ -502,7 +503,8 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 form_data['rank'] = rank form_data['min_age'] = min_age form_data['max_age'] = max_age - form_data['name'] = name + form_data['last_name'] = last_name + form_data['first_name'] = first_name form_data['badge'] = badge form_data['unit'] = unit form_data['unique_internal_identifier'] = unique_internal_identifier @@ -519,8 +521,10 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 form_data['max_age'] = request.args.get('max_age') if request.args.get('page'): page = int(request.args.get('page')) - if request.args.get('name'): - form_data['name'] = request.args.get('name') + if request.args.get('last_name'): + form_data['last_name'] = request.args.get('last_name') + if request.args.get('first_name'): + form_data['first_name'] = request.args.get('first_name') if request.args.get('badge'): form_data['badge'] = request.args.get('badge') if request.args.get('unit') and request.args.get('unit') != 'Not Sure': @@ -550,13 +554,26 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 'unit': [('Not Sure', 'Not Sure')] + unit_choices } + next_url=url_for('main.list_officer', department_id=department.id, + page=officers.next_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], + first_name=form_data['first_name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + prev_url=url_for('main.list_officer', department_id=department.id, + page=officers.prev_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], + first_name=form_data['first_name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + return render_template( 'list_officer.html', form=form, department=department, officers=officers, form_data=form_data, - choices=choices) + choices=choices, + next_url=next_url, + prev_url=prev_url) @main.route('/department//ranks') diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index 39ff84217..6f534eba2 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -117,10 +117,11 @@ class Officer(db.Model): def full_name(self): if self.middle_initial: + middle_initial = self.middle_initial + '.' if len(self.middle_initial) == 1 else self.middle_initial if self.suffix: - return '{} {}. {} {}'.format(self.first_name, self.middle_initial, self.last_name, self.suffix) + return '{} {} {} {}'.format(self.first_name, middle_initial, self.last_name, self.suffix) else: - return '{} {}. {}'.format(self.first_name, self.middle_initial, self.last_name) + return '{} {} {}'.format(self.first_name, middle_initial, self.last_name) if self.suffix: return '{} {} {}'.format(self.first_name, self.last_name, self.suffix) return '{} {}'.format(self.first_name, self.last_name) @@ -137,6 +138,20 @@ def gender_label(self): if self.gender == gender: return label + def job_title(self): + if self.assignments.all(): + return self.assignments\ + .order_by(self.assignments[0].__table__.c.star_date.desc())\ + .first()\ + .job.job_title + + def badge_number(self): + if self.assignments.all(): + return self.assignments\ + .order_by(self.assignments[0].__table__.c.star_date.desc())\ + .first()\ + .star_no + def __repr__(self): if self.unique_internal_identifier: return ''.format(self.id, diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index 6745d5ba1..b4670335e 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -9,13 +9,25 @@

{{ department.name|title }} Officers

-
+

Last name

-
+
- + +
+
+
+
+
+
+

First name

+
+
+
+
+
@@ -131,46 +143,28 @@

Age range

- {% with paginate=officers, - next_url=url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - prev_url=url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - location='top' %} + {% with paginate=officers, location='top' %} {% include "partials/paginate_nav.html" %} {% endwith %}
    {% for officer in officers.items %} - {% if officer.assignments.all() %} - {% set assignment = officer.assignments.order_by(officer.assignments[0].__table__.c.star_date.desc()).first() %} - {% if assignment is not none and assignment.star_no is not none %} - {% set star_no = "#" + assignment.star_no %} - {% set job_title = assignment.job.job_title %} - {% endif %} - {% endif %}
  • - {% include "partials/officer_name.html" %} - {{ star_no }} + {{ officer.full_name() }} + {{ officer.badge_number()|default('') }}

    Job Title
    -
    {{ job_title|default('Unknown') }}
    +
    {{ officer.job_title()|default('Unknown') }}
    Race
    {{ officer.race_label()|default('Unknown')|lower|title }}
    @@ -189,18 +183,7 @@

  • {% endfor %}
- {% with paginate=officers, - next_url=url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - prev_url=url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - location='bottom' %} + {% with paginate=officers, location='bottom' %} {% include "partials/paginate_nav.html" %} {% endwith %}
diff --git a/OpenOversight/app/templates/officer.html b/OpenOversight/app/templates/officer.html index 2ec81b83c..9d029ca3b 100644 --- a/OpenOversight/app/templates/officer.html +++ b/OpenOversight/app/templates/officer.html @@ -6,9 +6,9 @@
@@ -40,7 +40,7 @@

General Information Name - {% include 'partials/officer_name.html' %} + {{ officer.full_name() }} OpenOversight ID diff --git a/OpenOversight/app/templates/partials/officer_name.html b/OpenOversight/app/templates/partials/officer_name.html deleted file mode 100644 index 2b57058a4..000000000 --- a/OpenOversight/app/templates/partials/officer_name.html +++ /dev/null @@ -1,4 +0,0 @@ -{{ officer.first_name.lower()|title }} -{% if officer.middle_initial %}{{ officer.middle_initial }}{% if officer.middle_initial|length == 1 %}.{% endif %} {% endif %} -{{ officer.last_name|capfirst }} -{% if officer.suffix %}{{ officer.suffix }}. {% endif %} diff --git a/OpenOversight/app/templates/tagger_gallery.html b/OpenOversight/app/templates/tagger_gallery.html index 580350695..990762481 100644 --- a/OpenOversight/app/templates/tagger_gallery.html +++ b/OpenOversight/app/templates/tagger_gallery.html @@ -36,7 +36,7 @@

Matching Officers

+
+
+

Photo

+
+
+
+
+ + +
+
+
+
{{ wtf.form_field(form.submit, id="submit", button_map={'submit':'primary'}) }}
diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 3ab51847a..53ad21e6c 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -337,6 +337,17 @@ def filter_by_form(form, officer_query, department_id=None): form['rank'].append(None) officer_query = officer_query.filter(Job.job_title.in_(form['rank'])) + if form.get('photo') and all(photo in ['0', '1'] for photo in form['photo']): + face_officer_ids = set([face.officer_id for face in Face.query.all()]) + if '0' in form['photo'] and '1' not in form['photo']: + officer_query = officer_query.filter( + Officer.id.notin_(face_officer_ids) + ) + elif '1' in form['photo'] and '0' not in form['photo']: + officer_query = officer_query.filter( + Officer.id.in_(face_officer_ids) + ) + return officer_query From 36fe52f69fec1f1601ad24fb069de954bbcf7c5d Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Fri, 25 Sep 2020 18:09:07 -0400 Subject: [PATCH 14/32] flake8 fixes --- OpenOversight/app/main/forms.py | 4 ++-- OpenOversight/app/main/views.py | 20 ++++++++++---------- OpenOversight/app/utils.py | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index 4ae5d4b5c..e29e3f49b 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -39,11 +39,11 @@ class HumintContribution(Form): class FindOfficerForm(Form): last_name = StringField( 'last_name', default='', validators=[Regexp(r'\w*'), Length(max=50), - Optional()] + Optional()] ) first_name = StringField( 'first_name', default='', validators=[Regexp(r'\w*'), Length(max=50), - Optional()] + Optional()] ) badge = StringField('badge', default='', validators=[Regexp(r'\w*'), Length(max=10)]) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 408018f38..a12c9273a 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -557,16 +557,16 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 'unit': [('Not Sure', 'Not Sure')] + unit_choices } - next_url=url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], - min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], - first_name=form_data['first_name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) - prev_url=url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], - min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], - first_name=form_data['first_name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + next_url = url_for('main.list_officer', department_id=department.id, + page=officers.next_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], + first_name=form_data['first_name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + prev_url = url_for('main.list_officer', department_id=department.id, + page=officers.prev_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], + first_name=form_data['first_name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) return render_template( 'list_officer.html', diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 53ad21e6c..b5b493ac2 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -15,7 +15,7 @@ import sys from traceback import format_exc -from sqlalchemy import func +from sqlalchemy import func, or_ from sqlalchemy.sql.expression import cast import imghdr as imghdr from flask import current_app, url_for From f6d4bdee1f47aa5bc90d43716055f26f33fdaca6 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sat, 26 Sep 2020 14:21:19 -0400 Subject: [PATCH 15/32] Add job_title() and badge_number() methods to Officer class; move logic from list_officers.html -> view; remove officer_name.html partial template --- OpenOversight/app/main/views.py | 13 +++++- OpenOversight/app/models.py | 14 +++++++ OpenOversight/app/templates/list_officer.html | 41 +++---------------- OpenOversight/app/templates/officer.html | 6 +-- .../app/templates/partials/officer_name.html | 4 -- .../app/templates/tagger_gallery.html | 2 +- 6 files changed, 36 insertions(+), 44 deletions(-) delete mode 100644 OpenOversight/app/templates/partials/officer_name.html diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 463b8ec3e..a4cf7d30a 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -556,13 +556,24 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 'unit': [('Not Sure', 'Not Sure')] + unit_choices } + next_url = url_for('main.list_officer', department_id=department.id, + page=officers.next_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + prev_url = url_for('main.list_officer', department_id=department.id, + page=officers.prev_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + min_age=form_data['min_age'], max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + return render_template( 'list_officer.html', form=form, department=department, officers=officers, form_data=form_data, - choices=choices) + choices=choices, + next_url=next_url, + prev_url=prev_url) @main.route('/department//ranks') diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index 5df52a0ad..28dad066f 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -138,6 +138,20 @@ def gender_label(self): if self.gender == gender: return label + def job_title(self): + if self.assignments.all(): + return self.assignments\ + .order_by(self.assignments[0].__table__.c.star_date.desc())\ + .first()\ + .job.job_title + + def badge_number(self): + if self.assignments.all(): + return self.assignments\ + .order_by(self.assignments[0].__table__.c.star_date.desc())\ + .first()\ + .star_no + def __repr__(self): if self.unique_internal_identifier: return ''.format(self.id, diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index 70256d351..f362e62aa 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -132,46 +132,28 @@

Age range

- {% with paginate=officers, - next_url=url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - prev_url=url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - location='top' %} + {% with paginate=officers, location='top' %} {% include "partials/paginate_nav.html" %} {% endwith %}
    {% for officer in officers.items %} - {% if officer.assignments.all() %} - {% set assignment = officer.assignments.order_by(officer.assignments[0].__table__.c.star_date.desc()).first() %} - {% if assignment is not none and assignment.star_no is not none %} - {% set star_no = "#" + assignment.star_no %} - {% set job_title = assignment.job.job_title %} - {% endif %} - {% endif %}
  • - {% include "partials/officer_name.html" %} - {{ star_no }} + {{ officer.full_name() }} + {{ officer.badge_number()|default('') }}

    Job Title
    -
    {{ job_title|default('Unknown') }}
    +
    {{ officer.job_title()|default('Unknown') }}
    Race
    {{ officer.race_label()|default('Unknown')|lower|title }}
    @@ -190,18 +172,7 @@

  • {% endfor %}
- {% with paginate=officers, - next_url=url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - prev_url=url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], - gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], - max_age=form_data['max_age'], name=form_data['name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), - location='bottom' %} + {% with paginate=officers, location='bottom' %} {% include "partials/paginate_nav.html" %} {% endwith %}
diff --git a/OpenOversight/app/templates/officer.html b/OpenOversight/app/templates/officer.html index adb6ad304..cfe08a0d5 100644 --- a/OpenOversight/app/templates/officer.html +++ b/OpenOversight/app/templates/officer.html @@ -84,9 +84,9 @@
@@ -118,7 +118,7 @@

General Information Name - {% include 'partials/officer_name.html' %} + {{ officer.full_name() }} OpenOversight ID diff --git a/OpenOversight/app/templates/partials/officer_name.html b/OpenOversight/app/templates/partials/officer_name.html deleted file mode 100644 index 2b57058a4..000000000 --- a/OpenOversight/app/templates/partials/officer_name.html +++ /dev/null @@ -1,4 +0,0 @@ -{{ officer.first_name.lower()|title }} -{% if officer.middle_initial %}{{ officer.middle_initial }}{% if officer.middle_initial|length == 1 %}.{% endif %} {% endif %} -{{ officer.last_name|capfirst }} -{% if officer.suffix %}{{ officer.suffix }}. {% endif %} diff --git a/OpenOversight/app/templates/tagger_gallery.html b/OpenOversight/app/templates/tagger_gallery.html index 931ed522b..899433add 100644 --- a/OpenOversight/app/templates/tagger_gallery.html +++ b/OpenOversight/app/templates/tagger_gallery.html @@ -38,7 +38,7 @@

Matching Officers

+
+
+

Total pay

+
+
+
+
+
+
+
+
+
+
+
+ $ + +
+
+
+
+ $ + +
+
+
+
+
+

Unit

diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index c94c8ebcf..460fd303a 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -251,18 +251,27 @@ def upload_obj_to_s3(file_obj, dest_filename): def filter_by_form(form, officer_query, department_id=None): - # Some SQL acrobatics to left join only the most recent assignment per officer - row_num_col = func.row_number().over( + # Some SQL acrobatics to left join only the most recent assignment and salary per officer + assignment_row_num_col = func.row_number().over( partition_by=Assignment.officer_id, order_by=Assignment.star_date.desc() - ).label('row_num') - subq = db.session.query( + ).label('assignment_row_num') + assignment_subq = db.session.query( Assignment.officer_id, Assignment.job_id, Assignment.star_date, Assignment.star_no, Assignment.unit_id - ).add_columns(row_num_col).from_self().filter(row_num_col == 1).subquery() - officer_query = officer_query.outerjoin(subq) + ).add_columns(assignment_row_num_col).from_self().filter(assignment_row_num_col == 1).subquery() + salary_row_num_col = func.row_number().over( + partition_by=Salary.officer_id, order_by=Salary.year.desc() + ).label('salary_row_num') + salary_subq = db.session.query( + Salary.officer_id, + Salary.salary, + Salary.overtime_pay, + Salary.year, + ).add_columns(salary_row_num_col).from_self().filter(salary_row_num_col == 1).subquery() + officer_query = officer_query.outerjoin(assignment_subq).outerjoin(salary_subq) if form.get('last_name'): officer_query = officer_query.filter( @@ -278,37 +287,28 @@ def filter_by_form(form, officer_query, department_id=None): Officer.department_id == department_id ) if form.get('badge'): - if ',' in form['badge']: - or_clauses = [ - subq.c.assignments_star_no.ilike('%%{}%%'.format(star_no.strip())) - for star_no in form['badge'].split(',') - ] - officer_query = officer_query.filter(or_(*or_clauses)) - else: - officer_query = officer_query.filter( - subq.c.assignments_star_no.like('%%{}%%'.format(form['badge'])) - ) + or_clauses = [ + assignment_subq.c.assignments_star_no.ilike('%%{}%%'.format(star_no.strip())) + for star_no in form['badge'].split(',') + ] + officer_query = officer_query.filter(or_(*or_clauses)) if form.get('unit'): officer_query = officer_query.filter( - subq.c.assignments_unit_id == form['unit'] + assignment_subq.c.assignments_unit_id == form['unit'] ) - if form.get('unique_internal_identifier'): - if ',' in form['unique_internal_identifier']: - or_clauses = [ - Officer.unique_internal_identifier.ilike('%%{}%%'.format(uii.strip())) - for uii in form['unique_internal_identifier'].split(',') - ] - officer_query = officer_query.filter(or_(*or_clauses)) - else: - officer_query = officer_query.filter( - Officer.unique_internal_identifier.ilike('%%{}%%'.format(form['unique_internal_identifier'])) - ) + or_clauses = [ + Officer.unique_internal_identifier.ilike('%%{}%%'.format(uii.strip())) + for uii in form['unique_internal_identifier'].split(',') + ] + officer_query = officer_query.filter(or_(*or_clauses)) + race_values = [x for x, _ in RACE_CHOICES] if form.get('race') and all(race in race_values for race in form['race']): if 'Not Sure' in form['race']: form['race'].append(None) officer_query = officer_query.filter(Officer.race.in_(form['race'])) + gender_values = [x for x, _ in GENDER_CHOICES] if form.get('gender') and all(gender in gender_values for gender in form['gender']): if 'Not Sure' in form['gender']: @@ -341,6 +341,23 @@ def filter_by_form(form, officer_query, department_id=None): Officer.id.in_(face_officer_ids) ) + officer_query = officer_query.outerjoin(Officer.salaries) + if form.get('max_pay') and form.get('min_pay') and float(form['max_pay']) > float(form['min_pay']): + officer_query = officer_query.filter( + db.and_( + salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay >= float(form['min_pay']), + salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay <= float(form['max_pay']) + ) + ) + elif form.get('min_pay') and float(form['min_pay']) > 0 and not form.get('max_pay'): + officer_query = officer_query.filter( + salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay >= float(form['min_pay']) + ) + elif form.get('max_pay') and float(form['max_pay']) > 0 and not form.get('min_pay'): + officer_query = officer_query.filter( + salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay <= float(form['max_pay']) + ) + return officer_query From 62a4b079f4530b146586f54630bd0172ac353585 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 15:58:47 -0400 Subject: [PATCH 22/32] Added dropdown menu for sorting list_officers view by: last name, rank, total pay, salary, overtime --- OpenOversight/app/main/forms.py | 5 +- OpenOversight/app/main/views.py | 28 ++--- OpenOversight/app/models.py | 4 +- .../app/static/css/openoversight.css | 10 ++ OpenOversight/app/templates/list_officer.html | 113 +++++++++++------- .../app/templates/partials/paginate.html | 20 ---- OpenOversight/app/utils.py | 14 ++- .../b4145ba7d4c6_overtime_pay_default_0.py | 33 +++++ OpenOversight/tests/test_commands.py | 2 +- 9 files changed, 143 insertions(+), 86 deletions(-) delete mode 100644 OpenOversight/app/templates/partials/paginate.html create mode 100644 OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index 8e9ac0b60..e553504c8 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -23,9 +23,8 @@ def allowed_values(choices, empty_allowed=True): def validate_money(form, field): - test_str = field if isinstance(field, str) else str(field.data) - if not re.fullmatch(r'\d+(\.\d\d)?0*', test_str): - raise ValidationError('Invalid monetary value ' + test_str) + if not re.fullmatch(r'\d+(\.\d\d)?0*', str(field.data)): + raise ValidationError('Invalid monetary value') class HumintContribution(Form): diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 8d39a3f74..356506999 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -11,7 +11,6 @@ from flask import (abort, render_template, request, redirect, url_for, flash, current_app, jsonify, Response, Markup) from flask_login import current_user, login_required, login_user -from wtforms.validators import ValidationError from . import main from .. import limiter, sitemap @@ -28,7 +27,7 @@ FaceTag, AssignmentForm, DepartmentForm, AddOfficerForm, EditOfficerForm, IncidentForm, TextForm, EditTextForm, AddImageForm, EditDepartmentForm, BrowseForm, SalaryForm, - OfficerLinkForm, validate_money) + OfficerLinkForm) from .model_view import ModelView from .choices import GENDER_CHOICES, RACE_CHOICES, AGE_CHOICES from ..models import (db, Image, User, Face, Officer, Assignment, Department, @@ -499,8 +498,8 @@ def edit_department(department_id): @main.route('/department/') -def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16', max_age='100', last_name=None, - first_name=None, badge=None, unique_internal_identifier=None, unit=None, photo=[], min_pay=0, max_pay=None): +def list_officer(department_id, page=1, order=0, race=[], gender=[], rank=[], min_age='16', max_age='100', last_name=None, + first_name=None, badge=None, unique_internal_identifier=None, unit=None, photo=[], min_pay=None, max_pay=None): form = BrowseForm() form.rank.query = Job.query.filter_by(department_id=department_id, is_sworn_officer=True).order_by(Job.order.asc()).all() form_data = form.data @@ -530,6 +529,9 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 form_data['max_age'] = request.args.get('max_age') if request.args.get('page'): page = int(request.args.get('page')) + if request.args.get('order'): + order = int(request.args.get('order')) + form_data['order'] = order if request.args.get('last_name'): form_data['last_name'] = request.args.get('last_name') if request.args.get('first_name'): @@ -546,25 +548,17 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 form_data['gender'] = request.args.getlist('gender') if request.args.get('photo') and all(photo in ['0', '1'] for photo in request.args.getlist('photo')): form_data['photo'] = request.args.getlist('photo') - if request.args.get('min_pay'): + if request.args.get('min_pay') and re.fullmatch(r'\d+(\.\d\d)?', request.args.get('min_pay')): form_data['min_pay'] = request.args.get('min_pay') - if request.args.get('max_pay'): + if request.args.get('max_pay') and re.fullmatch(r'\d+(\.\d\d)?', request.args.get('max_pay')): form_data['max_pay'] = request.args.get('max_pay') - try: - if form_data['min_pay']: - validate_money(None, form_data['min_pay']) - if form_data['max_pay']: - validate_money(None, form_data['max_pay']) - except ValidationError: - abort(400) - unit_choices = [(unit.id, unit.descrip) for unit in Unit.query.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, is_sworn_officer=True).order_by(Job.order).all()] if request.args.get('rank') and all(rank in rank_choices for rank in request.args.getlist('rank')): form_data['rank'] = request.args.getlist('rank') - officers = filter_by_form(form_data, Officer.query, department_id).filter(Officer.department_id == department_id).order_by(Officer.last_name, Officer.first_name, Officer.id) + officers = filter_by_form(form_data, Officer.query, department_id, order).filter(Officer.department_id == department_id) officers = officers.paginate(page, OFFICERS_PER_PAGE, False) for officer in officers.items: officer_face = officer.face.order_by(Face.featured.desc()).first() @@ -579,12 +573,12 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 } next_url = url_for('main.list_officer', department_id=department.id, - page=officers.next_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + page=officers.next_num, order=order, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], first_name=form_data['first_name'], badge=form_data['badge'], unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) prev_url = url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], + page=officers.prev_num, order=order, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], first_name=form_data['first_name'], badge=form_data['badge'], unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index f03605988..d8106d319 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -176,12 +176,12 @@ class Salary(BaseModel): officer_id = db.Column(db.Integer, db.ForeignKey('officers.id', ondelete='CASCADE')) 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) + overtime_pay = db.Column(db.Numeric, index=True, unique=False, nullable=False, server_default='0') year = db.Column(db.Integer, index=True, unique=False, nullable=False) is_fiscal_year = db.Column(db.Boolean, index=False, unique=False, nullable=False) def __repr__(self): - return ''.format(self.officer_id, self.salary, self.overtime_pay, self.year) class Assignment(BaseModel): diff --git a/OpenOversight/app/static/css/openoversight.css b/OpenOversight/app/static/css/openoversight.css index 3d52e1296..c9ff3b4e1 100644 --- a/OpenOversight/app/static/css/openoversight.css +++ b/OpenOversight/app/static/css/openoversight.css @@ -514,6 +514,16 @@ tr:hover .row-actions { border: 0; } +.search-results .top-paginate > nav { + width: 100%; +} + +.search-results .top-sort .sort.form-group { + max-width: 250px; + margin: 0 0 0 auto; + display: block; +} + .console .button-explanation { height:35px; diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index 85746590d..3240572dc 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -190,50 +190,81 @@

Age range

- {% with paginate=officers, location='top' %} - {% include "partials/paginate_nav.html" %} - {% endwith %} -
    - {% for officer in officers.items %} -
  • -
    -
    - - {{ officer.full_name() }} - -
    -
    -

    - {{ officer.full_name() }} - {{ officer.badge_number()|default('') }} -

    -
    -
    -
    -
    Job Title
    -
    {{ officer.job_title()|default('Unknown') }}
    -
    Race
    -
    {{ officer.race_label()|default('Unknown')|lower|title }}
    -
    -
    -
    -
    -
    Gender
    -
    {{ officer.gender_label()|default('Unknown') }}
    -
    Number of Photos
    -
    {{ officer.face.count() }}
    -
    +
    +
    +
    +
    + + + +
    +
    +
    +
    + {% with paginate=officers, location='top' %} + {% include "partials/paginate_nav.html" %} + {% endwith %} +
    +
    +
      + {% for officer in officers.items %} +
    • +
      +
      + + {{ officer.full_name() }} + +
      +
      +

      + {{ officer.full_name() }} + {{ officer.badge_number()|default('') }} +

      +
      +
      +
      +
      Job Title
      +
      {{ officer.job_title()|default('Unknown') }}
      +
      Race
      +
      {{ officer.race_label()|default('Unknown')|lower|title }}
      +
      +
      +
      +
      +
      Gender
      +
      {{ officer.gender_label()|default('Unknown') }}
      +
      Number of Photos
      +
      {{ officer.face.count() }}
      +
      +
      -
    -
  • - {% endfor %} -
- {% with paginate=officers, location='bottom' %} - {% include "partials/paginate_nav.html" %} - {% endwith %} -
+ + {% endfor %} + + {% with paginate=officers, location='bottom' %} + {% include "partials/paginate_nav.html" %} + {% endwith %} +
{% endblock content %} diff --git a/OpenOversight/app/templates/partials/paginate.html b/OpenOversight/app/templates/partials/paginate.html deleted file mode 100644 index 82ba0502a..000000000 --- a/OpenOversight/app/templates/partials/paginate.html +++ /dev/null @@ -1,20 +0,0 @@ -
- - {% if officers.has_prev %} -
- {% include 'partials/officer_form_fields_hidden.html' %} - -
- {% endif %} - {% if officers.total > 0 %} -

Gallery {{ officers.page }} of {{ officers.pages }}

- {% endif %} - {% if officers.has_next %} -
- {% include 'partials/officer_form_fields_hidden.html' %} - -
- {% endif %} - -
- diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 460fd303a..a196e12bc 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -250,7 +250,7 @@ def upload_obj_to_s3(file_obj, dest_filename): return url -def filter_by_form(form, officer_query, department_id=None): +def filter_by_form(form, officer_query, department_id=None, order=0): # Some SQL acrobatics to left join only the most recent assignment and salary per officer assignment_row_num_col = func.row_number().over( partition_by=Assignment.officer_id, order_by=Assignment.star_date.desc() @@ -341,7 +341,6 @@ def filter_by_form(form, officer_query, department_id=None): Officer.id.in_(face_officer_ids) ) - officer_query = officer_query.outerjoin(Officer.salaries) if form.get('max_pay') and form.get('min_pay') and float(form['max_pay']) > float(form['min_pay']): officer_query = officer_query.filter( db.and_( @@ -358,6 +357,17 @@ def filter_by_form(form, officer_query, department_id=None): salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay <= float(form['max_pay']) ) + if order == 0: # Last name alphabetical + officer_query = officer_query.order_by(Officer.last_name, Officer.first_name, Officer.id) + elif order == 1: # Rank + officer_query = officer_query.order_by(Job.order.desc()) + elif order == 2: # Total pay + officer_query = officer_query.order_by((salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay).desc()) + elif order == 3: # Salary + officer_query = officer_query.order_by(salary_subq.c.salaries_salary.desc()) + elif order == 4: # Overtime pay + officer_query = officer_query.order_by(salary_subq.c.salaries_overtime_pay.desc()) + return officer_query diff --git a/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py b/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py new file mode 100644 index 000000000..0e7e92aea --- /dev/null +++ b/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py @@ -0,0 +1,33 @@ +"""overtime_pay default 0 + +Revision ID: b4145ba7d4c6 +Revises: 86eb228e4bc0 +Create Date: 2020-09-27 14:43:58.704560 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'b4145ba7d4c6' +down_revision = '86eb228e4bc0' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('salaries', 'overtime_pay', + existing_type=sa.NUMERIC(), + server_default='0', + nullable=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.alter_column('salaries', 'overtime_pay', + existing_type=sa.NUMERIC(), + nullable=True) + # ### end Alembic commands ### diff --git a/OpenOversight/tests/test_commands.py b/OpenOversight/tests/test_commands.py index 6e6b5bdb3..e3838cfbb 100644 --- a/OpenOversight/tests/test_commands.py +++ b/OpenOversight/tests/test_commands.py @@ -839,7 +839,7 @@ def test_advanced_csv_import__success(session, department_with_ranks, test_csv_d assert salary_2018.year == 2018 assert salary_2018.salary == 10000 assert salary_2018.is_fiscal_year is True - assert salary_2018.overtime_pay is None + assert salary_2018.overtime_pay == 0 assert salary_2019.salary == 10001 assignment_po, assignment_cap = sorted( From 73d5d79bbdbec75a6a08ac1228bfcf86ed5ae7b8 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 16:10:47 -0400 Subject: [PATCH 23/32] Forgot to change field name in Add Officer form javascript --- OpenOversight/app/static/js/add_officer.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OpenOversight/app/static/js/add_officer.js b/OpenOversight/app/static/js/add_officer.js index 76a115afc..837d12227 100644 --- a/OpenOversight/app/static/js/add_officer.js +++ b/OpenOversight/app/static/js/add_officer.js @@ -1,14 +1,14 @@ function set_jobs() { var dept_id = $('#department').val(); - // fetch jobs for dept_id and modify #job_titles var jobs_url = $('#add-officer-form').data('jobs-url'); var jobs = $.ajax({ url: jobs_url, data: {department_id: dept_id} }).done(function(jobs) { - $('#job_title').replaceWith(''); for (i = 0; i < jobs.length; i++) { - $('select#job_title').append( + $('select#job_id').append( $('').attr("value", jobs[i][0]).text(jobs[i][1]) ); } From e5611a9f27351602e76c18e40f0749bfa86514c9 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 18:29:33 -0400 Subject: [PATCH 24/32] Fix bugs in add_officer_profile() --- OpenOversight/app/utils.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index a196e12bc..8ef0d97b5 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -131,7 +131,8 @@ def add_officer_profile(form, current_user): gender=form.gender.data, birth_year=form.birth_year.data, employment_date=form.employment_date.data, - department_id=form.department.data.id) + department_id=form.department.data.id, + unique_internal_identifier=form.unique_internal_identifier.data) db.session.add(officer) db.session.commit() @@ -140,12 +141,13 @@ def add_officer_profile(form, current_user): else: officer_unit = None - assignment = Assignment(baseofficer=officer, - star_no=form.star_no.data, - job_id=form.job_id.data, - unit=officer_unit, - star_date=form.employment_date.data) - db.session.add(assignment) + if form.job_id.data: + assignment = Assignment(baseofficer=officer, + star_no=form.star_no.data, + job_id=form.job_id.data, + unit=officer_unit, + star_date=form.employment_date.data) + db.session.add(assignment) if form.links.data: for link in form.data['links']: # don't try to create with a blank string From 3bbd76e270a2bb200fd0c940bc33ec55c4e566dc Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 18:29:57 -0400 Subject: [PATCH 25/32] Added tests for new list_officer filters and sorting --- OpenOversight/app/models.py | 2 +- .../routes/test_officer_and_department.py | 157 +++++++++++++++++ OpenOversight/tests/test_models.py | 4 +- OpenOversight/tests/test_utils.py | 160 ++++++++++++++---- 4 files changed, 288 insertions(+), 35 deletions(-) diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py index d8106d319..71fec229b 100644 --- a/OpenOversight/app/models.py +++ b/OpenOversight/app/models.py @@ -212,7 +212,7 @@ class Unit(BaseModel): department = db.relationship('Department', backref='unit_types', order_by='Unit.descrip.asc()') def __repr__(self): - return 'Unit: {}'.format(self.descrip) + return ''.format(self.descrip) class Face(BaseModel): diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index 60defbe0b..b3a8a7e9e 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -1,4 +1,5 @@ # Routing and view tests +import re import csv import copy import json @@ -1390,6 +1391,162 @@ def test_browse_filtering_allows_good(client, mockdata, session): assert any("
Male
" in token for token in filter_list) +def test_browse_filtering_multiple_uiis(client, mockdata, session): + with current_app.test_request_context(): + department_id = Department.query.first().id + + # Add two officers + login_admin(client) + form = AddOfficerForm(first_name='Porkchops', + last_name='McBacon', + unique_internal_identifier='foo', + department=department_id) + data = process_form_data(form.data) + rv = client.post( + url_for('main.add_officer'), + data=data, + follow_redirects=True + ) + assert 'New Officer McBacon added' in rv.data.decode('utf-8') + + form = AddOfficerForm(first_name='Bacon', + last_name='McPorkchops', + unique_internal_identifier='bar', + department=department_id) + data = process_form_data(form.data) + rv = client.post( + url_for('main.add_officer'), + data=data, + follow_redirects=True + ) + assert 'New Officer McPorkchops added' in rv.data.decode('utf-8') + + # Check the officers were added to the database + assert Officer.query.filter_by(department_id=department_id).filter_by(unique_internal_identifier='foo').count() == 1 + assert Officer.query.filter_by(department_id=department_id).filter_by(unique_internal_identifier='bar').count() == 1 + + # Check that added officers appear when filtering for both UIIs + form = BrowseForm(unique_internal_identifier='foo,bar', race=None, gender=None) + data = process_form_data(form.data) + rv = client.get( + url_for('main.list_officer', department_id=department_id), + query_string=data, + follow_redirects=True + ) + assert 'McBacon' in rv.data.decode('utf-8') + assert 'McPorkchops' in rv.data.decode('utf-8') + assert 'foo' in rv.data.decode('utf-8') + assert 'bar' in rv.data.decode('utf-8') + + +def test_browse_sort_by_last_name(client, mockdata): + with current_app.test_request_context(): + department_id = Department.query.first().id + rv = client.get( + url_for('main.list_officer', department_id=department_id, order=0), + follow_redirects=True + ) + response = rv.data.decode('utf-8') + officer_ids = re.findall(r'', response) + assert officer_ids is not None and len(officer_ids) == int(current_app.config['OFFICERS_PER_PAGE']) + for idx, officer_id in enumerate(officer_ids): + if idx + 1 < len(officer_ids): + officer = Officer.query.filter_by(id=officer_id).one() + next_officer_id = officer_ids[idx + 1] + next_officer = Officer.query.filter_by(id=next_officer_id).one() + assert officer.last_name <= next_officer.last_name + + +def test_browse_sort_by_rank(client, mockdata): + with current_app.test_request_context(): + department_id = Department.query.first().id + rv = client.get( + url_for('main.list_officer', department_id=department_id, order=1), + follow_redirects=True + ) + response = rv.data.decode('utf-8') + officer_ids = re.findall(r'', response) + assert officer_ids is not None and len(officer_ids) == int(current_app.config['OFFICERS_PER_PAGE']) + for idx, officer_id in enumerate(officer_ids): + if idx + 1 < len(officer_ids): + officer = Officer.query.filter_by(id=officer_id).one() + next_officer_id = officer_ids[idx + 1] + next_officer = Officer.query.filter_by(id=next_officer_id).one() + officer_job = officer\ + .assignments\ + .order_by(Assignment.star_date.desc())\ + .first()\ + .job + next_officer_job = next_officer\ + .assignments\ + .order_by(Assignment.star_date.desc())\ + .first()\ + .job + assert officer_job.order >= next_officer_job.order + + +def test_browse_sort_by_total_pay(client, mockdata): + with current_app.test_request_context(): + department_id = Department.query.first().id + rv = client.get( + url_for('main.list_officer', department_id=department_id, order=2), + follow_redirects=True + ) + response = rv.data.decode('utf-8') + officer_ids = re.findall(r'', response) + assert officer_ids is not None and len(officer_ids) == int(current_app.config['OFFICERS_PER_PAGE']) + for idx, officer_id in enumerate(officer_ids): + if idx + 1 < len(officer_ids): + officer = Officer.query.filter_by(id=officer_id).one() + next_officer_id = officer_ids[idx + 1] + next_officer = Officer.query.filter_by(id=next_officer_id).one() + officer_salary = officer.salaries[0] + next_officer_salary = next_officer.salaries[0] + officer_total_pay = officer_salary.salary + officer_salary.overtime_pay + next_officer_total_pay = next_officer_salary.salary + next_officer_salary.overtime_pay + assert officer_total_pay >= next_officer_total_pay + + +def test_browse_sort_by_salary(client, mockdata): + with current_app.test_request_context(): + department_id = Department.query.first().id + rv = client.get( + url_for('main.list_officer', department_id=department_id, order=3), + follow_redirects=True + ) + response = rv.data.decode('utf-8') + officer_ids = re.findall(r'', response) + assert officer_ids is not None and len(officer_ids) == int(current_app.config['OFFICERS_PER_PAGE']) + for idx, officer_id in enumerate(officer_ids): + if idx + 1 < len(officer_ids): + officer = Officer.query.filter_by(id=officer_id).one() + next_officer_id = officer_ids[idx + 1] + next_officer = Officer.query.filter_by(id=next_officer_id).one() + officer_salary = officer.salaries[0] + next_officer_salary = next_officer.salaries[0] + assert officer_salary.salary >= next_officer_salary.salary + + +def test_browse_sort_by_overtime_pay(client, mockdata): + with current_app.test_request_context(): + department_id = Department.query.first().id + rv = client.get( + url_for('main.list_officer', department_id=department_id, order=4), + follow_redirects=True + ) + response = rv.data.decode('utf-8') + officer_ids = re.findall(r'', response) + assert officer_ids is not None and len(officer_ids) == int(current_app.config['OFFICERS_PER_PAGE']) + for idx, officer_id in enumerate(officer_ids): + if idx + 1 < len(officer_ids): + officer = Officer.query.filter_by(id=officer_id).one() + next_officer_id = officer_ids[idx + 1] + next_officer = Officer.query.filter_by(id=next_officer_id).one() + officer_salary = officer.salaries[0] + next_officer_salary = next_officer.salaries[0] + assert officer_salary.overtime_pay >= next_officer_salary.overtime_pay + + def test_admin_can_upload_photos_of_dept_officers(mockdata, client, session, test_jpg_BytesIO): with current_app.test_request_context(): login_admin(client) diff --git a/OpenOversight/tests/test_models.py b/OpenOversight/tests/test_models.py index 3681daa58..0f034f685 100644 --- a/OpenOversight/tests/test_models.py +++ b/OpenOversight/tests/test_models.py @@ -36,7 +36,7 @@ def test_face_repr(mockdata): def test_unit_repr(mockdata): unit = Unit.query.first() - assert unit.__repr__() == 'Unit: {}'.format(unit.descrip) + assert unit.__repr__() == ''.format(unit.descrip) def test_user_repr(mockdata): @@ -46,7 +46,7 @@ def test_user_repr(mockdata): def test_salary_repr(mockdata): salary = Salary.query.first() - assert salary.__repr__() == ''.format(salary.officer_id, salary.salary) def test_password_not_printed(mockdata): diff --git a/OpenOversight/tests/test_utils.py b/OpenOversight/tests/test_utils.py index 270c8116e..a7587f88b 100644 --- a/OpenOversight/tests/test_utils.py +++ b/OpenOversight/tests/test_utils.py @@ -13,91 +13,110 @@ def test_department_filter(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'race': ['Not Sure'], 'gender': ['Not Sure'], 'rank': ['Not Sure'], 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', 'dept': department, 'unique_internal_identifier': ''} ) - for element in results.all(): - assert element.department == department + for officer in officers.all(): + assert officer.department == department def test_race_filter_select_all_black_officers(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'race': ['BLACK'], 'dept': department} ) - for element in results.all(): - assert element.race in ('BLACK', 'Not Sure') + for officer in officers.all(): + assert officer.race in ('BLACK', 'Not Sure') def test_gender_filter_select_all_male_officers(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'gender': ['M'], 'dept': department} ) - for element in results.all(): - assert element.gender in ('M', 'Not Sure') + for officer in officers.all(): + assert officer.gender in ('M', 'Not Sure') def test_rank_filter_select_all_commanders(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'rank': ['Commander'], 'dept': department} ) - for element in results.all(): - assignment = element.assignments.first() + for officer in officers.all(): + assignment = officer.assignments.first() assert assignment.job.job_title in ('Commander', 'Not Sure') def test_rank_filter_select_all_police_officers(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'rank': ['Police Officer'], 'dept': department} ) - for element in results.all(): - assignment = element.assignments.first() + for officer in officers.all(): + assignment = officer.assignments.first() assert assignment.job.job_title in ('Police Officer', 'Not Sure') -def test_filter_by_name(mockdata): +def test_filter_by_last_name(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'last_name': 'J', 'dept': department} ) - for element in results.all(): - assert 'J' in element.last_name + for officer in officers.all(): + assert 'J' in officer.last_name + + +def test_filter_by_first_name(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'first_name': 'J', 'dept': department} + ) + for officer in officers.all(): + assert 'J' in officer.first_name def test_filters_do_not_exclude_officers_without_assignments(mockdata): department = OpenOversight.app.models.Department.query.first() officer = OpenOversight.app.models.Officer(first_name='Rachel', last_name='S', department=department, birth_year=1992) - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'last_name': 'S', 'dept': department} ) - assert officer in results.all() + assert officer in officers.all() def test_filter_by_badge_no(mockdata): department = OpenOversight.app.models.Department.query.first() - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'badge': '12', 'dept': department} ) - for element in results.all(): - assignment = element.assignments.first() - assert '12' in str(assignment.star_no) + for officer in officers.all(): + assert '12' in officer.badge_number() + + +def test_filter_by_multiple_badge_no(mockdata): + department = OpenOversight.app.models.Department.query.first() + target_officers = OpenOversight.app.models.Officer.query.filter_by(department_id=department.id).limit(2).all() + target_badge_nos = [officer.badge_number() for officer in target_officers] + officers = OpenOversight.app.utils.grab_officers( + {'badge': ','.join(target_badge_nos), 'dept': department} + ) + for officer in officers.all(): + assert officer.badge_number() in target_badge_nos def test_filter_by_full_unique_internal_identifier_returns_officers(mockdata): department = OpenOversight.app.models.Department.query.first() - target_unique_internal_id = OpenOversight.app.models.Officer.query.first().unique_internal_identifier - results = OpenOversight.app.utils.grab_officers( + target_unique_internal_id = OpenOversight.app.models.Officer.query.filter_by(department_id=department.id).first().unique_internal_identifier + officers = OpenOversight.app.utils.grab_officers( {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', 'dept': department, 'unique_internal_identifier': target_unique_internal_id} ) - for element in results: - returned_unique_internal_id = element.unique_internal_identifier + for officer in officers: + returned_unique_internal_id = officer.unique_internal_identifier assert returned_unique_internal_id == target_unique_internal_id @@ -105,16 +124,93 @@ def test_filter_by_partial_unique_internal_identifier_returns_officers(mockdata) department = OpenOversight.app.models.Department.query.first() identifier = OpenOversight.app.models.Officer.query.first().unique_internal_identifier partial_identifier = identifier[:len(identifier) // 2] - results = OpenOversight.app.utils.grab_officers( + officers = OpenOversight.app.utils.grab_officers( {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', 'dept': department, 'unique_internal_identifier': partial_identifier} ) - for element in results: - returned_identifier = element.unique_internal_identifier + for officer in officers: + returned_identifier = officer.unique_internal_identifier assert returned_identifier == identifier +def test_filter_by_multiple_unique_internal_identifiers_returns_officers(mockdata): + department = OpenOversight.app.models.Department.query.first() + target_officers = OpenOversight.app.models.Officer.query.filter_by(department_id=department.id).limit(2).all() + target_unique_internal_ids = [officer.unique_internal_identifier for officer in target_officers] + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': ','.join(target_unique_internal_ids)} + ) + for officer in officers: + returned_unique_internal_id = officer.unique_internal_identifier + assert returned_unique_internal_id in target_unique_internal_ids + + +def test_filter_by_photo_available(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': '', 'photo': ['1']} + ) + for officer in officers: + assert officer.face.count() > 0 + + +def test_filter_by_photo_not_available(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': '', 'photo': ['0']} + ) + for officer in officers: + assert officer.face.count() == 0 + + +def test_filter_min_pay(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': '', 'min_pay': '500000'} + ) + for officer in officers: + most_recent_salary = max(officer.salaries, key=lambda s: s.year) + total_pay = most_recent_salary.salary + most_recent_salary.overtime_pay + assert total_pay >= 500000 + + +def test_filter_max_pay(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': '', 'max_pay': '500000'} + ) + for officer in officers: + most_recent_salary = max(officer.salaries, key=lambda s: s.year) + total_pay = most_recent_salary.salary + most_recent_salary.overtime_pay + assert total_pay <= 500000 + + +def test_filter_min_and_max_pay(mockdata): + department = OpenOversight.app.models.Department.query.first() + officers = OpenOversight.app.utils.grab_officers( + {'race': 'Not Sure', 'gender': 'Not Sure', 'rank': 'Not Sure', + 'min_age': 16, 'max_age': 85, 'last_name': '', 'badge': '', + 'dept': department, 'unique_internal_identifier': '', + 'min_pay': '400000', 'max_pay': '500000'} + ) + for officer in officers: + most_recent_salary = max(officer.salaries, key=lambda s: s.year) + total_pay = most_recent_salary.salary + most_recent_salary.overtime_pay + assert total_pay <= 500000 + assert total_pay >= 400000 + + def test_compute_hash(mockdata): hash_result = OpenOversight.app.utils.compute_hash(b'bacon') expected_hash = '9cca0703342e24806a9f64e08c053dca7f2cd90f10529af8ea872afb0a0c77d4' From ff10608b41413c99cdb16eb92c19c57ab856f647 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 18:52:53 -0400 Subject: [PATCH 26/32] Added First and Last buttons to list_officer pagination in addition to Prev and Next --- OpenOversight/app/main/views.py | 16 ++++++---- .../app/static/css/openoversight.css | 8 +++++ .../app/templates/partials/paginate_nav.html | 32 +++++++++++++------ 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 356506999..5f97c9f2e 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -572,16 +572,16 @@ def list_officer(department_id, page=1, order=0, race=[], gender=[], rank=[], mi 'unit': [('Not Sure', 'Not Sure')] + unit_choices } - next_url = url_for('main.list_officer', department_id=department.id, + def gen_pagination_url(page): + return url_for('main.list_officer', department_id=department.id, page=officers.next_num, order=order, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], first_name=form_data['first_name'], badge=form_data['badge'], unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) - prev_url = url_for('main.list_officer', department_id=department.id, - page=officers.prev_num, order=order, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], - min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], - first_name=form_data['first_name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + prev_url = gen_pagination_url(page=officers.prev_num) + next_url = gen_pagination_url(page=officers.next_num) + first_url = gen_pagination_url(page=1) + last_url = gen_pagination_url(page=officers.pages) return render_template( 'list_officer.html', @@ -591,7 +591,9 @@ def list_officer(department_id, page=1, order=0, race=[], gender=[], rank=[], mi form_data=form_data, choices=choices, next_url=next_url, - prev_url=prev_url) + prev_url=prev_url, + first_url=first_url, + last_url=last_url) @main.route('/department//ranks') diff --git a/OpenOversight/app/static/css/openoversight.css b/OpenOversight/app/static/css/openoversight.css index c9ff3b4e1..c96e2a436 100644 --- a/OpenOversight/app/static/css/openoversight.css +++ b/OpenOversight/app/static/css/openoversight.css @@ -589,3 +589,11 @@ tr:hover .row-actions { display:block; } + +.pager .previous.first>a { + margin-right: 7px; +} + +.pager .next.last>a { + margin-left: 7px; +} \ No newline at end of file diff --git a/OpenOversight/app/templates/partials/paginate_nav.html b/OpenOversight/app/templates/partials/paginate_nav.html index fcce55215..9a800f2a6 100644 --- a/OpenOversight/app/templates/partials/paginate_nav.html +++ b/OpenOversight/app/templates/partials/paginate_nav.html @@ -1,6 +1,12 @@ From 2f85fa79639603c2ba749c7b13fa73fffe6557ec Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Sun, 27 Sep 2020 19:04:09 -0400 Subject: [PATCH 27/32] minor testing fixes --- OpenOversight/tests/test_models.py | 2 +- OpenOversight/tests/test_utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/OpenOversight/tests/test_models.py b/OpenOversight/tests/test_models.py index 0f034f685..924061de3 100644 --- a/OpenOversight/tests/test_models.py +++ b/OpenOversight/tests/test_models.py @@ -46,7 +46,7 @@ def test_user_repr(mockdata): def test_salary_repr(mockdata): salary = Salary.query.first() - assert salary.__repr__() == ''.format(salary.officer_id, salary.salary) + assert salary.__repr__() == ''.format(salary.officer_id, salary.salary, salary.overtime_pay, salary.year) def test_password_not_printed(mockdata): diff --git a/OpenOversight/tests/test_utils.py b/OpenOversight/tests/test_utils.py index a7587f88b..ff9454e46 100644 --- a/OpenOversight/tests/test_utils.py +++ b/OpenOversight/tests/test_utils.py @@ -254,7 +254,7 @@ def test_user_cannot_submit_invalid_file_extension(mockdata): def test_unit_choices(mockdata): unit_choices = [str(x) for x in OpenOversight.app.utils.unit_choices()] - assert 'Unit: Bureau of Organized Crime' in unit_choices + assert '' in unit_choices @patch('OpenOversight.app.utils.upload_obj_to_s3', MagicMock(return_value='https://s3-some-bucket/someaddress.jpg')) From 2abefccbe83aa8e24e1e4ab6ef11c6682db9cfde Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 29 Sep 2020 10:05:21 -0400 Subject: [PATCH 28/32] Fix to latest db migration, since it would fail if DB had salaries with null overtime_pay --- .../migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py | 1 + 1 file changed, 1 insertion(+) diff --git a/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py b/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py index 0e7e92aea..1d0427c1c 100644 --- a/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py +++ b/OpenOversight/migrations/versions/b4145ba7d4c6_overtime_pay_default_0.py @@ -18,6 +18,7 @@ def upgrade(): # ### commands auto generated by Alembic - please adjust! ### + op.execute('update salaries set overtime_pay = 0 where overtime_pay is null') op.alter_column('salaries', 'overtime_pay', existing_type=sa.NUMERIC(), server_default='0', From 3ecc2bdb8ff388d8a144de08f95ba2ea6da8d786 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 29 Sep 2020 10:06:59 -0400 Subject: [PATCH 29/32] Removed unncessary Bootstrap CSS include --- OpenOversight/app/templates/list_officer.html | 1 - 1 file changed, 1 deletion(-) diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index 3240572dc..d057baf1f 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -1,5 +1,4 @@ {% extends "base.html" %} -{% block head %}{% endblock %} {% import "bootstrap/wtf.html" as wtf %} {% block title %}Browse {{ department.name|title }} officers - OpenOversight{% endblock %} {% block meta %}{% endblock %} From 1a8b0544e405ed20dfcf01cd459290b3f86c8f0a Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 29 Sep 2020 10:20:33 -0400 Subject: [PATCH 30/32] Generated prev/next urls didn't include the new filters --- OpenOversight/app/main/views.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 2e62dbf02..b6e8d5104 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -576,10 +576,11 @@ def list_officer(department_id, page=1, order=0, race=[], gender=[], rank=[], mi def gen_pagination_url(page): return url_for('main.list_officer', department_id=department.id, - page=officers.next_num, order=order, race=form_data['race'], gender=form_data['gender'], rank=form_data['rank'], - min_age=form_data['min_age'], max_age=form_data['max_age'], last_name=form_data['last_name'], - first_name=form_data['first_name'], badge=form_data['badge'], - unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']) + page=officers.next_num, order=order, race=form_data['race'], gender=form_data['gender'], + rank=form_data['rank'], min_age=form_data['min_age'], max_age=form_data['max_age'], + last_name=form_data['last_name'], first_name=form_data['first_name'], badge=form_data['badge'], + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit'], + photo=form_data['photo'], min_pay=form_data['min_pay'], max_pay=form_data['max_pay']) prev_url = gen_pagination_url(page=officers.prev_num) next_url = gen_pagination_url(page=officers.next_num) first_url = gen_pagination_url(page=1) From 676830444ac282df76406d91544522b4ac05e1bd Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 29 Sep 2020 10:28:02 -0400 Subject: [PATCH 31/32] When sorting browse results, nulls last --- OpenOversight/app/utils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 9da1455c5..318e729a3 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -17,7 +17,7 @@ from distutils.util import strtobool from sqlalchemy import func, or_ -from sqlalchemy.sql.expression import cast +from sqlalchemy.sql.expression import cast, nullslast, desc import imghdr as imghdr from flask import current_app, url_for from flask_login import current_user @@ -363,13 +363,13 @@ def filter_by_form(form, officer_query, department_id=None, order=0): if order == 0: # Last name alphabetical officer_query = officer_query.order_by(Officer.last_name, Officer.first_name, Officer.id) elif order == 1: # Rank - officer_query = officer_query.order_by(Job.order.desc()) + officer_query = officer_query.order_by(nullslast(Job.order.desc())) elif order == 2: # Total pay - officer_query = officer_query.order_by((salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay).desc()) + officer_query = officer_query.order_by(nullslast(desc(salary_subq.c.salaries_salary + salary_subq.c.salaries_overtime_pay))) elif order == 3: # Salary - officer_query = officer_query.order_by(salary_subq.c.salaries_salary.desc()) + officer_query = officer_query.order_by(nullslast(salary_subq.c.salaries_salary.desc())) elif order == 4: # Overtime pay - officer_query = officer_query.order_by(salary_subq.c.salaries_overtime_pay.desc()) + officer_query = officer_query.order_by(nullslast(salary_subq.c.salaries_overtime_pay.desc())) return officer_query From f8185097724cd0d0234b55ec3ec1ebf61b9d8d49 Mon Sep 17 00:00:00 2001 From: Dan Staples Date: Tue, 29 Sep 2020 13:05:47 -0400 Subject: [PATCH 32/32] Bump libsqlite3 version from 3.27.2 -> 3.33.0 in Dockerfile so it supports "NULLS LAST" syntax --- dockerfiles/web/Dockerfile | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/dockerfiles/web/Dockerfile b/dockerfiles/web/Dockerfile index 20eee30db..8d4844e88 100644 --- a/dockerfiles/web/Dockerfile +++ b/dockerfiles/web/Dockerfile @@ -7,10 +7,14 @@ WORKDIR /usr/src/app ENV DEBIAN-FRONTEND noninteractive ENV DISPLAY=:1 ENV GECKODRIVER_VERSION="v0.26.0" -RUN echo "deb http://deb.debian.org/debian stretch-backports main" > /etc/apt/sources.list.d/backports.list +ENV SQLITE3_VERSION="3330000" RUN wget -O - https://deb.nodesource.com/setup_12.x | bash - -RUN apt-get update && apt-get install -y xvfb firefox-esr libpq-dev python3-dev nodejs && \ - apt-get install -y -t stretch-backports libsqlite3-0 && apt-get clean +RUN apt-get update && apt-get install -y xvfb firefox-esr libpq-dev python3-dev nodejs && apt-get clean +RUN wget https://www.sqlite.org/2020/sqlite-autoconf-${SQLITE3_VERSION}.tar.gz && \ + tar -xzf sqlite-autoconf-${SQLITE3_VERSION}.tar.gz && \ + cd sqlite-autoconf-${SQLITE3_VERSION} && \ + ./configure && make && make install && ldconfig && \ + cd .. && rm -r sqlite-autoconf-${SQLITE3_VERSION} RUN wget https://github.com/mozilla/geckodriver/releases/download/${GECKODRIVER_VERSION}/geckodriver-${GECKODRIVER_VERSION}-linux64.tar.gz RUN mkdir geckodriver