Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Officer unit view #783

Merged
merged 11 commits into from
Aug 19, 2020
4 changes: 4 additions & 0 deletions CONTRIB.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Use [PULL_REQUEST_TEMPLATE.md](/PULL_REQUEST_TEMPLATE.md) to create the descript

`flake8` is a tool for automated linting and style checks. Be sure to run `flake8` and fix any errors before submitting a PR.

You can run it with `make lint` to execute flake8 from the docker containers.

## Development Environment

You can use our Docker-compose environment to stand up a development OpenOversight.
Expand All @@ -29,6 +31,8 @@ $ docker exec -it openoversight_postgres_1 /bin/bash
# psql -d openoversight-dev -U openoversight
```

or run `make attach`.

Similarly to hop into the web container:

```
Expand Down
13 changes: 10 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@ populate: create_db ## Build and run containers
test: start ## Run tests
if [ -z "$(name)" ]; then \
if [ "$$(uname)" == "Darwin" ]; then \
FLASK_ENV=testing docker-compose run --rm web pytest -n $$(sysctl -n hw.logicalcpu) --dist=loadfile -v tests/; \
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n $$(sysctl -n hw.logicalcpu) --dist=loadfile -v tests/ app; \
else \
FLASK_ENV=testing docker-compose run --rm web pytest -n $$(nproc --all) --dist=loadfile -v tests/; \
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n $$(nproc --all) --dist=loadfile -v tests/ app; \
fi; \
else \
FLASK_ENV=testing docker-compose run --rm web pytest -v tests/ -k $(name); \
FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -v tests/ app -k $(name); \
fi

.PHONY: lint
lint: start
docker-compose run --rm web flake8

.PHONY: cleanassets
cleanassets:
rm -rf ./OpenOversight/app/static/dist/
Expand Down Expand Up @@ -76,3 +80,6 @@ help: ## Print this message and exit
@awk 'BEGIN {FS = ":.*?## "} /^[0-9a-zA-Z_-]+:.*?## / {printf "\033[36m%s\033[0m : %s\n", $$1, $$2}' $(MAKEFILE_LIST) \
| sort \
| column -s ':' -t

attach:
docker-compose exec postgres psql -h localhost -U openoversight openoversight-dev
14 changes: 14 additions & 0 deletions OpenOversight/app/auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ def login():
login_user(user, form.remember_me.data)
return redirect(request.args.get('next') or url_for('main.index'))
flash('Invalid username or password.')
else:
current_app.logger.info(form.errors)
return render_template('auth/login.html', form=form)


Expand Down Expand Up @@ -94,6 +96,8 @@ def register():
'auth/email/confirm', user=user, token=token)
flash('A confirmation email has been sent to you.')
return redirect(url_for('auth.login'))
else:
current_app.logger.info(form.errors)
return render_template('auth/register.html', form=form, jsloads=jsloads)


Expand Down Expand Up @@ -137,6 +141,8 @@ def change_password():
return redirect(url_for('main.index'))
else:
flash('Invalid password.')
else:
current_app.logger.info(form.errors)
return render_template("auth/change_password.html", form=form, jsloads=jsloads)


Expand All @@ -156,6 +162,8 @@ def password_reset_request():
flash('An email with instructions to reset your password has been '
'sent to you.')
return redirect(url_for('auth.login'))
else:
current_app.logger.info(form.errors)
return render_template('auth/reset_password.html', form=form)


Expand All @@ -173,6 +181,8 @@ def password_reset(token):
return redirect(url_for('auth.login'))
else:
return redirect(url_for('main.index'))
else:
current_app.logger.info(form.errors)
return render_template('auth/reset_password.html', form=form)


Expand All @@ -192,6 +202,8 @@ def change_email_request():
return redirect(url_for('main.index'))
else:
flash('Invalid email or password.')
else:
current_app.logger.info(form.errors)
return render_template("auth/change_email.html", form=form)


Expand Down Expand Up @@ -220,6 +232,8 @@ def change_dept():
db.session.commit()
flash('Updated!')
return redirect(url_for('main.index'))
else:
current_app.logger.info(form.errors)
return render_template('auth/change_dept_pref.html', form=form)


Expand Down
31 changes: 30 additions & 1 deletion OpenOversight/app/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,35 @@ def create_officer_from_row(row, department_id):
process_salary(row, officer, compare=False)


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added - there was a weird issue of failing unit tests on the first PR. I updated the test data to have datetime's for start_date + resign_date on assignments. Without this equality check, we would be == comparing datetime(2020, 01, 01) to '2020-01-01', which would fail and we would trigger spurious row updates and fail the tests.

