Merge pull request #3143 from alphagov/jobs-are-uploads

Merge ‘uploaded files’ and ‘uploads’ pages

app/main/views/uploads.py

@@ -19,6 +19,7 @@ from app import current_service, notification_api_client, service_api_client
 from app.extensions import antivirus_client
 from app.main import main
 from app.main.forms import PDFUploadForm
+from app.main.views.jobs import view_jobs
 from app.s3_client.s3_letter_upload_client import (
     get_letter_metadata,
     get_letter_pdf_and_metadata,
@@ -36,9 +37,9 @@ MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024  # 2MB
 
 
 @main.route("/services/<service_id>/uploads")
-@user_has_permissions('send_messages')
+@user_has_permissions()
 def uploads(service_id):
-    return render_template('views/uploads/index.html')
+    return view_jobs(service_id)
 
 
 @main.route("/services/<service_id>/upload-letter", methods=['GET', 'POST'])

app/models/service.py

@@ -462,6 +462,10 @@ class Service(JSONModel):
             key=lambda folder: folder['name'].lower(),
         )
 
+    @property
+    def can_upload_letters(self):
+        return self.has_permission('letter') and self.has_permission('upload_letters')
+
     @cached_property
     def all_template_folder_ids(self):
         return {folder['id'] for folder in self.all_template_folders}

app/navigation.py

@@ -633,9 +633,12 @@ class CaseworkNavigation(Navigation):
             'view_notifications',
             'view_notification',
         },
-        'uploaded-files': {
+        'uploads': {
             'view_jobs',
             'view_job',
+            'upload_letter',
+            'uploaded_letter_preview',
+            'uploads',
         },
     }
 
@@ -868,9 +871,6 @@ class CaseworkNavigation(Navigation):
         'two_factor_email_sent',
         'update_email_branding',
         'update_letter_branding',
-        'upload_letter',
-        'uploaded_letter_preview',
-        'uploads',
         'usage',
         'usage_for_all_services',
         'user_information',

app/templates/components/previous-next-navigation.html

@@ -1,32 +1,34 @@
 {% macro previous_next_navigation(previous_page, next_page) %}
-  <nav class="govuk-previous-and-next-navigation" role="navigation" aria-label="Pagination">
-    <ul class="group">
-      {% if previous_page %}
-        <li class="previous-page">
-          <a href="{{previous_page['url']}}" rel="previous" >
-            <span class="pagination-part-title">
-              <svg class="pagination-icon" xmlns="http://www.w3.org/2000/svg" height="13" width="17" viewBox="0 0 17 13">
-                <path fill="currentColor" d="m6.5938-0.0078125-6.7266 6.7266 6.7441 6.4062 1.377-1.449-4.1856-3.9768h12.896v-2h-12.984l4.2931-4.293-1.414-1.414z"/>
-              </svg>
-              {{previous_page['title']}}
-            </span>
-            <span class="pagination-label">{{previous_page['label']}}</span>
-          </a>
-        </li>
-      {% endif %}
-      {% if next_page %}
-        <li class="next-page">
-          <a href="{{next_page['url']}}" rel="next">
-            <span class="pagination-part-title">
-              {{next_page['title']}}
-              <svg class="pagination-icon" xmlns="http://www.w3.org/2000/svg" height="13" width="17" viewBox="0 0 17 13">
-                <path fill="currentColor" d="m10.107-0.0078125-1.4136 1.414 4.2926 4.293h-12.986v2h12.896l-4.1855 3.9766 1.377 1.4492 6.7441-6.4062-6.7246-6.7266z"/>
-              </svg>
-            </span>
-            <span class="pagination-label">{{next_page['label']}}</span>
-          </a>
-        </li>
-      {% endif %}
-    </ul>
-  </nav>
+  {% if previous_page or next_page %}
+    <nav class="govuk-previous-and-next-navigation" role="navigation" aria-label="Pagination">
+      <ul class="group">
+        {% if previous_page %}
+          <li class="previous-page">
+            <a href="{{previous_page['url']}}" rel="previous" >
+              <span class="pagination-part-title">
+                <svg class="pagination-icon" xmlns="http://www.w3.org/2000/svg" height="13" width="17" viewBox="0 0 17 13">
+                  <path fill="currentColor" d="m6.5938-0.0078125-6.7266 6.7266 6.7441 6.4062 1.377-1.449-4.1856-3.9768h12.896v-2h-12.984l4.2931-4.293-1.414-1.414z"/>
+                </svg>
+                {{previous_page['title']}}
+              </span>
+              <span class="pagination-label">{{previous_page['label']}}</span>
+            </a>
+          </li>
+        {% endif %}
+        {% if next_page %}
+          <li class="next-page">
+            <a href="{{next_page['url']}}" rel="next">
+              <span class="pagination-part-title">
+                {{next_page['title']}}
+                <svg class="pagination-icon" xmlns="http://www.w3.org/2000/svg" height="13" width="17" viewBox="0 0 17 13">
+                  <path fill="currentColor" d="m10.107-0.0078125-1.4136 1.414 4.2926 4.293h-12.986v2h12.896l-4.1855 3.9766 1.377 1.4492 6.7441-6.4062-6.7246-6.7266z"/>
+                </svg>
+              </span>
+              <span class="pagination-label">{{next_page['label']}}</span>
+            </a>
+          </li>
+        {% endif %}
+      </ul>
+    </nav>
+  {% endif %}
 {% endmacro %}

app/templates/main_nav.html

