Fix upload bugs (#434)

* Fix upload bugs

* Fix note deletion

* Log upload errors

OpenOversight/app/main/views.py

@@ -367,10 +367,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(
@@ -1061,7 +1058,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(
@@ -1075,7 +1072,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"):
@@ -1100,17 +1097,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")
 
@@ -1294,7 +1291,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(
@@ -1338,7 +1334,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])
@@ -1348,7 +1344,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),
@@ -1369,7 +1367,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):
  jsloads = ["js/cropper.js", "js/tagger.js"]
  if department_id:
  department = Department.query.filter_by(id=department_id).one()
@@ -1864,8 +1862,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,
@@ -1877,9 +1874,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:
@@ -1907,6 +1903,7 @@ def upload(department_id: int = 0, officer_id: int = 0):
  )
  except ValueError:
  # Raised if MIME type not allowed
+ current_app.logger.exception("Invalid data type!")
  return jsonify(error="Invalid data type!"), HTTPStatus.UNSUPPORTED_MEDIA_TYPE
 
  if image:

OpenOversight/app/models/users.py

@@ -4,5 +4,7 @@
 
 
 class AnonymousUser(AnonymousUserMixin):
+ id = None
+
  def is_admin_or_coordinator(self, department: Department) -> bool:
  return False

OpenOversight/app/templates/description_detail.html

@@ -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 }}</p>
+ </p>
+ </div>
+{% endblock content %}

OpenOversight/app/templates/note_delete.html

@@ -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>

OpenOversight/app/templates/note_detail.html

@@ -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 }}</p>
+ </p>
+ </div>
+{% endblock content %}

OpenOversight/tests/test_functional.py

@@ -42,6 +42,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(
@@ -377,12 +390,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}")
@@ -410,8 +418,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}")
@@ -435,3 +442,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.xdist_group
+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