From 7eb547a9e8e538050c8c28851a39abe7dcf23042 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 10:52:11 +0100 Subject: [PATCH 1/6] Put upload letters button on jobs page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is going to become the one true ‘Uploads’ page, so it need the sticky footer that takes users into the new upload letters journey. --- app/models/service.py | 4 ++++ app/templates/main_nav.html | 2 +- app/templates/views/dashboard/_jobs.html | 6 ++++-- app/templates/views/jobs/jobs.html | 5 +++++ tests/app/main/views/test_uploads.py | 8 ++++++-- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/app/models/service.py b/app/models/service.py index 3e3e93fd6..bf989cfeb 100644 --- a/app/models/service.py +++ b/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} diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 7f6a1fa55..88c1e4eca 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -15,7 +15,7 @@ {% endif %} {% endif %} {% endif %} - {% if current_user.has_permissions('send_messages') and current_service.has_permission('letter') and current_service.has_permission('upload_letters') %} + {% if current_user.has_permissions('send_messages') and current_service.can_upload_letters %}
  • Uploads
    • {% endif %}
  • Team members
    • diff --git a/app/templates/views/dashboard/_jobs.html b/app/templates/views/dashboard/_jobs.html index 30250b6ce..75a142a76 100644 --- a/app/templates/views/dashboard/_jobs.html +++ b/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() %}
    diff --git a/app/templates/views/jobs/jobs.html b/app/templates/views/jobs/jobs.html index 7d9bbba01..928ec0bec 100644 --- a/app/templates/views/jobs/jobs.html +++ b/app/templates/views/jobs/jobs.html @@ -11,5 +11,10 @@ {{ 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') %} +
    + Upload a letter +
    + {% endif %}
    {% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 891972037..ab4d355b2 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -8,9 +8,13 @@ from app.utils import normalize_spaces from tests.conftest import SERVICE_ONE_ID -def test_get_upload_hub_page(client_request): +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 From f9094cb98b044f6d0bf37decbd90dc5cc7f727d6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 10:52:33 +0100 Subject: [PATCH 2/6] Rename uploaded files to uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploads is the name that we’ve been using in the prototype. --- app/templates/main_nav.html | 2 +- app/templates/views/jobs/jobs.html | 4 ++-- tests/app/test_navigation.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 88c1e4eca..6adc195ef 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -11,7 +11,7 @@ {% if not current_user.has_permissions('view_activity') %}
  • Sent messages
    • {% if current_service.has_jobs %} -
  • Uploaded files
    • +
  • Uploads
    • {% endif %} {% endif %} {% endif %} diff --git a/app/templates/views/jobs/jobs.html b/app/templates/views/jobs/jobs.html index 928ec0bec..7866fa541 100644 --- a/app/templates/views/jobs/jobs.html +++ b/app/templates/views/jobs/jobs.html @@ -2,11 +2,11 @@ {% extends "withnav_template.html" %} {% block service_page_title %} - Uploaded files + Uploads {% endblock %} {% block maincolumn_content %} -

    Uploaded files

    +

    Uploads

    {{ scheduled_jobs|safe }} {% include 'views/dashboard/_jobs.html' %} diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 8cfb8c2ba..94bf7ec3b 100644 --- a/tests/app/test_navigation.py +++ b/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' ) From 93322d41e5cc33405f4b119d94b19c289cb1262e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 10:54:04 +0100 Subject: [PATCH 3/6] Make jobs and uploads page the same MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes these pages call the same code. We can resolve the multiple URLs once we’ve removed the feature flag. --- app/main/views/uploads.py | 3 ++- app/templates/views/uploads/index.html | 18 ------------------ tests/app/main/views/test_uploads.py | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 19 deletions(-) delete mode 100644 app/templates/views/uploads/index.html diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 4709b58b4..b4ff77d83 100644 --- a/app/main/views/uploads.py +++ b/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, @@ -38,7 +39,7 @@ MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @main.route("/services//uploads") @user_has_permissions('send_messages') def uploads(service_id): - return render_template('views/uploads/index.html') + return view_jobs(service_id) @main.route("/services//upload-letter", methods=['GET', 'POST']) diff --git a/app/templates/views/uploads/index.html b/app/templates/views/uploads/index.html deleted file mode 100644 index 0b2ad0480..000000000 --- a/app/templates/views/uploads/index.html +++ /dev/null @@ -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 %} -
    -
    - {{ page_header('Uploads') }} - -

    Upload a letter and Notify will print, pack and post it for you.

    - - Upload a letter -
    -
    -{% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index ab4d355b2..a45daaafc 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -8,6 +8,26 @@ from app.utils import normalize_spaces from tests.conftest import SERVICE_ONE_ID +@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, From 2ddc6216a55248f49a2f73b90eb20e97aa4725b9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 10:56:08 +0100 Subject: [PATCH 4/6] Let any user see the uploads page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even if you can’t send messages you might still want to see the messages that other people have sent. --- app/main/views/uploads.py | 2 +- app/templates/main_nav.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index b4ff77d83..665fce5a5 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -37,7 +37,7 @@ MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @main.route("/services//uploads") -@user_has_permissions('send_messages') +@user_has_permissions() def uploads(service_id): return view_jobs(service_id) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 6adc195ef..b81503ad8 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -15,7 +15,7 @@ {% endif %} {% endif %} {% endif %} - {% if current_user.has_permissions('send_messages') and current_service.can_upload_letters %} + {% if current_service.can_upload_letters %}
  • Uploads
    • {% endif %}
  • Team members
    • From 938b38d082402196b5cadb6f4ed0f1c9a3bf192f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 10:58:15 +0100 Subject: [PATCH 5/6] Let caseworkers see uploads page At the moment they can only see it if there are existing jobs. This commit lets them also see it if their service can upload letters, because caseworkers might be the ones uploading some letters. --- app/navigation.py | 8 ++++---- app/templates/main_nav.html | 13 +++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/navigation.py b/app/navigation.py index c7eafb68b..f0d30ffbd 100644 --- a/app/navigation.py +++ b/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', diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index b81503ad8..884a78441 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -8,15 +8,16 @@ {% endif %} {% if current_user.has_permissions() %}
  • Templates
    • - {% if not current_user.has_permissions('view_activity') %} + {% if current_user.has_permissions('view_activity') %} + {% if current_service.can_upload_letters %} +
  • Uploads
    • + {% endif %} + {% else %}
  • Sent messages
    • - {% if current_service.has_jobs %} -
  • Uploads
    • + {% if current_service.has_jobs or current_service.can_upload_letters %} +
  • Uploads
    • {% endif %} {% endif %} - {% endif %} - {% if current_service.can_upload_letters %} -
  • Uploads
    • {% endif %}
  • Team members
    • {% if current_user.has_permissions('manage_service', allow_org_user=True) %} From 7a55a5c9f2446850218917fb5413e462072b5d2f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 9 Oct 2019 14:10:29 +0100 Subject: [PATCH 6/6] Hide prev/next when neither prev nor next are set Otherwise it still takes up vertical space on the page. --- .../components/previous-next-navigation.html | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/app/templates/components/previous-next-navigation.html b/app/templates/components/previous-next-navigation.html index e7102d4a0..b73846c6a 100644 --- a/app/templates/components/previous-next-navigation.html +++ b/app/templates/components/previous-next-navigation.html @@ -1,32 +1,34 @@ {% macro previous_next_navigation(previous_page, next_page) %} - + {% if previous_page or next_page %} + + {% endif %} {% endmacro %}