Merge pull request #2487 from alphagov/split-account-update-role-and-…

…update-organisation-into-separate-pages

Split account update role & organisation page into two separate pages

app/controllers/account/role_organisations_controller.rb → app/controllers/account/organisations_controller.rb

@@ -1,37 +1,26 @@
-class Account::RoleOrganisationsController < ApplicationController
+class Account::OrganisationsController < ApplicationController
   layout "admin_layout"
 
   before_action :authenticate_user!
   before_action :authorise_user
 
-  def show; end
+  def edit; end
 
-  def update_organisation
+  def update
     organisation_id = params[:user][:organisation_id]
     organisation = Organisation.find(organisation_id)
 
     if UserUpdate.new(current_user, { organisation_id: }, current_user, user_ip_address).call
       redirect_to account_path, notice: "Your organisation is now #{organisation.name}"
     else
       flash[:alert] = "There was a problem changing your organisation."
-      render :show
-    end
-  end
-
-  def update_role
-    role = params[:user][:role]
-
-    if UserUpdate.new(current_user, { role: }, current_user, user_ip_address).call
-      redirect_to account_path, notice: "Your role is now #{role.humanize}"
-    else
-      flash[:alert] = "There was a problem changing your role."
-      render :show
+      render :edit
     end
   end
 
 private
 
   def authorise_user
-    authorize %i[account role_organisations]
+    authorize %i[account organisations]
   end
 end

app/controllers/account/roles_controller.rb

@@ -0,0 +1,25 @@
+class Account::RolesController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :authorise_user
+
+  def edit; end
+
+  def update
+    role = params[:user][:role]
+
+    if UserUpdate.new(current_user, { role: }, current_user, user_ip_address).call
+      redirect_to account_path, notice: "Your role is now #{role.humanize}"
+    else
+      flash[:alert] = "There was a problem changing your role."
+      render :edit
+    end
+  end
+
+private
+
+  def authorise_user
+    authorize %i[account roles]
+  end
+end

app/helpers/account_helper.rb

@@ -7,12 +7,19 @@ def two_step_verification_page_title
     end
   end
 
-  def role_organisation_page_title
-    if policy(%i[account role_organisations]).update_role? &&
-        policy(%i[account role_organisations]).update_organisation?
-      "Change your role or organisation"
+  def role_page_title
+    if policy(%i[account roles]).update?
+      "Change your role"
     else
-      "View your role and organisation"
+      "View your role"
+    end
+  end
+
+  def organisation_page_title
+    if policy(%i[account organisations]).update?
+      "Change your organisation"
+    else
+      "View your organisation"
     end
   end
 

app/policies/account/organisations_policy.rb

@@ -0,0 +1,9 @@
+class Account::OrganisationsPolicy < BasePolicy
+  def edit?
+    current_user.present?
+  end
+
+  def update?
+    current_user.govuk_admin?
+  end
+end

app/policies/account/roles_policy.rb

@@ -0,0 +1,9 @@
+class Account::RolesPolicy < BasePolicy
+  def edit?
+    current_user.present?
+  end
+
+  def update?
+    current_user.superadmin?
+  end
+end

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

@@ -0,0 +1,42 @@
+<% content_for :title, organisation_page_title %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "Settings",
+         url: account_path,
+       },
+       {
+         title: organisation_page_title,
+       }
+     ]
+   })
+%>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <% if policy(%i[account organisations]).update? %>
+      <%= form_for current_user, url: account_organisation_path do |f| %>
+        <%= render "govuk_publishing_components/components/select", {
+            id: "user_organisation_id",
+            name: "user[organisation_id]",
+            label: "Organisation",
+            options: options_for_your_organisation_select(current_user)
+            } %>
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Change organisation"
+        } %>
+      <% end %>
+    <% else %>
+      <%= render "govuk_publishing_components/components/inset_text", {
+        text: current_user_organisation_name,
+      } %>
+    <% end %>
+  </div>
+</div>

app/views/account/roles/edit.html.erb

@@ -0,0 +1,42 @@
+<% content_for :title, role_page_title %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "Settings",
+         url: account_path,
+       },
+       {
+         title: role_page_title,
+       }
+     ]
+   })
+%>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <% if policy(%i[account roles]).update? %>
+      <%= form_for current_user, url: account_role_path do |f| %>
+        <%= render "govuk_publishing_components/components/select", {
+            id: "user_role",
+            name: "user[role]",
+            label: "Role",
+            options: current_user.manageable_roles.map { |role| { text: role.humanize, value: role, selected: current_user.role == role } }
+            } %>
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Change role"
+        } %>
+      <% end %>
+    <% else %>
+      <%= render "govuk_publishing_components/components/inset_text", {
+        text: current_user.role.humanize,
+      } %>
+    <% end %>
+  </div>
+</div>

app/views/accounts/show.html.erb

@@ -46,8 +46,14 @@
     },
     {
       link: {
-        text: role_organisation_page_title,
-        path: account_role_organisation_path,
+        text: role_page_title,
+        path: edit_account_role_path,
+      },
+    },
+    {
+      link: {
+        text: organisation_page_title,
+        path: edit_account_organisation_path,
       },
     },
     {

config/routes.rb

@@ -63,10 +63,8 @@
     end
     resource :password, only: %i[edit update]
     resource :manage_permissions, only: %i[show update]
-    resource :role_organisation, only: [:show] do
-      patch :update_organisation
-      patch :update_role
-    end
+    resource :organisation, only: %i[edit update]
+    resource :role, only: %i[edit update]
   end
 
   resources :batch_invitations, only: %i[new create show] do

test/controllers/account/organisations_controller_test.rb

@@ -0,0 +1,31 @@
+require "test_helper"
+
+class Account::OrganisationsControllerTest < ActionController::TestCase
+  setup do
+    @organisation = create(:organisation)
+    create(:organisation)
+    @superadmin_user = create(:superadmin_user)
+    sign_in @superadmin_user
+  end
+
+  context "GET edit" do
+    should "display form with current organisation" do
+      get :edit
+
+      assert_select "form[action='#{account_organisation_path}']" do
+        assert_select "select[name='user[organisation_id]']", value: @superadmin_user.organisation_id
+      end
+    end
+  end
+
+  context "PUT update" do
+    should "display error when validation fails" do
+      UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false))
+
+      put :update, params: { user: { organisation_id: @organisation } }
+
+      assert_template :edit
+      assert_select "*[role='alert']", text: "There was a problem changing your organisation."
+    end
+  end
+end

test/controllers/account/roles_controller_test.rb

@@ -0,0 +1,29 @@
+require "test_helper"
+
+class Account::RolesControllerTest < ActionController::TestCase
+  setup do
+    @superadmin_user = create(:superadmin_user)
+    sign_in @superadmin_user
+  end
+
+  context "GET edit" do
+    should "display form with current role" do
+      get :edit
+
+      assert_select "form[action='#{account_role_path}']" do
+        assert_select "select[name='user[role]']", value: @superadmin_user.role
+      end
+    end
+  end
+
+  context "PUT update" do
+    should "display error when validation fails" do
+      UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false))
+
+      put :update, params: { user: { role: Roles::Normal.role_name } }
+
+      assert_template :edit
+      assert_select "*[role='alert']", text: "There was a problem changing your role."
+    end
+  end
+end