def is_equal(a, b):
"""exhaustive equality checking, originally to compare a sqlalchemy result object of various types to a csv string
Note: Stringifying covers object cases (as in the datetime example below)
>>> is_equal("1", 1) # string == int
True
>>> is_equal("foo", "bar") # string != other string
False
>>> is_equal(1, "1") # int == string
True
>>> is_equal(1.0, "1") # float == string
True
>>> is_equal(datetime(2020, 1, 1), "2020-01-01 00:00:00") # datetime == string
True
"""
def try_else_false(comparable):
try:
return comparable(a, b)
except TypeError:
return False
except ValueError:
return False

return any([
try_else_false(lambda _a, _b: str(_a) == str(_b)),
try_else_false(lambda _a, _b: int(_a) == int(_b)),
try_else_false(lambda _a, _b: float(_a) == float(_b))
])


def process_assignment(row, officer, compare=False):
assignment_fields = {
'required': ['job_title'],
Expand All @@ -254,7 +283,7 @@ def process_assignment(row, officer, compare=False):
for fieldname in assignment_fieldnames:
current = getattr(assignment, fieldname)
# Test if fields match between row and existing assignment
if (current and fieldname in row and row[fieldname] == current) or \
if (current and fieldname in row and is_equal(row[fieldname], current)) or \
(not current and (fieldname not in row or not row[fieldname])):
i += 1
if i == len(assignment_fieldnames):
Expand Down
1 change: 1 addition & 0 deletions OpenOversight/app/main/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class FindOfficerForm(Form):
unique_internal_identifier = StringField('unique_internal_identifier', default='', validators=[Regexp(r'\w*'), Length(max=55)])
dept = QuerySelectField('dept', validators=[DataRequired()],
query_factory=dept_choices, get_label='name')
unit = SelectField('unit', default='Not Sure', validators=[Optional()])
rank = StringField('rank', default='Not Sure', validators=[Optional()]) # Gets rewritten by Javascript
race = SelectField('race', default='Not Sure', choices=RACE_CHOICES,
validators=[AnyOf(allowed_values(RACE_CHOICES))])
Expand Down
4 changes: 3 additions & 1 deletion OpenOversight/app/main/model_view.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import datetime
from flask import render_template, redirect, request, url_for, flash, abort
from flask import render_template, redirect, request, url_for, flash, abort, current_app
from flask.views import MethodView
from flask_login import login_required, current_user
from ..auth.utils import ac_or_admin_required
Expand Down Expand Up @@ -56,6 +56,8 @@ def new(self, form=None):
db.session.commit()
flash('{} created!'.format(self.model_name))
return self.get_redirect_url(obj_id=new_obj.id)
else:
current_app.logger.info(form.errors)

return render_template('{}_new.html'.format(self.model_name), form=form)

Expand Down
56 changes: 41 additions & 15 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,15 @@ def get_officer():
race=form.data['race'] if form.data['race'] != 'Not Sure' else None,
gender=form.data['gender'] if form.data['gender'] != 'Not Sure' else None,
rank=form.data['rank'] if form.data['rank'] != 'Not Sure' else None,
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'],
badge=form.data['badge'],
unique_internal_identifier=form.data['unique_internal_identifier']),
code=302)
else:
current_app.logger.info(form.errors)
return render_template('input_find_officer.html', form=form, depts_dict=depts_dict, jsloads=jsloads)


Expand All @@ -105,6 +108,8 @@ def get_ooid():
form = FindOfficerIDForm()
if form.validate_on_submit():
return redirect(url_for('main.get_tagger_gallery'), code=307)
else:
current_app.logger.info(form.errors)
return render_template('input_find_ooid.html', form=form)


Expand All @@ -118,6 +123,8 @@ def get_started_labeling():
login_user(user, form.remember_me.data)
return redirect(request.args.get('next') or url_for('main.index'))
flash('Invalid username or password.')
else:
current_app.logger.info(form.errors)
departments = Department.query.all()
return render_template('label_data.html', departments=departments, form=form)

Expand Down Expand Up @@ -212,19 +219,20 @@ def add_assignment(officer_id):
flash('Officer not found')
abort(404)

if form.validate_on_submit() and (current_user.is_administrator or
(current_user.is_area_coordinator and
officer.department_id == current_user.ac_department_id)):
try:
add_new_assignment(officer_id, form)
flash('Added new assignment!')
except IntegrityError:
flash('Assignment already exists')
return redirect(url_for('main.officer_profile',
officer_id=officer_id), code=302)
elif current_user.is_area_coordinator and not officer.department_id == current_user.ac_department_id:
abort(403)

if form.validate_on_submit():
if (current_user.is_administrator
or (current_user.is_area_coordinator and officer.department_id == current_user.ac_department_id)):
try:
add_new_assignment(officer_id, form)
flash('Added new assignment!')
except IntegrityError:
flash('Assignment already exists')
return redirect(url_for('main.officer_profile',
officer_id=officer_id), code=302)
elif current_user.is_area_coordinator and not officer.department_id == current_user.ac_department_id:
abort(403)
else:
current_app.logger.info(form.errors)
return redirect(url_for('main.officer_profile', officer_id=officer_id))


