Merge pull request #1954 from alphagov/choose-accounts

Choose accounts

app/__init__.py

@@ -144,7 +144,7 @@ def create_app(application):
 
 def init_app(application):
     application.after_request(useful_headers_after_request)
-    application.after_request(save_service_after_request)
+    application.after_request(save_service_or_org_after_request)
     application.before_request(load_service_before_request)
     application.before_request(load_organisation_before_request)
     application.before_request(request_helper.check_proxy_header_before_request)
@@ -446,11 +446,17 @@ def load_organisation_before_request():
                         raise
 
 
-def save_service_after_request(response):
+def save_service_or_org_after_request(response):
     # Only save the current session if the request is 200
     service_id = request.view_args.get('service_id', None) if request.view_args else None
-    if response.status_code == 200 and service_id:
+    organisation_id = request.view_args.get('org_id', None) if request.view_args else None
-        session['service_id'] = service_id
+    if response.status_code == 200:
+        if service_id:
+            session['service_id'] = service_id
+            session['organisation_id'] = None
+        elif organisation_id:
+            session['service_id'] = None
+            session['organisation_id'] = organisation_id
     return response
 
 
@@ -482,7 +488,7 @@ def register_errorhandlers(application):  # noqa (C901 too complex)
 
     @application.errorhandler(HTTPError)
     def render_http_error(error):
