Merge pull request #554 from alphagov/add-created_by-to-jobs

Update the jobs and activity page to show the user that created the job.

app/main/views/dashboard.py

@@ -17,11 +17,9 @@ from flask_login import login_required
 
 from app.main import main
 from app import (
-    job_api_client,
     statistics_api_client,
     service_api_client,
-    template_statistics_client,
-    current_service
+    template_statistics_client
 )
 
 from app.utils import user_has_permissions

app/main/views/jobs.py

@@ -74,7 +74,8 @@ def view_job(service_id, job_id):
             template,
             prefix=current_service['name']
         ),
-        job_id=job_id
+        job_id=job_id,
+        created_by=job['created_by']['name']
     )
 
 
@@ -100,7 +101,8 @@ def view_job_updates(service_id, job_id):
         'status': render_template(
             'partials/jobs/status.html',
             uploaded_at=job['created_at'],
-            finished_at=job['updated_at'] if finished else None
+            finished_at=job['updated_at'] if finished else None,
+            created_by=job['created_by']['name']
         ),
     })
 
@@ -116,8 +118,6 @@ def view_notifications(service_id):
 
     filter_args = _parse_filter_args(request.args)
 
-    print(filter_args)
-
     notifications = notification_api_client.get_notifications_for_service(
         service_id=service_id,
         page=page,

app/templates/partials/jobs/status.html

@@ -16,5 +16,5 @@
       Started {{ uploaded_at|format_datetime }}
     </p>
   {% endif %}
-
+  Uploaded by {{ created_by }}
 </div>

app/templates/views/jobs/jobs.html

@@ -24,7 +24,7 @@
       caption="Recent activity",
       caption_visible=False,
       empty_message='You haven’t sent any notifications yet',
-      field_headings=['Job', 'Time', right_aligned_field_heading('Status')]
+      field_headings=['Job', 'Time', 'Uploaded by', right_aligned_field_heading('Status')]
     ) %}
       {% call field() %}
         <a href="{{ url_for('.view_job', service_id=current_service.id, job_id=item.id) }}">{{ item.original_file_name }}</a>
@@ -32,6 +32,9 @@
       {% call field() %}
         {{ item.created_at | format_datetime}}
       {% endcall %}
+      {% call field() %}
+        {{ item.created_by.name }}
+      {% endcall %}
       {% call field(align='right') %}
         {{ item.status }}
       {% endcall %}

tests/__init__.py

@@ -16,6 +16,7 @@ class TestClient(FlaskClient):
             mocker.patch('app.user_api_client.get_user', return_value=user)
             mocker.patch('app.events_api_client.create_event')
         if mocker and service:
+            session['service_id'] = service['id']
             mocker.patch('app.service_api_client.get_service', return_value={'data': service})
         login_user(user, remember=True)
 
@@ -115,6 +116,21 @@ def job_json():
     return data
 
 
