Browse Source

Merge f725fcd1e9 into cb77abf03b

pull/3015/merge
Manny Dinssa 2 days ago committed by GitHub
parent
commit
2bfc277607
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 21
      app/controllers/organisations_controller.rb
  2. 3
      app/frontend/controllers/index.js
  3. 27
      app/frontend/controllers/organisations_controller.js
  4. 52
      app/helpers/organisations_helper.rb
  5. 47
      app/models/organisation.rb
  6. 3
      app/services/exports/organisation_export_service.rb
  7. 27
      app/views/organisations/edit.html.erb
  8. 27
      app/views/organisations/new.html.erb
  9. 2
      app/views/organisations/show.html.erb
  10. 4
      config/locales/en.yml
  11. 6
      db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb
  12. 10
      db/migrate/20250227085622_add_new_question_fields_to_organisation.rb
  13. 4
      db/schema.rb
  14. 29
      lib/tasks/update_organisations_group_profit_status.rake
  15. 38
      spec/features/organisation_spec.rb
  16. 2
      spec/fixtures/files/organisations_group_profit_status_invalid.csv
  17. 2
      spec/fixtures/files/organisations_group_profit_status_valid.csv
  18. 62
      spec/helpers/organisations_helper_spec.rb
  19. 40
      spec/lib/tasks/update_organisations_group_profit_status_spec.rb

21
app/controllers/organisations_controller.rb

@ -91,6 +91,7 @@ class OrganisationsController < ApplicationController
selected_rent_periods = rent_period_params[:rent_periods].compact_blank
@organisation = Organisation.new(org_params)
if @organisation.save
@organisation.update!(group: assign_group_number(@organisation.id, org_params[:group_member_id])) if org_params[:group_member]
OrganisationRentPeriod.transaction do
selected_rent_periods.each { |period| OrganisationRentPeriod.create!(organisation: @organisation, rent_period: period) }
end
@ -142,6 +143,9 @@ class OrganisationsController < ApplicationController
end
flash[:notice] = I18n.t("organisation.reactivated", organisation: @organisation.name)
else
if org_params[:group_member] && org_params[:group_member_id]
@organisation.group = assign_group_number(@organisation.id, org_params[:group_member_id])
end
flash[:notice] = I18n.t("organisation.updated")
end
if rent_period_params[:rent_periods].present?
@ -341,7 +345,7 @@ private
end
def org_params
params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active)
params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no, :active, :profit_status, :group_member, :group_member_id)
end
def rent_period_params
@ -385,4 +389,19 @@ private
end
end
end
def assign_group_number(current_org_id, selected_org_id)
selected_org = Organisation.find_by(id: selected_org_id)
if selected_org&.group.present?
selected_org.group
else
next_group_number = next_available_group_number
selected_org.update!(group_member: true, group_member_id: current_org_id, group: next_group_number) if selected_org
next_group_number
end
end
def next_available_group_number
Organisation.maximum(:group).to_i + 1
end
end

3
app/frontend/controllers/index.js

@ -21,6 +21,8 @@ import TabsController from './tabs_controller.js'
import AddressSearchController from './address_search_controller.js'
import OrganisationsController from './organisations_controller.js'
application.register('accessible-autocomplete', AccessibleAutocompleteController)
application.register('conditional-filter', ConditionalFilterController)
application.register('conditional-question', ConditionalQuestionController)
@ -30,3 +32,4 @@ application.register('filter-layout', FilterLayoutController)
application.register('search', SearchController)
application.register('tabs', TabsController)
application.register('address-search', AddressSearchController)
application.register('organisations', OrganisationsController)

27
app/frontend/controllers/organisations_controller.js

