Browse Source

CLDC-1731 Allow data coordinators to view parent's schemes (#1637)

* Update permissions and scheme/location views

* Display parent organisation's schemes in the list

* Hide toggle scheme button from child organisation

* Update show location to use user_can_edit_scheme?

* Refactor user_allowed_action?

* remove redundant return
pull/1649/head
kosiakkatrina 2 years ago committed by GitHub
parent
commit
bf5d8442ce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 11
      app/controllers/locations_controller.rb
  2. 2
      app/controllers/organisations_controller.rb
  3. 6
      app/controllers/schemes_controller.rb
  4. 4
      app/helpers/locations_helper.rb
  5. 4
      app/views/locations/index.html.erb
  6. 4
      app/views/locations/show.html.erb
  7. 4
      app/views/schemes/show.html.erb
  8. 54
      spec/requests/locations_controller_spec.rb
  9. 54
      spec/requests/schemes_controller_spec.rb

11
app/controllers/locations_controller.rb

@ -4,7 +4,7 @@ class LocationsController < ApplicationController
before_action :authenticate_scope! before_action :authenticate_scope!
before_action :find_location, except: %i[create index] before_action :find_location, except: %i[create index]
before_action :find_scheme before_action :find_scheme
before_action :authenticate_action! before_action :authenticate_action!, only: %i[create update index new_deactivation deactivate_confirm deactivate postcode local_authority name units type_of_unit mobility_standards availability check_answers]
before_action :scheme_and_location_present, except: %i[create index] before_action :scheme_and_location_present, except: %i[create index]
include Modules::SearchFilter include Modules::SearchFilter
@ -21,6 +21,7 @@ class LocationsController < ApplicationController
end end
def postcode; end def postcode; end
def update; end
def update_postcode def update_postcode
@location.postcode = location_params[:postcode] @location.postcode = location_params[:postcode]
@ -225,11 +226,15 @@ private
end end
def authenticate_action! def authenticate_action!
if %w[create update index new_deactivation deactivate_confirm deactivate postcode local_authority name units type_of_unit mobility_standards availability check_answers].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) unless user_allowed_action?
render_not_found and return render_not_found
end end
end end
def user_allowed_action?
current_user.support? || current_user.organisation == @scheme&.owning_organisation || current_user.organisation.parent_organisations.exists?(@scheme&.owning_organisation_id)
end
def location_params def location_params
required_params = params.require(:location).permit(:postcode, :location_admin_district, :location_code, :name, :units, :type_of_unit, :mobility_type, "startdate(1i)", "startdate(2i)", "startdate(3i)").merge(scheme_id: @scheme.id) required_params = params.require(:location).permit(:postcode, :location_admin_district, :location_code, :name, :units, :type_of_unit, :mobility_type, "startdate(1i)", "startdate(2i)", "startdate(3i)").merge(scheme_id: @scheme.id)
required_params[:postcode] = PostcodeService.clean(required_params[:postcode]) if required_params[:postcode] required_params[:postcode] = PostcodeService.clean(required_params[:postcode]) if required_params[:postcode]

2
app/controllers/organisations_controller.rb

@ -19,7 +19,7 @@ class OrganisationsController < ApplicationController
end end
def schemes def schemes
all_schemes = Scheme.where(owning_organisation: @organisation).order_by_completion.order_by_service_name all_schemes = Scheme.where(owning_organisation: [@organisation] + @organisation.parent_organisations).order_by_completion.order_by_service_name
@pagy, @schemes = pagy(filtered_collection(all_schemes, search_term)) @pagy, @schemes = pagy(filtered_collection(all_schemes, search_term))
@searched = search_term.presence @searched = search_term.presence

6
app/controllers/schemes_controller.rb

@ -265,11 +265,15 @@ private
def authenticate_scope! def authenticate_scope!
head :unauthorized and return unless current_user.data_coordinator? || current_user.support? head :unauthorized and return unless current_user.data_coordinator? || current_user.support?
if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers edit_name deactivate].include?(action_name) && !((current_user.organisation == @scheme&.owning_organisation) || current_user.support?) if %w[show locations primary_client_group confirm_secondary_client_group secondary_client_group support details check_answers edit_name deactivate].include?(action_name) && !user_allowed_action?
render_not_found and return render_not_found and return
end end
end end
def user_allowed_action?
current_user.support? || current_user.organisation == @scheme&.owning_organisation || current_user.organisation.parent_organisations.exists?(@scheme&.owning_organisation_id)
end
def redirect_if_scheme_confirmed def redirect_if_scheme_confirmed
redirect_to @scheme if @scheme.confirmed? redirect_to @scheme if @scheme.confirmed?
end end

4
app/helpers/locations_helper.rb

@ -84,6 +84,10 @@ module LocationsHelper
end end
end end
def user_can_edit_scheme?(user, scheme)
user.support? || user.organisation == scheme.owning_organisation
end
private private
ActivePeriod = Struct.new(:from, :to) ActivePeriod = Struct.new(:from, :to)

