Merge pull request #1314 from alphagov/remove-phone-numbers-urls

Stop putting phone numbers and email addresses in URLs

app/assets/javascripts/updateContent.js

@@ -19,10 +19,14 @@
 
   var clearQueue = queue => (queue.length = 0);
 
-  var poll = function(renderer, resource, queue, interval) {
+  var poll = function(renderer, resource, queue, interval, form) {
 
     if (queue.push(renderer) === 1) $.ajax(
-      resource
+      resource,
+      {
+        'method': form ? 'post' : 'get',
+        'data': form ? $('#' + form).serialize() : {}
+      }
     ).done(
       response => flushQueue(queue, response)
     ).fail(
@@ -41,7 +45,8 @@
       getRenderer($(component)),
       $(component).data('resource'),
       getQueue($(component).data('resource')),
-      ($(component).data('interval-seconds') || 1.5) * 1000
+      ($(component).data('interval-seconds') || 1.5) * 1000,
+      $(component).data('form')
     );
 
   };

app/main/views/jobs.py

@@ -190,7 +190,7 @@ def view_job_updates(service_id, job_id):
     ))
 
 
-@main.route('/services/<service_id>/notifications/<message_type>')
+@main.route('/services/<service_id>/notifications/<message_type>', methods=['GET', 'POST'])
 @login_required
 @user_has_permissions('view_activity', admin_override=True)
 def view_notifications(service_id, message_type):
@@ -200,12 +200,12 @@ def view_notifications(service_id, message_type):
         message_type=message_type,
         status=request.args.get('status') or 'sending,delivered,failed',
         page=request.args.get('page', 1),
-        to=request.args.get('to'),
+        to=request.form.get('to', ''),
-        search_form=SearchNotificationsForm(to=request.args.get('to')),
+        search_form=SearchNotificationsForm(to=request.form.get('to', '')),
     )
 
 
-@main.route('/services/<service_id>/notifications/<message_type>.json')
+@main.route('/services/<service_id>/notifications/<message_type>.json', methods=['GET', 'POST'])
 @user_has_permissions('view_activity', admin_override=True)
 def get_notifications_as_json(service_id, message_type):
     return jsonify(get_notifications(
@@ -245,7 +245,7 @@ def get_notifications(service_id, message_type, status_override=None):
         template_type=[message_type],
         status=filter_args.get('status'),
         limit_days=current_app.config['ACTIVITY_STATS_LIMIT_DAYS'],
-        to=request.args.get('to'),
+        to=request.form.get('to', ''),
     )
 
     url_args = {

app/templates/components/ajax-block.html

@@ -1,10 +1,11 @@
-{% macro ajax_block(partials, url, key, interval=2, finished=False) %}
+{% macro ajax_block(partials, url, key, interval=2, finished=False, form='') %}
   {% if not finished %}
     <div
       data-module="update-content"
       data-resource="{{ url }}"
       data-key="{{ key }}"
       data-interval-seconds="{{ interval }}"
+      data-form="{{ form }}"
       aria-live="polite"
     >
   {% endif %}

app/templates/views/notifications.html

@@ -21,12 +21,11 @@
   ) }}
 
   <form
-    method="get"
+    method="post"
     action="{{ url_for('.view_notifications', service_id=current_service.id, message_type=message_type) }}"
     class="grid-row"
   >
     <div class="column-three-quarters">
-      <input type="hidden" name="status" value="{{ status }}">
       {{ textbox(
         search_form.to,
         width='1-1',
@@ -34,14 +33,21 @@
       ) }}
     </div>
     <div class="column-one-quarter align-button-with-textbox">
+      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
       <input type="submit" class="button" value="Search">
     </div>
   </form>
 
+  <form id="search-form" method="post">
+    <input type="hidden" name="to" value="{{ search_form.to.data }}">
+    <input type="hidden" name="csrf_token" value="{{ csrf_token() }}">
+  </form>
+
   {{ ajax_block(
     partials,
-    url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page, to=to),
+    url_for('.get_notifications_as_json', service_id=current_service.id, message_type=message_type, status=status, page=page),
-    'notifications'
+    'notifications',
+    form='search-form'
   ) }}
 
 {% endblock %}

