diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 3ed7ac7ff..0ed602f9c 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -9,9 +9,10 @@ def test_non_gov_user_cannot_see_add_service_button( mock_login, mock_get_non_govuser, api_nongov_user_active, + mock_get_organisations_and_services_for_user ): client.login(api_nongov_user_active) - response = client.get(url_for('main.choose_service')) + response = client.get(url_for('main.choose_account')) assert 'Add a new service' not in response.get_data(as_text=True) assert response.status_code == 200 diff --git a/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_accounts.py similarity index 76% rename from tests/app/main/views/test_choose_services.py rename to tests/app/main/views/test_choose_accounts.py index 44795d5c3..004c161eb 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_accounts.py @@ -2,14 +2,14 @@ from bs4 import BeautifulSoup from flask import url_for -def test_should_show_choose_services_page( +def test_should_show_choose_accounts_page( logged_in_client, mock_login, mock_get_user, api_user_active, mock_get_services, ): - response = logged_in_client.get(url_for('main.choose_service')) + response = logged_in_client.get(url_for('main.choose_account')) assert response.status_code == 200 resp_data = response.get_data(as_text=True) @@ -20,14 +20,14 @@ def test_should_show_choose_services_page( assert services['data'][1]['name'] in resp_data -def test_should_show_choose_services_page_if_no_services( +def test_should_show_choose_accounts_page_if_no_services( logged_in_client, mock_login, api_user_active, ): # if users last service has been archived there'll be no services # mock_login already patches get_services to return no data - response = logged_in_client.get(url_for('main.choose_service')) + response = logged_in_client.get(url_for('main.choose_account')) assert response.status_code == 200 resp_data = response.get_data(as_text=True) assert 'Choose service' in resp_data @@ -40,7 +40,7 @@ def test_redirect_if_only_one_service( api_user_active, mock_get_services_with_one_service, ): - response = logged_in_client.get(url_for('main.show_all_services_or_dashboard')) + response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) service = mock_get_services_with_one_service.side_effect()['data'][0] assert response.status_code == 302 @@ -52,10 +52,10 @@ def test_redirect_if_multiple_services( mock_login, api_user_active, ): - response = logged_in_client.get(url_for('main.show_all_services_or_dashboard')) + response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) def test_redirect_if_service_in_session( @@ -67,7 +67,7 @@ def test_redirect_if_service_in_session( ): with logged_in_client.session_transaction() as session: session['service_id'] = '147ad62a-2951-4fa1-9ca0-093cd1a52c52' - response = logged_in_client.get(url_for('main.show_all_services_or_dashboard')) + response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 assert response.location == url_for( @@ -81,7 +81,7 @@ def test_should_redirect_if_not_logged_in( logged_in_client, app_ ): - response = logged_in_client.get(url_for('main.show_all_services_or_dashboard')) + response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 assert url_for('main.index', _external=True) in response.location @@ -89,12 +89,12 @@ def test_should_redirect_if_not_logged_in( def test_should_show_back_to_service_link( logged_in_client ): - response = logged_in_client.get(url_for('main.choose_service')) + response = logged_in_client.get(url_for('main.choose_account')) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.select('.navigation-service a')[0]['href'] == ( - url_for('main.show_all_services_or_dashboard') + url_for('main.show_accounts_or_dashboard') ) @@ -105,7 +105,7 @@ def test_should_not_show_back_to_service_link_if_no_service_in_session( mock_get_services_with_no_services, ): client.login(api_user_active) - response = client.get(url_for('main.choose_service')) + response = client.get(url_for('main.choose_account')) assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 0c5b05ff6..eb08b0314 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -22,7 +22,7 @@ def test_non_logged_in_user_can_see_homepage( ) -def test_logged_in_user_redirects_to_choose_service( +def test_logged_in_user_redirects_to_choose_account( logged_in_client, api_user_active, mock_get_user, @@ -33,7 +33,7 @@ def test_logged_in_user_redirects_to_choose_service( assert response.status_code == 302 response = logged_in_client.get(url_for('main.sign_in', follow_redirects=True)) - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) @pytest.mark.parametrize('view', [ diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index 732ae86b6..93b5a425f 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -100,7 +100,7 @@ def test_should_sign_in_when_password_reset_is_successful_for_email_auth( response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 - assert response.location == url_for('.choose_service', _external=True) + assert response.location == url_for('.choose_account', _external=True) assert mock_get_user_by_email_request_password_reset.called assert mock_reset_failed_login_count.called diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 5757b8175..3a861c4ed 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -18,7 +18,7 @@ def test_render_register_returns_template_with_form(client): assert 'Create an account' in response.get_data(as_text=True) -def test_logged_in_user_redirects_to_choose_service( +def test_logged_in_user_redirects_to_choose_account( logged_in_client, api_user_active, mock_get_user_by_email, @@ -29,7 +29,7 @@ def test_logged_in_user_redirects_to_choose_service( assert response.status_code == 302 response = logged_in_client.get(url_for('main.sign_in', follow_redirects=True)) - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) @pytest.mark.parametrize('phone_number_to_register_with', [ @@ -78,7 +78,7 @@ def test_register_continue_handles_missing_session_sensibly( # session is not set response = client.get(url_for('main.registration_continue')) assert response.status_code == 302 - assert response.location == url_for('main.show_all_services_or_dashboard', _external=True) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) def test_process_register_returns_200_when_mobile_number_is_invalid( diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index c35a1807a..bcfaf98a4 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -38,12 +38,15 @@ def test_sign_in_explains_other_browser(logged_in_client, api_user_active, mocke assert 'We signed you out because you logged in to Notify on another device' in response.get_data(as_text=True) -def test_doesnt_redirect_to_sign_in_if_no_session_info(logged_in_client, api_user_active): +def test_doesnt_redirect_to_sign_in_if_no_session_info( + logged_in_client, api_user_active +): assert api_user_active.current_session_id is None + with logged_in_client.session_transaction() as session: session['current_session_id'] = None - response = logged_in_client.get(url_for('main.choose_service')) + response = logged_in_client.get(url_for('main.add_service')) assert response.status_code == 200 @@ -65,16 +68,16 @@ def test_redirect_to_sign_in_if_logged_in_from_other_browser( with logged_in_client.session_transaction() as session: session['current_session_id'] = str(cookie_sess_id) - response = logged_in_client.get(url_for('main.choose_service')) + response = logged_in_client.get(url_for('main.choose_account')) assert response.status_code == 302 - assert response.location == url_for('main.sign_in', next='/services', _external=True) + assert response.location == url_for('main.sign_in', next='/accounts', _external=True) -def test_logged_in_user_redirects_to_choose_service( +def test_logged_in_user_redirects_to_choose_account( logged_in_client ): response = logged_in_client.get(url_for('main.sign_in')) - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) @pytest.mark.parametrize('email_address, password', [ diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 3b28e5849..747dff750 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -90,7 +90,7 @@ def test_should_login_user_and_not_redirect_to_external_url( ) -def test_should_login_user_and_redirect_to_choose_services( +def test_should_login_user_and_redirect_to_choose_accounts( client, api_user_active, mock_get_user, @@ -106,7 +106,7 @@ def test_should_login_user_and_redirect_to_choose_services( data={'sms_code': '12345'}) assert response.status_code == 302 - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong( @@ -321,4 +321,4 @@ def test_two_factor_email_link_used_when_user_already_logged_in( url_for('main.two_factor_email', token=valid_token) ) assert response.status_code == 302 - assert response.location == url_for('main.choose_service', _external=True) + assert response.location == url_for('main.choose_account', _external=True) diff --git a/tests/conftest.py b/tests/conftest.py index ca7627b96..a70a7ce19 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2867,3 +2867,17 @@ def mock_update_organisation_name(mocker): return return mocker.patch('app.organisations_client.update_organisation_name', side_effect=_update_org_name) + + +@pytest.fixture +def mock_get_organisations_and_services_for_user(mocker, organisation_one, api_user_active): + def _get_orgs_and_services(user_id): + return { + 'organisations': [], + 'services_without_organisations': [] + } + + return mocker.patch( + 'app.user_api_client.get_organisations_and_services_for_user', + side_effect=_get_orgs_and_services + ) From 7e4c5c97b0433af9f7145d6d7ad91f74b0397f51 Mon Sep 17 00:00:00 2001 From: chrisw Date: Fri, 9 Mar 2018 10:38:33 +0000 Subject: [PATCH 15/23] adjusted lists for choose accounts page --- .../stylesheets/components/browse-list.scss | 19 ++++--- app/templates/views/choose-account.html | 52 +++++++------------ 2 files changed, 31 insertions(+), 40 deletions(-) diff --git a/app/assets/stylesheets/components/browse-list.scss b/app/assets/stylesheets/components/browse-list.scss index 26781ad65..14f7b59f8 100644 --- a/app/assets/stylesheets/components/browse-list.scss +++ b/app/assets/stylesheets/components/browse-list.scss @@ -1,18 +1,24 @@ .browse-list { + margin-bottom: $gutter-half; - margin-bottom: $gutter; + .browse-sub-list { + margin-top: $gutter-half; + margin-left: $gutter; + @include media('desktop') { + margin-left: $gutter * 2; + } + } - &-item { + &-item, + &-sub-item { list-style: none; - margin-bottom: $gutter-two-thirds; + margin-bottom: $gutter-half; } &-link { - @include bold-24; &-destructive { - @include bold-24; color: $error-colour; @@ -25,14 +31,11 @@ &:hover { color: $mellow-red; } - } - } &-hint { @include core-19; margin-top: 5px; } - } diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index 6fd63733e..e4f812935 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -9,45 +9,33 @@

Choose account

- - {% for org in organisations %} -
 
-
- -
- {% endfor %} - {% if services_without_organisations %} -
 
- - {% endif %} +
+ {% endif %} {% if can_add_service %} -
 
- + Add a new service… {% endif %} {% endblock %} From 50f75f9b8ddc1ecb5f6dd211cc78af2b6d87d3e8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Fri, 9 Mar 2018 13:58:45 +0000 Subject: [PATCH 16/23] add tests for choose_accounts --- tests/app/main/views/test_choose_accounts.py | 150 ++++++++++++------- 1 file changed, 93 insertions(+), 57 deletions(-) diff --git a/tests/app/main/views/test_choose_accounts.py b/tests/app/main/views/test_choose_accounts.py index 004c161eb..701085333 100644 --- a/tests/app/main/views/test_choose_accounts.py +++ b/tests/app/main/views/test_choose_accounts.py @@ -1,40 +1,104 @@ +import pytest from bs4 import BeautifulSoup from flask import url_for +from tests.conftest import normalize_spaces -def test_should_show_choose_accounts_page( - logged_in_client, - mock_login, - mock_get_user, - api_user_active, - mock_get_services, +SAMPLE_DATA = { + 'organisations': [ + { + 'name': 'org_1', + 'id': 'o1', + 'services': [ + {'name': 'org_service_1', 'id': 'os1'}, + {'name': 'org_service_2', 'id': 'os2'}, + {'name': 'org_service_3', 'id': 'os3'}, + ] + }, + { + 'name': 'org_2', + 'id': 'o2', + 'services': [ + {'name': 'org_service_4', 'id': 'os4'}, + ] + }, + { + 'name': 'org_3', + 'id': 'o3', + 'services': [] + } + ], + 'services_without_organisations': [ + {'name': 'service_1', 'id': 's1'}, + {'name': 'service_2', 'id': 's2'}, + {'name': 'service_3', 'id': 's3'}, + ] +} + + +@pytest.fixture +def mock_get_orgs_and_services(mocker): + return mocker.patch( + 'app.user_api_client.get_organisations_and_services_for_user', + return_value=SAMPLE_DATA + ) + + +def test_choose_account_should_show_choose_accounts_page( + client_request, + mock_get_orgs_and_services ): - response = logged_in_client.get(url_for('main.choose_account')) + resp = client_request.get('main.choose_account') + page = resp.find('div', {'id': 'content'}).main - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Choose service' in resp_data - services = mock_get_services.side_effect() - assert mock_get_services.called - assert services['data'][0]['name'] in resp_data - assert services['data'][1]['name'] in resp_data + assert normalize_spaces(page.h1.text) == 'Choose account' -def test_should_show_choose_accounts_page_if_no_services( - logged_in_client, - mock_login, - api_user_active, +def test_choose_account_should_show_choose_accounts_page_if_no_services( + client_request, + mock_get_orgs_and_services ): - # if users last service has been archived there'll be no services - # mock_login already patches get_services to return no data - response = logged_in_client.get(url_for('main.choose_account')) - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Choose service' in resp_data - assert 'Add a new service' in resp_data + mock_get_orgs_and_services.return_value = { + 'organisations': [], + 'services_without_organisations': [] + } + resp = client_request.get('main.choose_account') + page = resp.find('div', {'id': 'content'}).main + + links = page.findAll('a') + assert len(links) == 1 + add_service_link = links[0] + assert normalize_spaces(page.h1.text) == 'Choose account' + assert normalize_spaces(add_service_link.text) == 'Add a new service…' + assert add_service_link['href'] == url_for('main.add_service') -def test_redirect_if_only_one_service( +def test_choose_account_should_show_back_to_service_link( + client_request, + mock_get_orgs_and_services +): + resp = client_request.get('main.choose_account') + + page = resp.find('div', {'id': 'content'}) + back_to_service_link = page.find('div', {'class': 'navigation-service'}).a + + assert back_to_service_link['href'] == url_for('main.show_accounts_or_dashboard') + assert back_to_service_link.text == 'Back to service one' + + +def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_session( + client, + client_request, + mock_get_orgs_and_services +): + with client.session_transaction() as session: + session['service_id'] = None + page = client_request.get('main.choose_account') + + assert len(page.select('.navigation-service a')) == 0 + + +def test_show_accounts_or_dashboard_redirects_if_only_one_service( logged_in_client, mock_login, api_user_active, @@ -47,7 +111,7 @@ def test_redirect_if_only_one_service( assert response.location == url_for('main.service_dashboard', service_id=service['id'], _external=True) -def test_redirect_if_multiple_services( +def test_show_accounts_or_dashboard_redirects_if_multiple_services( logged_in_client, mock_login, api_user_active, @@ -58,12 +122,10 @@ def test_redirect_if_multiple_services( assert response.location == url_for('main.choose_account', _external=True) -def test_redirect_if_service_in_session( +def test_show_accounts_or_dashboard_redirects_if_service_in_session( logged_in_client, mock_login, api_user_active, - mock_get_services, - mock_get_service, ): with logged_in_client.session_transaction() as session: session['service_id'] = '147ad62a-2951-4fa1-9ca0-093cd1a52c52' @@ -77,36 +139,10 @@ def test_redirect_if_service_in_session( ) -def test_should_redirect_if_not_logged_in( +def test_show_accounts_or_dashboard_redirects_if_not_logged_in( logged_in_client, app_ ): response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 assert url_for('main.index', _external=True) in response.location - - -def test_should_show_back_to_service_link( - logged_in_client -): - response = logged_in_client.get(url_for('main.choose_account')) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.select('.navigation-service a')[0]['href'] == ( - url_for('main.show_accounts_or_dashboard') - ) - - -def test_should_not_show_back_to_service_link_if_no_service_in_session( - client, - api_user_active, - mock_get_user, - mock_get_services_with_no_services, -): - client.login(api_user_active) - response = client.get(url_for('main.choose_account')) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert len(page.select('.navigation-service a')) == 0 From b7b9b8dd90e6ac13c66c96ffdb305302d4395b11 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 12 Mar 2018 15:21:46 +0000 Subject: [PATCH 17/23] make sure mock login sets up session correctly --- tests/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/__init__.py b/tests/__init__.py index ed15d963d..501fe2a89 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -14,6 +14,7 @@ class TestClient(FlaskClient): def login(self, user, mocker=None, service=None): # Skipping authentication here and just log them in with self.session_transaction() as session: + session['current_session_id'] = user.current_session_id session['user_id'] = user.id if mocker: mocker.patch('app.user_api_client.get_user', return_value=user) @@ -63,6 +64,7 @@ def user_json( platform_admin=False, current_session_id='1234', organisations=[], + services=[] ): return { @@ -78,7 +80,8 @@ def user_json( 'max_failed_login_count': max_failed_login_count, 'platform_admin': platform_admin, 'current_session_id': current_session_id, - 'organisations': organisations + 'organisations': organisations, + 'services': services or permissions.keys() } From 8ef36f13f26afbddf72affa8d95134a795b634ef Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 13 Mar 2018 08:23:29 +0000 Subject: [PATCH 18/23] finish tests for choose account page --- app/templates/views/choose-account.html | 1 + tests/__init__.py | 4 +- tests/app/main/views/test_choose_accounts.py | 145 +++++++++++++++---- 3 files changed, 118 insertions(+), 32 deletions(-) diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index e4f812935..df673495e 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -34,6 +34,7 @@ {% endfor %}
{% endif %} + {% if can_add_service %} Add a new service… {% endif %} diff --git a/tests/__init__.py b/tests/__init__.py index 501fe2a89..5b69bb38c 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -64,7 +64,7 @@ def user_json( platform_admin=False, current_session_id='1234', organisations=[], - services=[] + services=None ): return { @@ -81,7 +81,7 @@ def user_json( 'platform_admin': platform_admin, 'current_session_id': current_session_id, 'organisations': organisations, - 'services': services or permissions.keys() + 'services': list(permissions.keys()) if services is None else services } diff --git a/tests/app/main/views/test_choose_accounts.py b/tests/app/main/views/test_choose_accounts.py index 701085333..781b4456e 100644 --- a/tests/app/main/views/test_choose_accounts.py +++ b/tests/app/main/views/test_choose_accounts.py @@ -1,8 +1,9 @@ import pytest -from bs4 import BeautifulSoup from flask import url_for +from tests import user_json from tests.conftest import normalize_spaces +from app.notify_client.models import User SAMPLE_DATA = { 'organisations': [ @@ -36,6 +37,14 @@ SAMPLE_DATA = { } +def user_with_orgs_and_services(num_orgs, num_services): + return User(user_json( + name='leo', + organisations=['org{}'.format(i) for i in range(1, num_orgs + 1)], + services=['service{}'.format(i) for i in range(1, num_services + 1)] + )) + + @pytest.fixture def mock_get_orgs_and_services(mocker): return mocker.patch( @@ -52,6 +61,34 @@ def test_choose_account_should_show_choose_accounts_page( page = resp.find('div', {'id': 'content'}).main assert normalize_spaces(page.h1.text) == 'Choose account' + outer_list_items = page.nav.ul.find_all('li', recursive=False) + + assert len(outer_list_items) == 6 + + # first org + assert outer_list_items[0].a.text == 'org_1' + assert outer_list_items[0].a['href'] == url_for('.organisation_dashboard', org_id='o1') + outer_list_orgs = outer_list_items[0].ul + assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_1 org_service_2 org_service_3' + + # second org + assert outer_list_items[1].a.text == 'org_2' + assert outer_list_items[1].a['href'] == url_for('.organisation_dashboard', org_id='o2') + outer_list_orgs = outer_list_items[1].ul + assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_4' + + # third org + assert outer_list_items[2].a.text == 'org_3' + assert outer_list_items[2].a['href'] == url_for('.organisation_dashboard', org_id='o3') + assert not outer_list_items[2].ul # org 3 has no services + + # orphaned services + assert outer_list_items[3].a.text == 'service_1' + assert outer_list_items[3].a['href'] == url_for('.service_dashboard', service_id='s1') + assert outer_list_items[4].a.text == 'service_2' + assert outer_list_items[4].a['href'] == url_for('.service_dashboard', service_id='s2') + assert outer_list_items[5].a.text == 'service_3' + assert outer_list_items[5].a['href'] == url_for('.service_dashboard', service_id='s3') def test_choose_account_should_show_choose_accounts_page_if_no_services( @@ -98,51 +135,99 @@ def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_se assert len(page.select('.navigation-service a')) == 0 -def test_show_accounts_or_dashboard_redirects_if_only_one_service( - logged_in_client, - mock_login, - api_user_active, - mock_get_services_with_one_service, -): - response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) +############################### + +@pytest.mark.parametrize('num_orgs,num_services,endpoint,endpoint_kwargs', [ + (0, 0, '.choose_account', {}), + (0, 2, '.choose_account', {}), + (1, 1, '.choose_account', {}), + (2, 0, '.choose_account', {}), + (0, 1, '.service_dashboard', {'service_id': 'service1'}), + (1, 0, '.organisation_dashboard', {'org_id': 'org1'}), +]) +def test_show_accounts_or_dashboard_redirects_to_choose_account_or_service_dashboard( + client, + mocker, + num_orgs, + num_services, + endpoint, + endpoint_kwargs +): + client.login(user_with_orgs_and_services(num_orgs=num_orgs, num_services=num_services), mocker=mocker) + + response = client.get(url_for('main.show_accounts_or_dashboard')) - service = mock_get_services_with_one_service.side_effect()['data'][0] assert response.status_code == 302 - assert response.location == url_for('main.service_dashboard', service_id=service['id'], _external=True) + assert response.location == url_for(endpoint, _external=True, **endpoint_kwargs) -def test_show_accounts_or_dashboard_redirects_if_multiple_services( - logged_in_client, - mock_login, - api_user_active, +def test_show_accounts_or_dashboard_redirects_if_service_in_session(client, mocker, mock_get_service): + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = 'service1' + session['organisation_id'] = None + + response = client.get(url_for('.show_accounts_or_dashboard')) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.service_dashboard', + service_id='service1', + _external=True + ) + + +def test_show_accounts_or_dashboard_redirects_if_org_in_session(client, mocker): + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = None + session['organisation_id'] = 'org1' + + response = client.get(url_for('.show_accounts_or_dashboard')) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.organisation_dashboard', + org_id='org1', + _external=True + ) + + +def test_show_accounts_or_dashboard_doesnt_redirect_to_service_dashboard_if_user_not_part_of_service_in_session( + client, + mocker, + mock_get_service ): - response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = 'service2' + session['organisation_id'] = None + + response = client.get(url_for('.show_accounts_or_dashboard')) assert response.status_code == 302 assert response.location == url_for('main.choose_account', _external=True) -def test_show_accounts_or_dashboard_redirects_if_service_in_session( - logged_in_client, - mock_login, - api_user_active, +def test_show_accounts_or_dashboard_doesnt_redirect_to_org_dashboard_if_user_not_part_of_org_in_session( + client, + mocker ): - with logged_in_client.session_transaction() as session: - session['service_id'] = '147ad62a-2951-4fa1-9ca0-093cd1a52c52' - response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = None + session['organisation_id'] = 'org2' + + response = client.get(url_for('.show_accounts_or_dashboard')) assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id='147ad62a-2951-4fa1-9ca0-093cd1a52c52', - _external=True - ) + assert response.location == url_for('main.choose_account', _external=True) def test_show_accounts_or_dashboard_redirects_if_not_logged_in( - logged_in_client, + client, app_ ): - response = logged_in_client.get(url_for('main.show_accounts_or_dashboard')) + response = client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 - assert url_for('main.index', _external=True) in response.location + assert response.location == url_for('main.index', _external=True) From a94fa5472d21aab8916a80087363eda1a5a0cc73 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 13 Mar 2018 09:02:09 +0000 Subject: [PATCH 19/23] platform admins can go back to any service --- app/main/views/choose_account.py | 4 +- tests/app/main/views/test_choose_accounts.py | 44 +++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/app/main/views/choose_account.py b/app/main/views/choose_account.py index 80fb4d143..9e846670e 100644 --- a/app/main/views/choose_account.py +++ b/app/main/views/choose_account.py @@ -26,11 +26,11 @@ def show_accounts_or_dashboard(): return redirect(url_for('.index')) service_id = session.get('service_id') - if any(service_id == x for x in current_user.services): + if service_id and (service_id in current_user.services or current_user.platform_admin): return redirect(url_for('.service_dashboard', service_id=service_id)) organisation_id = session.get('organisation_id') - if any(organisation_id == x for x in current_user.organisations): + if organisation_id and (organisation_id in current_user.organisations or current_user.platform_admin): return redirect(url_for('.organisation_dashboard', org_id=organisation_id)) if len(current_user.services) == 1 and not current_user.organisations: diff --git a/tests/app/main/views/test_choose_accounts.py b/tests/app/main/views/test_choose_accounts.py index 781b4456e..c6b645457 100644 --- a/tests/app/main/views/test_choose_accounts.py +++ b/tests/app/main/views/test_choose_accounts.py @@ -37,11 +37,12 @@ SAMPLE_DATA = { } -def user_with_orgs_and_services(num_orgs, num_services): +def user_with_orgs_and_services(num_orgs, num_services, platform_admin=False): return User(user_json( name='leo', organisations=['org{}'.format(i) for i in range(1, num_orgs + 1)], - services=['service{}'.format(i) for i in range(1, num_services + 1)] + services=['service{}'.format(i) for i in range(1, num_services + 1)], + platform_admin=platform_admin )) @@ -231,3 +232,42 @@ def test_show_accounts_or_dashboard_redirects_if_not_logged_in( response = client.get(url_for('main.show_accounts_or_dashboard')) assert response.status_code == 302 assert response.location == url_for('main.index', _external=True) + + +def test_show_accounts_or_dashboard_redirects_to_service_dashboard_if_platform_admin( + client, + mocker, + mock_get_service +): + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = 'service2' + session['organisation_id'] = None + + response = client.get(url_for('.show_accounts_or_dashboard')) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.service_dashboard', + service_id='service2', + _external=True + ) + + +def test_show_accounts_or_dashboard_redirects_to_org_dashboard_if_platform_admin( + client, + mocker +): + client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker) + with client.session_transaction() as session: + session['service_id'] = None + session['organisation_id'] = 'org2' + + response = client.get(url_for('.show_accounts_or_dashboard')) + + assert response.status_code == 302 + assert response.location == url_for( + 'main.organisation_dashboard', + org_id='org2', + _external=True + ) From a82ac13d4e0e09fbf9d2c16af6bf45d1b530d82a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 13 Mar 2018 09:29:09 +0000 Subject: [PATCH 20/23] split tests out into two separate files --- .../views/accounts/test_choose_accounts.py | 124 ++++++++++++++++++ .../test_show_accounts_or_dashbaord.py} | 124 ------------------ 2 files changed, 124 insertions(+), 124 deletions(-) create mode 100644 tests/app/main/views/accounts/test_choose_accounts.py rename tests/app/main/views/{test_choose_accounts.py => accounts/test_show_accounts_or_dashbaord.py} (54%) diff --git a/tests/app/main/views/accounts/test_choose_accounts.py b/tests/app/main/views/accounts/test_choose_accounts.py new file mode 100644 index 000000000..6dc0fc4df --- /dev/null +++ b/tests/app/main/views/accounts/test_choose_accounts.py @@ -0,0 +1,124 @@ +import pytest +from flask import url_for +from tests.conftest import normalize_spaces + +SAMPLE_DATA = { + 'organisations': [ + { + 'name': 'org_1', + 'id': 'o1', + 'services': [ + {'name': 'org_service_1', 'id': 'os1'}, + {'name': 'org_service_2', 'id': 'os2'}, + {'name': 'org_service_3', 'id': 'os3'}, + ] + }, + { + 'name': 'org_2', + 'id': 'o2', + 'services': [ + {'name': 'org_service_4', 'id': 'os4'}, + ] + }, + { + 'name': 'org_3', + 'id': 'o3', + 'services': [] + } + ], + 'services_without_organisations': [ + {'name': 'service_1', 'id': 's1'}, + {'name': 'service_2', 'id': 's2'}, + {'name': 'service_3', 'id': 's3'}, + ] +} + + +@pytest.fixture +def mock_get_orgs_and_services(mocker): + return mocker.patch( + 'app.user_api_client.get_organisations_and_services_for_user', + return_value=SAMPLE_DATA + ) + + +def test_choose_account_should_show_choose_accounts_page( + client_request, + mock_get_orgs_and_services +): + resp = client_request.get('main.choose_account') + page = resp.find('div', {'id': 'content'}).main + + assert normalize_spaces(page.h1.text) == 'Choose account' + outer_list_items = page.nav.ul.find_all('li', recursive=False) + + assert len(outer_list_items) == 6 + + # first org + assert outer_list_items[0].a.text == 'org_1' + assert outer_list_items[0].a['href'] == url_for('.organisation_dashboard', org_id='o1') + outer_list_orgs = outer_list_items[0].ul + assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_1 org_service_2 org_service_3' + + # second org + assert outer_list_items[1].a.text == 'org_2' + assert outer_list_items[1].a['href'] == url_for('.organisation_dashboard', org_id='o2') + outer_list_orgs = outer_list_items[1].ul + assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_4' + + # third org + assert outer_list_items[2].a.text == 'org_3' + assert outer_list_items[2].a['href'] == url_for('.organisation_dashboard', org_id='o3') + assert not outer_list_items[2].ul # org 3 has no services + + # orphaned services + assert outer_list_items[3].a.text == 'service_1' + assert outer_list_items[3].a['href'] == url_for('.service_dashboard', service_id='s1') + assert outer_list_items[4].a.text == 'service_2' + assert outer_list_items[4].a['href'] == url_for('.service_dashboard', service_id='s2') + assert outer_list_items[5].a.text == 'service_3' + assert outer_list_items[5].a['href'] == url_for('.service_dashboard', service_id='s3') + + +def test_choose_account_should_show_choose_accounts_page_if_no_services( + client_request, + mock_get_orgs_and_services +): + mock_get_orgs_and_services.return_value = { + 'organisations': [], + 'services_without_organisations': [] + } + resp = client_request.get('main.choose_account') + page = resp.find('div', {'id': 'content'}).main + + links = page.findAll('a') + assert len(links) == 1 + add_service_link = links[0] + assert normalize_spaces(page.h1.text) == 'Choose account' + assert normalize_spaces(add_service_link.text) == 'Add a new service…' + assert add_service_link['href'] == url_for('main.add_service') + + +def test_choose_account_should_show_back_to_service_link( + client_request, + mock_get_orgs_and_services +): + resp = client_request.get('main.choose_account') + + page = resp.find('div', {'id': 'content'}) + back_to_service_link = page.find('div', {'class': 'navigation-service'}).a + + assert back_to_service_link['href'] == url_for('main.show_accounts_or_dashboard') + assert back_to_service_link.text == 'Back to service one' + + +def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_session( + client, + client_request, + mock_get_orgs_and_services +): + with client.session_transaction() as session: + session['service_id'] = None + page = client_request.get('main.choose_account') + + assert len(page.select('.navigation-service a')) == 0 diff --git a/tests/app/main/views/test_choose_accounts.py b/tests/app/main/views/accounts/test_show_accounts_or_dashbaord.py similarity index 54% rename from tests/app/main/views/test_choose_accounts.py rename to tests/app/main/views/accounts/test_show_accounts_or_dashbaord.py index c6b645457..eb29398c4 100644 --- a/tests/app/main/views/test_choose_accounts.py +++ b/tests/app/main/views/accounts/test_show_accounts_or_dashbaord.py @@ -1,41 +1,9 @@ import pytest from flask import url_for from tests import user_json -from tests.conftest import normalize_spaces from app.notify_client.models import User -SAMPLE_DATA = { - 'organisations': [ - { - 'name': 'org_1', - 'id': 'o1', - 'services': [ - {'name': 'org_service_1', 'id': 'os1'}, - {'name': 'org_service_2', 'id': 'os2'}, - {'name': 'org_service_3', 'id': 'os3'}, - ] - }, - { - 'name': 'org_2', - 'id': 'o2', - 'services': [ - {'name': 'org_service_4', 'id': 'os4'}, - ] - }, - { - 'name': 'org_3', - 'id': 'o3', - 'services': [] - } - ], - 'services_without_organisations': [ - {'name': 'service_1', 'id': 's1'}, - {'name': 'service_2', 'id': 's2'}, - {'name': 'service_3', 'id': 's3'}, - ] -} - def user_with_orgs_and_services(num_orgs, num_services, platform_admin=False): return User(user_json( @@ -46,98 +14,6 @@ def user_with_orgs_and_services(num_orgs, num_services, platform_admin=False): )) -@pytest.fixture -def mock_get_orgs_and_services(mocker): - return mocker.patch( - 'app.user_api_client.get_organisations_and_services_for_user', - return_value=SAMPLE_DATA - ) - - -def test_choose_account_should_show_choose_accounts_page( - client_request, - mock_get_orgs_and_services -): - resp = client_request.get('main.choose_account') - page = resp.find('div', {'id': 'content'}).main - - assert normalize_spaces(page.h1.text) == 'Choose account' - outer_list_items = page.nav.ul.find_all('li', recursive=False) - - assert len(outer_list_items) == 6 - - # first org - assert outer_list_items[0].a.text == 'org_1' - assert outer_list_items[0].a['href'] == url_for('.organisation_dashboard', org_id='o1') - outer_list_orgs = outer_list_items[0].ul - assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_1 org_service_2 org_service_3' - - # second org - assert outer_list_items[1].a.text == 'org_2' - assert outer_list_items[1].a['href'] == url_for('.organisation_dashboard', org_id='o2') - outer_list_orgs = outer_list_items[1].ul - assert ' '.join(outer_list_orgs.stripped_strings) == 'org_service_4' - - # third org - assert outer_list_items[2].a.text == 'org_3' - assert outer_list_items[2].a['href'] == url_for('.organisation_dashboard', org_id='o3') - assert not outer_list_items[2].ul # org 3 has no services - - # orphaned services - assert outer_list_items[3].a.text == 'service_1' - assert outer_list_items[3].a['href'] == url_for('.service_dashboard', service_id='s1') - assert outer_list_items[4].a.text == 'service_2' - assert outer_list_items[4].a['href'] == url_for('.service_dashboard', service_id='s2') - assert outer_list_items[5].a.text == 'service_3' - assert outer_list_items[5].a['href'] == url_for('.service_dashboard', service_id='s3') - - -def test_choose_account_should_show_choose_accounts_page_if_no_services( - client_request, - mock_get_orgs_and_services -): - mock_get_orgs_and_services.return_value = { - 'organisations': [], - 'services_without_organisations': [] - } - resp = client_request.get('main.choose_account') - page = resp.find('div', {'id': 'content'}).main - - links = page.findAll('a') - assert len(links) == 1 - add_service_link = links[0] - assert normalize_spaces(page.h1.text) == 'Choose account' - assert normalize_spaces(add_service_link.text) == 'Add a new service…' - assert add_service_link['href'] == url_for('main.add_service') - - -def test_choose_account_should_show_back_to_service_link( - client_request, - mock_get_orgs_and_services -): - resp = client_request.get('main.choose_account') - - page = resp.find('div', {'id': 'content'}) - back_to_service_link = page.find('div', {'class': 'navigation-service'}).a - - assert back_to_service_link['href'] == url_for('main.show_accounts_or_dashboard') - assert back_to_service_link.text == 'Back to service one' - - -def test_choose_account_should_not_show_back_to_service_link_if_no_service_in_session( - client, - client_request, - mock_get_orgs_and_services -): - with client.session_transaction() as session: - session['service_id'] = None - page = client_request.get('main.choose_account') - - assert len(page.select('.navigation-service a')) == 0 - - -############################### - @pytest.mark.parametrize('num_orgs,num_services,endpoint,endpoint_kwargs', [ (0, 0, '.choose_account', {}), (0, 2, '.choose_account', {}), From d5108d041840ddd2b55ca9226f1506fd5571e982 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 13 Mar 2018 09:58:51 +0000 Subject: [PATCH 21/23] fix my awful rebasing skills --- app/main/views/organisations.py | 25 --- ....py => test_show_accounts_or_dashboard.py} | 0 .../views/organisations/test_organisations.py | 179 ------------------ 3 files changed, 204 deletions(-) rename tests/app/main/views/accounts/{test_show_accounts_or_dashbaord.py => test_show_accounts_or_dashboard.py} (100%) delete mode 100644 tests/app/main/views/organisations/test_organisations.py diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index ea73c6eb3..0010e5c5b 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -63,31 +63,6 @@ def organisation_dashboard(org_id): ) -@main.route("/organisations//edit", methods=['GET', 'POST']) -@login_required -@user_has_permissions() -def update_organisation(org_id): - org = organisations_client.get_organisation(org_id) - - form = CreateOrUpdateOrganisation() - - if form.validate_on_submit(): - organisations_client.update_organisation( - org_id=org_id, - name=form.name.data - ) - - return redirect(url_for('.organisations')) - - form.name.data = org['name'] - - return render_template( - 'views/organisations/organisation/update-organisation.html', - form=form, - organisation=org - ) - - @main.route("/organisations//users", methods=['GET']) @login_required @user_has_permissions() diff --git a/tests/app/main/views/accounts/test_show_accounts_or_dashbaord.py b/tests/app/main/views/accounts/test_show_accounts_or_dashboard.py similarity index 100% rename from tests/app/main/views/accounts/test_show_accounts_or_dashbaord.py rename to tests/app/main/views/accounts/test_show_accounts_or_dashboard.py diff --git a/tests/app/main/views/organisations/test_organisations.py b/tests/app/main/views/organisations/test_organisations.py deleted file mode 100644 index d51b1fa8f..000000000 --- a/tests/app/main/views/organisations/test_organisations.py +++ /dev/null @@ -1,179 +0,0 @@ -import pytest -from bs4 import BeautifulSoup -from flask import url_for -from tests.conftest import ORGANISATION_ID, normalize_spaces - - -@pytest.mark.parametrize('endpoint', ['.organisations', '.add_organisation']) -def test_global_organisation_pages_are_platform_admin_only(client_request, endpoint): - client_request.get( - endpoint, - _expected_status=403, - _test_page_title=False - ) - - -def test_organisation_page_shows_all_organisations( - logged_in_platform_admin_client, - mocker -): - orgs = [ - {'id': '1', 'name': 'Test 1', 'active': True}, - {'id': '2', 'name': 'Test 2', 'active': True}, - {'id': '3', 'name': 'Test 3', 'active': False}, - ] - - mocker.patch( - 'app.organisations_client.get_organisations', return_value=orgs - ) - response = logged_in_platform_admin_client.get( - url_for('.organisations') - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert normalize_spaces( - page.select_one('h1').text - ) == "Organisations" - - for index, org in enumerate(orgs): - assert page.select('a.browse-list-link')[index].text == org['name'] - if not org['active']: - assert page.select_one('.table-field-status-default,heading-medium').text == '- archived' - assert normalize_spaces((page.select('a.browse-list-link')[-1]).text) == 'Create an organisation' - - -def test_create_new_organisation( - logged_in_platform_admin_client, - mocker -): - mock_create_organisation = mocker.patch( - 'app.organisations_client.create_organisation' - ) - - org = {'name': 'new name'} - - logged_in_platform_admin_client.post( - url_for('.add_organisation'), - content_type='multipart/form-data', - data=org - ) - - mock_create_organisation.assert_called_once_with(name=org['name']) - - -def test_view_organisation_shows_the_correct_organisation( - logged_in_client, - mocker -): - org = {'id': ORGANISATION_ID, 'name': 'Test 1', 'active': True} - mocker.patch( - 'app.organisations_client.get_organisation', return_value=org - ) - mocker.patch( - 'app.organisations_client.get_organisation_services', return_value=[] - ) - - response = logged_in_client.get( - url_for('.organisation_dashboard', org_id=ORGANISATION_ID) - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert normalize_spaces(page.select_one('.heading-large').text) == org['name'] - - -def test_edit_organisation_shows_the_correct_organisation( - logged_in_client, - mocker -): - org = {'id': ORGANISATION_ID, 'name': 'Test 1', 'active': True} - mocker.patch( - 'app.organisations_client.get_organisation', return_value=org - ) - - response = logged_in_client.get( - url_for('.update_organisation', org_id=ORGANISATION_ID) - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert page.select_one('#name').attrs.get('value') == org['name'] - - -def test_update_organisation( - logged_in_client, - mocker, -): - org = {'name': 'new name'} - - mocker.patch( - 'app.organisations_client.get_organisation', return_value=org - ) - mock_update_organisation = mocker.patch( - 'app.organisations_client.update_organisation' - ) - - logged_in_client.post( - url_for('.update_organisation', org_id=ORGANISATION_ID), - content_type='multipart/form-data', - data=org - ) - - assert mock_update_organisation.called - mock_update_organisation.assert_called_once_with( - org_id=ORGANISATION_ID, - name=org['name'] - ) - - -def test_organisation_dashboard_shows_services( - logged_in_client, - mock_get_organisation, - mock_get_organisation_services, - mocker, -): - response = logged_in_client.get( - url_for('.organisation_dashboard', org_id=ORGANISATION_ID), - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert len(page.select('.browse-list-item')) == 3 - - for i in range(0, 3): - service_name = mock_get_organisation_services(ORGANISATION_ID)[i]['name'] - service_id = mock_get_organisation_services(ORGANISATION_ID)[i]['id'] - - assert normalize_spaces(page.select('.browse-list-item')[i].text) == service_name - assert normalize_spaces( - page.select('.browse-list-item a')[i]['href'] - ) == '/services/{}'.format(service_id) - - -def test_view_team_members( - logged_in_client, - mocker, - mock_get_organisation, - mock_get_users_for_organisation, - mock_get_invited_users_for_organisation, -): - response = logged_in_client.get( - url_for('.manage_org_users', org_id=ORGANISATION_ID), - ) - - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - for i in range(0, 2): - assert normalize_spaces( - page.select('.user-list-item .heading-small')[i].text - ) == 'Test User {}'.format(i + 1) - - assert normalize_spaces( - page.select('.tick-cross-list-edit-link')[1].text - ) == 'Cancel invitation' From 5f2e0d4e4911e6b5900b76e65ca068072e5b8a0a Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 14 Mar 2018 15:39:06 +0000 Subject: [PATCH 22/23] organisations can be edited by anyone who belongs to that org --- app/main/views/organisations.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 0010e5c5b..be2395b8c 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -160,7 +160,7 @@ def cancel_invited_org_user(org_id, invited_user_id): @main.route("/organisations//settings/", methods=['GET']) @login_required -@user_is_platform_admin +@user_has_permissions() def organisation_settings(org_id): return render_template( 'views/organisations/organisation/settings/index.html', @@ -169,7 +169,7 @@ def organisation_settings(org_id): @main.route("/organisations//settings/edit-name", methods=['GET', 'POST']) @login_required -@user_is_platform_admin +@user_has_permissions() def edit_organisation_name(org_id): form = RenameOrganisationForm() @@ -192,7 +192,7 @@ def edit_organisation_name(org_id): @main.route("/organisations//settings/edit-name/confirm", methods=['GET', 'POST']) @login_required -@user_is_platform_admin +@user_has_permissions() def confirm_edit_organisation_name(org_id): # Validate password for form def _check_password(pwd): From 666e77e699f462f6ca5b71f3693cc4c1f85f302f Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 15 Mar 2018 11:02:05 +0000 Subject: [PATCH 23/23] redirect (301 MOVED PERMANENTLY) from old choose endpoints --- app/main/views/choose_account.py | 11 +++++++++++ app/templates/views/choose-account.html | 4 ++-- tests/app/main/views/accounts/test_choose_accounts.py | 4 ++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/main/views/choose_account.py b/app/main/views/choose_account.py index 9e846670e..a7aa2da1f 100644 --- a/app/main/views/choose_account.py +++ b/app/main/views/choose_account.py @@ -1,11 +1,22 @@ from flask import redirect, render_template, session, url_for from flask_login import current_user, login_required +from werkzeug.routing import RequestRedirect from app import user_api_client from app.main import main from app.utils import is_gov_user +@main.route("/services") +def choose_service(): + raise RequestRedirect(url_for('.choose_account')) + + +@main.route("/services-or-dashboard") +def services_or_dashboard(): + raise RequestRedirect(url_for('.show_accounts_or_dashboard')) + + @main.route("/accounts") @login_required def choose_account(): diff --git a/app/templates/views/choose-account.html b/app/templates/views/choose-account.html index df673495e..7f7d793d9 100644 --- a/app/templates/views/choose-account.html +++ b/app/templates/views/choose-account.html @@ -1,13 +1,13 @@ {% extends "withoutnav_template.html" %} {% block per_page_title %} - Choose account + Choose service {% endblock %} {% block maincolumn_content %}

- Choose account + Choose service