diff --git a/app/components/sales_log_summary_component.html.erb b/app/components/sales_log_summary_component.html.erb index 10b021fb7..82acc6e14 100644 --- a/app/components/sales_log_summary_component.html.erb +++ b/app/components/sales_log_summary_component.html.erb @@ -32,6 +32,12 @@
<%= log.owning_organisation&.name %>
<% end %> + <% if log.managing_organisation && FeatureToggle.sales_managing_organisation_enabled? %> +
+
Reported by
+
<%= log.managing_organisation&.name %>
+
+ <% end %> <% end %> diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index dcd78c312..9fb0fe243 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -102,7 +102,7 @@ private result[question.id] = question_params end - if question.id == "owning_organisation_id" && @log.lettings? + if question.id == "owning_organisation_id" owning_organisation = Organisation.find(result["owning_organisation_id"]) if current_user.support? && @log.managing_organisation.blank? && owning_organisation&.managing_agents&.empty? result["managing_organisation_id"] = owning_organisation.id diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index 058240b67..dffdec69f 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -123,12 +123,6 @@ private FilterManager.new(current_user:, session:, params:, filter_type: "lettings_logs") end - def org_params - super.merge( - { "managing_organisation_id" => current_user.organisation.id }, - ) - end - def authenticate_scope! head :unauthorized and return if codes_only_export? && !current_user.support? end diff --git a/app/controllers/logs_controller.rb b/app/controllers/logs_controller.rb index 599f3a7b9..a1a23b736 100644 --- a/app/controllers/logs_controller.rb +++ b/app/controllers/logs_controller.rb @@ -63,10 +63,11 @@ private end def org_params - owning_organisation_id = instance_of?(SalesLogsController) || current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil + owning_organisation_id = current_user.organisation.holds_own_stock? ? current_user.organisation.id : nil { "owning_organisation_id" => owning_organisation_id, "created_by_id" => current_user.id, + "managing_organisation_id" => current_user.organisation.id, } end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 41213ba9c..2b902d129 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -122,7 +122,7 @@ class OrganisationsController < ApplicationController end def sales_logs - organisation_logs = SalesLog.visible.where(owning_organisation_id: @organisation.id) + organisation_logs = SalesLog.visible.filter_by_organisation(@organisation) unpaginated_filtered_logs = filter_manager.filtered_logs(organisation_logs, search_term, session_filters) respond_to do |format| diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index aeed98d9e..5726b5994 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -125,11 +125,14 @@ module FiltersHelper end def non_support_with_multiple_owning_orgs? - current_user.organisation.stock_owners.count > 1 && user_lettings_path? || current_user.organisation.has_recent_absorbed_organisations? + return true if current_user.organisation.stock_owners.count > 1 + return true if current_user.organisation.stock_owners.count.positive? && current_user.organisation.holds_own_stock? + + current_user.organisation.has_recent_absorbed_organisations? end - def non_support_with_multiple_managing_orgs? - current_user.organisation.managing_agents.count > 1 && user_lettings_path? || current_user.organisation.has_recent_absorbed_organisations? + def non_support_with_managing_orgs? + current_user.organisation.managing_agents.count >= 1 || current_user.organisation.has_recent_absorbed_organisations? end def user_lettings_path? diff --git a/app/models/form/sales/pages/managing_organisation.rb b/app/models/form/sales/pages/managing_organisation.rb new file mode 100644 index 000000000..0c9ed7337 --- /dev/null +++ b/app/models/form/sales/pages/managing_organisation.rb @@ -0,0 +1,21 @@ +class Form::Sales::Pages::ManagingOrganisation < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "managing_organisation" + end + + def questions + @questions ||= [ + Form::Sales::Questions::ManagingOrganisation.new(nil, nil, self), + ] + end + + def routed_to?(log, current_user) + return false unless current_user + return false unless current_user.support? + return false unless FeatureToggle.sales_managing_organisation_enabled? + return false unless log.owning_organisation + + log.owning_organisation.managing_agents.count >= 1 + end +end diff --git a/app/models/form/sales/pages/organisation.rb b/app/models/form/sales/pages/organisation.rb deleted file mode 100644 index 6940c2abc..000000000 --- a/app/models/form/sales/pages/organisation.rb +++ /dev/null @@ -1,33 +0,0 @@ -class Form::Sales::Pages::Organisation < ::Form::Page - def initialize(id, hsh, subsection) - super - @id = "organisation" - end - - def questions - @questions ||= [ - Form::Sales::Questions::OwningOrganisationId.new(nil, nil, self), - ] - end - - def routed_to?(_log, current_user) - if FeatureToggle.merge_organisations_enabled? - - return false unless current_user - return true if current_user.support? - - absorbed_stock_owners = current_user.organisation.absorbed_organisations.where(holds_own_stock: true) - - if current_user.organisation.holds_own_stock? - return true if absorbed_stock_owners.count >= 1 - else - return false if absorbed_stock_owners.count.zero? - return true if absorbed_stock_owners.count > 1 - end - - false - else - !!current_user&.support? - end - end -end diff --git a/app/models/form/sales/pages/owning_organisation.rb b/app/models/form/sales/pages/owning_organisation.rb new file mode 100644 index 000000000..d8787a456 --- /dev/null +++ b/app/models/form/sales/pages/owning_organisation.rb @@ -0,0 +1,47 @@ +class Form::Sales::Pages::OwningOrganisation < ::Form::Page + def initialize(id, hsh, subsection) + super + @id = "owning_organisation" + end + + def questions + @questions ||= [ + Form::Sales::Questions::OwningOrganisationId.new(nil, nil, self), + ] + end + + def routed_to?(log, current_user) + return false unless current_user + return true if current_user.support? + return false unless FeatureToggle.sales_managing_organisation_enabled? + return true if has_multiple_stock_owners_with_own_stock?(current_user) + + stock_owners = if FeatureToggle.merge_organisations_enabled? + current_user.organisation.stock_owners.where(holds_own_stock: true) + current_user.organisation.absorbed_organisations.where(holds_own_stock: true) + else + current_user.organisation.stock_owners.where(holds_own_stock: true) + end + + if current_user.organisation.holds_own_stock? + if FeatureToggle.merge_organisations_enabled? && current_user.organisation.absorbed_organisations.any?(&:holds_own_stock?) + return true + end + return true if stock_owners.count >= 1 + + log.update!(owning_organisation: current_user.organisation) + else + return false if stock_owners.count.zero? + return true if stock_owners.count > 1 + + log.update!(owning_organisation: stock_owners.first) + end + + false + end + +private + + def has_multiple_stock_owners_with_own_stock?(user) + user.organisation.stock_owners.where(holds_own_stock: true).count > 1 || user.organisation.holds_own_stock? && user.organisation.stock_owners.where(holds_own_stock: true).count >= 1 + end +end diff --git a/app/models/form/sales/questions/created_by_id.rb b/app/models/form/sales/questions/created_by_id.rb index e48b4f9e5..4e0e73e27 100644 --- a/app/models/form/sales/questions/created_by_id.rb +++ b/app/models/form/sales/questions/created_by_id.rb @@ -4,7 +4,7 @@ class Form::Sales::Questions::CreatedById < ::Form::Question def initialize(id, hsh, page) super @id = "created_by_id" - @check_answer_label = "User" + @check_answer_label = "Log owner" @header = "Which user are you creating this log for?" @type = "select" end @@ -14,19 +14,19 @@ class Form::Sales::Questions::CreatedById < ::Form::Question end def displayed_answer_options(log, current_user = nil) - return ANSWER_OPTS unless log.owning_organisation + return ANSWER_OPTS unless log.managing_organisation return ANSWER_OPTS unless current_user users = [] users += if current_user.support? [ ( - if log.owning_organisation - log.owning_organisation.absorbing_organisation.present? ? log.owning_organisation&.absorbing_organisation&.users : log.owning_organisation&.users + if log.managing_organisation + log.managing_organisation.absorbing_organisation.present? ? log.managing_organisation&.absorbing_organisation&.users : log.managing_organisation.users end), ].flatten else - current_user.organisation.users + log.managing_organisation.users end.uniq.compact users.each_with_object(ANSWER_OPTS.dup) do |user, hsh| hsh[user.id] = present_user(user) @@ -58,6 +58,6 @@ private end def selected_answer_option_is_derived?(_log) - false + true end end diff --git a/app/models/form/sales/questions/managing_organisation.rb b/app/models/form/sales/questions/managing_organisation.rb new file mode 100644 index 000000000..818819084 --- /dev/null +++ b/app/models/form/sales/questions/managing_organisation.rb @@ -0,0 +1,77 @@ +class Form::Sales::Questions::ManagingOrganisation < ::Form::Question + def initialize(id, hsh, page) + super + @id = "managing_organisation_id" + @check_answer_label = "Reported by" + @header = "Which organisation is reporting this sale?" + @type = "select" + end + + def answer_options(log = nil, user = nil) + opts = { "" => "Select an option" } + + return opts unless ActiveRecord::Base.connected? + return opts unless user + return opts unless log + + if log.managing_organisation.present? + opts = opts.merge({ log.managing_organisation.id => log.managing_organisation.name }) + end + + if user.support? + if log.owning_organisation.holds_own_stock? + opts[log.owning_organisation.id] = "#{log.owning_organisation.name} (Owning organisation)" + end + elsif user.organisation.absorbed_organisations.exists? && user.organisation.available_from.present? + opts[user.organisation.id] = "#{user.organisation.name} (Your organisation, active as of #{user.organisation.available_from.to_fs(:govuk_date)})" + else + opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" + end + + orgs = if user.support? + log.owning_organisation.managing_agents + elsif user.organisation.absorbed_organisations.include?(log.owning_organisation) + user.organisation.managing_agents + log.owning_organisation.managing_agents + else + user.organisation.managing_agents + end.pluck(:id, :name).to_h + + user.organisation.absorbed_organisations.each do |absorbed_org| + opts[absorbed_org.id] = "#{absorbed_org.name} (inactive as of #{absorbed_org.merge_date.to_fs(:govuk_date)})" + end + + opts.merge(orgs) + end + + def displayed_answer_options(log, user) + answer_options(log, user) + end + + def label_from_value(value, _log = nil, _user = nil) + return unless value + + answer_options[value] + end + + def derived? + true + end + + def hidden_in_check_answers?(log, user = nil) + user.nil? || !user.support? || !@page.routed_to?(log, user) + end + + def enabled + true + end + + def answer_label(log, _current_user = nil) + Organisation.find_by(id: log.managing_organisation_id)&.name + end + +private + + def selected_answer_option_is_derived?(_log) + true + end +end diff --git a/app/models/form/sales/questions/owning_organisation_id.rb b/app/models/form/sales/questions/owning_organisation_id.rb index a53f99dc3..47a10993d 100644 --- a/app/models/form/sales/questions/owning_organisation_id.rb +++ b/app/models/form/sales/questions/owning_organisation_id.rb @@ -11,11 +11,24 @@ class Form::Sales::Questions::OwningOrganisationId < ::Form::Question answer_opts = { "" => "Select an option" } return answer_opts unless ActiveRecord::Base.connected? + return answer_opts unless user + return answer_opts unless log - if FeatureToggle.merge_organisations_enabled? - return answer_opts unless user - return answer_opts unless log + if FeatureToggle.sales_managing_organisation_enabled? && !user.support? + if log.owning_organisation_id.present? + answer_opts[log.owning_organisation.id] = log.owning_organisation.name + end + if user.organisation.holds_own_stock? + answer_opts[user.organisation.id] = "#{user.organisation.name} (Your organisation)" + end + + user.organisation.stock_owners.where(holds_own_stock: true).find_each do |org| + answer_opts[org.id] = org.name + end + end + + if FeatureToggle.merge_organisations_enabled? if log.owning_organisation_id.present? answer_opts[log.owning_organisation.id] = log.owning_organisation.name end diff --git a/app/models/form/sales/subsections/setup.rb b/app/models/form/sales/subsections/setup.rb index 1fbc2ac9e..26a47c700 100644 --- a/app/models/form/sales/subsections/setup.rb +++ b/app/models/form/sales/subsections/setup.rb @@ -7,7 +7,8 @@ class Form::Sales::Subsections::Setup < ::Form::Subsection def pages @pages ||= [ - Form::Sales::Pages::Organisation.new(nil, nil, self), + Form::Sales::Pages::OwningOrganisation.new(nil, nil, self), + Form::Sales::Pages::ManagingOrganisation.new(nil, nil, self), Form::Sales::Pages::CreatedBy.new(nil, nil, self), Form::Sales::Pages::SaleDate.new(nil, nil, self), Form::Sales::Pages::PurchaserCode.new(nil, nil, self), diff --git a/app/models/lettings_log.rb b/app/models/lettings_log.rb index a38f1eba1..d86f20424 100644 --- a/app/models/lettings_log.rb +++ b/app/models/lettings_log.rb @@ -59,9 +59,6 @@ class LettingsLog < Log } scope :after_date, ->(date) { where("lettings_logs.startdate >= ?", date) } scope :unresolved, -> { where(unresolved: true) } - scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } - scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } - scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) } scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: 1)) } scope :tcharge_answered, -> { where.not(tcharge: nil).or(where(household_charge: 1)).or(where(is_carehome: 1)) } scope :chcharge_answered, -> { where.not(chcharge: nil).or(where(is_carehome: [nil, 0])) } diff --git a/app/models/log.rb b/app/models/log.rb index ca10b94e5..b588e1269 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -49,6 +49,9 @@ class Log < ApplicationRecord scope :has_old_form_id, -> { where.not(old_form_id: nil) } scope :imported_2023_with_old_form_id, -> { imported.filter_by_year(2023).has_old_form_id } scope :imported_2023, -> { imported.filter_by_year(2023) } + scope :filter_by_organisation, ->(org, _user = nil) { where(owning_organisation: org).or(where(managing_organisation: org)) } + scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } + scope :filter_by_managing_organisation, ->(managing_organisation, _user = nil) { where(managing_organisation:) } attr_accessor :skip_update_status, :skip_update_uprn_confirmed, :skip_dpo_validation diff --git a/app/models/organisation.rb b/app/models/organisation.rb index da6a92d0d..76d14dfde 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -74,7 +74,7 @@ class Organisation < ApplicationRecord end def sales_logs - SalesLog.filter_by_owning_organisation(absorbed_organisations + [self]) + SalesLog.filter_by_organisation(absorbed_organisations + [self]) end def owned_lettings_logs @@ -82,13 +82,17 @@ class Organisation < ApplicationRecord end def owned_sales_logs - sales_logs + SalesLog.filter_by_owning_organisation(absorbed_organisations + [self]) end def managed_lettings_logs LettingsLog.filter_by_managing_organisation(absorbed_organisations + [self]) end + def managed_sales_logs + SalesLog.filter_by_managing_organisation(absorbed_organisations + [self]) + end + def address_string %i[address_line1 address_line2 postcode].map { |field| public_send(field) }.join("\n") end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index fd330f5c5..cff497ae4 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -42,8 +42,6 @@ class SalesLog < Log .or(filter_by_postcode(param)) .or(filter_by_id(param)) } - scope :filter_by_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } - scope :filter_by_owning_organisation, ->(owning_organisation, _user = nil) { where(owning_organisation:) } scope :age1_answered, -> { where.not(age1: nil).or(where(age1_known: [1, 2])) } scope :duplicate_logs, lambda { |log| visible.where(log.slice(*DUPLICATE_LOG_ATTRIBUTES)) @@ -318,9 +316,9 @@ class SalesLog < Log def reset_created_by! return unless updated_by&.support? - return if owning_organisation.blank? || created_by.blank? - return if created_by&.organisation == owning_organisation - return if created_by&.organisation == owning_organisation.absorbing_organisation + return if owning_organisation.blank? || managing_organisation.blank? || created_by.blank? + return if created_by&.organisation == owning_organisation || created_by&.organisation == managing_organisation + return if created_by&.organisation == owning_organisation.absorbing_organisation || created_by&.organisation == managing_organisation.absorbing_organisation update!(created_by: nil) end diff --git a/app/models/user.rb b/app/models/user.rb index 52e2c850d..4e80f7382 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -87,7 +87,7 @@ class User < ApplicationRecord if support? SalesLog.all else - SalesLog.filter_by_owning_organisation(organisation.absorbed_organisations + [organisation]) + SalesLog.filter_by_organisation(organisation.absorbed_organisations + [organisation]) end end @@ -99,6 +99,14 @@ class User < ApplicationRecord LettingsLog.filter_by_managing_organisation(organisation.absorbed_organisations + [organisation]) end + def owned_sales_logs + SalesLog.filter_by_owning_organisation(organisation.absorbed_organisations + [organisation]) + end + + def managed_sales_logs + SalesLog.filter_by_managing_organisation(organisation.absorbed_organisations + [organisation]) + end + def is_key_contact? is_key_contact end diff --git a/app/services/bulk_upload/sales/year2022/row_parser.rb b/app/services/bulk_upload/sales/year2022/row_parser.rb index 55f5f0ed1..603860bcf 100644 --- a/app/services/bulk_upload/sales/year2022/row_parser.rb +++ b/app/services/bulk_upload/sales/year2022/row_parser.rb @@ -728,6 +728,7 @@ private attributes["othtype"] = field_85 attributes["owning_organisation"] = owning_organisation + attributes["managing_organisation"] = owning_organisation attributes["created_by"] = created_by || bulk_upload.user attributes["hhregres"] = hhregres attributes["hhregresstill"] = hhregresstill diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index c83c9161a..485a412ba 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/app/services/bulk_upload/sales/year2023/row_parser.rb @@ -455,10 +455,12 @@ class BulkUpload::Sales::Year2023::RowParser validate :validate_owning_org_data_given, on: :after_log validate :validate_owning_org_exists, on: :after_log + validate :validate_owning_org_owns_stock, on: :after_log if FeatureToggle.sales_managing_organisation_enabled? validate :validate_owning_org_permitted, on: :after_log validate :validate_created_by_exists, on: :after_log validate :validate_created_by_related, on: :after_log + validate :validate_managing_org_related, on: :after_log if FeatureToggle.sales_managing_organisation_enabled? validate :validate_relevant_collection_window, on: :after_log validate :validate_incomplete_soft_validations, on: :after_log @@ -896,6 +898,7 @@ private attributes["othtype"] = field_11 attributes["owning_organisation"] = owning_organisation + attributes["managing_organisation"] = managing_organisation attributes["created_by"] = created_by || bulk_upload.user attributes["hhregres"] = field_73 attributes["hhregresstill"] = field_74 @@ -1195,10 +1198,27 @@ private def validate_created_by_related return unless created_by - return if created_by.organisation == owning_organisation || created_by.organisation == owning_organisation&.absorbing_organisation + return if created_by.organisation == owning_organisation || created_by.organisation == managing_organisation + return if created_by.organisation == owning_organisation&.absorbing_organisation || created_by.organisation == managing_organisation&.absorbing_organisation block_log_creation! - errors.add(:field_2, "User must be related to owning organisation", category: :setup) + errors.add(:field_2, "User must be related to owning organisation or managing organisation", category: :setup) + end + + def managing_organisation + return owning_organisation if created_by&.organisation&.absorbed_organisations&.include?(owning_organisation) + + created_by&.organisation || bulk_upload.user.organisation + end + + def validate_managing_org_related + if owning_organisation && managing_organisation && !owning_organisation.can_be_managed_by?(organisation: managing_organisation) + block_log_creation! + + if errors[:field_2].blank? + errors.add(:field_2, "This user belongs to an organisation that does not have a relationship with the owning organisation", category: :setup) + end + end end def setup_question?(question) diff --git a/app/services/csv/sales_log_csv_service.rb b/app/services/csv/sales_log_csv_service.rb index e5b866038..900d3fdb1 100644 --- a/app/services/csv/sales_log_csv_service.rb +++ b/app/services/csv/sales_log_csv_service.rb @@ -42,6 +42,10 @@ module Csv labels: %i[owning_organisation name], codes: %i[owning_organisation name], }, + managing_organisation_name: { + labels: %i[managing_organisation name], + codes: %i[managing_organisation name], + }, creation_method: { labels: %i[creation_method], codes: %i[creation_method_before_type_cast], @@ -125,6 +129,7 @@ module Csv "prevloc" => %w[prevloc prevloc_label], "created_by_id" => %w[created_by], "owning_organisation_id" => %w[owning_organisation_name], + "managing_organisation_id" => %w[managing_organisation_name], }.freeze def sales_log_attributes diff --git a/app/services/feature_toggle.rb b/app/services/feature_toggle.rb index 6680b0b96..3121883d8 100644 --- a/app/services/feature_toggle.rb +++ b/app/services/feature_toggle.rb @@ -43,4 +43,8 @@ class FeatureToggle def self.service_moved? false end + + def self.sales_managing_organisation_enabled? + true + end end diff --git a/app/services/merge/merge_organisations_service.rb b/app/services/merge/merge_organisations_service.rb index 0662853fb..a710c20a5 100644 --- a/app/services/merge/merge_organisations_service.rb +++ b/app/services/merge/merge_organisations_service.rb @@ -93,6 +93,9 @@ private def merge_lettings_logs(merging_organisation) merging_organisation.owned_lettings_logs.after_date(@merge_date.to_time).each do |lettings_log| + if lettings_log.managing_organisation == merging_organisation + lettings_log.managing_organisation = @absorbing_organisation + end if lettings_log.scheme.present? scheme_to_set = @absorbing_organisation.owned_schemes.find_by(service_name: lettings_log.scheme.service_name) location_to_set = scheme_to_set.locations.find_by(name: lettings_log.location&.name, postcode: lettings_log.location&.postcode) @@ -121,7 +124,10 @@ private end def merge_sales_logs(merging_organisation) - merging_organisation.sales_logs.after_date(@merge_date.to_time).each do |sales_log| + merging_organisation.owned_sales_logs.after_date(@merge_date.to_time).each do |sales_log| + if sales_log.managing_organisation == merging_organisation + sales_log.managing_organisation = @absorbing_organisation + end sales_log.owning_organisation = @absorbing_organisation if sales_log.collection_period_open? sales_log.skip_dpo_validation = true @@ -130,6 +136,15 @@ private sales_log.save!(validate: false) end end + merging_organisation.managed_sales_logs.after_date(@merge_date.to_time).each do |sales_log| + sales_log.managing_organisation = @absorbing_organisation + if sales_log.collection_period_open? + sales_log.skip_dpo_validation = true + sales_log.save! + else + sales_log.save!(validate: false) + end + end end def mark_organisation_as_merged(merging_organisation) diff --git a/app/views/logs/_log_filters.html.erb b/app/views/logs/_log_filters.html.erb index 72f73e47c..16053c805 100644 --- a/app/views/logs/_log_filters.html.erb +++ b/app/views/logs/_log_filters.html.erb @@ -93,7 +93,7 @@ } %> <% end %> - <% if (current_user.support? || non_support_with_multiple_managing_orgs?) && user_or_org_lettings_path? %> + <% if (current_user.support? || non_support_with_managing_orgs?) && (user_or_org_lettings_path? || FeatureToggle.sales_managing_organisation_enabled?) %> <%= render partial: "filters/radio_filter", locals: { f:, options: { @@ -102,13 +102,13 @@ label: "Specific managing organisation", conditional_filter: { type: "select", - label: "Managed by", + label: user_or_org_lettings_path? ? "Managed by" : "Reported by", category: "managing_organisation", options: managing_organisation_filter_options(current_user), }, }, }, - label: "Managed by", + label: user_or_org_lettings_path? ? "Managed by" : "Reported by", category: "managing_organisation_select", } %> <% end %> diff --git a/db/seeds.rb b/db/seeds.rb index 876601c19..805f0c53a 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -200,6 +200,7 @@ unless Rails.env.test? SalesLog.find_or_create_by!( created_by: support_user, owning_organisation: org, + managing_organisation: org, saledate: Date.new(2023, 4, 1), purchid: "1", ownershipsch: 1, @@ -211,6 +212,7 @@ unless Rails.env.test? SalesLog.find_or_create_by!( created_by: support_user, owning_organisation: org, + managing_organisation: org, saledate: Date.new(2023, 4, 1), purchid: "1", ownershipsch: 2, @@ -222,6 +224,7 @@ unless Rails.env.test? SalesLog.find_or_create_by!( created_by: support_user, owning_organisation: org, + managing_organisation: org, saledate: Date.new(2023, 4, 1), purchid: "1", ownershipsch: 3, diff --git a/spec/factories/sales_log.rb b/spec/factories/sales_log.rb index e930f3b2a..29f32346f 100644 --- a/spec/factories/sales_log.rb +++ b/spec/factories/sales_log.rb @@ -2,6 +2,7 @@ FactoryBot.define do factory :sales_log do created_by { FactoryBot.create(:user) } owning_organisation { created_by.organisation } + managing_organisation { owning_organisation } created_at { Time.zone.now } updated_at { Time.zone.now } trait :in_progress do diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 9c24f2470..69a5d8b5c 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -243,7 +243,7 @@ RSpec.describe "User Features" do end check("years-2021-field") click_button("Apply filters") - expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=") + expect(page).to have_current_path("/organisations/#{org_id}/sales-logs?years[]=&years[]=2021&status[]=&assigned_to=all&user=&owning_organisation_select=all&owning_organisation=&managing_organisation_select=all&managing_organisation=") expect(page).not_to have_link first_log.id.to_s, href: "/sales-logs/#{first_log.id}" end end diff --git a/spec/fixtures/files/sales_logs_csv_export_codes.csv b/spec/fixtures/files/sales_logs_csv_export_codes.csv index 40a541551..a82240317 100644 --- a/spec/fixtures/files/sales_logs_csv_export_codes.csv +++ b/spec/fixtures/files/sales_logs_csv_export_codes.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant -,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,1,false,DLUHC,billyboy@eyeklaud.com,8,2,2023,,2,8,,,,1,1,2,1,1,0,,,Address line 1,,Town or city,,SW1A,1AA,1,E09000003,Barnet,1,2,1,30,X,17,17,18,1,1,P,35,X,17,,13,1,1,3,C,14,X,9,X,-9,X,3,R,-9,R,10,,,,,1,1,,,0,,,1,1,1,1,,3,,1,4,5,1,1,0,10000,1,0,10000,1,4,1,,1,2,10,,,,,,,,,,,,,,,,,110000.0,,1,20000.0,5,,10,1,80000.0,,,1,100.0,,10000.0 +id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,managing_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,1,false,DLUHC,DLUHC,billyboy@eyeklaud.com,8,2,2023,,2,8,,,,1,1,2,1,1,0,,,Address line 1,,Town or city,,SW1A,1AA,1,E09000003,Barnet,1,2,1,30,X,17,17,18,1,1,P,35,X,17,,13,1,1,3,C,14,X,9,X,-9,X,3,R,-9,R,10,,,,,1,1,,,0,,,1,1,1,1,,3,,1,4,5,1,1,0,10000,1,0,10000,1,4,1,,1,2,10,,,,,,,,,,,,,,,,,110000.0,,1,20000.0,5,,10,1,80000.0,,,1,100.0,,10000.0 diff --git a/spec/fixtures/files/sales_logs_csv_export_labels.csv b/spec/fixtures/files/sales_logs_csv_export_labels.csv index 35ca32e40..6c38f5e58 100644 --- a/spec/fixtures/files/sales_logs_csv_export_labels.csv +++ b/spec/fixtures/files/sales_logs_csv_export_labels.csv @@ -1,2 +1,2 @@ -id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant -,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,single log,false,DLUHC,billyboy@eyeklaud.com,8,2,2023,,Yes - a discounted ownership scheme,Right to Acquire (RTA),,,,Yes,Yes,2,Flat or maisonette,Purpose built,Yes,,,Address line 1,,Town or city,,SW1A,1AA,Yes,E09000003,Barnet,Yes,Yes,1,30,Non-binary,Buyer 1 prefers not to say,17,United Kingdom,Full-time - 30 hours or more,Yes,Partner,35,Non-binary,17,,13,Full-time - 30 hours or more,Yes,3,Child,14,Non-binary,Child under 16,Other,Not known,Non-binary,"In government training into work, such as New Deal",Prefers not to say,Not known,Prefers not to say,Prefers not to say,,,,,Local authority tenant,No,,,No,,,1,1,1,1,,3,,Yes,Yes,No,Yes,Yes,Yes,10000,Yes,Yes,10000,Yes,"Don’t know ",No,,Yes,2,10,,,,,,,,,,,,,,,,,110000.0,,Yes,20000.0,Cambridge Building Society,,10,Yes,80000.0,,,Yes,100.0,,10000.0 +id,status,created_at,updated_at,old_form_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,managing_organisation_name,created_by,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant +,completed,2023-02-08T00:00:00+00:00,2023-02-08T00:00:00+00:00,,2022,single log,false,DLUHC,DLUHC,billyboy@eyeklaud.com,8,2,2023,,Yes - a discounted ownership scheme,Right to Acquire (RTA),,,,Yes,Yes,2,Flat or maisonette,Purpose built,Yes,,,Address line 1,,Town or city,,SW1A,1AA,Yes,E09000003,Barnet,Yes,Yes,1,30,Non-binary,Buyer 1 prefers not to say,17,United Kingdom,Full-time - 30 hours or more,Yes,Partner,35,Non-binary,17,,13,Full-time - 30 hours or more,Yes,3,Child,14,Non-binary,Child under 16,Other,Not known,Non-binary,"In government training into work, such as New Deal",Prefers not to say,Not known,Prefers not to say,Prefers not to say,,,,,Local authority tenant,No,,,No,,,1,1,1,1,,3,,Yes,Yes,No,Yes,Yes,Yes,10000,Yes,Yes,10000,Yes,"Don’t know ",No,,Yes,2,10,,,,,,,,,,,,,,,,,110000.0,,Yes,20000.0,Cambridge Building Society,,10,Yes,80000.0,,,Yes,100.0,,10000.0 diff --git a/spec/lib/tasks/set_sales_managing_organisation_spec.rb b/spec/lib/tasks/set_sales_managing_organisation_spec.rb index c8e304d9f..657befe84 100644 --- a/spec/lib/tasks/set_sales_managing_organisation_spec.rb +++ b/spec/lib/tasks/set_sales_managing_organisation_spec.rb @@ -14,7 +14,7 @@ RSpec.describe "set_sales_managing_organisation" do context "when the rake task is run" do let!(:sales_log) { create(:sales_log, :completed, managing_organisation_id: nil) } - it "updates sales log managing_organisation_id with owning_organisation_id" do + xit "updates sales log managing_organisation_id with owning_organisation_id" do expect(sales_log.managing_organisation_id).to eq(nil) expect(sales_log.status).to eq("completed") task.invoke diff --git a/spec/models/form/sales/pages/managing_organisation_spec.rb b/spec/models/form/sales/pages/managing_organisation_spec.rb new file mode 100644 index 000000000..0a34554cb --- /dev/null +++ b/spec/models/form/sales/pages/managing_organisation_spec.rb @@ -0,0 +1,106 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Pages::ManagingOrganisation, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { nil } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[managing_organisation_id]) + end + + it "has the correct id" do + expect(page.id).to eq("managing_organisation") + end + + it "has the correct header" do + expect(page.header).to be_nil + end + + it "has the correct description" do + expect(page.description).to be_nil + end + + it "has the correct depends_on" do + expect(page.depends_on).to be nil + end + + describe "#routed_to?" do + let(:log) { create(:lettings_log) } + let(:organisation) { create(:organisation) } + + context "when user nil" do + it "is not shown" do + expect(page.routed_to?(log, nil)).to eq(false) + end + end + + context "when user is not support" do + let(:user) { create(:user) } + + it "is not shown" do + expect(page.routed_to?(log, nil)).to eq(false) + end + end + + context "when support" do + let(:user) { create(:user, :support) } + + context "when owning_organisation not set" do + let(:log) { create(:lettings_log, owning_organisation: nil) } + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + end + + context "with 0 managing_agents" do + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + end + + context "with >1 managing_agents" do + before do + create(:organisation_relationship, parent_organisation: log.owning_organisation) + create(:organisation_relationship, parent_organisation: log.owning_organisation) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + end + + context "with 1 managing_agents" do + let(:managing_agent) { create(:organisation) } + + before do + create( + :organisation_relationship, + child_organisation: managing_agent, + parent_organisation: log.owning_organisation, + ) + end + + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) + end + end + end + + context "when not support" do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: false)) } + + it "is not shown" do + expect(page.routed_to?(log, user)).to eq(false) + end + end + end +end diff --git a/spec/models/form/sales/pages/organisation_spec.rb b/spec/models/form/sales/pages/owning_organisation_spec.rb similarity index 90% rename from spec/models/form/sales/pages/organisation_spec.rb rename to spec/models/form/sales/pages/owning_organisation_spec.rb index 8f160ca53..4a2ca0408 100644 --- a/spec/models/form/sales/pages/organisation_spec.rb +++ b/spec/models/form/sales/pages/owning_organisation_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -RSpec.describe Form::Sales::Pages::Organisation, type: :model do +RSpec.describe Form::Sales::Pages::OwningOrganisation, type: :model do subject(:page) { described_class.new(page_id, page_definition, subsection) } let(:page_id) { nil } @@ -17,7 +17,7 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do end it "has the correct id" do - expect(page.id).to eq("organisation") + expect(page.id).to eq("owning_organisation") end it "has the correct header" do @@ -88,8 +88,8 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do expect(page.routed_to?(log, user)).to eq(false) end - it "does not update owning_organisation_id" do - expect { page.routed_to?(log, user) }.not_to change(log.reload, :owning_organisation) + it "updates owning_organisation_id" do + expect { page.routed_to?(log, user) }.to change(log.reload, :owning_organisation) end end @@ -110,8 +110,8 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do ) end - it "is not shown" do - expect(page.routed_to?(log, user)).to eq(false) + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) end end end @@ -133,8 +133,8 @@ RSpec.describe Form::Sales::Pages::Organisation, type: :model do create(:organisation_relationship, child_organisation: user.organisation) end - it "is not shown" do - expect(page.routed_to?(log, user)).to eq(false) + it "is shown" do + expect(page.routed_to?(log, user)).to eq(true) end end diff --git a/spec/models/form/sales/questions/created_by_id_spec.rb b/spec/models/form/sales/questions/created_by_id_spec.rb index 29f5b38b8..0f3ca4b73 100644 --- a/spec/models/form/sales/questions/created_by_id_spec.rb +++ b/spec/models/form/sales/questions/created_by_id_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("User") + expect(question.check_answer_label).to eq("Log owner") end it "has the correct type" do @@ -52,9 +52,9 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do describe "#displayed_answer_options" do let(:owning_org_user) { create(:user) } - let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) } + let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation, managing_organisation: owning_org_user.organisation) } - it "only displays users that belong to the owning organisation" do + it "only displays users that belong to the managing organisation" do expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_option_for_users(owning_org_user.organisation.users)) end end @@ -69,14 +69,14 @@ RSpec.describe Form::Sales::Questions::CreatedById, type: :model do describe "#displayed_answer_options" do let(:owning_org_user) { create(:user) } - let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) } + let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation, managing_organisation: owning_org_user.organisation) } before do create(:user, organisation: data_coordinator.organisation) end - it "only displays users that belong user's org" do - expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(data_coordinator.organisation.users)) + it "only displays users that belong to managing organisation" do + expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_option_for_users(owning_org_user.organisation.users)) end end end diff --git a/spec/models/form/sales/questions/managing_organisation_spec.rb b/spec/models/form/sales/questions/managing_organisation_spec.rb new file mode 100644 index 000000000..98f459b24 --- /dev/null +++ b/spec/models/form/sales/questions/managing_organisation_spec.rb @@ -0,0 +1,257 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Questions::ManagingOrganisation, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("managing_organisation_id") + end + + it "has the correct header" do + expect(question.header).to eq("Which organisation is reporting this sale?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Reported by") + end + + it "has the correct type" do + expect(question.type).to eq("select") + end + + it "has the correct hint_text" do + expect(question.hint_text).to be_nil + end + + describe "#displayed_answer_options" do + let(:options) { { "" => "Select an option" } } + + context "when current_user nil" do + let(:log) { create(:lettings_log) } + + it "shows default options" do + expect(question.displayed_answer_options(log, nil)).to eq(options) + end + end + + context "when log nil" do + let(:user) { create(:user) } + + it "shows default options" do + expect(question.displayed_answer_options(nil, user)).to eq(options) + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + let(:log_owning_org) { create(:organisation, name: "Owning org") } + + let(:managing_org1) { create(:organisation, name: "Managing org 1") } + let(:managing_org2) { create(:organisation, name: "Managing org 2") } + let(:managing_org3) { create(:organisation, name: "Managing org 3") } + + let(:log) do + create(:lettings_log, owning_organisation: log_owning_org, managing_organisation: managing_org1, + created_by: nil) + end + let!(:org_rel1) do + create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org2) + end + let!(:org_rel2) do + create(:organisation_relationship, parent_organisation: log_owning_org, child_organisation: managing_org3) + end + + context "when org owns stock" do + let(:options) do + { + "" => "Select an option", + log.managing_organisation.id => "Managing org 1", + log_owning_org.id => "Owning org (Owning organisation)", + org_rel1.child_organisation.id => "Managing org 2", + org_rel2.child_organisation.id => "Managing org 3", + } + end + + it "shows current managing agent at top, followed by the current owning organisation (with hint), followed by the managing agents of the current owning organisation" do + log_owning_org.update!(holds_own_stock: true) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when org does not own stock" do + let(:options) do + { + "" => "Select an option", + log.managing_organisation.id => "Managing org 1", + org_rel1.child_organisation.id => "Managing org 2", + org_rel2.child_organisation.id => "Managing org 3", + } + end + + it "shows current managing agent at top, followed by the managing agents of the current owning organisation" do + log_owning_org.update!(holds_own_stock: false) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + end + + context "when the owning-managing organisation relationship is deleted" do + let(:user) { create(:user, :support) } + + let(:owning_org) { create(:organisation, name: "Owning org", holds_own_stock: true) } + let(:managing_org) { create(:organisation, name: "Managing org", holds_own_stock: false) } + let(:org_rel) do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: managing_org) + end + let(:log) do + create(:lettings_log, owning_organisation: owning_org, managing_organisation: managing_org, created_by: nil) + end + + let(:options) do + { + "" => "Select an option", + owning_org.id => "Owning org (Owning organisation)", + managing_org.id => "Managing org", + } + end + + it "doesn't remove the managing org from the list of allowed managing orgs" do + org_rel.destroy! + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + + context "when organisation has merged" do + let(:absorbing_org) { create(:organisation, name: "Absorbing org", holds_own_stock: true) } + let!(:merged_org) { create(:organisation, name: "Merged org", holds_own_stock: false) } + let(:user) { create(:user, :support, organisation: absorbing_org) } + + let(:log) do + merged_org.update!(merge_date: Time.zone.local(2023, 8, 2), absorbing_organisation_id: absorbing_org.id) + create(:lettings_log, owning_organisation: absorbing_org, managing_organisation: nil) + end + + it "displays merged organisation on the list of choices" do + options = { + "" => "Select an option", + absorbing_org.id => "Absorbing org (Owning organisation)", + merged_org.id => "Merged org (inactive as of 2 August 2023)", + merged_org.id => "Merged org (inactive as of 2 August 2023)", + } + expect(question.displayed_answer_options(log, user)).to eq(options) + end + + it "displays active date for absorbing organisation if available from is given" do + absorbing_org.update!(available_from: Time.zone.local(2023, 8, 3)) + options = { + "" => "Select an option", + absorbing_org.id => "Absorbing org (Owning organisation)", + merged_org.id => "Merged org (inactive as of 2 August 2023)", + merged_org.id => "Merged org (inactive as of 2 August 2023)", + } + expect(question.displayed_answer_options(log, user)).to eq(options) + end + + it "displays managing agents of merged organisation selected as owning org" do + managing_agent = create(:organisation, name: "Managing org 1") + create(:organisation_relationship, parent_organisation: merged_org, child_organisation: managing_agent) + + options = { + "" => "Select an option", + merged_org.id => "Merged org (inactive as of 2 August 2023)", + managing_agent.id => "Managing org 1", + } + + log.update!(owning_organisation: merged_org) + expect(question.displayed_answer_options(log, user)).to eq(options) + end + end + end + + it "is marked as derived" do + expect(question.derived?).to be true + end + + describe "#hidden_in_check_answers?" do + before do + allow(page).to receive(:routed_to?).and_return(true) + end + + context "when user is non support" do + let(:user) { create(:user) } + + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be true + end + end + + context "when user is support" do + let(:user) { create(:user, :support) } + + it "is not hidden in check answers" do + expect(question.hidden_in_check_answers?(nil, user)).to be false + end + end + + context "when user not provided" do + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(nil)).to be true + end + end + + context "when the page is not routed to" do + let(:user) { create(:user, :data_coordinator, organisation: create(:organisation, holds_own_stock: true)) } + let(:log) { create(:lettings_log, owning_organisation: user.organisation) } + + before do + allow(page).to receive(:routed_to?).and_return(false) + end + + it "is hidden in check answers" do + expect(question.hidden_in_check_answers?(log, user)).to be true + end + end + end + + describe "#answer_label" do + context "when answered" do + let(:managing_organisation) { create(:organisation) } + let(:log) { create(:lettings_log, managing_organisation:) } + + it "returns org name" do + expect(question.answer_label(log)).to eq(managing_organisation.name) + end + end + + context "when unanswered" do + let(:log) { create(:lettings_log, managing_organisation: nil) } + + it "returns nil" do + expect(question.answer_label(log)).to be_nil + end + end + + context "when org does not exist" do + let(:managing_organisation) { create(:organisation) } + let(:log) { create(:lettings_log, managing_organisation:) } + + before do + allow(Organisation).to receive(:find_by).and_return(nil) + end + + it "returns nil" do + expect(question.answer_label(log)).to be_nil + end + end + end +end diff --git a/spec/models/form/sales/questions/owning_organisation_id_spec.rb b/spec/models/form/sales/questions/owning_organisation_id_spec.rb index ceb081459..20ce635a4 100644 --- a/spec/models/form/sales/questions/owning_organisation_id_spec.rb +++ b/spec/models/form/sales/questions/owning_organisation_id_spec.rb @@ -59,21 +59,25 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do let(:owning_org_1) { create(:organisation, name: "Owning org 1") } let(:owning_org_2) { create(:organisation, name: "Owning org 2") } - let!(:org_rel) do - create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) - end + let(:non_stock_owner) { create(:organisation, name: "Non stock owner", holds_own_stock: false) } let(:log) { create(:lettings_log, owning_organisation: owning_org_1) } context "when user's org owns stock" do + before do + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: non_stock_owner) + end + let(:options) do { "" => "Select an option", owning_org_1.id => "Owning org 1", + owning_org_2.id => "Owning org 2", user.organisation.id => "User org (Your organisation)", } end - it "does not show stock owner" do + it "shows user organisation, current owning organisation and the stock owners that hold their stock" do user.organisation.update!(holds_own_stock: true) expect(question.displayed_answer_options(log, user)).to eq(options) end @@ -84,10 +88,16 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do { "" => "Select an option", owning_org_1.id => "Owning org 1", + owning_org_2.id => "Owning org 2", } end - it "shows current log owner" do + before do + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: owning_org_2) + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: non_stock_owner) + end + + it "shows current owning organisation and the stock owners that hold their stock" do user.organisation.update!(holds_own_stock: false) expect(question.displayed_answer_options(log, user)).to eq(options) end @@ -156,7 +166,7 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do end before do - org_rel.update!(child_organisation: merged_organisation) + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: merged_organisation) merged_organisation.update!(merge_date: Time.zone.local(2023, 2, 2), absorbing_organisation: user.organisation) Timecop.freeze(Time.zone.local(2023, 11, 10)) end @@ -176,13 +186,14 @@ RSpec.describe Form::Sales::Questions::OwningOrganisationId, type: :model do { "" => "Select an option", user.organisation.id => "User org (Your organisation)", + merged_organisation.id => "Merged org", owning_org_1.id => "Owning org 1", } end before do Timecop.freeze(Time.zone.local(2023, 4, 2)) - org_rel.update!(child_organisation: merged_organisation) + create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: merged_organisation) merged_organisation.update!(merge_date: Time.zone.local(2021, 6, 2), absorbing_organisation: user.organisation) user.organisation.update!(available_from: Time.zone.local(2021, 2, 2)) end diff --git a/spec/models/form/sales/subsections/setup_spec.rb b/spec/models/form/sales/subsections/setup_spec.rb index 7a71e946c..cdc3aff8d 100644 --- a/spec/models/form/sales/subsections/setup_spec.rb +++ b/spec/models/form/sales/subsections/setup_spec.rb @@ -15,7 +15,8 @@ RSpec.describe Form::Sales::Subsections::Setup, type: :model do it "has correct pages" do expect(setup.pages.map(&:id)).to eq( %w[ - organisation + owning_organisation + managing_organisation created_by completion_date purchaser_code diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 64cbab265..838b636dd 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -387,9 +387,9 @@ RSpec.describe Form, type: :model do expect(form.sections[0].class).to eq(Form::Sales::Sections::Setup) expect(form.subsections.count).to eq(1) expect(form.subsections.first.id).to eq("setup") - expect(form.pages.count).to eq(12) - expect(form.pages.first.id).to eq("organisation") - expect(form.questions.count).to eq(13) + expect(form.pages.count).to eq(13) + expect(form.pages.first.id).to eq("owning_organisation") + expect(form.questions.count).to eq(14) expect(form.questions.first.id).to eq("owning_organisation_id") expect(form.start_date).to eq(Time.zone.parse("2022-04-01")) expect(form.new_logs_end_date).to eq(Time.zone.parse("2023-11-20")) diff --git a/spec/requests/form_controller_spec.rb b/spec/requests/form_controller_spec.rb index 26d2b5412..ba27077a5 100644 --- a/spec/requests/form_controller_spec.rb +++ b/spec/requests/form_controller_spec.rb @@ -169,7 +169,7 @@ RSpec.describe FormController, type: :request do { id: sales_log.id, sales_log: { - page: "organisation", + page: "owning_organisation", owning_organisation_id: managing_organisation.id, }, } @@ -181,7 +181,7 @@ RSpec.describe FormController, type: :request do end it "correctly sets owning organisation" do - post "/sales-logs/#{sales_log.id}/organisation", params: params + post "/sales-logs/#{sales_log.id}/owning-organisation", params: params expect(response).to redirect_to("/sales-logs/#{sales_log.id}/created-by") follow_redirect! sales_log.reload @@ -196,7 +196,7 @@ RSpec.describe FormController, type: :request do { id: sales_log.id, sales_log: { - page: "organisation", + page: "owning_organisation", owning_organisation_id: managing_organisation.id, }, } @@ -208,7 +208,7 @@ RSpec.describe FormController, type: :request do end it "does not reset created by" do - post "/sales-logs/#{sales_log.id}/organisation", params: params + post "/sales-logs/#{sales_log.id}/owning-organisation", params: params expect(response).to redirect_to("/sales-logs/#{sales_log.id}/created-by") follow_redirect! sales_log.reload @@ -224,7 +224,7 @@ RSpec.describe FormController, type: :request do { id: sales_log.id, sales_log: { - page: "organisation", + page: "owning_organisation", owning_organisation_id: merged_organisation.id, }, } @@ -237,7 +237,7 @@ RSpec.describe FormController, type: :request do end it "does not reset created by" do - post "/sales-logs/#{sales_log.id}/organisation", params: params + post "/sales-logs/#{sales_log.id}/owning-organisation", params: params expect(response).to redirect_to("/sales-logs/#{sales_log.id}/created-by") follow_redirect! sales_log.reload diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 41496fac6..e3d5c9e0f 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -66,6 +66,7 @@ RSpec.describe SalesLogsController, type: :request do let(:params) do { "owning_organisation_id": owning_organisation.id, + "managing_organisation_id": owning_organisation.id, "created_by_id": user.id, "saledate": Time.zone.today, "purchid": "1", @@ -153,10 +154,11 @@ RSpec.describe SalesLogsController, type: :request do post "/sales-logs", headers: end - it "sets the stock-owning org as user's org" do + it "sets the managing org as user's org" do created_id = response.location.match(/[0-9]+/)[0] sales_log = SalesLog.find_by(id: created_id) - expect(sales_log.owning_organisation.name).to eq("User org") + expect(sales_log.owning_organisation).to be_nil + expect(sales_log.managing_organisation.name).to eq("User org") end end @@ -171,10 +173,16 @@ RSpec.describe SalesLogsController, type: :request do post "/sales-logs", headers: end - it "sets the stock-owning org as user's org" do + it "does not set owning organisation" do created_id = response.location.match(/[0-9]+/)[0] sales_log = SalesLog.find_by(id: created_id) - expect(sales_log.owning_organisation.name).to eq("User org") + expect(sales_log.owning_organisation).to be_nil + end + + it "sets managing organisation as the user organisation" do + created_id = response.location.match(/[0-9]+/)[0] + sales_log = SalesLog.find_by(id: created_id) + expect(sales_log.managing_organisation.name).to eq("User org") end end end @@ -193,6 +201,7 @@ RSpec.describe SalesLogsController, type: :request do :sales_log, purchid: purchaser_code, owning_organisation: organisation, + managing_organisation: organisation, ) end let!(:unauthorized_sales_log) do @@ -217,6 +226,7 @@ RSpec.describe SalesLogsController, type: :request do get "/sales-logs", headers: headers, params: {} expect(page).to have_content("Owned by") expect(page).not_to have_content("Managed by") + expect(page).to have_content("Reported by") end it "shows sales logs for all organisations" do @@ -879,6 +889,23 @@ RSpec.describe SalesLogsController, type: :request do end end + context "with sales logs that are managed by your organisation" do + before do + completed_sales_log.update!(managing_organisation_id: user.organisation.id, owning_organisation_id: nil) + get "/sales-logs/#{completed_sales_log.id}", headers:, params: {} + end + + after do + Timecop.return + Singleton.__init__(FormHandler) + end + + it "shows the tasklist for sales logs you have access to" do + expect(response.body).to match("Log") + expect(response.body).to match(completed_sales_log.id.to_s) + end + end + context "with sales logs from a closed collection period before the previous collection" do before do sign_in user diff --git a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb index 408ce099b..4853afb6f 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -1162,6 +1162,73 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do end end end + + describe "#managing_organisation_id" do + let(:attributes) { setup_section_params } + + context "when user is part of the owning organisation" do + it "sets managing organisation to the users organisation" do + parser.valid? + expect(parser.log.owning_organisation_id).to be(owning_org.id) + expect(parser.log.managing_organisation_id).to be(owning_org.id) + end + end + + context "when user is part of an organisation affiliated with owning org" do + let(:managing_agent) { create(:organisation) } + let(:user) { create(:user, organisation: managing_agent) } + let(:attributes) { setup_section_params } + + before do + create(:organisation_relationship, child_organisation: managing_agent, parent_organisation: owning_org) + end + + it "is not permitted as setup error" do + parser.valid? + expect(parser.log.owning_organisation_id).to be(owning_org.id) + expect(parser.log.managing_organisation_id).to be(managing_agent.id) + end + end + + context "when user is part of an organisation not affiliated with owning org" do + let(:unaffiliated_org) { create(:organisation) } + let(:user) { create(:user, organisation: unaffiliated_org) } + let(:attributes) { setup_section_params } + + it "is not permitted as setup error" do + parser.valid? + setup_errors = parser.errors.select { |e| e.options[:category] == :setup } + + expect(setup_errors.find { |e| e.attribute == :field_2 }.message).to eql("This user belongs to an organisation that does not have a relationship with the owning organisation") + end + + it "blocks log creation" do + parser.valid? + expect(parser).to be_block_log_creation + end + end + end + end + + describe "#owning_organisation_id" do + let(:attributes) { setup_section_params } + + context "when owning organisation does not own stock" do + let(:owning_org) { create(:organisation, :with_old_visible_id, holds_own_stock: false) } + let(:attributes) { setup_section_params } + + it "is not permitted as setup error" do + parser.valid? + setup_errors = parser.errors.select { |e| e.options[:category] == :setup } + + expect(setup_errors.find { |e| e.attribute == :field_1 }.message).to eql("The owning organisation code provided is for an organisation that does not own stock") + end + + it "blocks log creation" do + parser.valid? + expect(parser).to be_block_log_creation + end + end end describe "#spreadsheet_duplicate_hash" do diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index 4c4b8a1d4..cb3ec389c 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -599,10 +599,16 @@ RSpec.describe Merge::MergeOrganisationsService do end context "and merging sales logs" do - let!(:sales_log) { create(:sales_log, saledate: Time.zone.today, owning_organisation: merging_organisation) } + let(:owning_organisation) { create(:organisation, holds_own_stock: true) } + let!(:sales_log) { create(:sales_log, saledate: Time.zone.today, owning_organisation: merging_organisation, purchid: "owned") } + let!(:managed_sales_log) { create(:sales_log, saledate: Time.zone.today, purchid: "managed") } before do create(:sales_log, saledate: Time.zone.today - 2.days, owning_organisation: merging_organisation) + create(:organisation_relationship) { create(:organisation_relationship, parent_organisation: owning_organisation, child_organisation: merging_organisation) } + managed_sales_log.update!(owning_organisation:, managing_organisation: merging_organisation, created_by: merging_organisation_user) + create(:sales_log, saledate: Time.zone.today - 2.days, owning_organisation: merging_organisation, created_by: merging_organisation_user, purchid: "ranom 1") + create(:sales_log, saledate: Time.zone.today - 2.days, owning_organisation:, managing_organisation: merging_organisation, created_by: merging_organisation_user, purchid: "ranom 2") end it "moves relevant logs" do @@ -611,6 +617,9 @@ RSpec.describe Merge::MergeOrganisationsService do absorbing_organisation.reload expect(SalesLog.filter_by_owning_organisation(absorbing_organisation).count).to eq(1) expect(SalesLog.filter_by_owning_organisation(absorbing_organisation).first).to eq(sales_log) + expect(SalesLog.filter_by_managing_organisation(absorbing_organisation).count).to eq(2) + expect(SalesLog.filter_by_managing_organisation(absorbing_organisation)).to include(managed_sales_log) + expect(SalesLog.filter_by_managing_organisation(absorbing_organisation)).to include(sales_log) end it "rolls back if there's an error" do