From 41bfd77ab4a5fa1f7a50d748973d0a4653e53082 Mon Sep 17 00:00:00 2001 From: F D Date: Mon, 3 Aug 2020 18:15:30 -0400 Subject: [PATCH 1/9] add attach/lint, add pick_date, add is_equal --- CONTRIB.md | 4 ++++ Makefile | 11 +++++++++-- OpenOversight/app/commands.py | 31 ++++++++++++++++++++++++++++++- OpenOversight/tests/conftest.py | 28 +++++++++++++++++++++++----- dockerfiles/web/Dockerfile | 1 + 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/CONTRIB.md b/CONTRIB.md index c27392043..b8eeba74a 100644 --- a/CONTRIB.md +++ b/CONTRIB.md @@ -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. @@ -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: ``` diff --git a/Makefile b/Makefile index 2752071ed..c407e5105 100644 --- a/Makefile +++ b/Makefile @@ -40,10 +40,14 @@ populate: create_db ## Build and run containers .PHONY: test test: start ## Run tests if [ -z "$(name)" ]; \ - then FLASK_ENV=testing docker-compose run --rm web pytest -n 4 --dist=loadfile -v tests/; \ - else FLASK_ENV=testing docker-compose run --rm web pytest -n 4 --dist=loadfile -v tests/ -k $(name); \ + then FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n 4 --dist=loadfile -v tests/ app; \ + else FLASK_ENV=testing docker-compose run --rm web pytest --doctest-modules -n 4 --dist=loadfile -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/ @@ -71,3 +75,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 \ No newline at end of file diff --git a/OpenOversight/app/commands.py b/OpenOversight/app/commands.py index 6a1e84911..84b02ce42 100644 --- a/OpenOversight/app/commands.py +++ b/OpenOversight/app/commands.py @@ -227,6 +227,35 @@ def create_officer_from_row(row, department_id): process_salary(row, officer, compare=False) +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'], @@ -252,7 +281,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): diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py index d8b30d95b..6d33033f0 100644 --- a/OpenOversight/tests/conftest.py +++ b/OpenOversight/tests/conftest.py @@ -16,7 +16,7 @@ from OpenOversight.app import create_app, models from OpenOversight.app.utils import merge_dicts -from OpenOversight.app.models import db as _db +from OpenOversight.app.models import db as _db, Unit, Job, Officer factory = Faker() @@ -39,6 +39,22 @@ def pick_birth_date(): return random.randint(1950, 2000) +def pick_date(seed: bytes = None, start_year=2000, end_year=2020): + # source: https://stackoverflow.com/questions/40351791/how-to-hash-strings-into-a-float-in-01 + # Wanted to deterministically create a date from a seed string (e.g. the hash or uuid on an officer object) + from struct import unpack + from hashlib import sha256 + + def bytes_to_float(b): + return float(unpack('L', sha256(b).digest()[:8])[0]) / 2 ** 64 + + if seed is None: + seed = str(uuid.uuid4()).encode('utf-8') + + return datetime.datetime(start_year, 1, 1, 00, 00, 00) \ + + datetime.timedelta(days=365 * (end_year - start_year) * bytes_to_float(seed)) + + def pick_race(): return random.choice(['WHITE', 'BLACK', 'HISPANIC', 'ASIAN', 'PACIFIC ISLANDER', 'Not Sure']) @@ -95,9 +111,11 @@ def generate_officer(): ) -def build_assignment(officer, unit, jobs): +def build_assignment(officer: Officer, unit: Unit, jobs: Job): return models.Assignment(star_no=pick_star(), job_id=random.choice(jobs).id, - officer=officer) + officer=officer, unit_id=unit.id, + star_date=pick_date(officer.full_name().encode('utf-8')), + resign_date=pick_date(officer.full_name().encode('utf-8'))) def build_note(officer, user): @@ -249,7 +267,8 @@ def add_mockdata(session): SEED = current_app.config['SEED'] random.seed(SEED) - unit1 = models.Unit(descrip="test") + unit1 = models.Unit(descrip="test", department_id=1) + session.add(unit1) test_images = [models.Image(filepath='/static/images/test_cop{}.png'.format(x + 1), department_id=1) for x in range(5)] + \ [models.Image(filepath='/static/images/test_cop{}.png'.format(x + 1), department_id=2) for x in range(5)] @@ -278,7 +297,6 @@ def add_mockdata(session): faces_dept2 = [assign_faces(officer, assigned_images_dept2) for officer in officers_dept2] faces1 = [f for f in faces_dept1 if f] faces2 = [f for f in faces_dept2 if f] - session.add(unit1) session.commit() session.add_all(assignments_dept1) session.add_all(assignments_dept2) diff --git a/dockerfiles/web/Dockerfile b/dockerfiles/web/Dockerfile index c92813d24..103dc7012 100644 --- a/dockerfiles/web/Dockerfile +++ b/dockerfiles/web/Dockerfile @@ -31,6 +31,7 @@ COPY package.json /usr/src/app/ RUN yarn COPY create_db.py test_data.py /usr/src/app/ +COPY .flake8 /usr/src/app/ EXPOSE 3000 ENV PATH="/usr/src/app/geckodriver:${PATH}" From 5f2b15c45dea6430bdef161decf837a6b340c6fe Mon Sep 17 00:00:00 2001 From: F D Date: Thu, 6 Aug 2020 15:47:01 -0400 Subject: [PATCH 2/9] update test data with more units, assign units correctly --- OpenOversight/tests/conftest.py | 36 ++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/OpenOversight/tests/conftest.py b/OpenOversight/tests/conftest.py index 6d33033f0..295f22683 100644 --- a/OpenOversight/tests/conftest.py +++ b/OpenOversight/tests/conftest.py @@ -1,4 +1,6 @@ import datetime +from typing import List + from flask import current_app from io import BytesIO import pytest @@ -111,9 +113,9 @@ def generate_officer(): ) -def build_assignment(officer: Officer, unit: Unit, jobs: Job): +def build_assignment(officer: Officer, units: List[Unit], jobs: Job): return models.Assignment(star_no=pick_star(), job_id=random.choice(jobs).id, - officer=officer, unit_id=unit.id, + officer=officer, unit_id=random.choice(units).id, star_date=pick_date(officer.full_name().encode('utf-8')), resign_date=pick_date(officer.full_name().encode('utf-8'))) @@ -149,10 +151,11 @@ def build_salary(officer): def assign_faces(officer, images): if random.uniform(0, 1) >= 0.5: - for num in range(1, len(images)): - return models.Face(officer_id=officer.id, - img_id=num, - original_image_id=random.choice(images).id) + # for num in range(1, len(images)): # <-- bug here, we always return on the first one, the for-loop is unneeded + img_id = random.choice(images).id + return models.Face(officer_id=officer.id, + img_id=img_id, + original_image_id=img_id) else: return False @@ -267,8 +270,15 @@ def add_mockdata(session): SEED = current_app.config['SEED'] random.seed(SEED) - unit1 = models.Unit(descrip="test", department_id=1) - session.add(unit1) + test_units = [ + models.Unit(descrip="test", department_id=1), + models.Unit(descrip='District 13', department_id=1), + models.Unit(descrip='Donut Devourers', department_id=1), + models.Unit(descrip='Bureau of Organized Crime', department_id=2), + models.Unit(descrip='Porky\'s BBQ: Rub Division', department_id=2) + ] + session.add_all(test_units) + session.commit() test_images = [models.Image(filepath='/static/images/test_cop{}.png'.format(x + 1), department_id=1) for x in range(5)] + \ [models.Image(filepath='/static/images/test_cop{}.png'.format(x + 1), department_id=2) for x in range(5)] @@ -289,8 +299,8 @@ def add_mockdata(session): jobs_dept1 = models.Job.query.filter_by(department_id=1).all() jobs_dept2 = models.Job.query.filter_by(department_id=2).all() - assignments_dept1 = [build_assignment(officer, unit1, jobs_dept1) for officer in officers_dept1] - assignments_dept2 = [build_assignment(officer, unit1, jobs_dept2) for officer in officers_dept2] + assignments_dept1 = [build_assignment(officer, test_units, jobs_dept1) for officer in officers_dept1] + assignments_dept2 = [build_assignment(officer, test_units, jobs_dept2) for officer in officers_dept2] salaries = [build_salary(officer) for officer in all_officers] faces_dept1 = [assign_faces(officer, assigned_images_dept1) for officer in officers_dept1] @@ -331,12 +341,6 @@ def add_mockdata(session): session.add(test_unconfirmed_user) session.commit() - test_units = [models.Unit(descrip='District 13', department_id=1), - models.Unit(descrip='Bureau of Organized Crime', - department_id=1)] - session.add_all(test_units) - session.commit() - test_addresses = [ models.Location( street_name='Test St', From 801ab740d9813f3993d57971bec7372a2aeb595d Mon Sep 17 00:00:00 2001 From: F D Date: Thu, 6 Aug 2020 15:47:35 -0400 Subject: [PATCH 3/9] update unit field in list_officer, update wtform --- OpenOversight/app/main/forms.py | 2 ++ OpenOversight/app/main/views.py | 13 ++++++++++--- OpenOversight/app/utils.py | 8 +++++++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index 30ade492d..cda397f3e 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -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') 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))]) @@ -384,6 +385,7 @@ class IncidentForm(DateFieldForm): class BrowseForm(Form): rank = QuerySelectField('rank', validators=[Optional()], get_label='job_title', get_pk=lambda job: job.job_title) # query set in view function + unit = SelectField('unit', default='Not Sure') name = StringField('Last name') badge = StringField('Badge number') unique_internal_identifier = StringField('Unique ID') diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index a9985f13d..6b261048f 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -478,7 +478,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): +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 @@ -489,6 +490,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']) @@ -507,6 +509,8 @@ 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')): @@ -514,16 +518,19 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 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') - officers = filter_by_form(form_data, Officer.query, department_id).filter(Officer.department_id == department_id).order_by(Officer.last_name).paginate(page, OFFICERS_PER_PAGE, False) + officers = filter_by_form(form_data, Officer.query, department_id).filter_by(department_id=department_id)\ + .order_by(Officer.last_name).paginate(page, OFFICERS_PER_PAGE, False) 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( diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 46806e14a..6baaf3f21 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -261,7 +261,8 @@ def filter_by_form(form, officer_query, department_id=None): Assignment.officer_id, Assignment.job_id, Assignment.star_date, - Assignment.star_no + 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) @@ -278,6 +279,11 @@ def filter_by_form(form, officer_query, department_id=None): 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'])) From cabec8bb3fffb898dab9b6cb8f9e7acda4eebef9 Mon Sep 17 00:00:00 2001 From: F D Date: Thu, 6 Aug 2020 15:47:45 -0400 Subject: [PATCH 4/9] add unit html --- OpenOversight/app/templates/list_officer.html | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/OpenOversight/app/templates/list_officer.html b/OpenOversight/app/templates/list_officer.html index ea57575cd..68fdd6993 100644 --- a/OpenOversight/app/templates/list_officer.html +++ b/OpenOversight/app/templates/list_officer.html @@ -92,6 +92,22 @@

Rank

+
+
+

Unit

+
+
+
+
+ +
+
+
+

Age range

@@ -120,12 +136,12 @@

Age range

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']), + 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']), + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), location='top' %} {% include "partials/paginate_nav.html" %} {% endwith %} @@ -179,12 +195,12 @@

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']), + 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']), + unique_internal_identifier=form_data['unique_internal_identifier'], unit=form_data['unit']), location='bottom' %} {% include "partials/paginate_nav.html" %} {% endwith %} From 419c45623912576f236f783215bb5b5688e8e1bf Mon Sep 17 00:00:00 2001 From: F D Date: Thu, 6 Aug 2020 15:57:55 -0400 Subject: [PATCH 5/9] remove unit from browse officer form --- OpenOversight/app/main/forms.py | 1 - 1 file changed, 1 deletion(-) diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index cda397f3e..4f5b2808d 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -385,7 +385,6 @@ class IncidentForm(DateFieldForm): class BrowseForm(Form): rank = QuerySelectField('rank', validators=[Optional()], get_label='job_title', get_pk=lambda job: job.job_title) # query set in view function - unit = SelectField('unit', default='Not Sure') name = StringField('Last name') badge = StringField('Badge number') unique_internal_identifier = StringField('Unique ID') From dced1e33270cd3e5451a50edd9dbb983ddc774d1 Mon Sep 17 00:00:00 2001 From: F D Date: Thu, 6 Aug 2020 16:30:10 -0400 Subject: [PATCH 6/9] reset officer department filter --- 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 6b261048f..e94642294 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -523,7 +523,7 @@ def list_officer(department_id, page=1, race=[], gender=[], rank=[], min_age='16 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_by(department_id=department_id)\ + officers = filter_by_form(form_data, Officer.query, department_id).filter(Officer.department_id == department_id)\ .order_by(Officer.last_name).paginate(page, OFFICERS_PER_PAGE, False) choices = { From 7e0d1b5f38c14423bf96f293f2263a62577901e7 Mon Sep 17 00:00:00 2001 From: F D Date: Sun, 9 Aug 2020 14:14:12 -0400 Subject: [PATCH 7/9] Add missing unit definitions to forms, add to list_officer url call --- OpenOversight/app/main/views.py | 1 + OpenOversight/app/utils.py | 6 +++++- OpenOversight/tests/routes/test_officer_and_department.py | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index e94642294..78c617d6a 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -91,6 +91,7 @@ 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'], diff --git a/OpenOversight/app/utils.py b/OpenOversight/app/utils.py index 6baaf3f21..1ce538358 100644 --- a/OpenOversight/app/utils.py +++ b/OpenOversight/app/utils.py @@ -1,3 +1,5 @@ +from typing import Optional + from future.utils import iteritems from urllib.request import urlopen @@ -69,7 +71,9 @@ def get_or_create(session, model, defaults=None, **kwargs): return instance, True -def unit_choices(): +def unit_choices(department_id: Optional[int] = None): + if department_id is not None: + return db.session.query(Unit).filter_by(department_id=department_id).all() return db.session.query(Unit).all() diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index 2bbad96b2..4c6f1351c 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -922,7 +922,7 @@ def test_ac_can_edit_officer_in_their_dept(mockdata, client, session): with current_app.test_request_context(): login_ac(client) department = Department.query.filter_by(id=AC_DEPT).first() - unit = random.choice(unit_choices()) + unit = random.choice(unit_choices(department.id)) first_name = 'Testier' last_name = 'OTester' middle_initial = 'R' From 943b62c11755e1a8244eca67cdddfa94e6afd968 Mon Sep 17 00:00:00 2001 From: F D Date: Sun, 9 Aug 2020 14:39:53 -0400 Subject: [PATCH 8/9] add unit to other forms for validation --- OpenOversight/app/main/forms.py | 2 +- OpenOversight/tests/routes/test_officer_and_department.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/OpenOversight/app/main/forms.py b/OpenOversight/app/main/forms.py index 4f5b2808d..a60d0f313 100644 --- a/OpenOversight/app/main/forms.py +++ b/OpenOversight/app/main/forms.py @@ -46,7 +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') + 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))]) diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index 4c6f1351c..79b229a79 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -732,7 +732,8 @@ def test_ac_can_add_new_officer_in_their_dept(mockdata, client, session): star_no=666, job_title=job.id, department=department.id, - birth_year=1990) + birth_year=1990, + unit=None) data = process_form_data(form.data) From 0e59d9b89c586817ff4446adc5520415d211ecbc Mon Sep 17 00:00:00 2001 From: F D Date: Sun, 9 Aug 2020 16:07:45 -0400 Subject: [PATCH 9/9] add logging; fix tests --- OpenOversight/app/auth/views.py | 14 ++++++ OpenOversight/app/main/model_view.py | 4 +- OpenOversight/app/main/views.py | 45 +++++++++++++------ .../routes/test_officer_and_department.py | 5 +-- 4 files changed, 51 insertions(+), 17 deletions(-) diff --git a/OpenOversight/app/auth/views.py b/OpenOversight/app/auth/views.py index a04aafb02..feb9393d1 100644 --- a/OpenOversight/app/auth/views.py +++ b/OpenOversight/app/auth/views.py @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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) diff --git a/OpenOversight/app/main/model_view.py b/OpenOversight/app/main/model_view.py index 092c31501..8be49a0c0 100644 --- a/OpenOversight/app/main/model_view.py +++ b/OpenOversight/app/main/model_view.py @@ -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 @@ -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) diff --git a/OpenOversight/app/main/views.py b/OpenOversight/app/main/views.py index 78c617d6a..4f76c2018 100644 --- a/OpenOversight/app/main/views.py +++ b/OpenOversight/app/main/views.py @@ -98,6 +98,8 @@ def get_officer(): 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) @@ -106,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) @@ -119,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) @@ -213,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)) @@ -254,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) @@ -309,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) @@ -412,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) @@ -475,6 +487,7 @@ 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) @@ -589,6 +602,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) @@ -614,6 +628,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) @@ -633,6 +648,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) @@ -740,6 +756,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, @@ -776,6 +794,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) diff --git a/OpenOversight/tests/routes/test_officer_and_department.py b/OpenOversight/tests/routes/test_officer_and_department.py index 79b229a79..6a0d58355 100644 --- a/OpenOversight/tests/routes/test_officer_and_department.py +++ b/OpenOversight/tests/routes/test_officer_and_department.py @@ -732,8 +732,7 @@ def test_ac_can_add_new_officer_in_their_dept(mockdata, client, session): star_no=666, job_title=job.id, department=department.id, - birth_year=1990, - unit=None) + birth_year=1990) data = process_form_data(form.data) @@ -758,7 +757,7 @@ def test_ac_can_add_new_officer_with_unit_in_their_dept(mockdata, client, sessio with current_app.test_request_context(): login_ac(client) department = Department.query.filter_by(id=AC_DEPT).first() - unit = random.choice(unit_choices()) + unit = random.choice(unit_choices(department_id=department.id)) first_name = 'Testy' last_name = 'OTester' middle_initial = 'R'