Merge pull request #2350 from alphagov/batch-invitations-two-step-flow

Break batch invitations creation into two steps

app/controllers/batch_invitation_permissions_controller.rb

@@ -0,0 +1,45 @@
+class BatchInvitationPermissionsController < ApplicationController
+  include UserPermissionsControllerMethods
+  before_action :authenticate_user!
+  before_action :load_batch_invitation
+  before_action :authorise_to_manage_permissions
+  before_action :prevent_updating
+
+  helper_method :applications_and_permissions
+
+  def new; end
+
+  def create
+    @batch_invitation.supported_permission_ids = params[:user][:supported_permission_ids] if params[:user]
+    grant_default_permissions(@batch_invitation)
+
+    @batch_invitation.save!
+
+    @batch_invitation.enqueue
+    flash[:notice] = "Scheduled invitation of #{@batch_invitation.batch_invitation_users.count} users"
+    redirect_to batch_invitation_path(@batch_invitation)
+  end
+
+private
+
+  def load_batch_invitation
+    @batch_invitation = current_user.batch_invitations.find(params[:batch_invitation_id])
+  end
+
+  def authorise_to_manage_permissions
+    authorize @batch_invitation, :manage_permissions?
+  end
+
+  def prevent_updating
+    if @batch_invitation.has_permissions?
+      flash[:alert] = "Permissions have already been set for this batch of users"
+      redirect_to batch_invitation_path(@batch_invitation)
+    end
+  end
+
+  def grant_default_permissions(batch_invitation)
+    SupportedPermission.default.each do |default_permission|
+      batch_invitation.grant_permission(default_permission)
+    end
+  end
+end

app/controllers/batch_invitations_controller.rb

@@ -1,10 +1,8 @@
 require "csv"
 
 class BatchInvitationsController < ApplicationController
-  include UserPermissionsControllerMethods
   before_action :authenticate_user!
 
-  helper_method :applications_and_permissions
   helper_method :recent_batch_invitations
 
   layout "admin_layout", only: %w[show]
@@ -16,8 +14,6 @@ def new
 
   def create
     @batch_invitation = BatchInvitation.new(user: current_user, organisation_id: params[:batch_invitation][:organisation_id])
-    @batch_invitation.supported_permission_ids = params[:user][:supported_permission_ids] if params[:user]
-    grant_default_permissions(@batch_invitation)
     authorize @batch_invitation
 
     unless file_uploaded?
@@ -64,9 +60,7 @@ def create
 
     @batch_invitation.save!
 
-    @batch_invitation.enqueue
-    flash[:notice] = "Scheduled invitation of #{@batch_invitation.batch_invitation_users.count} users"
-    redirect_to batch_invitation_path(@batch_invitation)
+    redirect_to new_batch_invitation_permissions_path(@batch_invitation)
   end
 
   def show
@@ -91,12 +85,6 @@ def file_uploaded?
     end
   end
 
-  def grant_default_permissions(batch_invitation)
-    SupportedPermission.default.each do |default_permission|
-      batch_invitation.grant_permission(default_permission)
-    end
-  end
-
   def batch_users_error_message(batch_user)
     e = batch_user.errors.first
 

app/helpers/batch_invitations_helper.rb

@@ -1,4 +1,12 @@
 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. " \
@@ -7,6 +15,8 @@ def batch_invite_status_message(batch_invitation)
         "users processed."
     elsif batch_invitation.all_successful?
       "#{batch_invitation.batch_invitation_users.count} users processed."
+    elsif !batch_invitation.has_permissions?
+      "Batch invitation doesn't have any permissions yet."
     else
       "#{pluralize(batch_invitation.batch_invitation_users.failed.count, 'error')} out of " \
         "#{batch_invitation.batch_invitation_users.count} " \

app/models/batch_invitation.rb

@@ -12,12 +12,16 @@ class BatchInvitation < ApplicationRecord
   validates :outcome, inclusion: { in: [nil, "success", "fail"] }
   validates :user_id, presence: true
 
+  def has_permissions?
+    batch_invitation_application_permissions.exists?
+  end
+
   def in_progress?
-    outcome.nil?
+    outcome.nil? && has_permissions?
   end
 
   def all_successful?
-    batch_invitation_users.failed.count.zero?
+    !outcome.nil? && batch_invitation_users.failed.count.zero?
   end
 
   def enqueue

app/policies/batch_invitation_policy.rb

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

app/views/batch_invitation_permissions/new.html.erb

@@ -0,0 +1,13 @@
+<% content_for :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' %>
+  <% end %>
+</div>

app/views/batch_invitations/new.html.erb

