Merge pull request #3658 from alphagov/use-scheduled-job-stats

Use new API endpoint for scheduled job stats

app/models/service.py

@@ -151,6 +151,12 @@ class Service(JSONModel):
             return []
         return ScheduledJobs(self.id)
 
+    @cached_property
+    def scheduled_job_stats(self):
+        if not self.has_jobs:
+            return {'count': 0}
+        return job_api_client.get_scheduled_job_stats(self.id)
+
     @cached_property
     def invited_users(self):
         return InvitedUsers(self.id)

app/notify_client/job_api_client.py

@@ -74,6 +74,11 @@ class JobApiClient(NotifyAdminAPIClient):
             reverse=True,
         )
 
+    def get_scheduled_job_stats(self, service_id):
+        return self.get(
+            url=f'/service/{service_id}/job/scheduled-job-stats'
+        )
+
     @cache.set('has_jobs-{service_id}')
     def has_jobs(self, service_id):
         return bool(self.get_jobs(service_id)['data'])

app/templates/views/dashboard/_upcoming.html

@@ -3,16 +3,16 @@
 {% from "components/show-more.html" import show_more %}
 
 <div class="ajax-block-container">
-  {% if current_service.scheduled_jobs %}
+  {% if current_service.scheduled_job_stats.count %}
     <h2 class="heading-medium heading-upcoming-jobs">
       In the next few days
     </h2>
     <a class="govuk-link govuk-link--no-visited-state banner-dashboard" href="{{ url_for('.uploads', service_id=current_service.id) }}">
       <span class="banner-dashboard-count">
-        {{ current_service.scheduled_jobs|length }}
+        {{ current_service.scheduled_job_stats.count }}
       </span>
       <span class="banner-dashboard-count-label">
-        {% if current_service.scheduled_jobs|length == 1 %}
+        {% if current_service.scheduled_job_stats.count == 1 %}
           file waiting to send
         {% else %}
           files waiting to send
@@ -20,7 +20,7 @@
       </span>
       <span class="banner-dashboard-meta">
         sending starts
-        {{ current_service.scheduled_jobs[-1].scheduled_for|format_datetime_relative }}
+        {{ current_service.scheduled_job_stats.soonest_scheduled_for|format_datetime_relative }}
       </span>
     </a>
   {% endif %}

tests/app/main/views/test_accept_invite.py

