Merge pull request #2540 from alphagov/split-out-edit-user-organisati…

…on-page-from-user-edit-page

Split out edit user organisation page from user edit page

app/controllers/users/organisations_controller.rb

@@ -0,0 +1,38 @@
+class Users::OrganisationsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :authorize_user
+  before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit]
+
+  def edit; end
+
+  def update
+    updater = UserUpdate.new(@user, user_params, current_user, user_ip_address)
+    if updater.call
+      redirect_to edit_user_path(@user), notice: "Updated user #{@user.email} successfully"
+    else
+      render :edit
+    end
+  end
+
+private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def authorize_user
+    authorize(@user)
+    authorize(@user, :assign_organisation?)
+  end
+
+  def user_params
+    params.require(:user).permit(*current_user.permitted_params.intersection([:organisation_id]))
+  end
+
+  def redirect_to_account_page_if_acting_on_own_user
+    redirect_to edit_account_organisation_path if current_user == @user
+  end
+end

app/controllers/users_controller.rb

@@ -36,8 +36,6 @@ def edit
   end
 
   def update
-    raise Pundit::NotAuthorizedError if params[:user][:organisation_id].present? && !policy(@user).assign_organisations?
-
     updater = UserUpdate.new(@user, user_params, current_user, user_ip_address)
     if updater.call
       redirect_to users_path, notice: "Updated user #{@user.email} successfully"
@@ -112,7 +110,7 @@ def user_params
   end
 
   def permitted_user_params
-    @permitted_user_params ||= params.require(:user).permit(:user, :organisation_id, :require_2sv, :skip_update_user_permissions, supported_permission_ids: []).to_h
+    @permitted_user_params ||= params.require(:user).permit(:require_2sv, :skip_update_user_permissions, supported_permission_ids: []).to_h
   end
 
   def filter_params

app/policies/user_policy.rb

@@ -7,7 +7,7 @@ def index?
   def new?
     %w[superadmin admin].include? current_user.role
   end
-  alias_method :assign_organisations?, :new?
+  alias_method :assign_organisation?, :new?
 
   # invitations#create
   alias_method :create?, :new?

app/views/users/_form_fields.html.erb

@@ -21,6 +21,15 @@
   <% end %>
 </p>
 
+<p>
+  <strong>Organisation:</strong> <%= @user.organisation.present? ? @user.organisation.name : Organisation::NONE %>
+  <% if policy(User).assign_organisation? %>
+    <%= link_to edit_user_organisation_path(@user) do %>
+      Change<span class="invisible"> organisation</span>
+    <% end %>
+  <% end %>
+</p>
+
 <% if policy(@user).mandate_2sv? %>
   <dl>
     <dt>Account security</dt>
@@ -69,13 +78,6 @@
   </dl>
 <% end %>
 
-<% if policy(User).assign_organisations? %>
-  <p class="form-group">
-    <%= f.label :organisation_id, "Organisation" %><br />
-    <%= f.select :organisation_id, organisation_options(f), organisation_select_options, { class: "chosen-select form-control", 'data-module' => 'chosen' } %>
-  </p>
-<% end %>
-
 <h2 class="add-vertical-margins"> <%= "Editable " if (current_user.publishing_manager? ) %>Permissions</h2>
 <%= render partial: "shared/user_permissions", locals: { user_object: f.object }%>
 

app/views/users/names/edit.html.erb

@@ -40,7 +40,7 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
-    <%= form_for @user, url: user_name_path(@user) do |f| %>
+    <%= form_for @user, url: user_name_path(@user) do %>
       <%= render "govuk_publishing_components/components/input", {
         label: {
           text: "Name"

app/views/users/organisations/edit.html.erb

@@ -0,0 +1,59 @@
+<% content_for :title_caption, "Manage other users" %>
+<% content_for :title, "Change organisation for #{@user.name}" %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "Users",
+         url: users_path,
+       },
+       {
+         title: @user.name,
+         url: edit_user_path(@user),
+       },
+       {
+         title: "Change organisation",
+       }
+     ]
+   })
+%>
+
+<% if @user.errors.count > 0 %>
+  <% content_for :error_summary do %>
+    <%= render "govuk_publishing_components/components/error_summary", {
+      title: "There is a problem",
+      items: @user.errors.map do |error|
+        {
+          text: error.full_message,
+          href: "#user_#{error.attribute}",
+        }
+      end,
+    } %>
+  <% end %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_for @user, url: user_organisation_path(@user) do %>
+      <%= render "govuk_publishing_components/components/select", {
+        id: "user_organisation_id",
+        name: "user[organisation_id]",
+        label: "Organisation",
+        options: options_for_organisation_select(selected: @user.organisation_id),
+        error_message: @user.errors[:organisation_id].any? ? @user.errors.full_messages_for(:organisation_id).to_sentence : nil
+      } %>
+      <div class="govuk-button-group">
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Change organisation",
+        } %>
+        <%= link_to "Cancel", edit_user_path(@user), class: "govuk-link govuk-link--no-visited-state" %>
+      </div>
+    <% end %>
+  </div>
+</div>

config/routes.rb

@@ -49,6 +49,7 @@
       delete :cancel_email_change
     end
     resource :role, only: %i[edit update], controller: "users/roles"
+    resource :organisation, only: %i[edit update], controller: "users/organisations"
   end
   get "user", to: "oauth_users#show"
 

lib/roles/organisation_admin.rb

@@ -7,7 +7,6 @@ def self.permitted_user_params
         :email,
         :password,
         :password_confirmation,
-        :organisation_id,
         :unconfirmed_email,
         :confirmation_token,
         :require_2sv,

lib/roles/super_organisation_admin.rb

@@ -7,7 +7,6 @@ def self.permitted_user_params
         :email,
         :password,
         :password_confirmation,
-        :organisation_id,
         :unconfirmed_email,
         :confirmation_token,
         :require_2sv,