Merge pull request #2361 from alphagov/migrate-batch-user-creation-to…

…-design-system

Migrate batch user creation to design system

app/controllers/batch_invitation_permissions_controller.rb

@@ -5,7 +5,7 @@ class BatchInvitationPermissionsController < ApplicationController
   before_action :authorise_to_manage_permissions
   before_action :prevent_updating
 
-  helper_method :applications_and_permissions
+  layout "admin_layout"
 
   def new; end
 

app/controllers/batch_invitations_controller.rb

@@ -3,9 +3,7 @@
 class BatchInvitationsController < ApplicationController
   before_action :authenticate_user!
 
-  helper_method :recent_batch_invitations
-
-  layout "admin_layout", only: %w[show]
+  layout "admin_layout"
 
   def new
     @batch_invitation = BatchInvitation.new(organisation_id: current_user.organisation_id)
@@ -44,10 +42,8 @@ def create
         batch_invitation: @batch_invitation,
         name: row["Name"],
         email: row["Email"],
+        organisation_slug: row["Organisation"],
       }
-      if policy(@batch_invitation).assign_organisation_from_csv?
-        batch_user_args[:organisation_slug] = row["Organisation"]
-      end
       batch_user = BatchInvitationUser.new(batch_user_args)
 
       unless batch_user.valid?
@@ -70,10 +66,6 @@ def show
 
 private
 
-  def recent_batch_invitations
-    @recent_batch_invitations ||= BatchInvitation.where("created_at > ?", 3.days.ago).order("created_at desc")
-  end
-
   def file_uploaded?
     if params[:batch_invitation].nil? || params[:batch_invitation][:user_names_and_emails].nil?
       false

app/helpers/batch_invitation_permissions_helper.rb

@@ -0,0 +1,16 @@
+module BatchInvitationPermissionsHelper
+  def formatted_permission_name(application_name, permission_name)
+    if permission_name == SupportedPermission::SIGNIN_NAME
+      "Has access to #{application_name}?"
+    else
+      permission_name
+    end
+  end
+
+  def permissions_for(application)
+    all_permissions = application.supported_permissions.grantable_from_ui
+    signin, others = all_permissions.partition(&:signin?)
+
+    signin + others.sort_by(&:name)
+  end
+end

app/helpers/batch_invitations_helper.rb

@@ -1,12 +1,4 @@
 module BatchInvitationsHelper
-  def batch_invite_status_link(batch_invitation, &block)
-    if !batch_invitation.has_permissions?
-      link_to(new_batch_invitation_permissions_path(batch_invitation), alt: "Edit this batch's permissions", &block)
-    else
-      link_to(batch_invitation_path(batch_invitation), alt: "View this batch", &block)
-    end
-  end
-
   def batch_invite_status_message(batch_invitation)
     if batch_invitation.in_progress?
       "In progress. " \

app/policies/batch_invitation_policy.rb

@@ -7,8 +7,4 @@ def new?
   alias_method :create?, :new?
   alias_method :show?, :new?
   alias_method :manage_permissions?, :new?
-
-  def assign_organisation_from_csv?
-    current_user.govuk_admin?
-  end
 end

app/views/batch_invitation_permissions/new.html.erb

@@ -1,13 +1,48 @@
 <% content_for :title, "Manage permissions for new users" %>
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Home",
+         url: root_path,
+       },
+       {
+         title: "Users",
+         url: users_path,
+       },
+       {
+         title: "Manage permissions for new users",
+       }
+     ]
+   })
+%>
 
-<div class="page-title">
-  <h1>Manage permissions for new users</h1>
-</div>
-
-<div class="well">
-  <%= form_for @batch_invitation, url: :batch_invitation_permissions, method: :post do |f| %>
-    <%= render partial: "shared/user_permissions", locals: { user_object: User.new } %>
-
-    <%= f.submit "Create users and send emails", :class => 'btn btn-success' %>
+<%= form_for @batch_invitation, url: :batch_invitation_permissions, method: :post do |f| %>
+<div class="govuk-accordion" data-module="govuk-accordion">
+  <% policy_scope(:user_permission_manageable_application).reject(&:retired?).each_with_index do |application, idx| %>
+  <div class="govuk-accordion__section">
+    <div class="govuk-accordion__section-header">
+      <h2 class="govuk-accordion__section-heading">
+        <span class="govuk-accordion__section-button" id="accordion-heading-<%= idx %>">
+          <%= application.name %>
+        </span>
+      </h2>
+    </div>
+    <div class="govuk-accordion__section-content" aria-labelledby="accordion-heading-<%= idx %>">
+      <%= render "govuk_publishing_components/components/checkboxes", {
+          name: "permissions_for_#{application.id}",
+          heading: "Permissions for #{application.name}",
+          items: permissions_for(application).map do |permission|
+            { label: formatted_permission_name(application.name, permission.name),
+              value: permission.id,
+              id: "user_application_#{application.id}_supported_permission_#{permission.id}",
+              name: "user[supported_permission_ids][]" }
+          end
+          } %>
+    </div>
+  </div>
   <% end %>
 </div>
