From ea6a5b6e7d748bc6f8b04957669ab9ebf50b55b2 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 6 Jul 2018 11:10:14 +0100 Subject: [PATCH 1/5] find_users_by_email view loads a page with search form and is linked to --- app/main/__init__.py | 1 + app/main/views/find_users.py | 16 ++++++++ .../views/find-users/find-users-by-email.html | 40 +++++++++++++++++++ .../views/platform-admin/_base_template.html | 1 + tests/app/main/views/test_find_users.py | 24 +++++++++++ 5 files changed, 82 insertions(+) create mode 100644 app/main/views/find_users.py create mode 100644 app/templates/views/find-users/find-users-by-email.html create mode 100644 tests/app/main/views/test_find_users.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 7510e236a..c42d86d65 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -26,6 +26,7 @@ from app.main.views import ( # noqa invites, feedback, providers, + find_users, platform_admin, letter_jobs, email_branding, diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py new file mode 100644 index 000000000..80135d9b0 --- /dev/null +++ b/app/main/views/find_users.py @@ -0,0 +1,16 @@ +from flask import abort, render_template, request, url_for +from flask_login import login_required + +from app.main import main +from app.utils import user_is_platform_admin +from app.main.forms import SearchUsersForm + + +@main.route("/find-users-by-email", methods=['GET']) +@login_required +@user_is_platform_admin +def find_users_by_email(): + return render_template( + 'views/find-users/find-users-by-email.html', + form=SearchUsersForm(), + ) diff --git a/app/templates/views/find-users/find-users-by-email.html b/app/templates/views/find-users/find-users-by-email.html new file mode 100644 index 000000000..650b5cdde --- /dev/null +++ b/app/templates/views/find-users/find-users-by-email.html @@ -0,0 +1,40 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/page-footer.html" import page_footer %} + +{% block per_page_title %} + Find users by e-mail +{% endblock %} + +{% block org_page_title %} + Find users by e-mail +{% endblock %} + +{% block platform_admin_content %} + +

+ Find users by e-mail +

+ +
+
+ {{ textbox( + form.search, + width='1-1', + label='Find users by e-mail' + ) }} +
+
+ + +
+
+ +
+ +
+ +{% endblock %} diff --git a/app/templates/views/platform-admin/_base_template.html b/app/templates/views/platform-admin/_base_template.html index 4434534f0..7ea7ce43c 100644 --- a/app/templates/views/platform-admin/_base_template.html +++ b/app/templates/views/platform-admin/_base_template.html @@ -20,6 +20,7 @@ ('Email branding', url_for('main.email_branding')), ('Letter jobs', url_for('main.letter_jobs')), ('Inbound SMS numbers', url_for('main.inbound_sms_admin')), + ('Find users by email', url_for('main.find_users_by_email')), ('Email Complaints', url_for('main.platform_admin_list_complaints')) ] %}
  • diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py new file mode 100644 index 000000000..0f061a53e --- /dev/null +++ b/tests/app/main/views/test_find_users.py @@ -0,0 +1,24 @@ +import pytest +from flask import url_for +from lxml import html +from app.main.views.find_users import find_users_by_email + +from tests.conftest import mock_get_user + +def test_find_users_by_email_page_loads_correctly( + client, + platform_admin_user, + mocker +): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + + client.login(platform_admin_user) + response = client.get(url_for('main.find_users_by_email')) + + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + header = document.xpath('//h1')[0].text + assert "Find users by e-mail" in header + assert len(document.xpath("//input[@type='search']")) > 0 From d1a05e2ec5d7d3c9d9f9d83ff0871a96453b8e37 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 6 Jul 2018 16:16:21 +0100 Subject: [PATCH 2/5] find_users_by_email view calls API and feeds results to template Template then displays the results. Page displays a message if no results found --- app/main/views/find_users.py | 10 ++++-- app/notify_client/user_api_client.py | 6 ++++ .../views/find-users/find-users-by-email.html | 17 +++++++++- tests/app/main/views/test_find_users.py | 33 +++++++++++++++++-- 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 80135d9b0..96992df19 100644 --- a/app/main/views/find_users.py +++ b/app/main/views/find_users.py @@ -1,16 +1,22 @@ from flask import abort, render_template, request, url_for from flask_login import login_required +from app import user_api_client from app.main import main from app.utils import user_is_platform_admin from app.main.forms import SearchUsersForm -@main.route("/find-users-by-email", methods=['GET']) +@main.route("/find-users-by-email", methods=['GET', 'POST']) @login_required @user_is_platform_admin def find_users_by_email(): + form = SearchUsersForm() + users_found = None + if form.validate_on_submit(): + users_found = user_api_client.find_users_by_full_or_partial_email(form.search.data)['data'] return render_template( 'views/find-users/find-users-by-email.html', - form=SearchUsersForm(), + form=form, + users_found=users_found ) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index c0e3a28ef..02b9bfd5b 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -179,6 +179,12 @@ class UserApiClient(NotifyAdminAPIClient): data = {'email': email_address} self.post(endpoint, data=data) + def find_users_by_full_or_partial_email(self, email_address): + endpoint = '/user/find-users-by-email' + data = {'email': email_address} + users = self.post(endpoint, data=data) + return users + def is_email_already_in_use(self, email_address): if self.get_user_by_email_or_none(email_address): return True diff --git a/app/templates/views/find-users/find-users-by-email.html b/app/templates/views/find-users/find-users-by-email.html index 650b5cdde..1ff768d0f 100644 --- a/app/templates/views/find-users/find-users-by-email.html +++ b/app/templates/views/find-users/find-users-by-email.html @@ -24,7 +24,7 @@ {{ textbox( form.search, width='1-1', - label='Find users by e-mail' + label='Find users by e-mail, or by partial e-mail' ) }}
    @@ -37,4 +37,19 @@ + {% if users_found %} + + {% elif users_found == [] %} +

    No users found.

    + {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index 0f061a53e..eddc35557 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -3,6 +3,7 @@ from flask import url_for from lxml import html from app.main.views.find_users import find_users_by_email +from tests import user_json from tests.conftest import mock_get_user def test_find_users_by_email_page_loads_correctly( @@ -11,14 +12,40 @@ def test_find_users_by_email_page_loads_correctly( mocker ): mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) - client.login(platform_admin_user) response = client.get(url_for('main.find_users_by_email')) - assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) header = document.xpath('//h1')[0].text assert "Find users by e-mail" in header assert len(document.xpath("//input[@type='search']")) > 0 + + +def test_find_users_by_email_displays_users_found( + client, + platform_admin_user, + mocker +): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [user_json()]}, autospec=True) + response = client.post(url_for('main.find_users_by_email', data=[{"email": "twilight.sparkle"}])) + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + assert "Test User" in document.text_content() + +def test_find_users_by_email_displays_message_if_no_users_found( + client, + platform_admin_user, + mocker +): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": []}, autospec=True) + response = client.post(url_for('main.find_users_by_email', data=[{"email": "twilight.sparkle"}])) + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + assert "No users found." in document.text_content() From 57e9c1d6e6f749c9e67847cd82ffafc89832af2e Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 10 Jul 2018 16:33:13 +0100 Subject: [PATCH 3/5] Validate against empty form submission for find_users_by_email This included: - creating a new form SearchUsersByEmailForm with validation on its search field - introducing 400 status to the view if the form does not validate - fixing the POST request data structure in the tests (it was incorrect before and uncaught due to lack of validation and mocking the response from the API. --- app/main/forms.py | 9 +++++++++ app/main/views/find_users.py | 9 ++++++--- tests/app/main/views/test_find_users.py | 18 ++++++++++++++++-- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 826080376..2a2e3a5df 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -853,6 +853,15 @@ class SearchTemplatesForm(StripWhitespaceForm): search = SearchField('Search by name') +class SearchUsersByEmailForm(StripWhitespaceForm): + + search = SearchField('Search by name or email address', + validators=[ + DataRequired("You need to enter full or partial e-mail address to search by.") + ] + ) + + class SearchUsersForm(StripWhitespaceForm): search = SearchField('Search by name or email address') diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 96992df19..13c98fb50 100644 --- a/app/main/views/find_users.py +++ b/app/main/views/find_users.py @@ -4,19 +4,22 @@ from flask_login import login_required from app import user_api_client from app.main import main from app.utils import user_is_platform_admin -from app.main.forms import SearchUsersForm +from app.main.forms import SearchUsersByEmailForm @main.route("/find-users-by-email", methods=['GET', 'POST']) @login_required @user_is_platform_admin def find_users_by_email(): - form = SearchUsersForm() + form = SearchUsersByEmailForm() users_found = None + status = 200 if form.validate_on_submit(): users_found = user_api_client.find_users_by_full_or_partial_email(form.search.data)['data'] + elif request.method == 'POST': + status = 400 return render_template( 'views/find-users/find-users-by-email.html', form=form, users_found=users_found - ) + ), status diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index eddc35557..ace7e803c 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -30,7 +30,7 @@ def test_find_users_by_email_displays_users_found( mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [user_json()]}, autospec=True) - response = client.post(url_for('main.find_users_by_email', data=[{"email": "twilight.sparkle"}])) + response = client.post(url_for('main.find_users_by_email'), data={"search": "twilight.sparkle"}) assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) @@ -44,8 +44,22 @@ def test_find_users_by_email_displays_message_if_no_users_found( mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": []}, autospec=True) - response = client.post(url_for('main.find_users_by_email', data=[{"email": "twilight.sparkle"}])) + response = client.post(url_for('main.find_users_by_email'), data={"search": "twilight.sparkle"}) assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) assert "No users found." in document.text_content() + + +def test_find_users_by_email_validates_against_empty_search_submission( + client, + platform_admin_user, + mocker +): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + response = client.post(url_for('main.find_users_by_email'), data={"search": ""}) + assert response.status_code == 400 + + document = html.fromstring(response.get_data(as_text=True)) + assert "You need to enter full or partial e-mail address to search by." in document.text_content() From 4cd465753a870951b8322bf275924d93d964df0e Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 10 Jul 2018 17:24:20 +0100 Subject: [PATCH 4/5] Add view that displays user information, including: - name - email - phone number - services - last login - failed login attempts if any The view can be accessed from results of find_users_by_email logged_in_at added to User serialization on admin frontend as a part of this work --- app/main/forms.py | 7 +- app/main/views/find_users.py | 15 ++- app/navigation.py | 10 +- app/notify_client/models.py | 1 + .../views/find-users/find-users-by-email.html | 14 +-- .../views/find-users/user-information.html | 43 +++++++ tests/app/main/views/test_find_users.py | 115 +++++++++++++++--- 7 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 app/templates/views/find-users/user-information.html diff --git a/app/main/forms.py b/app/main/forms.py index 2a2e3a5df..f0f7d790a 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -855,10 +855,11 @@ class SearchTemplatesForm(StripWhitespaceForm): class SearchUsersByEmailForm(StripWhitespaceForm): - search = SearchField('Search by name or email address', + search = SearchField( + 'Search by name or email address', validators=[ - DataRequired("You need to enter full or partial e-mail address to search by.") - ] + DataRequired("You need to enter full or partial email address to search by.") + ], ) diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 13c98fb50..23a961afe 100644 --- a/app/main/views/find_users.py +++ b/app/main/views/find_users.py @@ -1,10 +1,10 @@ -from flask import abort, render_template, request, url_for +from flask import render_template, request from flask_login import login_required from app import user_api_client from app.main import main -from app.utils import user_is_platform_admin from app.main.forms import SearchUsersByEmailForm +from app.utils import user_is_platform_admin @main.route("/find-users-by-email", methods=['GET', 'POST']) @@ -23,3 +23,14 @@ def find_users_by_email(): form=form, users_found=users_found ), status + + +@main.route("/users/", methods=['GET']) +@login_required +@user_is_platform_admin +def user_information(user_id): + user = user_api_client.get_user(user_id) + return render_template( + 'views/find-users/user-information.html', + user=user + ) diff --git a/app/navigation.py b/app/navigation.py index 4860e4adf..076af3d12 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -76,15 +76,17 @@ class HeaderNavigation(Navigation): 'add_organisation', 'create_email_branding', 'email_branding', + 'find_users_by_email', 'live_services', 'organisations', 'platform_admin', + 'platform_admin_list_complaints', 'suspend_service', 'trial_services', 'update_email_branding', + 'user_information', 'view_provider', 'view_providers', - 'platform_admin_list_complaints', }, 'sign-in': { 'sign_in', @@ -399,6 +401,7 @@ class MainNavigation(Navigation): 'error', 'features', 'feedback', + 'find_users_by_email', 'forgot_password', 'get_example_csv', 'get_notifications_as_json', @@ -472,6 +475,7 @@ class MainNavigation(Navigation): 'two_factor_email', 'two_factor_email_sent', 'update_email_branding', + 'user_information', 'user_profile', 'user_profile_email', 'user_profile_email_authenticate', @@ -567,6 +571,7 @@ class CaseworkNavigation(Navigation): 'error', 'features', 'feedback', + 'find_users_by_email', 'forgot_password', 'get_example_csv', 'get_notifications_as_json', @@ -687,6 +692,7 @@ class CaseworkNavigation(Navigation): 'two_factor_email_sent', 'update_email_branding', 'usage', + 'user_information', 'user_profile', 'user_profile_email', 'user_profile_email_authenticate', @@ -789,6 +795,7 @@ class OrgNavigation(Navigation): 'error', 'features', 'feedback', + 'find_users_by_email', 'forgot_password', 'get_example_csv', 'get_notifications_as_json', @@ -908,6 +915,7 @@ class OrgNavigation(Navigation): 'two_factor_email_sent', 'update_email_branding', 'usage', + 'user_information', 'user_profile', 'user_profile_email', 'user_profile_email_authenticate', diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 3ccdce48b..c3d2564a2 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -59,6 +59,7 @@ class User(UserMixin): self.failed_login_count = fields.get('failed_login_count') self.state = fields.get('state') self.max_failed_login_count = max_failed_login_count + self.logged_in_at = fields.get('logged_in_at') self.platform_admin = fields.get('platform_admin') self.current_session_id = fields.get('current_session_id') self.services = fields.get('services', []) diff --git a/app/templates/views/find-users/find-users-by-email.html b/app/templates/views/find-users/find-users-by-email.html index 1ff768d0f..7ad9939bb 100644 --- a/app/templates/views/find-users/find-users-by-email.html +++ b/app/templates/views/find-users/find-users-by-email.html @@ -2,17 +2,13 @@ {% from "components/page-footer.html" import page_footer %} {% block per_page_title %} - Find users by e-mail -{% endblock %} - -{% block org_page_title %} - Find users by e-mail + Find users by email {% endblock %} {% block platform_admin_content %}

    - Find users by e-mail + Find users by email

    @@ -42,8 +38,8 @@
      {% for user in users_found %}
    • - {{ user.email_address }} -

      {{ user.name }}

      + {{ user.email_address }} +

      {{ user.name }}

      • {% endfor %} diff --git a/app/templates/views/find-users/user-information.html b/app/templates/views/find-users/user-information.html new file mode 100644 index 000000000..5c9281615 --- /dev/null +++ b/app/templates/views/find-users/user-information.html @@ -0,0 +1,43 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/page-footer.html" import page_footer %} + +{% block per_page_title %} + User information for {{ user.name }} +{% endblock %} + +{% block platform_admin_content %} +
      +
      +

      + {{ user.name }} +

      +

      {{ user.email_address }}

      +

      {{ user.mobile_number }}

      +

      Services

      + +

      Last login

      + {% if not user.logged_in_at %} +

      This person has never logged in

      + {% else %} +

      Last logged in + +

      + {% endif %} + {% if user.failed_login_count > 0 %} +

      + {{ user.failed_login_count }} failed login attempts +

      + {% endif %} +
      +
      +{% endblock %} diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index ace7e803c..9d3394776 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -1,15 +1,15 @@ -import pytest from flask import url_for from lxml import html -from app.main.views.find_users import find_users_by_email +from app.notify_client.user_api_client import User from tests import user_json from tests.conftest import mock_get_user + def test_find_users_by_email_page_loads_correctly( - client, - platform_admin_user, - mocker + client, + platform_admin_user, + mocker ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) @@ -17,29 +17,54 @@ def test_find_users_by_email_page_loads_correctly( assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) - header = document.xpath('//h1')[0].text - assert "Find users by e-mail" in header + assert document.xpath("//h1/text()[normalize-space()='Find users by email']") assert len(document.xpath("//input[@type='search']")) > 0 def test_find_users_by_email_displays_users_found( - client, - platform_admin_user, - mocker + client, + platform_admin_user, + mocker ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) - mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [user_json()]}, autospec=True) + mocker.patch( + 'app.user_api_client.find_users_by_full_or_partial_email', + return_value={"data": [user_json()]}, + autospec=True, + ) response = client.post(url_for('main.find_users_by_email'), data={"search": "twilight.sparkle"}) assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) - assert "Test User" in document.text_content() + assert document.xpath("//a/text()[normalize-space()='test@gov.uk']") + assert document.xpath("//p/text()[normalize-space()='Test User']") + + +def test_find_users_by_email_displays_multiple_users( + client, + platform_admin_user, + mocker +): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [ + user_json(name="Apple Jack"), + user_json(name="Apple Bloom") + ]}, autospec=True) + response = client.post(url_for('main.find_users_by_email'), data={"search": "apple"}) + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + + assert document.xpath("//p/text()[normalize-space()='Apple Jack']") + assert document.xpath("//p/text()[normalize-space()='Apple Bloom']") + def test_find_users_by_email_displays_message_if_no_users_found( - client, - platform_admin_user, - mocker + client, + platform_admin_user, + mocker ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) @@ -48,13 +73,13 @@ def test_find_users_by_email_displays_message_if_no_users_found( assert response.status_code == 200 document = html.fromstring(response.get_data(as_text=True)) - assert "No users found." in document.text_content() + assert document.xpath("//p/text()[normalize-space()='No users found.']") def test_find_users_by_email_validates_against_empty_search_submission( - client, - platform_admin_user, - mocker + client, + platform_admin_user, + mocker ): mock_get_user(mocker, user=platform_admin_user) client.login(platform_admin_user) @@ -62,4 +87,54 @@ def test_find_users_by_email_validates_against_empty_search_submission( assert response.status_code == 400 document = html.fromstring(response.get_data(as_text=True)) - assert "You need to enter full or partial e-mail address to search by." in document.text_content() + expected_message = "You need to enter full or partial email address to search by." + assert document.xpath( + "//span[contains(@class, 'error-message') and normalize-space(text()) = '{}']".format(expected_message) + ) + + +def test_user_information_page_shows_information_about_user( + client, + platform_admin_user, + mocker +): + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + User(user_json(name="Apple Bloom", services=[ + {"id": 1, "name": "Fresh Orchard Juice"}, + {"id": 2, "name": "Nature Therapy"}, + ])) + ], autospec=True) + client.login(platform_admin_user) + response = client.get(url_for('main.user_information', user_id=345)) + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + + assert document.xpath("//h1/text()[normalize-space()='Apple Bloom']") + assert document.xpath("//p/text()[normalize-space()='test@gov.uk']") + assert document.xpath("//p/text()[normalize-space()='+447700900986']") + + assert document.xpath("//h2/text()[normalize-space()='Services']") + assert document.xpath("//p/text()[normalize-space()='Fresh Orchard Juice']") + assert document.xpath("//p/text()[normalize-space()='Nature Therapy']") + + assert document.xpath("//h2/text()[normalize-space()='Last login']") + assert not document.xpath("//p/text()[normalize-space()='0 failed login attempts']") + + +def test_user_information_page_displays_if_there_are_failed_login_attempts( + client, + platform_admin_user, + mocker +): + mocker.patch('app.user_api_client.get_user', side_effect=[ + platform_admin_user, + User(user_json(name="Apple Bloom", failed_login_count=2)) + ], autospec=True) + client.login(platform_admin_user) + response = client.get(url_for('main.user_information', user_id=345)) + assert response.status_code == 200 + + document = html.fromstring(response.get_data(as_text=True)) + assert document.xpath("//p/text()[normalize-space()='2 failed login attempts']") From 8258de084c0b72230aeb0d6b870d0992e13a48d8 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 16 Jul 2018 15:04:03 +0100 Subject: [PATCH 5/5] Change part of the tests to use client_request Two tests retained the old syntax because of mocker conflict: when logging in as a user through client_request, it sets up a side_effect on user_api_client.get_user to the user you log in as. If you later want to set return_value for get_user to something else, problems start :d. --- .../views/find-users/find-users-by-email.html | 2 +- tests/app/main/views/test_find_users.py | 93 +++++++++---------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/app/templates/views/find-users/find-users-by-email.html b/app/templates/views/find-users/find-users-by-email.html index 7ad9939bb..2237b70c5 100644 --- a/app/templates/views/find-users/find-users-by-email.html +++ b/app/templates/views/find-users/find-users-by-email.html @@ -46,6 +46,6 @@
    {% elif users_found == [] %} -

    No users found.

    +

    No users found.

    {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index 9d3394776..6f2f51c4d 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -3,94 +3,87 @@ from lxml import html from app.notify_client.user_api_client import User from tests import user_json -from tests.conftest import mock_get_user -def test_find_users_by_email_page_loads_correctly( - client, - platform_admin_user, - mocker -): - mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) - response = client.get(url_for('main.find_users_by_email')) - assert response.status_code == 200 +def test_find_users_by_email_page_loads_correctly(client_request, platform_admin_user): + client_request.login(platform_admin_user) + document = client_request.get('main.find_users_by_email') - document = html.fromstring(response.get_data(as_text=True)) - assert document.xpath("//h1/text()[normalize-space()='Find users by email']") - assert len(document.xpath("//input[@type='search']")) > 0 + assert document.h1.text.strip() == 'Find users by email' + assert len(document.find_all('input', {'type': 'search'})) > 0 def test_find_users_by_email_displays_users_found( - client, + client_request, platform_admin_user, mocker ): - mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) + client_request.login(platform_admin_user) mocker.patch( 'app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [user_json()]}, autospec=True, ) - response = client.post(url_for('main.find_users_by_email'), data={"search": "twilight.sparkle"}) - assert response.status_code == 200 + document = client_request.post( + 'main.find_users_by_email', + _data={"search": "twilight.sparkle"}, + _expected_status=200 + ) - document = html.fromstring(response.get_data(as_text=True)) - assert document.xpath("//a/text()[normalize-space()='test@gov.uk']") - assert document.xpath("//p/text()[normalize-space()='Test User']") + assert any(element.text.strip() == 'test@gov.uk' for element in document.find_all( + 'a', {'class': 'browse-list-link'}, href=True) + ) + assert any(element.text.strip() == 'Test User' for element in document.find_all('p', {'class': 'browse-list-hint'})) + + assert document.find('a', {'class': 'browse-list-link'}).text.strip() == 'test@gov.uk' + assert document.find('p', {'class': 'browse-list-hint'}).text.strip() == 'Test User' def test_find_users_by_email_displays_multiple_users( - client, + client_request, platform_admin_user, mocker ): - mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) - mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": [ - user_json(name="Apple Jack"), - user_json(name="Apple Bloom") - ]}, autospec=True) - response = client.post(url_for('main.find_users_by_email'), data={"search": "apple"}) - assert response.status_code == 200 + client_request.login(platform_admin_user) + mocker.patch( + 'app.user_api_client.find_users_by_full_or_partial_email', + return_value={"data": [user_json(name="Apple Jack"), user_json(name="Apple Bloom")]}, + autospec=True, + ) + document = client_request.post('main.find_users_by_email', _data={"search": "apple"}, _expected_status=200) - document = html.fromstring(response.get_data(as_text=True)) - - assert document.xpath("//p/text()[normalize-space()='Apple Jack']") - assert document.xpath("//p/text()[normalize-space()='Apple Bloom']") + assert any( + element.text.strip() == 'Apple Jack' for element in document.find_all('p', {'class': 'browse-list-hint'}) + ) + assert any( + element.text.strip() == 'Apple Bloom' for element in document.find_all('p', {'class': 'browse-list-hint'}) + ) def test_find_users_by_email_displays_message_if_no_users_found( - client, + client_request, platform_admin_user, mocker ): - mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) + client_request.login(platform_admin_user) mocker.patch('app.user_api_client.find_users_by_full_or_partial_email', return_value={"data": []}, autospec=True) - response = client.post(url_for('main.find_users_by_email'), data={"search": "twilight.sparkle"}) - assert response.status_code == 200 + document = client_request.post( + 'main.find_users_by_email', _data={"search": "twilight.sparkle"}, _expected_status=200 + ) - document = html.fromstring(response.get_data(as_text=True)) - assert document.xpath("//p/text()[normalize-space()='No users found.']") + assert document.find('p', {'class': 'browse-list-hint'}).text.strip() == 'No users found.' def test_find_users_by_email_validates_against_empty_search_submission( - client, + client_request, platform_admin_user, mocker ): - mock_get_user(mocker, user=platform_admin_user) - client.login(platform_admin_user) - response = client.post(url_for('main.find_users_by_email'), data={"search": ""}) - assert response.status_code == 400 + client_request.login(platform_admin_user) + document = client_request.post('main.find_users_by_email', _data={"search": ""}, _expected_status=400) - document = html.fromstring(response.get_data(as_text=True)) expected_message = "You need to enter full or partial email address to search by." - assert document.xpath( - "//span[contains(@class, 'error-message') and normalize-space(text()) = '{}']".format(expected_message) - ) + assert document.find('span', {'class': 'error-message'}).text.strip() == expected_message def test_user_information_page_shows_information_about_user(