+def job_json_with_created_by(service_id=None, job_id=None):
+    data = {
+        'id': job_id if job_id else str(generate_uuid()),
+        'service': service_id if service_id else str(generate_uuid()),
+        'template': 1,
+        'original_file_name': 'thisisatest.csv',
+        'created_at': str(datetime.datetime.now().time()),
+        'notification_count': 1,
+        'notifications_sent': 1,
+        'status': '',
+        'created_by': {'name': 'Test User'}
+    }
+    return data
+
+
 def notification_json(service_id,
                       job=None,
                       template=None,

tests/app/main/views/test_jobs.py

@@ -2,22 +2,20 @@ from flask import url_for
 from bs4 import BeautifulSoup
 import json
 from app.utils import generate_notifications_csv
-from tests import notification_json
+from tests import notification_json, job_json_with_created_by
+from tests.conftest import fake_uuid
+from tests.conftest import mock_get_job as mock_get_job1
 
 
 def test_should_return_list_of_all_jobs(app_,
                                         service_one,
-                                        api_user_active,
-                                        mock_get_user,
-                                        mock_get_user_by_email,
-                                        mock_login,
-                                        mock_get_service,
+                                        active_user_with_permissions,
                                         mock_get_jobs,
-                                        mock_has_permissions):
+                                        mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
-            response = client.get(url_for('main.view_jobs', service_id=101))
+            client.login(active_user_with_permissions, mocker, service_one)
+            response = client.get(url_for('main.view_jobs', service_id=service_one['id']))
 
         assert response.status_code == 200
         page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -29,53 +27,40 @@ def test_should_return_list_of_all_jobs(app_,
 def test_should_show_page_for_one_job(
     app_,
     service_one,
-    api_user_active,
-    mock_login,
-    mock_get_user,
-    mock_get_user_by_email,
-    mock_get_service,
+    active_user_with_permissions,
     mock_get_service_template,
-    job_data,
-    mock_get_job,
-    mock_get_notifications,
-    mock_has_permissions
+    mocker,
+    mock_get_notifications
 ):
-    service_id = job_data['service']
-    job_id = job_data['id']
-    file_name = job_data['original_file_name']
-
+    data = job_json_with_created_by(service_id=service_one['id'])
+    job_id = data['id']
+    file_name = data['original_file_name']
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
-            response = client.get(url_for('main.view_job', service_id=service_id, job_id=job_id))
+            client.login(active_user_with_permissions, mocker, service_one)
+            mock_get_job1(mocker=mocker, job_data=data)
+            response = client.get(url_for('main.view_job', service_id=service_one['id'], job_id=job_id))
 
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        assert "Test Service: Your vehicle tax is about to expire" in content
+        assert "{}: Your vehicle tax is about to expire".format(service_one['name']) in content
         assert file_name in content
 
 
 def test_should_show_updates_for_one_job_as_json(
     app_,
     service_one,
-    api_user_active,
-    mock_login,
-    mock_get_user,
-    mock_get_user_by_email,
-    mock_get_service,
-    mock_get_service_template,
-    job_data,
-    mock_get_job,
+    active_user_with_permissions,
     mock_get_notifications,
-    mock_has_permissions
+    mocker
 ):
-    service_id = job_data['service']
-    job_id = job_data['id']
-
+    job_id = fake_uuid()
+    data = job_json_with_created_by(job_id, service_one['id'])
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
-            response = client.get(url_for('main.view_job_updates', service_id=service_id, job_id=job_id))
+            client.login(active_user_with_permissions, mocker, service_one)
+            mock_get_job1(mocker=mocker, job_data=data)
+            response = client.get(url_for('main.view_job_updates', service_id=service_one['id'], job_id=job_id))
 
         assert response.status_code == 200
         content = json.loads(response.get_data(as_text=True))
@@ -84,20 +69,17 @@ def test_should_show_updates_for_one_job_as_json(
         assert 'Recipient' in content['notifications']
         assert 'Status' in content['notifications']
         assert 'Started' in content['status']
+        assert 'Uploaded by Test User' in content['status']
 
 
 def test_should_show_notifications_for_a_service(app_,
                                                  service_one,
-                                                 api_user_active,
-                                                 mock_login,
-                                                 mock_get_user,
-                                                 mock_get_user_by_email,
-                                                 mock_get_service,
+                                                 active_user_with_permissions,
                                                  mock_get_notifications,
-                                                 mock_has_permissions):
+                                                 mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for('main.view_notifications', service_id=service_one['id']))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
@@ -115,16 +97,12 @@ def test_should_show_notifications_for_a_service(app_,
 
 def test_can_view_only_sms_notifications_for_a_service(app_,
                                                        service_one,
-                                                       api_user_active,
-                                                       mock_login,
-                                                       mock_get_user,
-                                                       mock_get_user_by_email,
-                                                       mock_get_service,
+                                                       active_user_with_permissions,
                                                        mock_get_notifications,
-                                                       mock_has_permissions):
+                                                       mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
@@ -147,16 +125,12 @@ def test_can_view_only_sms_notifications_for_a_service(app_,
 
 def test_can_view_only_email_notifications_for_a_service(app_,
                                                          service_one,
-                                                         api_user_active,
-                                                         mock_login,
-                                                         mock_get_user,
-                                                         mock_get_user_by_email,
-                                                         mock_get_service,
+                                                         active_user_with_permissions,
                                                          mock_get_notifications,
-                                                         mock_has_permissions):
+                                                         mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
@@ -179,16 +153,12 @@ def test_can_view_only_email_notifications_for_a_service(app_,
 
 def test_can_view_successful_notifications_for_a_service(app_,
                                                          service_one,
-                                                         api_user_active,
-                                                         mock_login,
-                                                         mock_get_user,
-                                                         mock_get_user_by_email,
-                                                         mock_get_service,
+                                                         active_user_with_permissions,
                                                          mock_get_notifications,
-                                                         mock_has_permissions):
+                                                         mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
@@ -209,16 +179,12 @@ def test_can_view_successful_notifications_for_a_service(app_,
 
 def test_can_view_failed_notifications_for_a_service(app_,
                                                      service_one,
-                                                     api_user_active,
-                                                     mock_login,
-                                                     mock_get_user,
-                                                     mock_get_user_by_email,
-                                                     mock_get_service,
+                                                     active_user_with_permissions,
                                                      mock_get_notifications,
-                                                     mock_has_permissions):
+                                                     mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
@@ -240,17 +206,13 @@ def test_can_view_failed_notifications_for_a_service(app_,
 def test_can_view_failed_combination_of_notification_type_and_status(
     app_,
     service_one,
-    api_user_active,
-    mock_login,
-    mock_get_user,
-    mock_get_user_by_email,
-    mock_get_service,
+    active_user_with_permissions,
     mock_get_notifications,
-    mock_has_permissions
+    mocker
 ):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],
@@ -265,16 +227,12 @@ def test_can_view_failed_combination_of_notification_type_and_status(
 
 def test_should_show_notifications_for_a_service_with_next_previous(app_,
                                                                     service_one,
-                                                                    api_user_active,
-                                                                    mock_login,
-                                                                    mock_get_user,
-                                                                    mock_get_user_by_email,
-                                                                    mock_get_service,
+                                                                    active_user_with_permissions,
                                                                     mock_get_notifications_with_previous_next,
-                                                                    mock_has_permissions):
+                                                                    mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for('main.view_notifications', service_id=service_one['id'], page=2))
         assert response.status_code == 200
         content = response.get_data(as_text=True)
@@ -286,16 +244,12 @@ def test_should_show_notifications_for_a_service_with_next_previous(app_,
 
 def test_should_download_notifications_for_a_service(app_,
                                                      service_one,
-                                                     api_user_active,
-                                                     mock_login,
-                                                     mock_get_user,
-                                                     mock_get_user_by_email,
-                                                     mock_get_service,
+                                                     active_user_with_permissions,
                                                      mock_get_notifications,
-                                                     mock_has_permissions):
+                                                     mocker):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             response = client.get(url_for(
                 'main.view_notifications',
                 service_id=service_one['id'],

tests/app/main/views/test_register.py

@@ -82,7 +82,6 @@ def test_should_return_200_when_email_is_not_gov_uk(app_,
                                                  'password': 'validPassword!'})
 
     assert response.status_code == 200
-    print(response.get_data(as_text=True))
     assert 'Enter a central government email address' in response.get_data(as_text=True)
 
 

tests/app/main/views/test_send.py

@@ -4,7 +4,7 @@ from io import BytesIO
 from bs4 import BeautifulSoup
 from flask import url_for
 from unittest.mock import ANY
-from tests import validate_route_permission
+from tests import validate_route_permission, job_json_with_created_by
 
 template_types = ['email', 'sms']
 
@@ -260,32 +260,32 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
 def test_create_job_should_call_api(
     app_,
     service_one,
-    api_user_active,
-    mock_login,
+    active_user_with_permissions,
     job_data,
     mock_create_job,
     mock_get_job,
     mock_get_notifications,
-    mock_get_service,
     mock_get_service_template,
-    mock_has_permissions
+    mocker
 ):
-
     service_id = service_one['id']
     job_id = job_data['id']
     original_file_name = job_data['original_file_name']
     template_id = job_data['template']
     notification_count = job_data['notification_count']
 
+    from tests.conftest import mock_get_job as mock_get_job1
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             with client.session_transaction() as session:
                 session['upload_data'] = {'original_file_name': original_file_name,
                                           'template_id': template_id,
                                           'notification_count': notification_count,
                                           'valid': True}
             url = url_for('main.start_job', service_id=service_one['id'], upload_id=job_id)
+            data = job_json_with_created_by(service_id=service_one['id'], job_id=job_data['id'])
+            mock_get_job1(mocker=mocker, job_data=data)
             response = client.post(url, data=job_data, follow_redirects=True)
 
         assert response.status_code == 200
@@ -296,9 +296,7 @@ def test_create_job_should_call_api(
 def test_check_messages_should_revalidate_file_when_uploading_file(
     app_,
     service_one,
-    api_user_active,
-    mock_login,
-    mock_get_service,
+    active_user_with_permissions,
     job_data,
     mock_create_job,
     mock_get_service_template,
@@ -321,7 +319,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
     )
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions, mocker, service_one)
             with client.session_transaction() as session:
                 session['upload_data'] = {'original_file_name': 'invalid.csv',
                                           'template_id': job_data['template'],

tests/conftest.py

@@ -14,8 +14,8 @@ from . import (
     notification_json,
     invite_json,
     sample_uuid,
-    generate_uuid
-)
+    generate_uuid,
+    job_json_with_created_by)
 from app.notify_client.models import (
     User,
     InvitedUser
@@ -745,9 +745,7 @@ def mock_get_jobs(mocker):
     def _get_jobs(service_id):
         data = []
         for i in range(5):
-            job_data = job_json()
-            job_data['id'] = str(generate_uuid())
-            job_data['service'] = service_id
+            job_data = job_json_with_created_by(service_id=service_id)
             data.append(job_data)
         return {"data": data}
     return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs)