-
Notifications
You must be signed in to change notification settings - Fork 78
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
Fix upload bug #1095
Fix upload bug #1095
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -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( | ||
|
@@ -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"): | ||
|
@@ -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): | ||
Comment on lines
-1111
to
+1108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer minimal assumptions when possible, but it's not a deal breaker. |
||
if not department_id: | ||
department_id = request.args.get("department_id") | ||
|
||
|
@@ -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( | ||
|
@@ -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>", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good find! |
||
methods=[HTTPMethod.GET, HTTPMethod.POST], | ||
) | ||
@main.route("/cop_face/image/<int:image_id>", methods=[HTTPMethod.GET, HTTPMethod.POST]) | ||
|
@@ -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), | ||
|
@@ -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: | ||
|
@@ -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, | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the line that was causing issues |
||
@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: | ||
|
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> | ||
michplunkett marked this conversation as resolved.
Show resolved
Hide resolved
|
||
<p class="lead"> | ||
<p>{{ obj.text_contents | markdown }}</p> | ||
</p> | ||
</div> | ||
{% endblock content %} |
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 %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( | ||
|
@@ -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}") | ||
|
@@ -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}") | ||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noice. |
||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find.