diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index a9081dd3f..0d18b0246 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -19,8 +19,31 @@ class OrganisationsController < ApplicationController render "show" end + def edit + if current_user.data_coordinator? + render "edit", layout: "application" + else + head :unauthorized + end + end + + def update + if current_user.data_coordinator? + if @organisation.update(org_params) + flash[:notice] = I18n.t("organisation.updated") + redirect_to details_organisation_path(@organisation) + end + else + head :unauthorized + end + end + private + def org_params + params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone) + end + def authenticate_scope! head :not_found if current_user.organisation != @organisation end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index efd72bae5..6b63b612b 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -23,15 +23,15 @@ class Organisation < ApplicationRecord end def display_attributes - { - name: name, - address: address_string, - telephone_number: phone, - type: "Org type", - local_authorities_operated_in: local_authorities, - holds_own_stock: holds_own_stock, - other_stock_owners: other_stock_owners, - managing_agents: managing_agents, - } + [ + { name: "name", value: name, editable: true }, + { name: "address", value: address_string, editable: true }, + { name: "telephone_number", value: phone, editable: true }, + { name: "type", value: "Org type", editable: false }, + { name: "local_authorities_operated_in", value: local_authorities, editable: false }, + { name: "holds_own_stock", value: holds_own_stock, editable: false }, + { name: "other_stock_owners", value: other_stock_owners, editable: false }, + { name: "managing_agents", value: managing_agents, editable: false }, + ] end end diff --git a/app/views/organisations/edit.html.erb b/app/views/organisations/edit.html.erb new file mode 100644 index 000000000..43d8dab9a --- /dev/null +++ b/app/views/organisations/edit.html.erb @@ -0,0 +1,45 @@ +<% content_for :title, "Change #{@organisation.name}’s details" %> + +<% content_for :before_content do %> + <%= govuk_back_link( + text: 'Back', + href: :back, + ) %> +<% end %> + +<%= form_for(@organisation, as: :organisation, html: { method: :patch }) do |f| %> +
+
+

+ <%= content_for(:title) %> +

+ + <%= f.govuk_text_field :name, + autocomplete: "name" + %> + + <%= f.govuk_text_field :address_line1, + label: { text: "Address line 1" }, + autocomplete: "address-line1" + %> + + <%= f.govuk_text_field :address_line2, + label: { text: "Address line 2" }, + autocomplete: "address-line2" + %> + + <%= f.govuk_text_field :postcode, + autocomplete: "postal-code", + width: 10 + %> + + <%= f.govuk_phone_field :phone, + label: { text: "Telephone number" }, + autocomplete: "tel", + width: 20 + %> + + <%= f.govuk_submit "Save changes" %> +
+
+<% end %> diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb index 35a8a0079..c549605e6 100644 --- a/app/views/organisations/show.html.erb +++ b/app/views/organisations/show.html.erb @@ -7,11 +7,22 @@
<%= govuk_summary_list do |summary_list| %> - <% @organisation.display_attributes.each do |attr, val| %> - <%= summary_list.row do |row| - row.key { attr.to_s.humanize } - row.value { simple_format(val.to_s, {}, wrapper_tag: "div") } - end %> + <% @organisation.display_attributes.each do |attr| %> + + <% if current_user.data_coordinator? && attr[:editable] %> + <%= summary_list.row do |row| + row.key { attr[:name].to_s.humanize } + row.value { simple_format(attr[:value].to_s, {}, wrapper_tag: "div") } + row.action(visually_hidden_text: 'name', href: edit_organisation_path, html_attributes: { 'data-qa': "change-#{attr[:name]}" }) + end %> + <% else %> + <%= summary_list.row do |row| + row.key { attr[:name].to_s.humanize } + row.value { simple_format(attr[:value].to_s, {}, wrapper_tag: "div") } + row.action() + end %> + <% end %> + <% end %> <% end %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 957b67284..bdf920ebe 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -31,3 +31,5 @@ en: service_name: "Log social housing lettings and sales (CORE)" + organisation: + updated: "Organisation details updated" diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index d086452ac..ee1cb29ed 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -6,6 +6,8 @@ RSpec.describe OrganisationsController, type: :request do let(:headers) { { "Accept" => "text/html" } } let(:page) { Capybara::Node::Simple.new(response.body) } let(:user) { FactoryBot.create(:user, :data_coordinator) } + let(:new_value) { "Test Name 35" } + let(:params) { { id: organisation.id, organisation: { name: new_value } } } context "a not signed in user" do describe "#show" do @@ -74,6 +76,11 @@ RSpec.describe OrganisationsController, type: :request do expected_html = "

Details" expect(response.body).to include(expected_html) end + + it "has a change details link" do + expected_html = "data-qa=\"change-name\" href=\"/organisations/#{organisation.id}/edit\"" + expect(response.body).to include(expected_html) + end end context "organisation that are not in scope for the user, i.e. that they do not belong to" do @@ -127,6 +134,66 @@ RSpec.describe OrganisationsController, type: :request do end end end + + context "#edit" do + context "organisation that the user belongs to" do + before do + sign_in user + get "/organisations/#{organisation.id}/edit", headers: headers, params: {} + end + + it "shows an edit form" do + expect(response.body).to include("Change #{organisation.name}’s details") + expect(page).to have_field("organisation-name-field") + expect(page).to have_field("organisation-phone-field") + end + end + + context "organisation that the user does not belong to" do + before do + sign_in user + get "/organisations/#{unauthorised_organisation.id}/edit", headers: headers, params: {} + end + + it "returns a 404 not found" do + expect(response).to have_http_status(:not_found) + end + end + end + + context "#update" do + context "organisation that the user belongs to" do + before do + sign_in user + patch "/organisations/#{organisation.id}", headers: headers, params: params + end + + it "updates the org" do + organisation.reload + expect(organisation.name).to eq(new_value) + end + + it "redirects to the organisation details page" do + expect(response).to redirect_to("/organisations/#{organisation.id}/details") + end + + it "shows a success banner" do + follow_redirect! + expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") + end + end + + context "organisation that the user does not belong to" do + before do + sign_in user + patch "/organisations/#{unauthorised_organisation.id}", headers: headers, params: {} + end + + it "returns a 404 not found" do + expect(response).to have_http_status(:not_found) + end + end + end end context "As a data provider user" do @@ -154,6 +221,11 @@ RSpec.describe OrganisationsController, type: :request do expected_html = "

Details" expect(response.body).to include(expected_html) end + + it "does not have a change details link" do + expected_html = "data-qa=\"change-name\" href=\"/organisations/#{organisation.id}/edit\"" + expect(response.body).not_to include(expected_html) + end end context "organisation that are not in scope for the user, i.e. that they do not belong to" do @@ -178,6 +250,28 @@ RSpec.describe OrganisationsController, type: :request do expect(response).to have_http_status(:unauthorized) end end + + context "#edit" do + before do + sign_in user + get "/organisations/#{organisation.id}/edit", headers: headers, params: {} + end + + it "redirects to home" do + expect(response).to have_http_status(:unauthorized) + end + end + + context "#update" do + before do + sign_in user + patch "/organisations/#{organisation.id}", headers: headers, params: params + end + + it "redirects to home" do + expect(response).to have_http_status(:unauthorized) + end + end end end end