From bd54dbb40ca85dd72037ba993a4a7de564b580d7 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Tue, 27 Feb 2018 17:18:26 +0000
Subject: [PATCH] remove unnecessary invocations of has_permissions(...,
 any_=True)

when added to a service, all users are given the view_activity
permission. So, if that's included in the list, we don't need `any_`,
and we don't need any of the other permissions.
---
 app/main/views/templates.py                   | 32 ++-----------------
 app/notify_client/user_api_client.py          |  2 +-
 app/templates/main_nav.html                   |  6 ++--
 .../views/templates/choose-reply.html         |  2 +-
 app/templates/views/templates/choose.html     |  2 +-
 tests/app/main/views/test_send.py             |  2 +-
 tests/app/main/views/test_templates.py        |  2 +-
 7 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index 0888b7098..08c88db63 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -43,11 +43,7 @@ page_headings = {
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
-    admin_override=True, any_=True
+    admin_override=True,
 )
 def view_template(service_id, template_id):
     template = service_api_client.get_service_template(service_id, str(template_id))['data']
@@ -81,11 +77,7 @@ def view_template(service_id, template_id):
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
-    admin_override=True, any_=True
+    admin_override=True,
 )
 def start_tour(service_id, template_id):
 
@@ -110,12 +102,7 @@ def start_tour(service_id, template_id):
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
     admin_override=True,
-    any_=True,
 )
 def choose_template(service_id, template_type='all'):
     templates = service_api_client.get_service_templates(service_id)['data']
@@ -181,12 +168,7 @@ def _view_template_version(service_id, template_id, version, letters_as_pdf=Fals
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
     admin_override=True,
-    any_=True
 )
 def view_template_version(service_id, template_id, version):
     return render_template(
@@ -199,12 +181,7 @@ def view_template_version(service_id, template_id, version):
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
     admin_override=True,
-    any_=True
 )
 def view_template_version_preview(service_id, template_id, version, filetype):
     db_template = service_api_client.get_service_template(service_id, template_id, version=version)['data']
@@ -515,12 +492,7 @@ def redact_template(service_id, template_id):
 @login_required
 @user_has_permissions(
     'view_activity',
-    'send_texts',
-    'send_emails',
-    'manage_templates',
-    'manage_api_keys',
     admin_override=True,
-    any_=True
 )
 def view_template_versions(service_id, template_id):
     return render_template(
diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py
index 8b9e5643a..dc45a2b46 100644
--- a/app/notify_client/user_api_client.py
+++ b/app/notify_client/user_api_client.py
@@ -133,7 +133,7 @@ class UserApiClient(NotifyAdminAPIClient):
     def get_count_of_users_with_permission(self, service_id, permission):
         return len([
             user for user in self.get_users_for_service(service_id)
-            if user.has_permissions(permission, any_=True)
+            if user.has_permissions(permission)
         ])
 
     def get_users_for_organisation(self, org_id):
diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index dd8c02b52..a77db2ae5 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -44,14 +44,12 @@
 <nav class="navigation">
   <ul>
     <li><a href="{{ url_for('.service_dashboard', service_id=current_service.id) }}">Dashboard</a></li>
-  {% if current_user.has_permissions('view_activity', 'manage_templates', 'manage_api_keys', admin_override=True, any_=True) %}
+  {% if current_user.has_permissions('view_activity', admin_override=True) %}
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id) }}">Templates</a></li>
   {% endif %}
+  <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}">Team members</a></li>
   {% if current_user.has_permissions('manage_users', 'manage_settings', admin_override=True) %}
-    <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}">Team members</a></li>
     <li><a href="{{ url_for('.usage', service_id=current_service.id) }}">Usage</a></li>
-  {% elif current_user.has_permissions('view_activity') %}
-    <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}">Team members</a></li>
   {% endif %}
   {% if current_user.has_permissions('manage_api_keys', 'manage_settings', admin_override=True, any_=True) %}
     <li><a href="{{ url_for('.service_settings', service_id=current_service.id) }}">Settings</a></li>
diff --git a/app/templates/views/templates/choose-reply.html b/app/templates/views/templates/choose-reply.html
index 57218d6a3..07706038a 100644
--- a/app/templates/views/templates/choose-reply.html
+++ b/app/templates/views/templates/choose-reply.html
@@ -14,7 +14,7 @@
 
   {% if not templates %}
 
-    {% if current_user.has_permissions('manage_templates', any_=True) %}
+    {% if current_user.has_permissions('manage_templates') %}
        <p class="bottom-gutter">
          You need a template before you can send text messages.
        </p>
diff --git a/app/templates/views/templates/choose.html b/app/templates/views/templates/choose.html
index fea067ea3..4eea222d0 100644
--- a/app/templates/views/templates/choose.html
+++ b/app/templates/views/templates/choose.html
@@ -14,7 +14,7 @@
 
     <h1 class="heading-large">Templates</h1>
 
-    {% if current_user.has_permissions('manage_templates', any_=True) %}
+    {% if current_user.has_permissions('manage_templates') %}
        <p class="bottom-gutter">
          You need a template before you can send
          {% if 'letter' in current_service.permissions %}
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index c349c6d8d..4a50d6764 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -1752,7 +1752,7 @@ def test_route_permissions(
             service_id=service_one['id'],
             template_id=fake_uuid
         ),
-        ['send_texts', 'send_emails', 'send_letters'],
+        ['view_activity', 'send_texts', 'send_emails', 'send_letters'],
         api_user_active,
         service_one)
 
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py
index 3c7adafbb..09f0534d3 100644
--- a/tests/app/main/views/test_templates.py
+++ b/tests/app/main/views/test_templates.py
@@ -139,7 +139,7 @@ def test_should_be_able_to_view_a_template_with_links(
     permissions,
     links_to_be_shown,
 ):
-    active_user_with_permissions._permissions[service_one['id']] = permissions
+    active_user_with_permissions._permissions[service_one['id']] = permissions + ['view_activity']
     client.login(active_user_with_permissions, mocker, service_one)
 
     response = client.get(url_for(