Skip to content

Commit

Permalink
Fix upload bug (#1095)
Browse files Browse the repository at this point in the history
## Description of Changes
*Cherry-pick of
OrcaCollective#434

* Fixed bug introduced in
#1018 where anonymous
uploads were silently failing due to misplaced `@login_required`
decorator
* Audited all other redirect routes
* Fixed note_detail.html and description_detail.html

## Notes for Deployment
None!

## Screenshots (if appropriate)
N/A

## Tests and Linting
 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.
  • Loading branch information
sea-kelp authored Apr 8, 2024
1 parent d2c6dd4 commit 429869c
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 48 deletions.
30 changes: 13 additions & 17 deletions OpenOversight/app/main/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,7 @@ def sitemap_officers():
yield "main.officer_profile", {"officer_id": officer.id}


@main.route(
"/officer/<int:officer_id>/assignment/new",
methods=[HTTPMethod.GET, HTTPMethod.POST],
)
@main.route("/officer/<int:officer_id>/assignment/new", methods=[HTTPMethod.POST])
@ac_or_admin_required
def redirect_add_assignment(officer_id: int):
return redirect(
Expand Down Expand Up @@ -1059,7 +1056,7 @@ def list_officer(


@main.route("/department/<int:department_id>/ranks")
def redirect_get_dept_ranks(department_id: int = 0, is_sworn_officer: bool = False):
def redirect_get_dept_ranks(department_id: int, is_sworn_officer: bool = False):
flash(FLASH_MSG_PERMANENT_REDIRECT)
return redirect(
url_for(
Expand All @@ -1073,7 +1070,7 @@ def redirect_get_dept_ranks(department_id: int = 0, is_sworn_officer: bool = Fal

@main.route("/departments/<int:department_id>/ranks")
@main.route("/ranks")
def get_dept_ranks(department_id: int = 0, is_sworn_officer: bool = False):
def get_dept_ranks(department_id: Optional[int] = None, is_sworn_officer: bool = False):
if not department_id:
department_id = request.args.get("department_id")
if request.args.get("is_sworn_officer"):
Expand All @@ -1098,17 +1095,17 @@ def get_dept_ranks(department_id: int = 0, is_sworn_officer: bool = False):


@main.route("/department/<int:department_id>/units")
def redirect_get_dept_units(department_id: int = 0):
def redirect_get_dept_units(department_id: int):
flash(FLASH_MSG_PERMANENT_REDIRECT)
return redirect(
url_for("main.get_dept_ranks", department_id=department_id),
url_for("main.get_dept_units", department_id=department_id),
code=HTTPStatus.PERMANENT_REDIRECT,
)


@main.route("/departments/<int:department_id>/units")
@main.route("/units")
def get_dept_units(department_id: int = 0):
def get_dept_units(department_id: Optional[int] = None):
if not department_id:
department_id = request.args.get("department_id")

Expand Down Expand Up @@ -1292,7 +1289,6 @@ def delete_tag(tag_id: int):

@main.route("/tag/set_featured/<int:tag_id>", methods=[HTTPMethod.POST])
@login_required
@ac_or_admin_required
def redirect_set_featured_tag(tag_id: int):
flash(FLASH_MSG_PERMANENT_REDIRECT)
return redirect(
Expand Down Expand Up @@ -1342,7 +1338,7 @@ def leaderboard():


@main.route(
"/cop_face/department/<int:department_id>/images/<int:image_id>",
"/cop_face/department/<int:department_id>/image/<int:image_id>",
methods=[HTTPMethod.GET, HTTPMethod.POST],
)
@main.route("/cop_face/image/<int:image_id>", methods=[HTTPMethod.GET, HTTPMethod.POST])
Expand All @@ -1352,7 +1348,9 @@ def leaderboard():
)
@main.route("/cop_face/", methods=[HTTPMethod.GET, HTTPMethod.POST])
@login_required
def redirect_label_data(department_id: int = 0, image_id: int = 0):
def redirect_label_data(
department_id: Optional[int] = None, image_id: Optional[int] = None
):
flash(FLASH_MSG_PERMANENT_REDIRECT)
return redirect(
url_for("main.label_data", department_id=department_id, image_id=image_id),
Expand All @@ -1373,7 +1371,7 @@ def redirect_label_data(department_id: int = 0, image_id: int = 0):
)
@main.route("/cop_faces/", methods=[HTTPMethod.GET, HTTPMethod.POST])
@login_required
def label_data(department_id: int = 0, image_id: int = 0):
def label_data(department_id: Optional[int] = None, image_id: Optional[int] = None):
if department_id:
department = Department.query.filter_by(id=department_id).one()
if image_id:
Expand Down Expand Up @@ -1842,8 +1840,7 @@ def submit_officer_images(officer_id: int):
"/upload/department/<int:department_id>/officer/<int:officer_id>",
methods=[HTTPMethod.POST],
)
@login_required
def redirect_upload(department_id: int = 0, officer_id: int = 0):
def redirect_upload(department_id: int, officer_id: Optional[int] = None):
return redirect(
url_for("main.upload", department_id=department_id, officer_id=officer_id),
code=HTTPStatus.PERMANENT_REDIRECT,
Expand All @@ -1855,9 +1852,8 @@ def redirect_upload(department_id: int = 0, officer_id: int = 0):
"/upload/departments/<int:department_id>/officers/<int:officer_id>",
methods=[HTTPMethod.POST],
)
@login_required
@limiter.limit("250/minute")
def upload(department_id: int = 0, officer_id: int = 0):
def upload(department_id: int, officer_id: Optional[int] = None):
if officer_id:
officer = Officer.query.filter_by(id=officer_id).first()
if not officer:
Expand Down
2 changes: 2 additions & 0 deletions OpenOversight/app/models/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,7 @@


class AnonymousUser(AnonymousUserMixin):
id = None

def is_admin_or_coordinator(self, department: Department) -> bool:
return False
21 changes: 10 additions & 11 deletions OpenOversight/app/templates/description_detail.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
{% extends "base.html" %}
{% block page_title %}
Description Details
{% endblock page_title %}
{% block form %}
<p>
For officer with OOID {{ form.officer_id.data }}.
<br>
{{ form.description }}
</p>
{{ super() }}
{% endblock form %}
{% block content %}
<div class="container theme-showcase" role="main">
<div class="page-header">
<h1>Viewing Description {{ obj.id }} on Officer {{ obj.officer_id }}</h1>
</div>
<p class="lead">
<p>{{ obj.text_contents | markdown }}</p>
</p>
</div>
{% endblock content %}
2 changes: 1 addition & 1 deletion OpenOversight/app/templates/note_delete.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ <h1>Delete Note on officer {{ obj.officer_id }}</h1>
<p class="lead">
Are you sure you want to delete this note?
This cannot be undone.
<form action="{{ url_for('main.note_api_edit', obj_id=obj.id, officer_id=obj.officer_id) }}"
<form action="{{ url_for('main.note_api_delete', obj_id=obj.id, officer_id=obj.officer_id) }}"
method="post">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<button class="btn btn-danger" type="submit">Delete</button>
Expand Down
21 changes: 10 additions & 11 deletions OpenOversight/app/templates/note_detail.html
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
{% extends "base.html" %}
{% block page_title %}
Note Details
{% endblock page_title %}
{% block form %}
<p>
For officer with OOID {{ form.officer_id.data }}.
<br>
{{ form.description }}
</p>
{{ super() }}
{% endblock form %}
{% block content %}
<div class="container theme-showcase" role="main">
<div class="page-header">
<h1>Viewing Note {{ obj.id }} on Officer {{ obj.officer_id }}</h1>
</div>
<p class="lead">
<p>{{ obj.text_contents | markdown }}</p>
</p>
</div>
{% endblock content %}
70 changes: 62 additions & 8 deletions OpenOversight/tests/test_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ def login_admin(browser, server_port):
wait_for_element(browser, By.TAG_NAME, "body")


def logout(browser, server_port):
browser.get(f"http://localhost:{server_port}/auth/logout")
wait_for_page_load(browser)


def submit_image_to_dropzone(browser, img_path):
# Submit files in selenium: https://stackoverflow.com/a/61566075
wait_for_element(browser, By.CLASS_NAME, "dz-hidden-input")
upload = browser.find_element(By.CLASS_NAME, "dz-hidden-input")
upload.send_keys(img_path)
wait_for_element(browser, By.CLASS_NAME, "dz-success")


def wait_for_element(browser, locator, text, timeout=10):
try:
element_present = expected_conditions.presence_of_element_located(
Expand Down Expand Up @@ -359,12 +372,7 @@ def test_image_classification_and_tagging(mockdata, browser, server_port):
wait_for_page_load(browser)

Select(browser.find_element("id", "department")).select_by_value(dept_id)

# Submit files in selenium: https://stackoverflow.com/a/61566075
wait_for_element(browser, By.CLASS_NAME, "dz-hidden-input")
upload = browser.find_element(By.CLASS_NAME, "dz-hidden-input")
upload.send_keys(img_path)
wait_for_element(browser, By.CLASS_NAME, "dz-success")
submit_image_to_dropzone(browser, img_path)

# 4. Classify the uploaded image
browser.get(f"http://localhost:{server_port}/sort/departments/{dept_id}")
Expand Down Expand Up @@ -392,8 +400,7 @@ def test_image_classification_and_tagging(mockdata, browser, server_port):
assert "Tag added to database" in page_text

# 6. Log out as admin
browser.get(f"http://localhost:{server_port}/auth/logout")
wait_for_page_load(browser)
logout(browser, server_port)

# 7. Check that the tag appears on the officer page
browser.get(f"http://localhost:{server_port}/officers/{officer_id}")
Expand All @@ -417,3 +424,50 @@ def test_image_classification_and_tagging(mockdata, browser, server_port):
>= frame.location["y"] + frame.size["height"]
)
assert image.location["y"] <= frame.location["y"]


@pytest.mark.skip("Enable once real file upload in tests is supported.")
def test_anonymous_user_can_upload_image(mockdata, browser, server_port):
test_dir = os.path.dirname(os.path.realpath(__file__))
img_path = os.path.join(test_dir, "images/200Cat.jpeg")

login_admin(browser, server_port)

# 1. Create new department as admin (to avoid mockdata)
browser.get(f"http://localhost:{server_port}/departments/new")
wait_for_page_load(browser)
browser.find_element(By.ID, "name").send_keys("Auburn Police Department")
browser.find_element(By.ID, "short_name").send_keys("APD")
Select(browser.find_element(By.ID, "state")).select_by_value("WA")
browser.find_element(By.ID, "submit").click()
wait_for_page_load(browser)

# 2. Log out
logout(browser, server_port)

# 3. Upload image
browser.get(f"http://localhost:{server_port}/submit")
wait_for_page_load(browser)

dept_select = Select(browser.find_element("id", "department"))
dept_select.select_by_visible_text("[WA] Auburn Police Department")
dept_id = dept_select.first_selected_option.get_attribute("value")

submit_image_to_dropzone(browser, img_path)

# 4. Login as admin again
login_admin(browser, server_port)

# 5. Check that there is 1 image to classify
browser.get(f"http://localhost:{server_port}/sort/departments/{dept_id}")
wait_for_page_load(browser)

page_text = browser.find_element(By.TAG_NAME, "body").text
assert "Do you see uniformed law enforcement officers in the photo?" in page_text

browser.find_element(By.ID, "answer-yes").click()
wait_for_page_load(browser)

# 6. All images tagged!
page_text = browser.find_element(By.TAG_NAME, "body").text
assert "All images have been classified!" in page_text

0 comments on commit 429869c

Please sign in to comment.