@ -0,0 +1,27 @@
import { Controller } from '@hotwired/stimulus'
export default class extends Controller {
updateProfitStatusOptions (event) {
const profitStatusSelect = document.getElementById('organisation-profit-status-field')
if (!profitStatusSelect) return
const localAuthorityOption = profitStatusSelect.querySelector('option[value="local_authority"]')
const nonProfitOption = profitStatusSelect.querySelector('option[value="non_profit"]')
const profitOption = profitStatusSelect.querySelector('option[value="profit"]')
const providerType = event.target.value
profitStatusSelect.disabled = false
localAuthorityOption.hidden = false
nonProfitOption.hidden = false
profitOption.hidden = false
if (providerType === 'LA') {
profitStatusSelect.value = 'local_authority'
nonProfitOption.hidden = true
profitOption.hidden = true
} else if (providerType === 'PRP') {
profitStatusSelect.value = ''
localAuthorityOption.hidden = true
}
}
}

52
app/helpers/organisations_helper.rb

@ -9,18 +9,27 @@ module OrganisationsHelper
end
end
def display_organisation_attributes(organisation)
[
def display_organisation_attributes(user, organisation)
attributes = [
{ name: "Organisation ID", value: "ORG#{organisation.id}", editable: false },
{ name: "Address", value: organisation.address_string, editable: true },
{ name: "Telephone number", value: organisation.phone, editable: true },
{ name: "Registration number", value: organisation.housing_registration_no || "", editable: false },
{ name: "Type of provider", value: organisation.display_provider_type, editable: false },
{ name: "Owns housing stock", value: organisation.holds_own_stock ? "Yes" : "No", editable: false },
{ name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet },
{ name: "Data Sharing Agreement" },
{ name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false },
{ name: "Part of group", value: organisation.group_member ? "Yes" : "No", editable: user.support? },
]
if organisation.group_member
attributes << { name: "Group number", value: "GROUP#{organisation.group}", editable: user.support? }
end
attributes << { name: "For profit", value: organisation.display_profit_status, editable: user.support? }
attributes << { name: "Rent periods", value: organisation.rent_period_labels, editable: true, format: :bullet }
attributes << { name: "Data Sharing Agreement" }
attributes << { name: "Status", value: status_tag(organisation.status) + delete_organisation_text(organisation), editable: false }
attributes
end
def organisation_name_row(user:, organisation:, summary_list:)
@ -64,7 +73,40 @@ module OrganisationsHelper
def organisation_details_link_message(attribute)
text = lowercase_first_letter(attribute[:name])
return "Add #{text}" if attribute[:name] == "Rent periods"
return "Select profit status" if attribute[:name] == "For profit"
"Enter #{text}"
end
def group_organisation_options
null_option = [OpenStruct.new(id: "", name: "Select an option", group: nil)]
organisations = Organisation.visible.map { |org| OpenStruct.new(id: org.id, name: group_organisation_options_name(org)) }
null_option + organisations
end
def group_organisation_options_name(org)
"#{org.name}#{group_organisation_options_group_text(org)}"
end
def group_organisation_options_group_text(org)
return "" unless org.oldest_group_member
" (GROUP#{org.oldest_group_member&.group})"
end
def profit_status_options(provider_type = nil)
null_option = [OpenStruct.new(id: "", name: "Select an option")]
profit_statuses = Organisation::PROFIT_STATUS.map do |key, _value|
OpenStruct.new(id: key, name: Organisation::DISPLAY_PROFIT_STATUS[key])
end
case provider_type
when "LA"
profit_statuses.select! { |option| option.id == :local_authority }
when "PRP"
profit_statuses.reject! { |option| option.id == :local_authority }
end
null_option + profit_statuses
end
end

47
app/models/organisation.rb

@ -54,13 +54,27 @@ class Organisation < ApplicationRecord
PRP: 2,
}.freeze
PROFIT_STATUS = {
non_profit: 1,
profit: 2,
local_authority: 3,
}.freeze
enum :provider_type, PROVIDER_TYPE
enum :profit_status, PROFIT_STATUS
attribute :group_member, :boolean
attr_accessor :skip_group_member_validation
before_save :clear_group_member_fields_if_not_group_member
alias_method :la?, :LA?
validates :name, presence: { message: I18n.t("validations.organisation.name_missing") }
validates :name, uniqueness: { case_sensitive: false, message: I18n.t("validations.organisation.name_not_unique") }
validates :provider_type, presence: { message: I18n.t("validations.organisation.provider_type_missing") }
validates :group_member_id, presence: { message: I18n.t("validations.organisation.group_missing") }, if: -> { group_member? && !skip_group_member_validation }
validate :validate_profit_status
def self.find_by_id_on_multiple_fields(id)
return if id.nil?
@ -142,6 +156,12 @@ class Organisation < ApplicationRecord
DISPLAY_PROVIDER_TYPE[provider_type.to_sym]
end
DISPLAY_PROFIT_STATUS = { "non_profit": "Non-profit", "profit": "Profit", "local_authority": "Local authority" }.freeze
def display_profit_status
DISPLAY_PROFIT_STATUS.fetch(profit_status&.to_sym, "")
end
def has_managing_agents?
managing_agents.count.positive?
end
@ -232,4 +252,31 @@ class Organisation < ApplicationRecord
def has_visible_schemes?
owned_schemes.visible.count.positive?
end
def oldest_group_member
return nil if group.blank?
Organisation.visible.where(group:).order(:created_at).first
end
private
def validate_profit_status
return if profit_status.nil?
if provider_type == "LA" && profit_status != "local_authority"
errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_be_LA"))
end
if provider_type == "PRP" && profit_status == "local_authority"
errors.add(:profit_status, I18n.t("validations.organisation.profit_status.must_not_be_LA"))
end
end
def clear_group_member_fields_if_not_group_member
unless group_member
self.group_member_id = nil
self.group = nil
end
end
end

