Merge pull request #2930 from alphagov/only-show-new-permissions-form…

…-where-there-are-some

Only show new permissions form when there are any

app/views/shared/_permissions_forms.html.erb

@@ -3,12 +3,14 @@
     <% unless split_assigned_and_unassigned_permissions %>
       <%= render "shared/edit_permissions_form", { **shared_permissions_form_locals, permissions: permissions } %>
     <% else %>
-      <%= render "shared/add_permissions_with_autocomplete_form", {
-        action: shared_permissions_form_locals[:action],
-        cancel_path: shared_permissions_form_locals[:cancel_path],
-        assigned_permissions: assigned_permissions,
-        unassigned_permission_options: unassigned_permission_options
-      } %>
+      <% if unassigned_permission_options.present? %>
+        <%= render "shared/add_permissions_with_autocomplete_form", {
+          action: shared_permissions_form_locals[:action],
+          cancel_path: shared_permissions_form_locals[:cancel_path],
+          assigned_permissions: assigned_permissions,
+          unassigned_permission_options: unassigned_permission_options
+        } %>
+      <% end %>
 
       <% if assigned_permissions.present? %>
         <%= render "shared/edit_permissions_form", { 

test/integration/account_applications_test.rb

@@ -224,7 +224,7 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest
       end
     end
 
-    context "when the user already has some permissions" do
+    context "when the user already has some but not all permissions" do
       should "show the new and current permissions forms" do
         user = create(:superadmin_user)
 
@@ -256,6 +256,37 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest
       end
     end
 
+    context "when the user has all permissions" do
+      should "only show the current permissions form" do
+        user = create(:superadmin_user)
+
+        visit root_path
+        signin_with(user)
+
+        app = create(
+          :application,
+          name: "MyApp",
+          with_supported_permissions: %w[Gotta catch 'em all I know it's my destiny],
+        )
+        app.signin_permission.update!(delegatable: true)
+        user.grant_application_signin_permission(app)
+        user.grant_application_permissions(app, %w[Gotta catch 'em all I know it's my destiny])
+
+        # when I visit the path to edit my permissions for the app
+        visit account_applications_path
+        click_on "Update permissions for MyApp"
+        assert_current_url edit_account_application_permissions_path(app)
+
+        # the new permissions form exists with autocomplete and select elements
+        assert_no_selector "#new_permission_id"
+
+        # the current permissions form does not exist
+        assert_selector "legend", text: "Current permissions"
+        assert_selector ".govuk-hint", text: "Clear the checkbox and save changes to remove a permission."
+        assert_selector ".govuk-button", text: "Update permissions"
+      end
+    end
+
     context "when the user has no permissions" do
       should "only show the new permissions form" do
         user = create(:superadmin_user)

test/integration/granting_permissions_test.rb

@@ -286,7 +286,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest
       end
     end
 
-    context "when the user already has some permissions" do
+    context "when the user already has some but not all permissions" do
       should "show the new and current permissions forms" do
         admin = create(:superadmin_user)
         user = create(:user)
@@ -318,6 +318,38 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest
       end
     end
 
+    context "when the user has all permissions" do
+      should "only show the current permissions form" do
+        admin = create(:superadmin_user)
+        user = create(:user)
+        visit root_path
+        signin_with(admin)
+
+        app = create(
+          :application,
+          name: "MyApp",
+          with_supported_permissions: %w[Gotta catch 'em all I know it's my destiny],
+        )
+        @adding_permission = SupportedPermission.find_by(name: "adding")
+        user.grant_application_signin_permission(app)
+        user.grant_application_permissions(app, %w[Gotta catch 'em all I know it's my destiny])
+
+        # when I visit the path to edit the user's permissions for the app
+        visit edit_user_path(user)
+        click_on "Manage permissions"
+        click_on "Update permissions for MyApp"
+        assert_current_url edit_user_application_permissions_path(user, app)
+
+        # the new permissions form exists
+        assert_no_selector "#new_permission_id", visible: false
+
+        # the current permissions form does not exist
+        assert_selector "legend", text: "Current permissions"
+        assert_selector ".govuk-hint", text: "Clear the checkbox and save changes to remove a permission."
+        assert_selector ".govuk-button", text: "Update permissions"
+      end
+    end
+
     context "when the user has no permissions" do
       should "only show the new permissions form" do
         admin = create(:superadmin_user)
@@ -330,7 +362,6 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest
           name: "MyApp",
           with_supported_permissions: %w[never-1 never-2 never-3 never-4 never-5 gonna make you cry],
         )
-        @adding_permission = SupportedPermission.find_by(name: "adding")
         user.grant_application_signin_permission(app)
 
         # when I visit the path to edit the user's permissions for the app