[CLDC-2202] Allow coordinators to set created_by (#1533)

* [CLDC-2202] Allow coordinators to set created_by

* Scope user selection when data coordinator

* Remove managing_for_other_user_enabled

* Move sales created_by page and question out of common

* Address comments

- only select required users
- remove not needed CYA checks

app/models/form/common/pages/created_by.rb

@@ -1,16 +0,0 @@
-class Form::Common::Pages::CreatedBy < ::Form::Page
-  def initialize(id, hsh, subsection)
-    super
-    @id = "created_by"
-  end
-
-  def questions
-    @questions ||= [
-      Form::Common::Questions::CreatedById.new(nil, nil, self),
-    ]
-  end
-
-  def routed_to?(_log, current_user)
-    !!current_user&.support?
-  end
-end

app/models/form/common/questions/created_by_id.rb

@@ -1,46 +0,0 @@
-class Form::Common::Questions::CreatedById < ::Form::Question
-  def initialize(id, hsh, page)
-    super
-    @id = "created_by_id"
-    @check_answer_label = "User"
-    @header = "Which user are you creating this log for?"
-    @type = "select"
-  end
-
-  def answer_options
-    answer_opts = { "" => "Select an option" }
-    return answer_opts unless ActiveRecord::Base.connected?
-
-    User.select(:id, :name).each_with_object(answer_opts) do |user, hsh|
-      hsh[user.id] = user.name
-      hsh
-    end
-  end
-
-  def displayed_answer_options(log, _user = nil)
-    return answer_options unless log.owning_organisation
-
-    user_ids = log.owning_organisation.users.pluck(:id) + [""]
-    answer_options.select { |k, _v| user_ids.include?(k) }
-  end
-
-  def label_from_value(value, _log = nil, _user = nil)
-    return unless value
-
-    answer_options[value]
-  end
-
-  def hidden_in_check_answers?(_log, current_user)
-    !current_user.support?
-  end
-
-  def derived?
-    true
-  end
-
-private
-
-  def selected_answer_option_is_derived?(_log)
-    false
-  end
-end

app/models/form/lettings/pages/created_by.rb

@@ -11,6 +11,9 @@ class Form::Lettings::Pages::CreatedBy < ::Form::Page
   end
 
   def routed_to?(_log, current_user)
-    !!current_user&.support?
+    return true if current_user&.support?
+    return true if current_user&.data_coordinator?
+
+    false
   end
 end

app/models/form/lettings/questions/created_by_id.rb

@@ -1,4 +1,6 @@
 class Form::Lettings::Questions::CreatedById < ::Form::Question
+  ANSWER_OPTS = { "" => "Select an option" }.freeze
+
   def initialize(id, hsh, page)
     super
     @id = "created_by_id"
@@ -8,33 +10,32 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
   end
 
   def answer_options
-    answer_opts = { "" => "Select an option" }
+    ANSWER_OPTS
-    return answer_opts unless ActiveRecord::Base.connected?
+  end
 
-    User.select(:id, :name, :email).each_with_object(answer_opts) do |user, hsh|
+  def displayed_answer_options(log, current_user = nil)
-      hsh[user.id] = "#{user.name} (#{user.email})"
+    return ANSWER_OPTS unless current_user
+
+    users = []
+    users += if current_user.support?
+               [
+                 (log.owning_organisation&.users if log.owning_organisation),
+                 (log.managing_organisation&.users if log.managing_organisation),
+               ].flatten
+             else
+               current_user.organisation.users
+             end.uniq.compact
+
+    users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
+      hsh[user.id] = present_user(user)
       hsh
     end
   end
 
-  def displayed_answer_options(log, _user = nil)
-    return answer_options unless log.owning_organisation && log.managing_organisation
-
-    user_ids = [""]
-    user_ids += log.owning_organisation.users.pluck(:id)
-    user_ids += log.managing_organisation.users.pluck(:id)
-
-    answer_options.select { |k, _v| user_ids.include?(k) }
-  end
-
   def label_from_value(value, _log = nil, _user = nil)
     return unless value
 
-    answer_options[value]
+    present_user(User.find(value))
-  end
-
-  def hidden_in_check_answers?(_log, current_user)
-    !current_user.support?
   end
 
   def derived?
@@ -43,6 +44,10 @@ class Form::Lettings::Questions::CreatedById < ::Form::Question
 
 private
 
+  def present_user(user)
+    "#{user.name} (#{user.email})"
+  end
+
   def selected_answer_option_is_derived?(_log)
     true
   end

app/models/form/lettings/subsections/setup.rb

@@ -8,12 +8,11 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection
 
   def pages
     @pages ||= [
-      organisation_page,
+      Form::Lettings::Pages::StockOwner.new(nil, nil, self),
-      stock_owner_page,
       Form::Lettings::Pages::MinRentValueCheck.new("stock_owner_min_rent_value_check", nil, self),
       Form::Lettings::Pages::MaxRentValueCheck.new("stock_owner_max_rent_value_check", nil, self),
-      managing_organisation_page,
+      Form::Lettings::Pages::ManagingOrganisation.new(nil, nil, self),
-      created_by_page,
+      Form::Lettings::Pages::CreatedBy.new(nil, nil, self),
       Form::Lettings::Pages::NeedsType.new(nil, nil, self),
       Form::Lettings::Pages::Scheme.new(nil, nil, self),
       Form::Lettings::Pages::Location.new(nil, nil, self),
@@ -34,32 +33,4 @@ class Form::Lettings::Subsections::Setup < ::Form::Subsection
   def enabled?(_lettings_log)
     true
   end
-
-private
-
-  def organisation_page
-    return if FeatureToggle.managing_for_other_user_enabled?
-
-    Form::Common::Pages::Organisation.new(nil, nil, self)
-  end
-
-  def stock_owner_page
-    return unless FeatureToggle.managing_for_other_user_enabled?
-
-    Form::Lettings::Pages::StockOwner.new(nil, nil, self)
-  end
-
-  def managing_organisation_page
-    return unless FeatureToggle.managing_for_other_user_enabled?
-
-    Form::Lettings::Pages::ManagingOrganisation.new(nil, nil, self)
-  end
-
-  def created_by_page
-    if FeatureToggle.managing_for_other_user_enabled?
-      Form::Lettings::Pages::CreatedBy.new(nil, nil, self)
-    else
-      Form::Common::Pages::CreatedBy.new(nil, nil, self)
-    end
-  end
 end

app/models/form/sales/pages/created_by.rb

@@ -0,0 +1,19 @@
+class Form::Sales::Pages::CreatedBy < ::Form::Page
+  def initialize(id, hsh, subsection)
+    super
+    @id = "created_by"
+  end
+
+  def questions
+    @questions ||= [
+      Form::Sales::Questions::CreatedById.new(nil, nil, self),
+    ]
+  end
+
+  def routed_to?(_log, current_user)
+    return true if current_user&.support?
+    return true if current_user&.data_coordinator?
+
+    false
+  end
+end

app/models/form/sales/questions/created_by_id.rb

@@ -0,0 +1,54 @@
+class Form::Sales::Questions::CreatedById < ::Form::Question
+  ANSWER_OPTS = { "" => "Select an option" }.freeze
+
+  def initialize(id, hsh, page)
+    super
+    @id = "created_by_id"
+    @check_answer_label = "User"
+    @header = "Which user are you creating this log for?"
+    @type = "select"
+  end
+
+  def answer_options
+    ANSWER_OPTS
+  end
+
+  def displayed_answer_options(log, current_user = nil)
+    return ANSWER_OPTS unless log.owning_organisation
+    return ANSWER_OPTS unless current_user
+
+    users = current_user.support? ? log.owning_organisation.users : current_user.organisation.users
+
+    users.each_with_object(ANSWER_OPTS.dup) do |user, hsh|
+      hsh[user.id] = present_user(user)
+      hsh
+    end
+  end
+
+  def label_from_value(value, _log = nil, _user = nil)
+    return unless value
+
+    present_user(User.find(value))
+  end
+
+  def hidden_in_check_answers?(_log, current_user)
+    return false if current_user.support?
+    return false if current_user.data_coordinator?
+
+    true
+  end
+
+  def derived?
+    true
+  end
+
+private
+
+  def present_user(user)
+    "#{user.name} (#{user.email})"
+  end
+
+  def selected_answer_option_is_derived?(_log)
+    false
+  end
+end

app/models/form/sales/subsections/setup.rb

@@ -8,7 +8,7 @@ class Form::Sales::Subsections::Setup < ::Form::Subsection
   def pages
     @pages ||= [
       Form::Common::Pages::Organisation.new(nil, nil, self),
-      Form::Common::Pages::CreatedBy.new(nil, nil, self),
+      Form::Sales::Pages::CreatedBy.new(nil, nil, self),
       Form::Sales::Pages::SaleDate.new(nil, nil, self),
       Form::Sales::Pages::SaleDateCheck.new(nil, nil, self),
       Form::Sales::Pages::PurchaserCode.new(nil, nil, self),

app/services/feature_toggle.rb

@@ -28,10 +28,6 @@ class FeatureToggle
     true
   end
 
-  def self.managing_for_other_user_enabled?
-    true
-  end
-
   def self.bulk_upload_lettings_logs?
     true
   end

spec/features/lettings_log_spec.rb

@@ -240,10 +240,12 @@ RSpec.describe "Lettings Log Features" do
 
     context "when completing the setup log section" do
       context "and there is at most 1 potential stock owner" do
-        it "does not include the owning organisation and created by questions" do
+        it "does not include the owning organisation and includes the created by questions" do
           visit("/lettings-logs")
           click_button("Create a new lettings log")
           click_link("Set up this lettings log")
+          select(user.name, from: "lettings-log-created-by-id-field")
+          click_button("Save and continue")
           log_id = page.current_path.scan(/\d/).join
           expect(page).to have_current_path("/lettings-logs/#{log_id}/needs-type")
           visit("lettings-logs/#{log_id}/setup/check-answers")

spec/models/form/common/questions/created_by_id_spec.rb

@@ -1,82 +0,0 @@
-require "rails_helper"
-
-RSpec.describe Form::Common::Questions::CreatedById, 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) }
-  let(:user_1) { FactoryBot.create(:user, name: "first user") }
-  let(:user_2) { FactoryBot.create(:user, name: "second user") }
-  let!(:expected_answer_options) do
-    {
-      "" => "Select an option",
-      user_1.id => user_1.name,
-      user_2.id => user_2.name,
-    }
-  end
-
-  it "has correct page" do
-    expect(question.page).to eq(page)
-  end
-
-  it "has the correct id" do
-    expect(question.id).to eq("created_by_id")
-  end
-
-  it "has the correct header" do
-    expect(question.header).to eq("Which user are you creating this log for?")
-  end
-
-  it "has the correct check_answer_label" do
-    expect(question.check_answer_label).to eq("User")
-  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
-
-  it "has the correct answer options" do
-    expect(question.answer_options).to eq(expected_answer_options)
-  end
-
-  it "is marked as derived" do
-    expect(question.derived?).to be true
-  end
-
-  context "when the current user is support" do
-    let(:support_user) { FactoryBot.build(:user, :support) }
-
-    it "is shown in check answers" do
-      expect(question.hidden_in_check_answers?(nil, support_user)).to be false
-    end
-  end
-
-  context "when the current user is not support" do
-    let(:user) { FactoryBot.build(:user) }
-
-    it "is not shown in check answers" do
-      expect(question.hidden_in_check_answers?(nil, user)).to be true
-    end
-  end
-
-  context "when the owning organisation is already set" do
-    let(:lettings_log) { FactoryBot.create(:lettings_log, owning_organisation: user_2.organisation) }
-    let(:expected_answer_options) do
-      {
-        "" => "Select an option",
-        user_2.id => user_2.name,
-      }
-    end
-
-    it "only displays users that belong to that organisation" do
-      expect(question.displayed_answer_options(lettings_log)).to eq(expected_answer_options)
-    end
-  end
-end

spec/models/form/lettings/pages/created_by_spec.rb

@@ -22,9 +22,15 @@ RSpec.describe Form::Lettings::Pages::CreatedBy, type: :model do
       end
     end
 
-    context "when not support" do
+    context "when data coordinator" do
+      it "is shown" do
+        expect(page.routed_to?(nil, create(:user, :data_coordinator))).to eq(true)
+      end
+    end
+
+    context "when data provider" do
       it "is not shown" do
-        expect(page.routed_to?(nil, create(:user, :data_coordinator))).to eq(false)
+        expect(page.routed_to?(nil, create(:user))).to eq(false)
       end
     end
   end

spec/models/form/lettings/questions/created_by_id_spec.rb

@@ -8,16 +8,6 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
   let(:page) { instance_double(Form::Page) }
   let(:subsection) { instance_double(Form::Subsection) }
   let(:form) { instance_double(Form) }
-  let(:user_1) { create(:user, name: "first user") }
-  let(:user_2) { create(:user, name: "second user") }
-  let(:user_3) { create(:user, name: "third user") }
-  let!(:expected_answer_options) do
-    {
-      "" => "Select an option",
-      user_1.id => "#{user_1.name} (#{user_1.email})",
-      user_2.id => "#{user_2.name} (#{user_2.email})",
-    }
-  end
 
   it "has correct page" do
     expect(question.page).to eq(page)
@@ -44,7 +34,7 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
   end
 
   it "has the correct answer options" do
-    expect(question.answer_options).to eq(expected_answer_options)
+    expect(question.answer_options).to eq({ "" => "Select an option" })
   end
 
   it "is marked as derived" do
@@ -52,40 +42,51 @@ RSpec.describe Form::Lettings::Questions::CreatedById, type: :model do
   end
 
   context "when the current user is support" do
-    let(:support_user) { FactoryBot.build(:user, :support) }
+    let(:owning_org_user) { create(:user) }
-
+    let(:managing_org_user) { create(:user) }
-    it "is shown in check answers" do
+    let(:support_user) { create(:user, :support, organisation: owning_org_user.organisation) }
-      expect(question.hidden_in_check_answers?(nil, support_user)).to be false
+
+    describe "#displayed_answer_options" do
+      let(:lettings_log) do
+        create(:lettings_log, created_by: support_user, owning_organisation: owning_org_user.organisation, managing_organisation: managing_org_user.organisation)
+      end
+
+      let(:expected_answer_options) do
+        {
+          "" => "Select an option",
+          managing_org_user.id => "#{managing_org_user.name} (#{managing_org_user.email})",
+          owning_org_user.id => "#{owning_org_user.name} (#{owning_org_user.email})",
+          support_user.id => "#{support_user.name} (#{support_user.email})",
+        }
+      end
+
+      it "only displays users that belong to owning and managing organisations" do
+        expect(question.displayed_answer_options(lettings_log, support_user)).to eq(expected_answer_options)
+      end
     end
   end
 
-  context "when the current user is not support" do
+  context "when the current user is data_coordinator" do
-    let(:user) { FactoryBot.build(:user) }
+    let(:managing_org_user) { create(:user) }
+    let(:data_coordinator) { create(:user, :data_coordinator) }
 
-    it "is not shown in check answers" do
+    describe "#displayed_answer_options" do
-      expect(question.hidden_in_check_answers?(nil, user)).to be true
+      let(:lettings_log) do
-    end
+        create(:lettings_log, created_by: data_coordinator, owning_organisation: data_coordinator.organisation, managing_organisation: managing_org_user.organisation)
-  end
+      end
 
-  context "when the owning organisation is already set" do
+      let(:user_in_same_org) { create(:user, organisation: data_coordinator.organisation) }
-    let(:lettings_log) do
+
-      create(
+      let(:expected_answer_options) do
-        :lettings_log,
+        {
-        created_by: user_2,
+          "" => "Select an option",
-        owning_organisation: user_2.organisation,
+          data_coordinator.id => "#{data_coordinator.name} (#{data_coordinator.email})",
-        managing_organisation: user_3.organisation,
+        }
-      )
+      end
-    end
-    let(:expected_answer_options) do
-      {
-        "" => "Select an option",
-        user_2.id => "#{user_2.name} (#{user_2.email})",
-        user_3.id => "#{user_3.name} (#{user_3.email})",
-      }
-    end
 
-    it "only displays users that belong to that organisation" do
+      it "only displays users that belong user's org" do
-      expect(question.displayed_answer_options(lettings_log)).to eq(expected_answer_options)
+        expect(question.displayed_answer_options(lettings_log, data_coordinator)).to eq(expected_answer_options)
+      end
     end
   end
 end

spec/models/form/sales/pages/created_by_spec.rb

@@ -1,6 +1,6 @@
 require "rails_helper"
 
-RSpec.describe Form::Common::Pages::CreatedBy, type: :model do
+RSpec.describe Form::Sales::Pages::CreatedBy, type: :model do
   subject(:page) { described_class.new(page_id, page_definition, subsection) }
 
   let(:page_id) { nil }
@@ -34,15 +34,23 @@ RSpec.describe Form::Common::Pages::CreatedBy, type: :model do
   end
 
   context "when the current user is a support user" do
-    let(:support_user) { FactoryBot.build(:user, :support) }
+    let(:support_user) { build(:user, :support) }
 
     it "is shown" do
       expect(page.routed_to?(lettings_log, support_user)).to be true
     end
   end
 
-  context "when the current user is not a support user" do
+  context "when the current user is a data coordinator" do
-    let(:user) { FactoryBot.build(:user) }
+    let(:support_user) { build(:user, :data_coordinator) }
+
+    it "is shown" do
+      expect(page.routed_to?(lettings_log, support_user)).to be true
+    end
+  end
+
+  context "when the current user is a data provider" do
+    let(:user) { build(:user) }
 
     it "is not shown" do
       expect(page.routed_to?(lettings_log, user)).to be false

spec/models/form/sales/questions/created_by_id_spec.rb

@@ -0,0 +1,88 @@
+require "rails_helper"
+
+RSpec.describe Form::Sales::Questions::CreatedById, 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("created_by_id")
+  end
+
+  it "has the correct header" do
+    expect(question.header).to eq("Which user are you creating this log for?")
+  end
+
+  it "has the correct check_answer_label" do
+    expect(question.check_answer_label).to eq("User")
+  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
+
+  it "is marked as derived" do
+    expect(question.derived?).to be true
+  end
+
+  context "when the current user is support" do
+    let(:support_user) { build(:user, :support) }
+
+    it "is shown in check answers" do
+      expect(question.hidden_in_check_answers?(nil, support_user)).to be false
+    end
+
+    describe "#displayed_answer_options" do
+      let(:owning_org_user) { create(:user) }
+      let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) }
+      let(:expected_answer_options) do
+        {
+          "" => "Select an option",
+          owning_org_user.id => "#{owning_org_user.name} (#{owning_org_user.email})",
+        }
+      end
+
+      it "only displays users that belong to the owning organisation" do
+        expect(question.displayed_answer_options(sales_log, support_user)).to eq(expected_answer_options)
+      end
+    end
+  end
+
+  context "when the current user is data_coordinator" do
+    let(:data_coordinator) { create(:user, :data_coordinator) }
+
+    it "is shown in check answers" do
+      expect(question.hidden_in_check_answers?(nil, data_coordinator)).to be false
+    end
+
+    describe "#displayed_answer_options" do
+      let(:owning_org_user) { create(:user) }
+      let(:sales_log) { create(:sales_log, owning_organisation: owning_org_user.organisation) }
+      let!(:user_in_same_org) { create(:user, organisation: data_coordinator.organisation) }
+
+      let(:expected_answer_options) do
+        {
+          "" => "Select an option",
+          user_in_same_org.id => "#{user_in_same_org.name} (#{user_in_same_org.email})",
+          data_coordinator.id => "#{data_coordinator.name} (#{data_coordinator.email})",
+        }
+      end
+
+      it "only displays users that belong user's org" do
+        expect(question.displayed_answer_options(sales_log, data_coordinator)).to eq(expected_answer_options)
+      end
+    end
+  end
+end