3
app/services/exports/organisation_export_service.rb

@ -74,8 +74,7 @@ module Exports
attribute_hash["provider_type"] = organisation.provider_type_before_type_cast
attribute_hash["merge_date"] = organisation.merge_date&.iso8601
attribute_hash["available_from"] = organisation.available_from&.iso8601
attribute_hash["profit_status"] = nil # will need update when we add the field to the org
attribute_hash["group"] = nil # will need update when we add the field to the org
attribute_hash["profit_status"] = organisation.profit_status_before_type_cast
attribute_hash["status"] = organisation.status
attribute_hash["active"] = attribute_hash["status"] == :active

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

@ -29,6 +29,33 @@
autocomplete: "tel",
width: 20 %>
<% if current_user.support? %>
<%= f.govuk_radio_buttons_fieldset :group_member,
legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %>
<%= f.govuk_radio_button :group_member, true,
label: { text: "Yes" },
"data-controller": "conditional-question",
"data-action": "click->conditional-question#displayConditional",
"data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %>
<%= f.govuk_collection_select :group_member_id,
group_organisation_options,
:id,
:name,
label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" },
options: { disabled: [""], selected: @organisation.oldest_group_member&.id || "" },
"data-controller": %w[accessible-autocomplete conditional-filter] %>
<% end %>
<%= f.govuk_radio_button :group_member, false, label: { text: "No" } %>
<% end %>
<%= f.govuk_collection_select :profit_status,
profit_status_options(@organisation.provider_type),
:id,
:name,
label: { text: "Is the organisation for profit?", size: "m" },
options: { disabled: [""], selected: @organisation.profit_status || "" } %>
<% end %>
<%= f.govuk_check_boxes_fieldset :rent_periods,
legend: { text: "What are the rent periods for the organisation?" },
hint: { text: "It is not possible to deselect rent periods that are used in logs" } do %>

27
app/views/organisations/new.html.erb