4
app/views/locations/index.html.erb

@ -64,7 +64,9 @@
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% if user_can_edit_scheme?(current_user, @scheme) %>
<%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %> <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %>
<% end %>
</div> </div>
</div> </div>
@ -118,7 +120,9 @@
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
<% if user_can_edit_scheme?(current_user, @scheme) %>
<%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %> <%= govuk_button_to "Add a location", scheme_locations_path(@scheme), method: "post", secondary: true %>
<% end %>
<% end %> <% end %>

4
app/views/locations/show.html.erb

@ -16,12 +16,12 @@
<%= summary_list.row do |row| %> <%= summary_list.row do |row| %>
<% row.key { attr[:name] } %> <% row.key { attr[:name] } %>
<% row.value { attr[:attribute].eql?("status") ? status_tag(attr[:value]) : details_html(attr) } %> <% row.value { attr[:attribute].eql?("status") ? status_tag(attr[:value]) : details_html(attr) } %>
<% row.action(text: "Change", href: scheme_location_name_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "name" %> <% row.action(text: "Change", href: scheme_location_name_path(@scheme, @location, referrer: "details")) if attr[:attribute] == "name" && user_can_edit_scheme?(current_user, @scheme) %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
</div> </div>
</div> </div>
<% if FeatureToggle.location_toggle_enabled? %> <% if FeatureToggle.location_toggle_enabled? && user_can_edit_scheme?(current_user, @scheme) %>
<%= toggle_location_link(@location) %> <%= toggle_location_link(@location) %>
<% end %> <% end %>

4
app/views/schemes/show.html.erb

@ -22,7 +22,7 @@
<%= summary_list.row do |row| %> <%= summary_list.row do |row| %>
<% row.key { attr[:name] } %> <% row.key { attr[:name] } %>
<% row.value { details_html(attr) } %> <% row.value { details_html(attr) } %>
<% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] %> <% row.action(text: "Change", href: scheme_edit_name_path(scheme_id: @scheme.id)) if attr[:edit] && user_can_edit_scheme?(current_user, @scheme) %>
<% end %> <% end %>
<% end %> <% end %>
<% end %> <% end %>
@ -32,6 +32,6 @@
</div> </div>
<% end %> <% end %>
<% if FeatureToggle.scheme_toggle_enabled? %> <% if FeatureToggle.scheme_toggle_enabled? && user_can_edit_scheme?(current_user, @scheme) %>
<%= toggle_scheme_link(@scheme) %> <%= toggle_scheme_link(@scheme) %>
<% end %> <% end %>

54
spec/requests/locations_controller_spec.rb

@ -145,7 +145,7 @@ RSpec.describe LocationsController, type: :request do
end end
end end
it "shows locations with correct data wben the new locations layout feature toggle is enabled" do it "shows locations with correct data when the new locations layout feature toggle is enabled" do
locations.each do |location| locations.each do |location|
expect(page).to have_content(location.id) expect(page).to have_content(location.id)
expect(page).to have_content(location.postcode) expect(page).to have_content(location.postcode)
@ -154,7 +154,7 @@ RSpec.describe LocationsController, type: :request do
end end
end end
it "shows locations with correct data wben the new locations layout feature toggle is disabled" do it "shows locations with correct data when the new locations layout feature toggle is disabled" do
allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false) allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false)
get "/schemes/#{scheme.id}/locations" get "/schemes/#{scheme.id}/locations"
locations.each do |location| locations.each do |location|
@ -248,6 +248,30 @@ RSpec.describe LocationsController, type: :request do
expect(page).to have_title(expected_title) expect(page).to have_title(expected_title)
end end
end end
context "when coordinator attempts to see scheme belonging to a parent organisation" do
let(:parent_organisation) { FactoryBot.create(:organisation) }
let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: parent_organisation) }
let!(:locations) { FactoryBot.create_list(:location, 3, scheme:, startdate: Time.zone.local(2022, 4, 1)) }
before do
create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation)
get "/schemes/#{scheme.id}/locations"
end
it "shows all the locations" do
locations.each do |location|
expect(page).to have_content(location.id)
expect(page).to have_content(location.postcode)
expect(page).to have_content(location.name)
expect(page).to have_content(location.status)
end
end
it "does not allow adding new locations" do
expect(page).not_to have_button("Add a location")
end
end
end end
context "when signed in as a support user" do context "when signed in as a support user" do
@ -261,16 +285,17 @@ RSpec.describe LocationsController, type: :request do
get "/schemes/#{scheme.id}/locations" get "/schemes/#{scheme.id}/locations"
end end
it "shows locations with correct data wben the new locations layout feature toggle is enabled" do it "shows locations with correct data when the new locations layout feature toggle is enabled" do
locations.each do |location| locations.each do |location|
expect(page).to have_content(location.id) expect(page).to have_content(location.id)
expect(page).to have_content(location.postcode) expect(page).to have_content(location.postcode)
expect(page).to have_content(location.name) expect(page).to have_content(location.name)
expect(page).to have_content(location.status) expect(page).to have_content(location.status)
end end
expect(page).to have_button("Add a location")
end end
it "shows locations with correct data wben the new locations layout feature toggle is disabled" do it "shows locations with correct data when the new locations layout feature toggle is disabled" do
allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false) allow(FeatureToggle).to receive(:location_toggle_enabled?).and_return(false)
get "/schemes/#{scheme.id}/locations" get "/schemes/#{scheme.id}/locations"
locations.each do |location| locations.each do |location|
@ -1677,6 +1702,27 @@ RSpec.describe LocationsController, type: :request do
expect(page).not_to have_link("Deactivate this location") expect(page).not_to have_link("Deactivate this location")
end end
end end
context "and are viewing their parent organisation's location" do
let(:parent_organisation) { FactoryBot.create(:organisation) }
let!(:scheme) { FactoryBot.create(:scheme, owning_organisation: parent_organisation) }
let!(:location) { FactoryBot.create(:location, scheme:) }
let(:add_deactivations) {}
before do
create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation)
end
it "shows the location" do
expect(page).to have_content("Location name")
expect(page).to have_content(location.name)
end
it "does not allow editing the location" do
expect(page).not_to have_link("Change")
expect(page).not_to have_link("Deactivate this location", href: "/schemes/#{scheme.id}/locations/#{location.id}/new-deactivation")
end
end
end end
end end