-        application.logger.error("API {} failed with status {} message {}".format(
+        application.logger.warning("API {} failed with status {} message {}".format(
             error.response.url if error.response else 'unknown',
             error.status_code,
             error.message
@@ -495,7 +501,13 @@ def register_errorhandlers(application):  # noqa (C901 too complex)
                 msg = list(itertools.chain(*[error.message[x] for x in error.message.keys()]))
             resp = make_response(render_template("error/400.html", message=msg))
             return useful_headers_after_request(resp)
-        elif error_code not in [401, 404, 403, 410, 500]:
+        elif error_code not in [401, 404, 403, 410]:
+            # probably a 500 or 503
+            application.logger.exception("API {} failed with status {} message {}".format(
+                error.response.url if error.response else 'unknown',
+                error.status_code,
+                error.message
+            ))
             error_code = 500
         return _error_response(error_code)
 

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;
   }
-
 }

app/config.py

@@ -99,6 +99,7 @@ class Test(Development):
     CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test'
     NOTIFY_ENVIRONMENT = 'test'
+    API_HOST_NAME = 'http://you-forgot-to-mock-an-api-call-to'
     TEMPLATE_PREVIEW_API_HOST = 'http://localhost:9999'
 
 

app/main/__init__.py

@@ -20,7 +20,7 @@ from app.main.views import (  # noqa
     new_password,
     styleguide,
     user_profile,
-    choose_service,
+    choose_account,
     api_keys,
     manage_users,
     invites,

app/main/views/choose_account.py

@@ -0,0 +1,53 @@
+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():
+    orgs_and_services = user_api_client.get_organisations_and_services_for_user(current_user)
+
+    return render_template(
+        'views/choose-account.html',
+        organisations=orgs_and_services['organisations'],
+        services_without_organisations=orgs_and_services['services_without_organisations'],
+        can_add_service=is_gov_user(current_user.email_address)
+    )
+
+
+@main.route("/accounts-or-dashboard")
+def show_accounts_or_dashboard():
+
+    if not current_user.is_authenticated:
+        return redirect(url_for('.index'))
+
+    service_id = session.get('service_id')
+    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 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:
+        return redirect(url_for('.service_dashboard', service_id=current_user.services[0]))
+
+    if len(current_user.organisations) == 1 and not current_user.services:
+        return redirect(url_for('.organisation_dashboard', org_id=current_user.organisations[0]))
+
+    return redirect(url_for('.choose_account'))

app/main/views/choose_service.py

@@ -1,35 +0,0 @@
-from flask import redirect, render_template, session, url_for
-from flask_login import current_user, login_required
-
-from app import service_api_client
-from app.main import main
-from app.notify_client.service_api_client import ServicesBrowsableItem
-from app.utils import is_gov_user
-
-
-@main.route("/services")
-@login_required
-def choose_service():
-    return render_template(
-        'views/choose-service.html',
-        services=[ServicesBrowsableItem(x) for x in
-                  service_api_client.get_active_services({'user_id': current_user.id})['data']],
-        can_add_service=is_gov_user(current_user.email_address)
-    )
-
-
-@main.route("/services-or-dashboard")
-def show_all_services_or_dashboard():
-
-    if not current_user.is_authenticated:
-        return redirect(url_for('.index'))
-
-    services = service_api_client.get_active_services({'user_id': current_user.id})['data']
-
-    if 1 == len(services):
-        return redirect(url_for('.service_dashboard', service_id=services[0]['id']))
-    else:
-        service_id = session.get('service_id', None)
-        if any([service_id == x['id'] for x in services]):
-            return redirect(url_for('.service_dashboard', service_id=service_id))
-        return redirect(url_for('.choose_service'))

app/main/views/index.py

@@ -15,7 +15,7 @@ from app.utils import AgreementInfo
 @main.route('/')
 def index():
     if current_user and current_user.is_authenticated:
-        return redirect(url_for('main.choose_service'))
+        return redirect(url_for('main.choose_account'))
     return render_template('views/signedout.html')
 
 

app/main/views/organisations.py

@@ -17,7 +17,7 @@ from app.main.forms import (
     RenameOrganisationForm,
     SearchUsersForm,
 )
-from app.utils import user_is_platform_admin
+from app.utils import user_has_permissions, user_is_platform_admin
 
 
 @main.route("/organisations", methods=['GET'])
@@ -53,7 +53,7 @@ def add_organisation():
 
 @main.route("/organisations/<org_id>", methods=['GET'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def organisation_dashboard(org_id):
     organisation_services = organisations_client.get_organisation_services(org_id)
 
@@ -65,7 +65,7 @@ def organisation_dashboard(org_id):
 
 @main.route("/organisations/<org_id>/users", methods=['GET'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def manage_org_users(org_id):
     users = sorted(
         user_api_client.get_users_for_organisation(org_id=org_id) + [
@@ -85,7 +85,7 @@ def manage_org_users(org_id):
 
 @main.route("/organisations/<org_id>/users/invite", methods=['GET', 'POST'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def invite_org_user(org_id):
     form = InviteOrgUserForm(
         invalid_email_address=current_user.email_address
@@ -109,7 +109,7 @@ def invite_org_user(org_id):
 
 @main.route("/organisations/<org_id>/users/<user_id>", methods=['GET', 'POST'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def edit_user_org_permissions(org_id, user_id):
     user = user_api_client.get_user(user_id)
 
@@ -121,7 +121,7 @@ def edit_user_org_permissions(org_id, user_id):
 
 @main.route("/organisations/<org_id>/users/<user_id>/delete", methods=['GET', 'POST'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def remove_user_from_organisation(org_id, user_id):
     user = user_api_client.get_user(user_id)
     if request.method == 'POST':
@@ -151,7 +151,7 @@ def remove_user_from_organisation(org_id, user_id):
 
 @main.route("/organisations/<org_id>/cancel-invited-user/<invited_user_id>", methods=['GET'])
 @login_required
-@user_is_platform_admin
+@user_has_permissions()
 def cancel_invited_org_user(org_id, invited_user_id):
     org_invite_api_client.cancel_invited_user(org_id=org_id, invited_user_id=invited_user_id)
 
@@ -160,7 +160,7 @@ def cancel_invited_org_user(org_id, invited_user_id):
 
 @main.route("/organisations/<org_id>/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/<org_id>/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/<org_id>/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):

app/main/views/register.py

@@ -16,7 +16,7 @@ from app.main.views.verify import activate_user
 @main.route('/register', methods=['GET', 'POST'])
 def register():
     if current_user and current_user.is_authenticated:
-        return redirect(url_for('main.choose_service'))
+        return redirect(url_for('main.choose_account'))
 
     form = RegisterUserForm()
     if form.validate_on_submit():
@@ -101,5 +101,5 @@ def _do_registration(form, send_sms=True, send_email=True, organisation_id=None)
 @main.route('/registration-continue')
 def registration_continue():
     if not session.get('user_details'):
-        return redirect(url_for('.show_all_services_or_dashboard'))
+        return redirect(url_for('.show_accounts_or_dashboard'))
     return render_template('views/registration-continue.html')

app/main/views/sign_in.py

@@ -18,7 +18,7 @@ from app.main.forms import LoginForm
 @main.route('/sign-in', methods=(['GET', 'POST']))
 def sign_in():
     if current_user and current_user.is_authenticated:
-        return redirect(url_for('main.choose_service'))
+        return redirect(url_for('main.choose_account'))
 
     form = LoginForm()
 

app/main/views/two_factor.py

@@ -117,4 +117,4 @@ def redirect_when_logged_in(user_id):
     if len(services) == 1:
         return redirect(url_for('main.service_dashboard', service_id=services[0]['id']))
     else:
-        return redirect(url_for('main.choose_service'))
+        return redirect(url_for('main.choose_account'))

app/notify_client/models.py

@@ -68,6 +68,7 @@ class User(UserMixin):
         self.max_failed_login_count = max_failed_login_count
         self.platform_admin = fields.get('platform_admin')
         self.current_session_id = fields.get('current_session_id')
+        self.services = fields.get('services', [])
         self.organisations = fields.get('organisations', [])
 
     def _set_permissions(self, permissions_by_service):

app/notify_client/service_api_client.py

@@ -1,9 +1,6 @@
 from __future__ import unicode_literals
 
-from flask import url_for
-
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user
-from app.utils import BrowsableItem
 
 
 class ServiceAPIClient(NotifyAdminAPIClient):
@@ -426,21 +423,3 @@ class ServiceAPIClient(NotifyAdminAPIClient):
             "updated_by_id": user_id
         }
         return self.post("/service/{}/delivery-receipt-api".format(service_id), data)
-
-
-class ServicesBrowsableItem(BrowsableItem):
-    @property
-    def title(self):
-        return self._item['name']
-
-    @property
-    def link(self):
-        return url_for('main.service_dashboard', service_id=self._item['id'])
-
-    @property
-    def destructive(self):
-        return False
-
-    @property
-    def hint(self):
-        return None

app/notify_client/user_api_client.py

@@ -185,3 +185,7 @@ class UserApiClient(NotifyAdminAPIClient):
         endpoint = '/user/{}/change-email-verification'.format(user_id)
         data = {'email': new_email}
         self.post(endpoint, data)
+
+    def get_organisations_and_services_for_user(self, user):
+        endpoint = '/user/{}/organisations-and-services'.format(user.id)
+        return self.get(endpoint)

app/templates/admin_template.html

@@ -63,7 +63,7 @@
 
 {% set global_header_text = "GOV.UK <span id='product-name'>Notify</span>"|safe %}
 
-{% set homepage_url = url_for('main.show_all_services_or_dashboard') %}
+{% set homepage_url = url_for('main.show_accounts_or_dashboard') %}
 
 {% block content %}
   {% block fullwidth_content %}{% endblock %}

app/templates/org_template.html

@@ -10,7 +10,7 @@
       <div class="navigation-service-name">
         {{ current_org.name }}
       </div>
-      <a href="{{ url_for('main.choose_service') }}" class="navigation-service-switch">Switch service</a>
+      <a href="{{ url_for('main.choose_account') }}" class="navigation-service-switch">Switch service</a>
     </div>
     <div class="grid-row">
       <div class="column-one-quarter">

app/templates/views/all-services.html

@@ -1,16 +0,0 @@
-{% extends "withoutnav_template.html" %}
-{% from "components/browse-list.html" import browse_list %}
-
-{% block per_page_title %}
-  All services
-{% endblock %}
-
-{% block maincolumn_content %}
-
-  <h1 class="heading-large">
-    All services
-  </h1>
-
-  {{ browse_list(services) }}
-
-{% endblock %}

app/templates/views/choose-account.html

@@ -0,0 +1,42 @@
+{% extends "withoutnav_template.html" %}
+
+{% block per_page_title %}
+  Choose service
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">
+    Choose service
+  </h1>
+  <nav class="browse-list">
+    <ul>
+      {% for org in organisations %}
+        <li class="browse-list-item">
+          <a href="{{ url_for('.organisation_dashboard', org_id=org.id) }}" class="browse-list-link">{{ org.name }}</a>
+          {% if org.services %}
+            <ul class="browse-sub-list">
+              {% for item in org.services %}
+                <li class="browse-list-sub-item">
+                  <a href="{{ url_for('.service_dashboard', service_id=item.id) }}" class="browse-list-link">{{ item.name }}</a>
+                </li>
+              {% endfor %}
+            </ul>
+          {% endif %}
+        </li>
+        <div class ="keyline-block"></div>
+      {% endfor %}
+      {% if services_without_organisations %}
+        {% for item in services_without_organisations %}
+          <li class="browse-list-item">
+            <a href="{{ url_for('.service_dashboard', service_id=item.id) }}" class="browse-list-link">{{ item.name }}</a>
+          </li>
+        {% endfor %}
+        <div class ="keyline-block"></div>
+      {% endif %}
+    </ul>
+  {% if can_add_service %}
+    <a href="{{ url_for('.add_service') }}" class="browse-list-link">Add a new service…</a>
+  {% endif %}
+
+{% endblock %}

app/templates/views/choose-service.html

@@ -1,25 +0,0 @@
-{% extends "withoutnav_template.html" %}
-{% from "components/browse-list.html" import browse_list %}
-
-{% block per_page_title %}
-  Choose service
-{% endblock %}
-
-{% block maincolumn_content %}
-
-  <h1 class="heading-large">
-    Choose service
-  </h1>
-
-  {{ browse_list(services) }}
-  {% if can_add_service %}
-    {{ browse_list([
-      {
-        'title': 'Add a new service…',
-        'link': url_for('.add_service')
-      },
-    ]) }}
-  {% endif %}
-
-
-{% endblock %}

app/templates/views/service-settings.html

@@ -1,6 +1,5 @@
 {% extends "withnav_template.html" %}
 {% from "components/banner.html" import banner_wrapper %}
-{% from "components/browse-list.html" import browse_list %}
 {% from "components/table.html" import mapping_table, row, text_field, optional_text_field, edit_field, field, boolean_field with context %}
 
 {% block service_page_title %}

app/templates/withnav_template.html

@@ -10,7 +10,7 @@
       <div class="navigation-service-name">
         {{ current_service.name }}
       </div>
-      <a href="{{ url_for('main.choose_service') }}" class="navigation-service-switch">Switch service</a>
+      <a href="{{ url_for('main.choose_account') }}" class="navigation-service-switch">Switch service</a>
     </div>
     <div class="grid-row">
       {% if help %}

app/templates/withoutnav_template.html

@@ -4,7 +4,7 @@
   <div id="content">
     {% if current_service %}
     <div class="navigation-service">
-      <a href="{{ url_for('main.show_all_services_or_dashboard') }}">Back to {{ current_service.name }}</a>
+      <a href="{{ url_for('main.show_accounts_or_dashboard') }}">Back to {{ current_service.name }}</a>
     </div>
     {% endif %}
     <main role="main">

app/utils.py

@@ -33,32 +33,6 @@ FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technic
 REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES
 
 
-class BrowsableItem(object):
-    """
-    Maps for the template browse-list.
-    """
-
-    def __init__(self, item, *args, **kwargs):
-        self._item = item
-        super(BrowsableItem, self).__init__()
-
-    @property
-    def title(self):
-        pass
-
-    @property
-    def link(self):
-        pass
-
-    @property
-    def hint(self):
-        pass
-
-    @property
-    def destructive(self):
-        pass
-
-
 def user_has_permissions(*permissions, **permission_kwargs):
     def wrap(func):
         @wraps(func)

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=None
 
 ):
     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': list(permissions.keys()) if services is None else services
     }
 
 

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 service'
+    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 service'
+    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

tests/app/main/views/accounts/test_show_accounts_or_dashboard.py

@@ -0,0 +1,149 @@
+import pytest
+from flask import url_for
+from tests import user_json
+
+from app.notify_client.models import User
+
+
+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)],
+        platform_admin=platform_admin
+    ))
+
+
+@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'))
+
+    assert response.status_code == 302
+    assert response.location == url_for(endpoint, _external=True, **endpoint_kwargs)
+
+
+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
+):
+    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_doesnt_redirect_to_org_dashboard_if_user_not_part_of_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'] = 'org2'
+
+    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_not_logged_in(
+    client,
+    app_
+):
+    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
+    )

tests/app/main/views/organisations/test_organisation_invites.py

@@ -5,7 +5,7 @@ import pytest
 from bs4 import BeautifulSoup
 from flask import url_for
 from notifications_python_client.errors import HTTPError
-from tests.conftest import normalize_spaces
+from tests.conftest import ORGANISATION_ID, normalize_spaces
 
 from app.notify_client.models import InvitedOrgUser
 
@@ -42,11 +42,10 @@ def test_organisation_page_shows_all_organisations(
 
 
 def test_view_organisation_shows_the_correct_organisation(
-    logged_in_platform_admin_client,
+    logged_in_client,
-    fake_uuid,
     mocker
 ):
-    org = {'id': fake_uuid, 'name': 'Test 1', 'active': True}
+    org = {'id': ORGANISATION_ID, 'name': 'Test 1', 'active': True}
     mocker.patch(
         'app.organisations_client.get_organisation', return_value=org
     )
@@ -54,8 +53,8 @@ def test_view_organisation_shows_the_correct_organisation(
         'app.organisations_client.get_organisation_services', return_value=[]
     )
 
-    response = logged_in_platform_admin_client.get(
+    response = logged_in_client.get(
-        url_for('.organisation_dashboard', org_id=fake_uuid)
+        url_for('.organisation_dashboard', org_id=ORGANISATION_ID)
     )
 
     assert response.status_code == 200
@@ -85,14 +84,14 @@ def test_create_new_organisation(
 
 
 def test_organisation_services_show(
-    logged_in_platform_admin_client,
+    logged_in_client,
     mock_get_organisation,
     mock_get_organisation_services,
     mocker,
     fake_uuid,
 ):
-    response = logged_in_platform_admin_client.get(
+    response = logged_in_client.get(
-        url_for('.organisation_dashboard', org_id=mock_get_organisation['id']),
+        url_for('.organisation_dashboard', org_id=ORGANISATION_ID),
     )
 
     assert response.status_code == 200
@@ -111,15 +110,15 @@ def test_organisation_services_show(
 
 
 def test_view_team_members(
-    logged_in_platform_admin_client,
+    logged_in_client,
     mocker,
     mock_get_organisation,
     mock_get_users_for_organisation,
     mock_get_invited_users_for_organisation,
     fake_uuid
 ):
-    response = logged_in_platform_admin_client.get(
+    response = logged_in_client.get(
-        url_for('.manage_org_users', org_id=fake_uuid),
+        url_for('.manage_org_users', org_id=ORGANISATION_ID),
     )
 
     assert response.status_code == 200
@@ -136,11 +135,10 @@ def test_view_team_members(
 
 
 def test_invite_org_user(
-    logged_in_platform_admin_client,
+    logged_in_client,
     mocker,
     mock_get_organisation,
     sample_org_invite,
-    fake_uuid
 ):
 
     mock_invite_org_user = mocker.patch(
@@ -148,27 +146,26 @@ def test_invite_org_user(
         return_value=InvitedOrgUser(**sample_org_invite)
     )
 
-    logged_in_platform_admin_client.post(
+    logged_in_client.post(
-        url_for('.invite_org_user', org_id=mock_get_organisation['id']),
+        url_for('.invite_org_user', org_id=ORGANISATION_ID),
         data={'email_address': 'test@example.gov.uk'}
     )
 
     mock_invite_org_user.assert_called_once_with(
         sample_org_invite['invited_by'],
-        '{}'.format(mock_get_organisation['id']),
+        '{}'.format(ORGANISATION_ID),
         'test@example.gov.uk',
     )
 
 
 def test_invite_org_user_errors_when_same_email_as_inviter(
-    logged_in_platform_admin_client,
+    client_request,
     mocker,
     mock_get_organisation,
     sample_org_invite,
-    fake_uuid
 ):
     new_org_user_data = {
-        'email_address': 'platform@admin.gov.uk',
+        'email_address': 'test@user.gov.uk',
     }
 
     mock_invite_org_user = mocker.patch(
@@ -176,14 +173,13 @@ def test_invite_org_user_errors_when_same_email_as_inviter(
         return_value=InvitedOrgUser(**sample_org_invite)
     )
 
-    response = logged_in_platform_admin_client.post(
+    page = client_request.post(
-        url_for('.invite_org_user', org_id=mock_get_organisation['id']),
+        '.invite_org_user',
-        data=new_org_user_data
+        org_id=ORGANISATION_ID,
+        _data=new_org_user_data,
+        _follow_redirects=True
     )
 
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-
     assert mock_invite_org_user.called is False
     assert normalize_spaces(page.select_one('.error-message').text) == 'You can’t send an invitation to yourself'
 
@@ -226,7 +222,7 @@ def test_cancelled_invite_opened_by_user(
     ) == 'If you need access to Org 1, you’ll have to ask them to invite you again.'
 
     mock_get_user.assert_called_once_with(fake_uuid)
-    mock_get_organisation.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af')
+    mock_get_organisation.assert_called_once_with(ORGANISATION_ID)
 
 
 def test_user_invite_already_accepted(
@@ -238,7 +234,7 @@ def test_user_invite_already_accepted(
     assert response.status_code == 302
     assert response.location == url_for(
         'main.organisation_dashboard',
-        org_id='596364a0-858e-42c8-9062-a8fe822260af',
+        org_id=ORGANISATION_ID,
         _external=True
     )
 
@@ -250,20 +246,19 @@ def test_existing_user_invite_already_is_member_of_organisation(
     mock_get_users_for_organisation,
     mock_accept_org_invite,
     mock_add_user_to_organisation,
-    fake_uuid
 ):
     response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken'))
 
     assert response.status_code == 302
     assert response.location == url_for(
         'main.organisation_dashboard',
-        org_id='596364a0-858e-42c8-9062-a8fe822260af',
+        org_id=ORGANISATION_ID,
         _external=True
     )
 
-    mock_accept_org_invite.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af', ANY)
+    mock_accept_org_invite.assert_called_once_with(ORGANISATION_ID, ANY)
     mock_get_user_by_email.assert_called_once_with('invited_user@test.gov.uk')
-    mock_get_users_for_organisation.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af')
+    mock_get_users_for_organisation.assert_called_once_with(ORGANISATION_ID)
 
 
 def test_existing_user_invite_not_a_member_of_organisation(
@@ -273,22 +268,21 @@ def test_existing_user_invite_not_a_member_of_organisation(
     mock_get_users_for_organisation,
     mock_accept_org_invite,
     mock_add_user_to_organisation,
-    fake_uuid
 ):
     response = client.get(url_for('main.accept_org_invite', token='thisisnotarealtoken'))
 
     assert response.status_code == 302
     assert response.location == url_for(
         'main.organisation_dashboard',
-        org_id='596364a0-858e-42c8-9062-a8fe822260af',
+        org_id=ORGANISATION_ID,
         _external=True
     )
 
-    mock_accept_org_invite.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af', ANY)
+    mock_accept_org_invite.assert_called_once_with(ORGANISATION_ID, ANY)
     mock_get_user_by_email.assert_called_once_with('invited_user@test.gov.uk')
-    mock_get_users_for_organisation.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af')
+    mock_get_users_for_organisation.assert_called_once_with(ORGANISATION_ID)
     mock_add_user_to_organisation.assert_called_once_with(
-        '596364a0-858e-42c8-9062-a8fe822260af',
+        ORGANISATION_ID,
         '6ce466d0-fd6a-11e5-82f5-e0accb9d11a6'
     )
 
@@ -306,7 +300,7 @@ def test_user_accepts_invite(
 
     mock_check_org_invite_token.assert_called_once_with('thisisnotarealtoken')
     mock_dont_get_user_by_email.assert_called_once_with('invited_user@test.gov.uk')
-    mock_get_users_for_organisation.assert_called_once_with('596364a0-858e-42c8-9062-a8fe822260af')
+    mock_get_users_for_organisation.assert_called_once_with(ORGANISATION_ID)
 
 
 def test_registration_from_org_invite_404s_if_user_not_in_session(

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
 

tests/app/main/views/test_choose_services.py

@@ -1,112 +0,0 @@
-from bs4 import BeautifulSoup
-from flask import url_for
-
-
-def test_should_show_choose_services_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'))
-
-    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
-
-
-def test_should_show_choose_services_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'))
-    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
-
-
-def test_redirect_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_all_services_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)
-
-
-def test_redirect_if_multiple_services(
-    logged_in_client,
-    mock_login,
-    api_user_active,
-):
-    response = logged_in_client.get(url_for('main.show_all_services_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for('main.choose_service', _external=True)
-
-
-def test_redirect_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'
-    response = logged_in_client.get(url_for('main.show_all_services_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(
-        'main.service_dashboard',
-        service_id='147ad62a-2951-4fa1-9ca0-093cd1a52c52',
-        _external=True
-    )
-
-
-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'))
-    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_service'))
-
-    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')
-    )
-
-
-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_service'))
-
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert len(page.select('.navigation-service a')) == 0

tests/app/main/views/test_index.py

@@ -23,7 +23,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,
@@ -34,7 +34,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', [

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
 

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(

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', [

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)

tests/conftest.py

@@ -704,6 +704,7 @@ def mock_update_service_raise_httperror_duplicate_name(mocker):
 
 SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
 SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
+ORGANISATION_ID = "c011fa40-4cbe-4524-b415-dde2f421bd9c"
 
 
 @pytest.fixture(scope='function')
@@ -1165,7 +1166,7 @@ def active_user_with_permissions(fake_uuid):
                                                   'view_activity']},
                  'platform_admin': False,
                  'auth_type': 'sms_auth',
-                 'organisations': []
+                 'organisations': [ORGANISATION_ID]
                  }
     user = User(user_data)
     return user
@@ -2692,7 +2693,7 @@ def mock_update_service_callback_api(mocker):
 
 @pytest.fixture(scope='function')
 def organisation_one(api_user_active):
-    return organisation_json('596364a0-858e-42c8-9062-a8fe822260af', 'organisation one', [api_user_active.id])
+    return organisation_json(ORGANISATION_ID, 'organisation one', [api_user_active.id])
 
 
 @pytest.fixture(scope='function')
@@ -2866,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
+    )