tests/app/main/views/test_activity.py

@@ -70,14 +70,27 @@ def test_can_show_notifications(
     to_argument,
     expected_to_argument,
 ):
-    response = logged_in_client.get(url_for(
+    if expected_to_argument:
-        'main.view_notifications',
+        response = logged_in_client.post(
-        service_id=service_one['id'],
+            url_for(
-        message_type=message_type,
+                'main.view_notifications',
-        status=status_argument,
+                service_id=service_one['id'],
-        page=page_argument,
+                message_type=message_type,
-        to=to_argument,
+                status=status_argument,
-    ))
+                page=page_argument,
+            ),
+            data={
+                'to': to_argument
+            }
+        )
+    else:
+        response = logged_in_client.get(url_for(
+            'main.view_notifications',
+            service_id=service_one['id'],
+            message_type=message_type,
+            status=status_argument,
+            page=page_argument,
+        ))
     assert response.status_code == 200
     content = response.get_data(as_text=True)
     notifications = notification_json(service_one['id'])
@@ -97,8 +110,7 @@ def test_can_show_notifications(
         assert query_dict['status'] == [status_argument]
     if expected_page_argument:
         assert query_dict['page'] == [str(expected_page_argument)]
-    if to_argument:
+    assert 'to' not in query_dict
-        assert query_dict['to'] == [to_argument]
 
     mock_get_notifications.assert_called_with(
         limit_days=7,
@@ -119,20 +131,25 @@ def test_can_show_notifications(
     assert json_content.keys() == {'counts', 'notifications'}
 
 
-@pytest.mark.parametrize("initial_query_arguments, expected_status_field_value, expected_search_box_contents", [
+@pytest.mark.parametrize((
+    'initial_query_arguments,'
+    'form_post_data,'
+    'expected_search_box_contents'
+), [
     (
         {
             'message_type': 'sms',
         },
-        'sending,delivered,failed',
+        {},
         '',
     ),
     (
         {
             'message_type': 'sms',
+        },
+        {
             'to': '+33(0)5-12-34-56-78',
         },
-        'sending,delivered,failed',
         '+33(0)5-12-34-56-78',
     ),
     (
@@ -140,9 +157,10 @@ def test_can_show_notifications(
             'status': 'failed',
             'message_type': 'email',
             'page': '99',
+        },
+        {
             'to': 'test@example.com',
         },
-        'failed',
         'test@example.com',
     ),
 ])
@@ -151,17 +169,21 @@ def test_search_recipient_form(
     mock_get_notifications,
     mock_get_detailed_service,
     initial_query_arguments,
-    expected_status_field_value,
+    form_post_data,
     expected_search_box_contents,
 ):
-    response = logged_in_client.get(url_for(
+    response = logged_in_client.post(
-        'main.view_notifications',
+        url_for(
-        service_id=SERVICE_ONE_ID,
+            'main.view_notifications',
-        **initial_query_arguments
+            service_id=SERVICE_ONE_ID,
-    ))
+            **initial_query_arguments
+        ),
+        data=form_post_data
+    )
     assert response.status_code == 200
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
+    assert page.find("form")['method'] == 'post'
     action_url = page.find("form")['action']
     url = urlparse(action_url)
     assert url.path == '/services/{}/notifications/{}'.format(
@@ -171,8 +193,11 @@ def test_search_recipient_form(
     query_dict = parse_qs(url.query)
     assert query_dict == {}
 
-    assert page.find("input", {'name': 'status'})['value'] == expected_status_field_value
+    recipient_inputs = page.select("input[name=to]")
-    assert page.find("input", {'name': 'to'})['value'] == expected_search_box_contents
+    assert(len(recipient_inputs) == 2)
+
+    for field in recipient_inputs:
+        assert field['value'] == expected_search_box_contents
 
 
 def test_should_show_notifications_for_a_service_with_next_previous(