diff --git a/app/controllers/batch_invitations_controller.rb b/app/controllers/batch_invitations_controller.rb index d3f8686e6b..b51a54d87d 100644 --- a/app/controllers/batch_invitations_controller.rb +++ b/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 diff --git a/app/views/batch_invitations/new.html.erb b/app/views/batch_invitations/new.html.erb index e95b890198..59da3281c5 100644 --- a/app/views/batch_invitations/new.html.erb +++ b/app/views/batch_invitations/new.html.erb @@ -66,10 +66,6 @@ Winston Churchill,winston@example.com <% end %> -

Permissions for the users

- - <%= 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 %> diff --git a/test/controllers/batch_invitations_controller_test.rb b/test/controllers/batch_invitations_controller_test.rb index a4a4a2791d..d516c80d82 100644 --- a/test/controllers/batch_invitations_controller_test.rb +++ b/test/controllers/batch_invitations_controller_test.rb @@ -43,12 +43,11 @@ def users_csv(filename = "users.csv") context "POST create" do should "create a BatchInvitation and BatchInvitationUsers" do - app = create(:application) - post :create, params: { batch_invitation: { user_names_and_emails: users_csv }, user: { supported_permission_ids: [app.signin_permission.id] } } + create(:application) + post :create, params: { batch_invitation: { user_names_and_emails: users_csv } } bi = BatchInvitation.last assert_not_nil bi - assert_equal [app.signin_permission], bi.supported_permissions expected_names_and_emails = [["Arthur Dent", "a@hhg.com"], ["Tricia McMillan", "t@hhg.com"]] assert_equal( expected_names_and_emails, @@ -57,9 +56,7 @@ def users_csv(filename = "users.csv") end should "store the organisation to invite users to" do - post :create, - params: { user: { supported_permission_ids: [] }, - batch_invitation: { user_names_and_emails: users_csv, organisation_id: 3 } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv, organisation_id: 3 } } bi = BatchInvitation.last @@ -70,8 +67,7 @@ def users_csv(filename = "users.csv") should "store organisation info from the uploaded CSV when logged in as an admin" do @user.update!(role: Roles::Admin.role_name) post :create, - params: { user: { supported_permission_ids: [] }, - batch_invitation: { user_names_and_emails: users_csv("users_with_orgs.csv"), organisation_id: 3 } } + params: { batch_invitation: { user_names_and_emails: users_csv("users_with_orgs.csv"), organisation_id: 3 } } bi = BatchInvitation.last @@ -85,8 +81,7 @@ def users_csv(filename = "users.csv") should "store organisation info from the uploaded CSV when logged in as a superadmin" do @user.update!(role: Roles::Superadmin.role_name) post :create, - params: { user: { supported_permission_ids: [] }, - batch_invitation: { user_names_and_emails: users_csv("users_with_orgs.csv"), organisation_id: 3 } } + params: { batch_invitation: { user_names_and_emails: users_csv("users_with_orgs.csv"), organisation_id: 3 } } bi = BatchInvitation.last @@ -97,34 +92,15 @@ def users_csv(filename = "users.csv") assert_equal "cabinet-office", bi.batch_invitation_users[2].organisation_slug end - should "queue a job to do the processing" do - assert_enqueued_jobs 2 do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv }, user: { supported_permission_ids: [] } } - end - end - - should "send an email to signon-alerts" do - perform_enqueued_jobs do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv }, user: { supported_permission_ids: [] } } - - 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: { user_names_and_emails: users_csv }, user: { supported_permission_ids: [] } } + should "redirect to the batch invitation permissions page and show a flash message" do + post :create, params: { batch_invitation: { user_names_and_emails: users_csv } } - assert_match(/Scheduled invitation of 2 users/i, flash[:notice]) - assert_redirected_to "/batch_invitations/#{BatchInvitation.last.id}" + assert_redirected_to "/batch_invitations/#{BatchInvitation.last.id}/permissions/new" end context "no file uploaded" do should "redisplay the form and show a flash message" do - post :create, params: { batch_invitation: { user_names_and_emails: nil }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: nil } } assert_template :new assert_match(/You must upload a file/i, flash[:alert]) @@ -133,7 +109,7 @@ def users_csv(filename = "users.csv") context "the CSV contains one or more email addresses that aren't valid" do should "redisplay the form and show a flash message" do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv("users_with_non_valid_emails.csv") }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv("users_with_non_valid_emails.csv") } } assert_template :new assert_match(/One or more emails were invalid/i, flash[:alert]) @@ -142,7 +118,7 @@ def users_csv(filename = "users.csv") context "the CSV has all the fields, but not in the expected order" do should "process the fields by name" do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv("reversed_users.csv") }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv("reversed_users.csv") } } bi = BatchInvitation.last assert_not_nil bi.batch_invitation_users.find_by(email: "a@hhg.com") @@ -152,7 +128,7 @@ def users_csv(filename = "users.csv") context "the CSV has no data rows" do should "redisplay the form and show a flash message" do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv("empty_users.csv") }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv("empty_users.csv") } } assert_template :new assert_match(/no rows/i, flash[:alert]) @@ -161,7 +137,7 @@ def users_csv(filename = "users.csv") context "the CSV format is invalid" do should "redisplay the form and show a flash message" do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv("invalid_users.csv") }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv("invalid_users.csv") } } assert_template :new assert_match(/Couldn't understand that file/i, flash[:alert]) @@ -170,7 +146,7 @@ def users_csv(filename = "users.csv") context "the CSV has no headers?" do should "redisplay the form and show a flash message" do - post :create, params: { batch_invitation: { user_names_and_emails: users_csv("no_headers_users.csv") }, user: { supported_permission_ids: [] } } + post :create, params: { batch_invitation: { user_names_and_emails: users_csv("no_headers_users.csv") } } assert_template :new assert_match(/must have headers/i, flash[:alert]) diff --git a/test/integration/batch_inviting_users_test.rb b/test/integration/batch_inviting_users_test.rb index fea5ca3eae..518d1be28d 100644 --- a/test/integration/batch_inviting_users_test.rb +++ b/test/integration/batch_inviting_users_test.rb @@ -88,6 +88,8 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest visit new_batch_invitation_path path = Rails.root.join("test/fixtures/users.csv") attach_file("Choose a CSV file of users with names and email addresses", path) + click_button "Manage permissions for new users" + uncheck "Has access to #{support_app.name}?" check "Has access to #{@application.name}?" unselect "reader", from: "Permissions for #{@application.name}" @@ -124,11 +126,12 @@ def perform_batch_invite_with_user(user, application, organisation:, fixture_fil visit new_batch_invitation_path path = Rails.root.join("test/fixtures", fixture_file) attach_file("Choose a CSV file of users with names and email addresses", path) - check "Has access to #{application.name}?" if organisation select organisation.name, from: "Organisation" end + click_button "Manage permissions for new users" + check "Has access to #{application.name}?" click_button "Create users and send emails" assert_response_contains("Creating a batch of users")