@ -48,6 +48,8 @@
:id,
:name,
label: { text: "Organisation type", size: "m" },
"data-controller": "organisations",
"data-action": "change->organisations#updateProfitStatusOptions",
options: { disabled: [""], selected: @organisation.provider_type || "" } %>
<%= f.govuk_collection_radio_buttons :holds_own_stock,
@ -56,6 +58,31 @@
:name,
legend: { text: "Does the organisation hold its own stock?", size: "m" } %>
<%= f.govuk_radio_buttons_fieldset :group_member,
legend: { text: "Is this organisation part of a housing provider group structure?", size: "m" } do %>
<%= f.govuk_radio_button :group_member, true,
label: { text: "Yes" },
"data-controller": "conditional-question",
"data-action": "click->conditional-question#displayConditional",
"data-info": { conditional_questions: { group: [true] }, type: "organisation" }.to_json do %>
<%= f.govuk_collection_select :group_member_id,
group_organisation_options,
:id,
:name,
label: { text: "Search for an organisation that is part of the same group as this organisation", size: "m" },
options: { disabled: [""], selected: @organisation.oldest_group_member&.id || "" },
"data-controller": %w[accessible-autocomplete conditional-filter] %>
<% end %>
<%= f.govuk_radio_button :group_member, false, label: { text: "No" } %>
<% end %>
<%= f.govuk_collection_select :profit_status,
profit_status_options,
:id,
:name,
label: { text: "Is the organisation for profit?", size: "m" },
options: { disabled: [""], selected: @organisation.profit_status || "" } %>
<%= f.govuk_check_boxes_fieldset :rent_periods,
legend: { text: "What are the rent periods for the organisation?" } do %>
<% @rent_periods.map do |key, period| %>

2
app/views/organisations/show.html.erb

@ -15,7 +15,7 @@
<div class="govuk-grid-column-two-thirds-from-desktop">
<%= govuk_summary_list do |summary_list| %>
<%= organisation_name_row(user: current_user, organisation: @organisation, summary_list:) %>
<% display_organisation_attributes(@organisation).each do |attr| %>
<% display_organisation_attributes(current_user, @organisation).each do |attr| %>
<% if attr[:name] == "Data Sharing Agreement" %>
<%= data_sharing_agreement_row(organisation: @organisation, user: current_user, summary_list:) %>
<% else %>

4
config/locales/en.yml

@ -220,6 +220,10 @@ en:
name_missing: "Enter the name of the organisation."
name_not_unique: "An organisation with this name already exists. Use the organisation that already exists or add a location or other identifier to the name. For example: Organisation name (City)."
provider_type_missing: "Select the organisation type."
group_missing: "Select a group member."
profit_status:
must_be_LA: "This organisation is a local authority, it's profit status must also be local authority."
must_not_be_LA: "This organisation is a private registered provider, it's profit status cannot be local authority."
stock_owner:
blank: "You must choose a stock owner."
already_added: "You have already added this stock owner."

6
db/migrate/20250219122817_add_manual_address_entry_selected_to_logs.rb

@ -1,6 +0,0 @@
class AddManualAddressEntrySelectedToLogs < ActiveRecord::Migration[7.2]
def change
add_column :sales_logs, :manual_address_entry_selected, :boolean, default: false
add_column :lettings_logs, :manual_address_entry_selected, :boolean, default: false
end
end

10
db/migrate/20250227085622_add_new_question_fields_to_organisation.rb

@ -0,0 +1,10 @@
class AddNewQuestionFieldsToOrganisation < ActiveRecord::Migration[7.2]
def change
change_table :organisations, bulk: true do |t|
t.integer :profit_status
t.boolean :group_member
t.integer :group_member_id
t.integer :group
end
end
end

4
db/schema.rb

@ -559,6 +559,10 @@ ActiveRecord::Schema[7.2].define(version: 2025_04_16_111741) do
t.datetime "available_from"
t.datetime "discarded_at"
t.datetime "schemes_deduplicated_at"
t.integer "profit_status"
t.boolean "group_member"
t.integer "group_member_id"
t.integer "group"
t.index ["absorbing_organisation_id"], name: "index_organisations_on_absorbing_organisation_id"
t.index ["name"], name: "index_organisations_on_name", unique: true
t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true

29
lib/tasks/update_organisations_group_profit_status.rake

