From 17a4d8ef3bc42cd3d95692c57d44d0c41070043e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 2 Feb 2017 09:15:48 +0000 Subject: [PATCH] Use boolean logic instead of any/all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using and/or over any/all has a couple of advantages: - it's a bit quicker - it won't evaluate the second half at all if the first half fails – if it is in business hours, and convert_to_boolean would raise, with your use of all we'd throw a 500, whereas if we had or, business_hours would trip and we'd skip over the second half without worrying about exceptions any and all are designed for use with variable length args eg `any(x for x in thing())` --- app/main/views/feedback.py | 14 +++++++------- tests/app/main/views/test_feedback.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index ce8143815..05b9f6fe7 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -50,15 +50,15 @@ def feedback(ticket_type): severe = request.args.get('severe') - urgent = any(( - in_business_hours(), + urgent = ( + in_business_hours() or (ticket_type == 'problem' and convert_to_boolean(severe)) - )) + ) - anonymous = all(( - (not form.email_address.data), - (not current_user.is_authenticated), - )) + anonymous = ( + (not form.email_address.data) and + (not current_user.is_authenticated) + ) if needs_triage(ticket_type, severe): session['feedback_message'] = form.feedback.data diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index e9477c139..3e7689a2a 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -157,7 +157,7 @@ def test_passes_user_details_through_flow( ]) @pytest.mark.parametrize('ticket_type, expected_response, expected_redirect, expected_error', [ ('problem', 200, no_redirect(), element.Tag), - ('question', 302, partial(url_for, 'main.thanks', urgent=True, anonymous=True), type(None)), + ('question', 302, partial(url_for, 'main.thanks', anonymous=True, urgent=True), type(None)), ]) def test_email_address_required_for_problems( client,