Merge pull request #4231 from alphagov/allow-editing-pending-users

Allow editing of pending users

app/models/user.py

@@ -151,7 +151,7 @@ class User(JSONModel, UserMixin):
         return session.get('current_session_id') != self.current_session_id
 
     def activate(self):
-        if self.state == 'pending':
+        if self.is_pending:
             user_data = user_api_client.activate_user(self.id)
             return self.__class__(user_data['data'])
         else:
@@ -192,6 +192,10 @@ class User(JSONModel, UserMixin):
     def is_active(self):
         return self.state == 'active'
 
+    @property
+    def is_pending(self):
+        return self.state == 'pending'
+
     @property
     def is_gov_user(self):
         return is_gov_user(self.email_address)
@@ -440,6 +444,13 @@ class User(JSONModel, UserMixin):
     def complete_webauthn_login_attempt(self, is_successful=True):
         return user_api_client.complete_webauthn_login_attempt(self.id, is_successful)
 
+    def is_editable_by(self, other_user):
+        if other_user == self:
+            return False
+        if self.is_active or self.is_pending:
+            return True
+        return False
+
 
 class InvitedUser(JSONModel):
 
@@ -575,6 +586,9 @@ class InvitedUser(JSONModel):
         # only used on the manage users page to display the count, so okay to not be fully fledged for now
         return [{'id': x} for x in self.folder_permissions]
 
+    def is_editable_by(self, other):
+        return False
+
 
 class InvitedOrgUser(JSONModel):
 

app/templates/views/manage-users.html

@@ -73,7 +73,7 @@
               {% if current_user.has_permissions('manage_service') %}
                 {% if user.status == 'pending' %}
                   <a class="user-list-edit-link govuk-link govuk-link--no-visited-state" href="{{ url_for('.cancel_invited_user', service_id=current_service.id, invited_user_id=user.id)}}">Cancel invitation<span class="govuk-visually-hidden"> for {{ user.email_address }}</span></a>
-                {% elif user.state == 'active' and current_user.id != user.id %}
+                {% elif user.is_editable_by(current_user) %}
                   <a class="user-list-edit-link govuk-link govuk-link--no-visited-state" href="{{ url_for('.edit_user_permissions', service_id=current_service.id, user_id=user.id)}}">Change details<span class="govuk-visually-hidden"> for {{ user.name }} {{ user.email_address }}</span></a>
                 {% endif %}
               {% endif %}

tests/app/main/views/test_manage_users.py

@@ -151,6 +151,45 @@ def test_should_show_overview_page(
     mock_get_users.assert_called_once_with(SERVICE_ONE_ID)
 
 
+@pytest.mark.parametrize('state', (
+    'active', 'pending',
+))
+def test_should_show_change_details_link(
+    client_request,
+    mocker,
+    mock_get_invites_for_service,
+    mock_get_template_folders,
+    service_one,
+    active_user_with_permissions,
+    active_caseworking_user,
+    state,
+):
+    current_user = active_user_with_permissions
+
+    other_user = active_caseworking_user
+    other_user['id'] = uuid.uuid4()
+    other_user['email_address'] = 'zzzzzzz@example.gov.uk'
+    other_user['state'] = state
+
+    mocker.patch('app.user_api_client.get_user', return_value=current_user)
+    mocker.patch('app.models.user.Users.client_method', return_value=[
+        current_user,
+        other_user,
+    ])
+
+    page = client_request.get('main.manage_users', service_id=SERVICE_ONE_ID)
+    link = page.select('.user-list-item')[-1].select_one('a')
+
+    assert normalize_spaces(link.text) == (
+        'Change details for Test User zzzzzzz@example.gov.uk'
+    )
+    assert link['href'] == url_for(
+        '.edit_user_permissions',
+        service_id=SERVICE_ONE_ID,
+        user_id=other_user['id'],
+    )
+
+
 @pytest.mark.parametrize('number_of_users', (
     pytest.param(7, marks=pytest.mark.xfail),
     pytest.param(8),