@ -0,0 +1,29 @@
namespace :data_update do
desc "Update organisations with data from a CSV file"
task :update_organisations, [:csv_path] => :environment do |_task, args|
require "csv"
csv_path = args[:csv_path]
unless csv_path
Rails.logger.error "Please provide the path to the CSV file. Example: rake data_update:update_organisations[csv_path]"
exit
end
CSV.foreach(csv_path, headers: true) do |row|
organisation = Organisation.find_by(id: row["id"].to_i)
if organisation
organisation.skip_group_member_validation = true
organisation.update!(
profit_status: row["profit_status"].to_i,
group_member: true,
group: row["group"].to_i,
)
Rails.logger.info "Updated ORG#{row['id']}"
else
Rails.logger.warn "Organisation with ID #{row['id']} not found"
end
end
Rails.logger.info "Organisation update task completed"
end
end

38
spec/features/organisation_spec.rb

@ -43,6 +43,11 @@ RSpec.describe "User Features" do
expect(page).to have_current_path("/organisations/#{org_id}/details")
end
it "does not allow coordinator users to edit their organisation's group and profit status" do
expect(page).to have_no_link("Change part of group")
expect(page).to have_no_link("Select profit status")
end
context "and the organisation does not hold housing stock" do
before do
organisation.update(holds_own_stock: false)
@ -365,5 +370,38 @@ RSpec.describe "User Features" do
expect(page).not_to have_link("Schemes", href: "/schemes", count: 2)
end
end
context "and is creating a new organisation" do
before do
visit("/organisations")
click_link("Create a new organisation")
end
it "displays the new organisation form" do
expect(page).to have_content("Create a new organisation")
expect(page).to have_field("organisation[name]", type: "text")
expect(page).to have_field("organisation[address_line1]", type: "text")
expect(page).to have_field("organisation[address_line2]", type: "text")
expect(page).to have_field("organisation[postcode]", type: "text")
expect(page).to have_field("organisation[phone]")
expect(page).to have_field("organisation[housing_registration_no]", type: "text")
expect(page).to have_select("organisation[provider_type]")
expect(page).to have_field("organisation[holds_own_stock]", type: "radio")
expect(page).to have_field("organisation[group_member]", type: "radio")
expect(page).to have_select("organisation[profit_status]")
expect(page).to have_button("Create organisation")
end
end
context "when viewing a specific organisation's details page" do
before do
visit("/organisations/#{org_id}/details")
end
it "allows the support user to edit the organisation's group and profit status" do
expect(page).to have_link("Change part of group")
expect(page).to have_link("Select profit status")
end
end
end
end

2
spec/fixtures/files/organisations_group_profit_status_invalid.csv vendored

@ -0,0 +1,2 @@
id,profit_status,group
2000,2,4
1 id profit_status group
2 2000 2 4

2
spec/fixtures/files/organisations_group_profit_status_valid.csv vendored

@ -0,0 +1,2 @@
id,profit_status,group
234,3,2
1 id profit_status group
2 234 3 2

62
spec/helpers/organisations_helper_spec.rb