54
spec/requests/schemes_controller_spec.rb

@ -39,6 +39,9 @@ RSpec.describe SchemesController, type: :request do
let(:user) { FactoryBot.create(:user, :data_coordinator) } let(:user) { FactoryBot.create(:user, :data_coordinator) }
before do before do
schemes.each do |scheme|
scheme.update!(owning_organisation: user.organisation)
end
sign_in user sign_in user
get "/schemes" get "/schemes"
end end
@ -47,6 +50,33 @@ RSpec.describe SchemesController, type: :request do
follow_redirect! follow_redirect!
expect(path).to match("/organisations/#{user.organisation.id}/schemes") expect(path).to match("/organisations/#{user.organisation.id}/schemes")
end end
it "shows a list of schemes for the organisation" do
follow_redirect!
schemes.each do |scheme|
expect(page).to have_content(scheme.id_to_display)
end
end
context "when parent organisation has schemes" do
let(:parent_organisation) { FactoryBot.create(:organisation) }
let!(:parent_schemes) { FactoryBot.create_list(:scheme, 5, owning_organisation: parent_organisation) }
before do
create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation)
parent_schemes.each do |scheme|
FactoryBot.create(:location, scheme:)
end
get "/schemes"
end
it "shows parent organisation schemes" do
follow_redirect!
parent_schemes.each do |scheme|
expect(page).to have_content(scheme.id_to_display)
end
end
end
end end
context "when signed in as a support user" do context "when signed in as a support user" do
@ -233,9 +263,6 @@ RSpec.describe SchemesController, type: :request do
expect(page).to have_content(specific_scheme.id_to_display) expect(page).to have_content(specific_scheme.id_to_display)
expect(page).to have_content(specific_scheme.service_name) expect(page).to have_content(specific_scheme.service_name)
expect(page).to have_content(specific_scheme.sensitive) expect(page).to have_content(specific_scheme.sensitive)
expect(page).to have_content(specific_scheme.id_to_display)
expect(page).to have_content(specific_scheme.service_name)
expect(page).to have_content(specific_scheme.sensitive)
expect(page).to have_content(specific_scheme.scheme_type) expect(page).to have_content(specific_scheme.scheme_type)
expect(page).to have_content(specific_scheme.registered_under_care_act) expect(page).to have_content(specific_scheme.registered_under_care_act)
expect(page).to have_content(specific_scheme.primary_client_group) expect(page).to have_content(specific_scheme.primary_client_group)
@ -306,6 +333,27 @@ RSpec.describe SchemesController, type: :request do
end end
end end
end end
context "when coordinator attempts to see scheme belonging to a parent organisation" do
let(:parent_organisation) { FactoryBot.create(:organisation) }
let!(:specific_scheme) { FactoryBot.create(:scheme, owning_organisation: parent_organisation) }
before do
FactoryBot.create(:location, scheme: specific_scheme)
create(:organisation_relationship, parent_organisation:, child_organisation: user.organisation)
get "/schemes/#{specific_scheme.id}"
end
it "shows the scheme" do
expect(page).to have_content(specific_scheme.id_to_display)
end
it "does not allow editing the scheme" do
expect(page).not_to have_link("Change")
expect(page).not_to have_content("Reactivate this scheme")
expect(page).not_to have_content("Deactivate this scheme")
end
end
end end
context "when signed in as a support user" do context "when signed in as a support user" do

Loading…
Cancel
Save