@@ -143,7 +143,7 @@ def test_accepting_invite_removes_invite_from_session(
     mock_accept_invite,
     mock_get_service_templates,
     mock_get_template_statistics,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_template_folders,
     mock_get_usage,
@@ -503,7 +503,7 @@ def test_new_invited_user_verifies_and_added_to_service(
     mock_get_service,
     mock_get_service_templates,
     mock_get_template_statistics,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_has_permissions,
     mock_get_users_by_service,
     mock_get_service_statistics,

tests/app/main/views/test_dashboard.py

@@ -139,7 +139,7 @@ def test_get_started(
     client_request,
     mocker,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
@@ -164,7 +164,7 @@ def test_get_started_is_hidden_once_templates_exist(
     client_request,
     mocker,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
@@ -189,7 +189,7 @@ def test_inbound_messages_not_visible_to_service_without_permissions(
     mocker,
     service_one,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -215,6 +215,7 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_messages(
     service_one,
     mock_get_service_templates_when_no_templates_exist,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -242,6 +243,7 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_no_messages(
     service_one,
     mock_get_service_templates_when_no_templates_exist,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -484,7 +486,7 @@ def test_returned_letters_not_visible_if_service_has_no_returned_letters(
     mocker,
     service_one,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -520,6 +522,7 @@ def test_returned_letters_shows_count_of_recently_returned_letters(
     service_one,
     mock_get_service_templates_when_no_templates_exist,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -579,6 +582,7 @@ def test_returned_letters_only_counts_recently_returned_letters(
     service_one,
     mock_get_service_templates_when_no_templates_exist,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_service_statistics,
     mock_get_template_statistics,
     mock_get_usage,
@@ -610,7 +614,7 @@ def test_should_show_recent_templates_on_dashboard(
     client_request,
     mocker,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
@@ -664,7 +668,7 @@ def test_should_not_show_recent_templates_on_dashboard_if_only_one_template_used
     client_request,
     mocker,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_service_statistics,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
@@ -821,6 +825,7 @@ def test_should_show_upcoming_jobs_on_dashboard(
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_inbound_sms_summary,
@@ -830,9 +835,8 @@ def test_should_show_upcoming_jobs_on_dashboard(
         'main.service_dashboard',
         service_id=SERVICE_ONE_ID,
     )
-    second_call = mock_get_jobs.call_args_list[1]
-    assert second_call[0] == (SERVICE_ONE_ID,)
-    assert second_call[1]['statuses'] == ['scheduled']
+    mock_get_jobs.assert_called_once_with(SERVICE_ONE_ID)
+    mock_get_scheduled_job_stats.assert_called_once_with(SERVICE_ONE_ID)
 
     assert normalize_spaces(
         page.select_one('main h2').text
@@ -852,6 +856,54 @@ def test_should_show_upcoming_jobs_on_dashboard(
     )
 
 
+def test_should_not_show_upcoming_jobs_on_dashboard_if_count_is_0(
+    mocker,
+    client_request,
+    mock_get_service_templates,
+    mock_get_template_statistics,
+    mock_get_service_statistics,
+    mock_has_jobs,
+    mock_get_usage,
+    mock_get_free_sms_fragment_limit,
+    mock_get_inbound_sms_summary,
+    mock_get_returned_letter_statistics_with_no_returned_letters,
+):
+    mocker.patch('app.job_api_client.get_scheduled_job_stats', return_value={
+        'count': 0,
+        'soonest_scheduled_for': None,
+    })
+    page = client_request.get(
+        'main.service_dashboard',
+        service_id=SERVICE_ONE_ID,
+    )
+    mock_has_jobs.assert_called_once_with(SERVICE_ONE_ID)
+    assert 'In the next few days' not in page.select_one('main').text
+    assert 'files waiting to send ' not in page.select_one('main').text
+
+
+def test_should_not_show_upcoming_jobs_on_dashboard_if_service_has_no_jobs(
+    mocker,
+    client_request,
+    mock_get_service_templates,
+    mock_get_template_statistics,
+    mock_get_service_statistics,
+    mock_has_no_jobs,
+    mock_get_scheduled_job_stats,
+    mock_get_usage,
+    mock_get_free_sms_fragment_limit,
+    mock_get_inbound_sms_summary,
+    mock_get_returned_letter_statistics_with_no_returned_letters,
+):
+    page = client_request.get(
+        'main.service_dashboard',
+        service_id=SERVICE_ONE_ID,
+    )
+    mock_has_no_jobs.assert_called_once_with(SERVICE_ONE_ID)
+    assert mock_get_scheduled_job_stats.called is False
+    assert 'In the next few days' not in page.select_one('main').text
+    assert 'files waiting to send ' not in page.select_one('main').text
+
+
 @pytest.mark.parametrize('permissions', (
     ['email', 'sms'],
     ['email', 'sms', 'letter'],
@@ -878,7 +930,7 @@ def test_correct_font_size_for_big_numbers(
     mock_get_service_templates,
     mock_get_template_statistics,
     mock_get_service_statistics,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
@@ -915,6 +967,7 @@ def test_should_not_show_jobs_on_dashboard_for_users_with_uploads_page(
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_jobs,
+    mock_get_scheduled_job_stats,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_inbound_sms_summary,
@@ -1145,7 +1198,7 @@ def test_menu_send_messages(
     api_user_active,
     service_one,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1181,7 +1234,7 @@ def test_menu_send_messages_when_service_does_not_have_upload_letters_permission
     api_user_active,
     service_one,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1207,7 +1260,7 @@ def test_menu_manage_service(
     api_user_active,
     service_one,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1239,7 +1292,7 @@ def test_menu_manage_api_keys(
     api_user_active,
     service_one,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1269,7 +1322,7 @@ def test_menu_all_services_for_platform_admin_user(
     platform_admin_user,
     service_one,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1301,7 +1354,7 @@ def test_route_for_service_permissions(
     mock_get_service,
     mock_get_user,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_template_statistics,
     mock_get_service_statistics,
     mock_get_usage,
@@ -1357,7 +1410,7 @@ def test_service_dashboard_updates_gets_dashboard_totals(
     mock_get_service_templates,
     mock_get_template_statistics,
     mock_get_service_statistics,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_inbound_sms_summary,
@@ -1541,7 +1594,7 @@ def test_org_breadcrumbs_do_not_show_if_service_has_no_org(
     client_request,
     mock_get_template_statistics,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
@@ -1555,7 +1608,7 @@ def test_org_breadcrumbs_do_not_show_if_user_is_not_an_org_member(
     mocker,
     client,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     active_caseworking_user,
     client_request,
     mock_get_template_folders,
@@ -1579,7 +1632,7 @@ def test_org_breadcrumbs_show_if_user_is_a_member_of_the_services_org(
     mocker,
     mock_get_template_statistics,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
@@ -1609,7 +1662,7 @@ def test_org_breadcrumbs_do_not_show_if_user_is_a_member_of_the_services_org_but
     mocker,
     mock_get_template_statistics,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
@@ -1634,7 +1687,7 @@ def test_org_breadcrumbs_show_if_user_is_platform_admin(
     mocker,
     mock_get_template_statistics,
     mock_get_service_templates_when_no_templates_exist,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
@@ -1668,7 +1721,7 @@ def test_should_show_usage_on_dashboard(
     service_one,
     mock_get_service_templates,
     mock_get_template_statistics,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_get_usage,
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,

tests/app/main/views/test_sign_out.py

@@ -24,7 +24,7 @@ def test_sign_out_user(
     mock_get_user_by_email,
     mock_login,
     mock_get_service_templates,
-    mock_get_jobs,
+    mock_has_no_jobs,
     mock_has_permissions,
     mock_get_template_statistics,
     mock_get_service_statistics,

tests/conftest.py

@@ -1798,12 +1798,12 @@ def mock_get_letter_job_in_progress(mocker, api_user_active):
 
 @pytest.fixture(scope='function')
 def mock_has_jobs(mocker):
-    mocker.patch('app.job_api_client.has_jobs', return_value=True)
+    return mocker.patch('app.job_api_client.has_jobs', return_value=True)
 
 
 @pytest.fixture(scope='function')
 def mock_has_no_jobs(mocker):
-    mocker.patch('app.job_api_client.has_jobs', return_value=False)
+    return mocker.patch('app.job_api_client.has_jobs', return_value=False)
 
 
 @pytest.fixture(scope='function')
@@ -1842,6 +1842,15 @@ def mock_get_jobs(mocker, api_user_active):
     return mocker.patch('app.job_api_client.get_jobs', side_effect=_get_jobs)
 
 
+@pytest.fixture(scope='function')
+def mock_get_scheduled_job_stats(mocker, api_user_active):
+    return mocker.patch('app.job_api_client.get_scheduled_job_stats', return_value={
+        # These values match the return value of `mock_get_jobs`
+        'count': 2,
+        'soonest_scheduled_for': '2016-01-01 11:09:00'
+    })
+
+
 @pytest.fixture(scope='function')
 def mock_get_uploads(mocker, api_user_active):
     def _get_uploads(service_id, limit_days=None, statuses=None, page=1):