@ -3,10 +3,12 @@ require "rails_helper"
RSpec.describe OrganisationsHelper do
include TagHelper
describe "display_organisation_attributes" do
let(:organisation) { create(:organisation, :la, :holds_own_stock, address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", housing_registration_no: 1234, organisation_rent_periods: []) }
let(:support_user) { create(:user, :support) }
let(:coordinator_user) { create(:user, :data_coordinator) }
let(:organisation) { create(:organisation, :la, :holds_own_stock, address_line1: "2 Marsham Street", address_line2: "London", postcode: "SW1P 4DF", housing_registration_no: 1234, organisation_rent_periods: [], group_member: true, group_member_id: 99, group: 1) }
it "has the correct values" do
expect(display_organisation_attributes(organisation)).to eq(
it "has the correct values and editable status for support users" do
expect(display_organisation_attributes(support_user, organisation)).to eq(
[{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" },
{ editable: true,
name: "Address",
@ -15,6 +17,28 @@ RSpec.describe OrganisationsHelper do
{ editable: false, name: "Registration number", value: "1234" },
{ editable: false, name: "Type of provider", value: "Local authority" },
{ editable: false, name: "Owns housing stock", value: "Yes" },
{ editable: true, name: "Part of group", value: "Yes" },
{ editable: true, name: "Group number", value: "GROUP1" },
{ editable: true, name: "For profit", value: "" },
{ editable: true, format: :bullet, name: "Rent periods", value: [] },
{ name: "Data Sharing Agreement" },
{ editable: false, name: "Status", value: status_tag(organisation.status) }],
)
end
it "has the correct values and editable status for non-support users" do
expect(display_organisation_attributes(coordinator_user, organisation)).to eq(
[{ editable: false, name: "Organisation ID", value: "ORG#{organisation.id}" },
{ editable: true,
name: "Address",
value: "2 Marsham Street\nLondon\nSW1P 4DF" },
{ editable: true, name: "Telephone number", value: nil },
{ editable: false, name: "Registration number", value: "1234" },
{ editable: false, name: "Type of provider", value: "Local authority" },
{ editable: false, name: "Owns housing stock", value: "Yes" },
{ editable: false, name: "Part of group", value: "Yes" },
{ editable: false, name: "Group number", value: "GROUP1" },
{ editable: false, name: "For profit", value: "" },
{ editable: true, format: :bullet, name: "Rent periods", value: [] },
{ name: "Data Sharing Agreement" },
{ editable: false, name: "Status", value: status_tag(organisation.status) }],
@ -56,4 +80,36 @@ RSpec.describe OrganisationsHelper do
end
end
end
describe "#group_organisation_options_name" do
let(:older_org) { create(:organisation, group_member: true, group_member_id: 123, group: 9) }
let(:org) { create(:organisation, name: "Org1", group_member: true, group_member_id: older_org.id) }
it "returns the organisation name with group text" do
allow(org).to receive(:oldest_group_member).and_return(older_org)
name = helper.group_organisation_options_name(org)
expect(name).to eq("Org1 (GROUP9)")
end
end
describe "#profit_status_options" do
it "returns a list of profit statuses with a null option" do
options = helper.profit_status_options
expect(options.map(&:name)).to include("Select an option")
end
context "when provider type is LA" do
it "returns only the local authority option" do
options = helper.profit_status_options("LA")
expect(options.map(&:id)).to eq(["", :local_authority])
end
end
context "when provider type is PRP" do
it "excludes the local authority option" do
options = helper.profit_status_options("PRP")
expect(options.map(&:id)).not_to include(:local_authority)
end
end
end
end

40
spec/lib/tasks/update_organisations_group_profit_status_spec.rb

@ -0,0 +1,40 @@
require "rails_helper"
require "rake"
RSpec.describe "data_update:update_organisations", type: :task do
let(:task) { Rake::Task["data_update:update_organisations"] }
before do
Rake.application.rake_require("tasks/update_organisations_group_profit_status")
Rake::Task.define_task(:environment)
task.reenable
end
context "when the CSV file is valid" do
let!(:organisation) { create(:organisation, id: 234) }
let(:csv_path) { Rails.root.join("spec/fixtures/files/organisations_group_profit_status_valid.csv") }
it "updates the organisation fields" do
expect {
task.invoke(csv_path.to_s)
}.to change { organisation.reload.profit_status }.to("local_authority")
.and change { organisation.reload.group }.to(2)
end
end
context "when the organisation is not found" do
let(:csv_path) { Rails.root.join("spec/fixtures/files/organisations_group_profit_status_invalid.csv") }
it "logs a warning" do
expect(Rails.logger).to receive(:warn).with("Organisation with ID 2000 not found")
task.invoke(csv_path.to_s)
end
end
context "when the CSV path is not provided" do
it "logs an error and exits" do
expect(Rails.logger).to receive(:error).with("Please provide the path to the CSV file. Example: rake data_update:update_organisations[csv_path]")
expect { task.invoke }.to raise_error(SystemExit)
end
end
end
Loading…
Cancel
Save