Expand Down Expand Up @@ -253,6 +261,8 @@ def edit_assignment(officer_id, assignment_id):
assignment = edit_existing_assignment(assignment, form)
flash('Edited officer assignment ID {}'.format(assignment.id))
return redirect(url_for('main.officer_profile', officer_id=officer_id))
else:
current_app.logger.info(form.errors)
return render_template('edit_assignment.html', form=form)


Expand Down Expand Up @@ -308,6 +318,8 @@ def edit_salary(officer_id, salary_id):
db.session.commit()
flash('Edited officer salary ID {}'.format(salary.id))
return redirect(url_for('main.officer_profile', officer_id=officer_id))
else:
current_app.logger.info(form.errors)
return render_template('add_edit_salary.html', form=form, update=True)


Expand Down Expand Up @@ -411,6 +423,7 @@ def add_department():
flash('Department {} already exists'.format(form.name.data))
return redirect(url_for('main.get_started_labeling'))
else:
current_app.logger.info(form.errors)
return render_template('add_edit_department.html', form=form, jsloads=jsloads)


Expand Down Expand Up @@ -474,11 +487,13 @@ def edit_department(department_id):
flash('Department {} edited'.format(department.name))
return redirect(url_for('main.list_officer', department_id=department.id))
else:
current_app.logger.info(form.errors)
return render_template('add_edit_department.html', form=form, update=True, jsloads=jsloads)


@main.route('/department/<int:department_id>')
def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16', max_age='100', name=None, badge=None, unique_internal_identifier=None):
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):
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
Expand All @@ -489,6 +504,7 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16
form_data['max_age'] = max_age
form_data['name'] = name
form_data['badge'] = badge
form_data['unit'] = unit
form_data['unique_internal_identifier'] = unique_internal_identifier

OFFICERS_PER_PAGE = int(current_app.config['OFFICERS_PER_PAGE'])
Expand All @@ -507,13 +523,16 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16
form_data['name'] = request.args.get('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':
form_data['unit'] = int(request.args.get('unit'))
if request.args.get('unique_internal_identifier'):
form_data['unique_internal_identifier'] = request.args.get('unique_internal_identifier')
if request.args.get('race') and all(race in [rc[0] for rc in RACE_CHOICES] for race in request.args.getlist('race')):
form_data['race'] = request.args.getlist('race')
if request.args.get('gender') and all(gender in [gc[0] for gc in GENDER_CHOICES] for gender in request.args.getlist('gender')):
form_data['gender'] = request.args.getlist('gender')

unit_choices = [(unit.id, unit.descrip) for unit in Unit.query.filter_by(department_id=department_id).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')
Expand All @@ -527,7 +546,8 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16
choices = {
'race': RACE_CHOICES,
'gender': GENDER_CHOICES,
'rank': [(rc, rc) for rc in rank_choices]
'rank': [(rc, rc) for rc in rank_choices],
'unit': [('Not Sure', 'Not Sure')] + unit_choices
}

return render_template(
Expand Down Expand Up @@ -585,6 +605,7 @@ def add_officer():
flash('New Officer {} added to OpenOversight'.format(officer.last_name))
return redirect(url_for('main.submit_officer_images', officer_id=officer.id))
else:
current_app.logger.info(form.errors)
return render_template('add_officer.html', form=form, jsloads=jsloads)


Expand All @@ -610,6 +631,7 @@ def edit_officer(officer_id):
flash('Officer {} edited'.format(officer.last_name))
return redirect(url_for('main.officer_profile', officer_id=officer.id))
else:
current_app.logger.info(form.errors)
return render_template('edit_officer.html', form=form, jsloads=jsloads)


Expand All @@ -629,6 +651,7 @@ def add_unit():
flash('New unit {} added to OpenOversight'.format(unit.descrip))
return redirect(url_for('main.get_started_labeling'))
else:
current_app.logger.info(form.errors)
return render_template('add_unit.html', form=form)


Expand Down Expand Up @@ -769,6 +792,8 @@ def label_data(department_id=None, image_id=None):
flash('There was a problem saving this tag. Please try again later.')
else:
flash('Tag already exists between this officer and image! Tag not added.')
else:
current_app.logger.info(form.errors)

return render_template('cop_face.html', form=form,
image=image, path=proper_path,
Expand Down Expand Up @@ -805,6 +830,7 @@ def get_tagger_gallery(page=1):
form=form,
form_data=form_data)
else:
current_app.logger.info(form.errors)
return redirect(url_for('main.get_ooid'), code=307)


Expand Down
Loading