Remove separate function for live service check

When we get a support ticket we need to check whether a user has any
live services.

We have a method for this on the user model now, so we don’t need a
separate function in the feedback code.

It wasn’t very well tested so I’ve adapted the old tests from the
feedback view to work against the method on the user model too.

app/main/views/feedback.py

@@ -5,7 +5,7 @@ from flask import redirect, render_template, request, session, url_for
 from flask_login import current_user
 from govuk_bank_holidays.bank_holidays import BankHolidays
 
-from app import convert_to_boolean, current_service, service_api_client
+from app import convert_to_boolean, current_service
 from app.extensions import zendesk_client
 from app.main import main
 from app.main.forms import (
@@ -205,19 +205,12 @@ def is_bank_holiday(time):
     return bank_holidays.is_holiday(time.date())
 
 
-def has_live_services(user_id):
-    return any(
-        service['restricted'] is False
-        for service in service_api_client.get_services({'user_id': user_id})['data']
-    )
-
-
 def needs_triage(ticket_type, severe):
     return all((
         ticket_type != QUESTION_TICKET_TYPE,
         severe is None,
         (
-            not current_user.is_authenticated or has_live_services(current_user.id)
+            not current_user.is_authenticated or current_user.live_services
         ),
         not in_business_hours(),
     ))

tests/app/main/views/test_feedback.py

@@ -1,12 +1,12 @@
 from functools import partial
-from unittest.mock import ANY
+from unittest.mock import ANY, PropertyMock
 
 import pytest
 from bs4 import BeautifulSoup, element
 from flask import url_for
 from freezegun import freeze_time
 
-from app.main.views.feedback import has_live_services, in_business_hours
+from app.main.views.feedback import in_business_hours
 from app.models.feedback import (
     GENERAL_TICKET_TYPE,
     PROBLEM_TICKET_TYPE,
@@ -70,6 +70,7 @@ def test_get_support_index_page_when_signed_out(
 ])
 def test_choose_support_type(
     client_request,
+    mock_get_non_empty_organisations_and_services_for_user,
     support_type,
     expected_h1
 ):
@@ -171,6 +172,7 @@ def test_passed_non_logged_in_user_details_through_flow(client, mocker, ticket_t
 @pytest.mark.parametrize('ticket_type', [PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE])
 def test_passes_user_details_through_flow(
     client_request,
+    mock_get_non_empty_organisations_and_services_for_user,
     mocker,
     ticket_type,
     data
@@ -279,6 +281,7 @@ def test_email_address_must_be_valid_if_provided_to_support_form(
 ])
 def test_urgency(
     client_request,
+    mock_get_non_empty_organisations_and_services_for_user,
     mocker,
     ticket_type,
     severe,
@@ -346,7 +349,11 @@ def test_redirects_to_triage(
     expected_status,
     expected_redirect,
 ):
-    mocker.patch('app.main.views.feedback.has_live_services', return_value=has_live_services)
+    mocker.patch(
+        'app.models.user.User.live_services',
+        new_callable=PropertyMock,
+        return_value=[{}, {}] if has_live_services else [],
+    )
     mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours)
     if logged_in:
         client.login(api_user_active)
@@ -376,7 +383,7 @@ def test_doesnt_lose_message_if_post_across_closing(
     mocker,
 ):
 
-    mocker.patch('app.main.views.feedback.has_live_services', return_value=True)
+    mocker.patch('app.models.user.User.live_services', return_value=True)
     mocker.patch('app.main.views.feedback.in_business_hours', return_value=False)
 
     page = client_request.post(
@@ -400,18 +407,6 @@ def test_doesnt_lose_message_if_post_across_closing(
         assert 'feedback_message' not in session
 
 
-def test_has_live_services(mock_get_services):
-    assert has_live_services(12345) is True
-
-
-def test_has_live_services_when_there_are_no_services(mock_get_services_with_no_services):
-    assert has_live_services(12345) is False
-
-
-def test_has_live_services_when_service_is_not_live(mock_get_services_with_one_service):
-    assert has_live_services(12345) is False
-
-
 @pytest.mark.parametrize('when, is_in_business_hours', [
 
     ('2016-06-06 09:29:59+0100', False),  # opening time, summer and winter
@@ -482,6 +477,7 @@ def test_triage_redirects_to_correct_url(
 ])
 def test_back_link_from_form(
     client_request,
+    mock_get_non_empty_organisations_and_services_for_user,
     extra_args,
     expected_back_link,
 ):
@@ -544,7 +540,7 @@ def test_should_be_shown_the_bat_email(
     active_user_with_permissions,
     mocker,
     service_one,
-    mock_get_services,
+    mock_get_non_empty_organisations_and_services_for_user,
     is_in_business_hours,
     severe,
     expected_status_code,
@@ -596,7 +592,7 @@ def test_should_be_shown_the_bat_email_for_general_questions(
     active_user_with_permissions,
     mocker,
     service_one,
-    mock_get_services,
+    mock_get_non_empty_organisations_and_services_for_user,
     severe,
     expected_status_code,
     expected_redirect,

tests/app/models/test_user.py

@@ -70,3 +70,39 @@ def test_platform_admin_flag_set_in_session(client, mocker, is_platform_admin, v
     mocker.patch.dict('app.models.user.session', values=session_dict, clear=True)
 
     assert User({'platform_admin': is_platform_admin}).platform_admin == expected_result
+
+
+def test_has_live_services(
+    client_request,
+    mock_get_non_empty_organisations_and_services_for_user,
+    fake_uuid,
+):
+    user = User({
+        'id': fake_uuid,
+        'platform_admin': False,
+    })
+    assert len(user.live_services) == 5
+    for service in user.live_services:
+        assert service.live
+
+
+def test_has_live_services_when_there_are_no_services(
+    client_request,
+    mock_get_organisations_and_services_for_user,
+    fake_uuid,
+):
+    assert User({
+        'id': fake_uuid,
+        'platform_admin': False,
+    }).live_services == []
+
+
+def test_has_live_services_when_service_is_not_live(
+    client_request,
+    mock_get_empty_organisations_and_one_service_for_user,
+    fake_uuid,
+):
+    assert User({
+        'id': fake_uuid,
+        'platform_admin': False,
+    }).live_services == []

tests/conftest.py

@@ -3672,6 +3672,7 @@ def mock_get_empty_organisations_and_one_service_for_user(mocker, organisation_o
             'services': [{
                 'name': 'Only service',
                 'id': SERVICE_TWO_ID,
+                'restricted': True,
             }]
         }