@@ -17,7 +17,7 @@
       <% recent_batch_invitations.each do |batch_invitation| %>
         <tr>
           <td>
-            <%= link_to(batch_invitation_path(batch_invitation), alt: "View this batch") do %>
+            <%= 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>
@@ -66,10 +66,6 @@ Winston Churchill,winston@example.com
       <% end %>
     </div>
 
-    <h2>Permissions for the users</h2>
-
-    <%= render partial: "shared/user_permissions", locals: { user_object: User.new } %>
-
-    <%= f.submit "Create users and send emails", :class => 'btn btn-success' %>
+    <%= f.submit "Manage permissions for new users", :class => 'btn btn-success' %>
   <% end %>
 </div>

config/routes.rb

@@ -54,7 +54,12 @@
     resources :applications, only: [:index]
   end
 
-  resources :batch_invitations, only: %i[new create show]
+  resources :batch_invitations, only: %i[new create show] do
+    resource :permissions,
+             only: %i[new create],
+             controller: :batch_invitation_permissions
+  end
+
   resources :bulk_grant_permission_sets, only: %i[new create show]
   resources :organisations, only: %i[index edit update]
   resources :suspensions, only: %i[edit update]

test/controllers/batch_invitation_permissions_controller_test.rb

@@ -0,0 +1,103 @@
+require "test_helper"
+
+class BatchInvitationPermissionsControllerTest < ActionController::TestCase
+  include ActiveJob::TestHelper
+
+  setup do
+    @user = create(:admin_user)
+    sign_in @user
+
+    @app = create(:application, name: "Profound Publisher")
+
+    @batch_invitation = create(:batch_invitation, user: @user)
+    create(
+      :batch_invitation_user,
+      name: "Darayavaush Ayers",
+      email: "darayavaush.ayers@department.gov.uk",
+      batch_invitation: @batch_invitation,
+    )
+    create(
+      :batch_invitation_user,
+      name: "Precious Kumar",
+      email: "precious.kumar@department.gov.uk",
+      batch_invitation: @batch_invitation,
+    )
+  end
+
+  context "GET new" do
+    should "not allow access if batch invitation already has permissions" do
+      @batch_invitation.supported_permission_ids = [@app.signin_permission.id]
+      @batch_invitation.save!
+
+      get :new, params: { batch_invitation_id: @batch_invitation.id }
+
+      assert_match(/Permissions have already been set for this batch of users/, flash[:alert])
+      assert_redirected_to "/batch_invitations/#{@batch_invitation.id}"
+    end
+
+    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
+    end
+  end
+
+  context "POST create" do
+    should "not accept submission if batch invitation already has permissions" do
+      @batch_invitation.supported_permission_ids = [@app.signin_permission.id]
+      @batch_invitation.save!
+
+      post :create, params: { batch_invitation_id: @batch_invitation.id }
+
+      assert_match(/Permissions have already been set for this batch of users/, flash[:alert])
+      assert_redirected_to "/batch_invitations/#{@batch_invitation.id}"
+    end
+
+    should "grant selected permissions and default permissions to BatchInvitation" do
+      support_app = create(:application, name: "Support")
+      support_app.signin_permission.update!(default: true)
+
+      post :create, params: {
+        batch_invitation_id: @batch_invitation.id,
+        user: { supported_permission_ids: [@app.signin_permission.id] },
+      }
+
+      assert_equal [@app.signin_permission, support_app.signin_permission],
+                   @batch_invitation.supported_permissions
+    end
+
+    context "with no permissions selected" do
+      should "still grant default permissions to BatchInvitation" do
+        support_app = create(:application, name: "Support")
+        support_app.signin_permission.update!(default: true)
+
+        post :create, params: { batch_invitation_id: @batch_invitation.id }
+
+        assert_equal [support_app.signin_permission],
+                     @batch_invitation.supported_permissions
+      end
+    end
+
+    should "send an email to signon-alerts" do
+      perform_enqueued_jobs do
+        post :create, params: { batch_invitation_id: @batch_invitation.id }
+
+        email = ActionMailer::Base.deliveries.detect do |m|
+          m.to.any? { |to| to =~ /signon-alerts@.*\.gov\.uk/ }
+        end
+        assert_not_nil email
+        assert_equal "[SIGNON] #{@user.name} created a batch of 2 users in development", email.subject
+      end
+    end
+
+    should "redirect to the batch invitation page and show a flash message" do
+      post :create, params: { batch_invitation_id: @batch_invitation.id }
+
+      assert_match(/Scheduled invitation of 2 users/i, flash[:notice])
+      assert_redirected_to "/batch_invitations/#{@batch_invitation.id}"
+    end
+  end
+end