@@ -8,15 +8,16 @@
   {% endif %}
   {% if current_user.has_permissions() %}
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}" {{ main_navigation.is_selected('templates') }}>Templates</a></li>
-    {% if not current_user.has_permissions('view_activity') %}
+    {% if current_user.has_permissions('view_activity') %}
+      {% if current_service.can_upload_letters %}
+        <li><a href="{{ url_for('main.uploads', service_id=current_service.id) }}" {{ main_navigation.is_selected('uploads') }}>Uploads</a></li>
+      {% endif %}
+    {% else %}
       <li><a href="{{ url_for('.view_notifications', service_id=current_service.id, status='sending,delivered,failed') }}" {{ casework_navigation.is_selected('sent-messages') }}>Sent messages</a></li>
-      {% if current_service.has_jobs %}
-        <li><a href="{{ url_for('.view_jobs', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploaded-files') }}>Uploaded files</a></li>
+      {% if current_service.has_jobs or current_service.can_upload_letters %}
+        <li><a href="{{ url_for('main.uploads', service_id=current_service.id) }}" {{ casework_navigation.is_selected('uploads') }}>Uploads</a></li>
       {% endif %}
     {% endif %}
-  {% endif %}
-  {% if current_user.has_permissions('send_messages') and current_service.has_permission('letter') and current_service.has_permission('upload_letters') %}
-    <li><a href="{{ url_for('main.uploads', service_id=current_service.id) }}" {{ main_navigation.is_selected('uploads') }}>Uploads</a></li>
   {% endif %}
     <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}" {{ main_navigation.is_selected('team-members') }}>Team members</a></li>
   {% if current_user.has_permissions('manage_service', allow_org_user=True) %}

app/templates/views/dashboard/_jobs.html

@@ -6,14 +6,16 @@
     jobs,
     caption="Recent files uploaded",
     caption_visible=False,
-    empty_message='You have not uploaded any files recently',
+    empty_message=(
+      'Upload a letter and Notify will print, pack and post it for you.' if current_service.can_upload_letters else 'You have not uploaded any files recently'
+    ),
     field_headings=[
       'File',
       'Sending',
       'Delivered',
       'Failed'
     ],
-    field_headings_visible=True
+    field_headings_visible=True if jobs else False
   ) %}
     {% call row_heading() %}
       <div class="file-list">

app/templates/views/jobs/jobs.html

@@ -2,14 +2,19 @@
 {% extends "withnav_template.html" %}
 
 {% block service_page_title %}
-  Uploaded files
+  Uploads
 {% endblock %}
 
 {% block maincolumn_content %}
-    <h1 class="heading-large">Uploaded files</h1>
+    <h1 class="heading-large">Uploads</h1>
     <div class="dashboard">
       {{ scheduled_jobs|safe }}
       {% include 'views/dashboard/_jobs.html' %}
       {{ previous_next_navigation(prev_page, next_page) }}
+      {% if current_service.can_upload_letters and current_user.has_permissions('send_messages') %}
+        <div class="js-stick-at-bottom-when-scrolling">
+          <a href="{{ url_for('.upload_letter', service_id=current_service.id) }}" class="button-secondary" role="button">Upload a letter</a>
+        </div>
+      {% endif %}
     </div>
 {% endblock %}

app/templates/views/uploads/index.html

@@ -1,18 +0,0 @@
-{% extends "withnav_template.html" %}
-{% from "components/page-header.html" import page_header %}
-
-{% block service_page_title %}
-  Uploads
-{% endblock %}
-
-{% block maincolumn_content %}
-<div class="grid-row">
-  <div class="column-five-sixths">
-    {{ page_header('Uploads') }}
-
-    <p>Upload a letter and Notify will print, pack and post it for you.</p>
-
-    <a href="{{ url_for('.upload_letter', service_id=current_service.id) }}" class="button-secondary">Upload a letter</a>
-  </div>
-</div>
-{% endblock %}

tests/app/main/views/test_uploads.py

@@ -8,9 +8,33 @@ from app.utils import normalize_spaces
 from tests.conftest import SERVICE_ONE_ID
 
 
-def test_get_upload_hub_page(client_request):
+@pytest.mark.parametrize('extra_permissions', (
+    [],
+    ['letter'],
+    ['upload_letters'],
+    pytest.param(
+        ['letter', 'upload_letters'],
+        marks=pytest.mark.xfail(raises=AssertionError),
+    ),
+))
+def test_no_upload_letters_button_without_permission(
+    client_request,
+    service_one,
+    mock_get_jobs,
+    extra_permissions,
+):
+    service_one['permissions'] += extra_permissions
     page = client_request.get('main.uploads', service_id=SERVICE_ONE_ID)
+    assert not page.find('a', text='Upload a letter')
 
+
+def test_get_upload_hub_page(
+    client_request,
+    service_one,
+    mock_get_jobs,
+):
+    service_one['permissions'] += ['letter', 'upload_letters']
+    page = client_request.get('main.uploads', service_id=SERVICE_ONE_ID)
     assert page.find('h1').text == 'Uploads'
     assert page.find('a', text='Upload a letter').attrs['href'] == url_for(
         'main.upload_letter', service_id=SERVICE_ONE_ID

tests/app/test_navigation.py

@@ -180,5 +180,5 @@ def test_caseworkers_see_jobs_nav_if_jobs_exist(
     )
     page = client_request.get('main.choose_template', service_id=SERVICE_ONE_ID)
     assert normalize_spaces(page.select_one('#content nav').text) == (
-        'Templates Sent messages Uploaded files Team members'
+        'Templates Sent messages Uploads Team members'
     )