+<%= render "govuk_publishing_components/components/button", { text: "Create users and send emails" } %>
+<% end %>

app/views/batch_invitations/new.html.erb

@@ -1,71 +1,85 @@
 <% content_for :title, "Upload a batch of users" %>
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Home",
+         url: root_path,
+       },
+       {
+         title: "Users",
+         url: users_path,
+       },
+       {
+         title: "Upload a batch of users",
+       }
+     ]
+   })
+%>
 
-<div class="page-title">
-  <h1>Upload a batch of users</h1>
-</div>
+<div>
+  <%= form_for @batch_invitation, multipart: true do |f| %>
+    <%= render "govuk_publishing_components/components/fieldset", {
+        legend_text: "Upload a CSV",
+        heading_level: 2,
+        heading_size: "s"
+        } do %>
+
+      <%= render "govuk_publishing_components/components/hint", { text:
+          "The format of the CSV should be as follows:" } %>
+
+      <pre>
+        Name,Email,Organisation
+        Jane Smith,jane@example.com,government-digital-service
+        Winston Churchill,winston@example.com,cabinet-office
+      </pre>
+
+      <p class="gem-c-hint govuk-hint">The values in the Organisation
+        column should be the slug of the organisation the user will be
+        assigned to.  If the value is blank, the user will be assigned
+        to the Organisation selected in the drop-down below.  If the
+        value is provided, but is not a valid slug, the user will not
+        be invited.  You can find the slug for an organisation on
+        <%= link_to 'the list of organisations', organisations_path
+        %>.</p>
+
+      <%= render "govuk_publishing_components/components/hint", { text:
+          "Any fields in the CSV other than those shown above will be
+           ignored." } %>
+
+      <%= render "govuk_publishing_components/components/file_upload", {
+          label: {
+            text: "Upload a CSV file"
+          },
+          name: "batch_invitation[user_names_and_emails]",
+          id: "batch_invitation_user_names_and_emails",
+          accept: "text/csv",
+          } %>
+    <% end %>
+
+    <%= render "govuk_publishing_components/components/fieldset", {
+        legend_text: "Organisation",
+        heading_level: 2,
+        heading_size: "s"
+        } do %>
 
-<% if recent_batch_invitations.any? %>
-  <h3 class="add-bottom-margin">Recent batches</h3>
-  <table class="recent-batches table table-bordered">
-    <thead>
-      <tr class="table-header">
-        <th>Summary</th>
-        <th>Status</th>
-      </tr>
-    </thead>
-    <tbody>
-      <% recent_batch_invitations.each do |batch_invitation| %>
-        <tr>
-          <td>
-            <%= batch_invite_status_link(batch_invitation) do %>
-              <%= batch_invitation.batch_invitation_users.count %> users by <%= batch_invitation.user.name %> at <%= batch_invitation.created_at.to_fs(:govuk_date) %>
-            <% end %>
-          </td>
-          <td><%= batch_invite_status_message(batch_invitation) %></td>
-        </tr>
-      <% end %>
-    </tbody>
-  </table>
-<% end %>
+      <%= render "govuk_publishing_components/components/hint", {
+          text: "If the uploaded CSV doesn't contain an Organisation
+          column, or the value is blank for a row, the user will be
+          assigned to this organisation instead."  } %>
 
-<div class="well">
-  <%= form_for @batch_invitation do |f| %>
-    <h2>Upload a CSV</h2>
-    <div class="row">
-      <div class="col-md-5">
-        <%= f.label :user_names_and_emails, "Choose a CSV file of users with names and email addresses" %>
-        <%= f.file_field(:user_names_and_emails, :accept => 'text/csv', required: true) %>
-      </div>
-      <div class="col-md-5 pull-right">
-        <p>The format of the CSV should be as follows:</p>
-        <% if policy(f.object).assign_organisation_from_csv? %>
-          <pre>
-Name,Email,Organisation
-Jane Smith,jane@example.com,government-digital-service
-Winston Churchill,winston@example.com,cabinet-office
-          </pre>
-          <p class="help-block">The values in the Organisation column should be the slug of the organisation the user will be assigned to.  If the value is blank, the user will be assigned to the Organisation selected in the drop-down below.  If the value is provided, but is not a valid slug, the user will not be invited.  You can find the slug for an organisation on <%= link_to 'the list of organisations', organisations_path %>.</p>
-        <% else %>
-          <pre>
-Name,Email
-Jane Smith,jane@example.com
-Winston Churchill,winston@example.com
-          </pre>
-        <% end %>
-        <p class="help-block">Any fields in the CSV other than those shown above will be ignored.</p>
-      </div>
-    </div>
+      <%= render "govuk_publishing_components/components/select", {
+          id: "batch_invitation_organisation_id",
+          name: "batch_invitation[organisation_id]",
+          label: "Organisation",
+          options: policy_scope(Organisation).map { |organisation| { text: organisation.name_with_abbreviation, value: organisation.id } }
+          } %>
 
-    <div class="form-group">
-      <%= f.label :organisation_id, "Organisation" %>
-      <%= f.select :organisation_id, organisation_options(f), organisation_select_options, { class: "chosen-select", 'data-module' => 'chosen' } %>
-      <% if policy(f.object).assign_organisation_from_csv? %>
-        <p class="help-block">If the uploaded CSV doesn't contain an Organisation column, or the value is blank for a row, the user will be assigned to this organisation instead.</p>
-      <% else %>
-        <p class="help-block">All users in the CSV will be assigned to the organisation selected here.</p>
-      <% end %>
-    </div>
+    <% end %>
 
-    <%= f.submit "Manage permissions for new users", :class => 'btn btn-success' %>
+    <%= render "govuk_publishing_components/components/button", {
+        text: "Manage permissions for new users"
+        } %>
   <% end %>
 </div>

test/controllers/batch_invitation_permissions_controller_test.rb

@@ -7,7 +7,7 @@ class BatchInvitationPermissionsControllerTest < ActionController::TestCase
     @user = create(:admin_user)
     sign_in @user
 
-    @app = create(:application, name: "Profound Publisher")
+    @app = create(:application, name: "Profound Publisher", with_supported_permissions: %w[reader])
 
     @batch_invitation = create(:batch_invitation, user: @user)
     create(
@@ -38,10 +38,8 @@ class BatchInvitationPermissionsControllerTest < ActionController::TestCase
     should "allow selection of application permissions to grant to users" do
       get :new, params: { batch_invitation_id: @batch_invitation.id }
 
-      assert_select "table#editable-permissions" do
-        assert_select "td", "Has access to Profound Publisher?"
-        assert_select "td", "Permissions for Profound Publisher"
-      end
+      assert_select "label", "Has access to Profound Publisher?"
+      assert_select "label", "reader"
     end
   end
 

test/controllers/batch_invitations_controller_test.rb

@@ -19,20 +19,6 @@ def users_csv(filename = "users.csv")
       assert_select "input[type=file]"
     end
 
-    context "some batches created recently" do
-      setup do
-        @bi = create(:batch_invitation, :in_progress)
-        create(:batch_invitation_user, batch_invitation: @bi)
-      end
-
-      should "show a table summarising them" do
-        get :new
-        assert_select "table.recent-batches tbody tr", count: 1
-        assert_select "table.recent-batches tbody td", "1 users by #{@bi.user.name} at #{@bi.created_at.to_fs(:govuk_date)}"
-        assert_select "table.recent-batches tbody td", "In progress. 0 of 1 users processed."
-      end
-    end
-
     should "allow selection of an organisation to invite users to" do
       organisation = create(:organisation)
       get :new

test/helpers/batch_invitation_permissions_helper_test.rb

@@ -0,0 +1,38 @@
+require "test_helper"
+
+class BatchInvitationPermissionsHelperTest < ActionView::TestCase
+  context "#formatted_permission_name" do
+    should "return the permission name if permission is not the signin permission" do
+      assert_equal "Editor", formatted_permission_name("Whitehall", "Editor")
+    end
+
+    should "include the application name if permission is the signin permission" do
+      assert_equal "Has access to Whitehall?", formatted_permission_name("Whitehall", SupportedPermission::SIGNIN_NAME)
+    end
+  end
+
+  context "#permissions_for" do
+    should "return all of the supported permissions that are grantable from the ui" do
+      application = create(:application,
+                           name: "Whitehall",
+                           with_supported_permissions: ["Editor", SupportedPermission::SIGNIN_NAME],
+                           with_supported_permissions_not_grantable_from_ui: ["Not grantable"])
+
+      permission_names = permissions_for(application).map(&:name)
+
+      assert permission_names.include?("Editor")
+      assert permission_names.include?(SupportedPermission::SIGNIN_NAME)
+      assert_not permission_names.include?("Not grantable")
+    end
+
+    should "sort the permissions alphabetically by name, but with the signin permission first" do
+      application = create(:application,
+                           name: "Whitehall",
+                           with_supported_permissions: ["Writer", "Editor", SupportedPermission::SIGNIN_NAME])
+
+      permission_names = permissions_for(application).map(&:name)
+
+      assert_equal [SupportedPermission::SIGNIN_NAME, "Editor", "Writer"], permission_names
+    end
+  end
+end

test/helpers/batch_invitations_helper_test.rb

@@ -93,18 +93,4 @@ class BatchInvitationsHelperTest < ActionView::TestCase
       end
     end
   end
-
-  context "#batch_invite_status_link" do
-    should "link to show the batch when it has permissions" do
-      batch_invitation = create(:batch_invitation, :has_permissions, outcome: "success")
-
-      assert_includes batch_invite_status_link(batch_invitation) {}, batch_invitation_path(batch_invitation)
-    end
-
-    should "link to show edit the permissions when it has no permissions" do
-      batch_invitation = create(:batch_invitation)
-
-      assert_includes batch_invite_status_link(batch_invitation) {}, new_batch_invitation_permissions_path(batch_invitation)
-    end
-  end
 end