From 905ec1a4e116c5933d82f6700c59b483fffab105 Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Sun, 29 Dec 2024 14:26:58 -0500
Subject: [PATCH 1/7] Fix duplicate occurrence point forms bug

---
 .../hmis/app/models/hmis/form/definition.rb   |  6 ++
 .../hmis/app/models/hmis/hud/enrollment.rb    | 62 +++++++-------
 .../spec/factories/hmis/form/definitions.rb   |  2 +-
 .../spec/models/hmis/hud/enrollment_spec.rb   | 85 ++++++++++++-------
 4 files changed, 91 insertions(+), 64 deletions(-)

diff --git a/drivers/hmis/app/models/hmis/form/definition.rb b/drivers/hmis/app/models/hmis/form/definition.rb
index 224084418bb..fc058fea128 100644
--- a/drivers/hmis/app/models/hmis/form/definition.rb
+++ b/drivers/hmis/app/models/hmis/form/definition.rb
@@ -603,6 +603,12 @@ def updates_client_or_enrollment?
     link_id_item_hash.values.find { |item| ['ENROLLMENT', 'CLIENT'].include?(item.mapping&.record_type) }.present?
   end
 
+  def collects_enrollment_field?(field_name)
+    link_id_item_hash.values.find do |item|
+      item.mapping&.record_type == 'ENROLLMENT' && item.mapping&.field_name == field_name.to_s.camelize(:lower)
+    end.present?
+  end
+
   # Find and/or initialize CustomDataElementDefinitions that are collected by this form.
   # For application forms, we now rely on PublishFormDefinition to generate CDEDs, but
   # this is still used in test fixtures and in PublishExternalFormsJob.
diff --git a/drivers/hmis/app/models/hmis/hud/enrollment.rb b/drivers/hmis/app/models/hmis/hud/enrollment.rb
index 5be74b87114..57f6e5f4017 100644
--- a/drivers/hmis/app/models/hmis/hud/enrollment.rb
+++ b/drivers/hmis/app/models/hmis/hud/enrollment.rb
@@ -339,49 +339,51 @@ def data_collection_features
     end.compact
   end
 
+  # Occurrence Point Forms that are enabled for this Enrollment.
+  # Returns array of OpenStructs, which are resolved by the HmisSchema::OccurrencePointForm GQL type.
   def occurrence_point_forms
     # Get definitions for Occurrence Point forms, including inactive/retired (but excluding drafts)
     definitions = Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published_or_retired.latest_versions
-    # Get cdeds that this enrollment has CDE record(s) for. Do this in advance so we don't make extra trips to db
-    cdeds_this_enrollment_has = custom_data_element_definitions.pluck(:key).to_set
 
-    definitions.map do |definition|
-      # Choose the most specific instance for this enrollment
+    # Filter to only those Occurrence Point Forms that are enabled
+    structs = definitions.map do |definition|
+      # Choose the most specific Instance that enables this FormDefinition for this Enrollment
       best_instance = definition.instances.active.detect_best_instance_for_enrollment(enrollment: self)
-
-      # Check for legacy data. Skip the calculation if there is a current instance
-      has_legacy_data = best_instance ? false : legacy_occurrence_point_data?(definition, cdeds_this_enrollment_has)
-
-      next unless best_instance || has_legacy_data
+      next unless best_instance
 
       OpenStruct.new(
-        legacy: has_legacy_data && !best_instance,
+        legacy: false, # not legacy, because there is an active Form Instance enabling it
         definition: definition,
-        data_collected_about: best_instance&.data_collected_about || 'ALL_CLIENTS',
+        data_collected_about: best_instance.data_collected_about || 'ALL_CLIENTS',
       )
     end.compact
-  end
-
-  private def legacy_occurrence_point_data?(definition, cdeds_this_enrollment_has)
-    definition.walk_definition_nodes(as_open_struct: true) do |item|
-      next unless item.mapping.present?
 
-      record_type = item.mapping&.record_type
-      field_name = item.mapping&.field_name&.underscore
-      custom_field_key = item.mapping&.custom_field_key
-
-      next unless record_type == 'ENROLLMENT' || custom_field_key
-
-      if record_type && field_name
-        # Example: if this item collects `move_in_date` and the Enrollment has a Move-in Date value, then we want to show this form on the Enrollment Dashboard (even though it isn't "enabled" via an instance)
-        return true if respond_to?(field_name) && send(field_name).present?
-      elsif custom_field_key
-        # For simplicity, for now, just look for CDEDs where the owner is this Enrollment
-        return true if cdeds_this_enrollment_has.include?(custom_field_key)
-      end
+    # Add legacy forms to ensure that HUD Data Elements are not hidden.
+    # In the event that an Enrollment has a MoveInDate, for example, but there is no active form that collects it,
+    # we still need to show it so that user can see the data and perform data correction.
+    [
+      # [<Enrollment field>, <default form identifier that should be used if field has any data>]
+      [:move_in_date, 'move_in_date'],
+      [:date_of_engagement, 'date_of_engagement'],
+      [:date_of_path_status, 'path_status'],
+    ].each do |(field, identifier)|
+      # field has no data on this enrollment, skip
+      next unless send(field).present?
+      # field is already collected by an enabled form, skip
+      next if structs.find { |s| s.definition.collects_enrollment_field?(field) }
+
+      # find default form to use
+      form = definitions.find { |fd| fd.identifier == identifier && fd.managed_in_version_control? }
+      raise "Unexpected: #{field} present, but default #{identifier} form not found" unless form
+
+      structs << OpenStruct.new(
+        legacy: true, # legacy because there is no Form Instance enabling this data collection
+        definition: form,
+        data_collected_about: 'ALL_CLIENTS',
+      )
     end
 
-    false
+    structs
   end
 
   def save_new_enrollment!
diff --git a/drivers/hmis/spec/factories/hmis/form/definitions.rb b/drivers/hmis/spec/factories/hmis/form/definitions.rb
index 0284e97417e..fbb1fa250b1 100644
--- a/drivers/hmis/spec/factories/hmis/form/definitions.rb
+++ b/drivers/hmis/spec/factories/hmis/form/definitions.rb
@@ -852,7 +852,7 @@
   end
 
   factory :occurrence_point_form, parent: :hmis_form_definition do
-    identifier { 'move_in_date' }
+    identifier { 'move_in_date_form' }
     role { :OCCURRENCE_POINT }
     definition do
       JSON.parse(<<~JSON)
diff --git a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
index f2ed08163e0..9b9e7f9ef17 100644
--- a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
+++ b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
@@ -273,14 +273,14 @@
     let(:legacy_expected_struct) do
       have_attributes(
         legacy: true,
-        definition: definition,
+        definition: have_attributes(identifier: 'move_in_date'), # default form seeded by JsonForms.seed_all
         data_collected_about: 'ALL_CLIENTS',
       )
     end
 
     before(:all) do
-      Hmis::Form::Definition.delete_all
-      Hmis::Form::Instance.delete_all
+      # seed default FormDefinitions so that the default move_in_date form is present
+      ::HmisUtil::JsonForms.seed_all
     end
 
     it 'does not return the form when no instance exists' do
@@ -297,7 +297,7 @@
       expect(hoh_enrollment.occurrence_point_forms).to be_empty
     end
 
-    context 'when there is no instance, but there is legacy data' do
+    context 'when there is no instance, but Enrollment has a MoveInDate value' do
       let!(:spouse_enrollment) do
         create(
           :hmis_hud_enrollment,
@@ -309,7 +309,7 @@
         )
       end
 
-      it 'does return the form' do
+      it 'does return the default move_in_date form' do
         expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
       end
 
@@ -319,7 +319,7 @@
         let!(:instance3) { create(:hmis_form_instance, role: role, project_type: 4, active: true, definition: definition) }
         let!(:inactive_instance) { create(:hmis_form_instance, role: role, project_type: 6, active: false, definition: definition) }
 
-        it 'returns the form, with no duplicates' do
+        it 'returns the default move_in_date form, with no duplicates' do
           expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
         end
       end
@@ -327,7 +327,7 @@
       context 'when a draft version of the form does not collect the same data' do
         let!(:draft_definition) { create(:occurrence_point_form, version: 2, status: :draft) }
 
-        it 'returns the form' do
+        it 'returns the default move_in_date form' do
           expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
         end
       end
@@ -382,32 +382,7 @@
           )
         end
 
-        it 'returns the form for non-HoH' do
-          expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
-        end
-      end
-
-      context 'when legacy data exists on a CDED' do
-        let!(:definition_json) do
-          {
-            'item': [
-              {
-                'text': 'Foo data element',
-                'type': 'STRING',
-                'link_id': 'foo',
-                'mapping': {
-                  'custom_field_key': 'foo',
-                  'record_type': 'ENROLLMENT',
-                },
-              },
-            ],
-          }
-        end
-        let!(:definition) { create :occurrence_point_form, definition: definition_json }
-        let!(:cded) { create :hmis_custom_data_element_definition, key: 'foo', data_source: ds1, owner_type: 'Hmis::Hud::Enrollment', repeats: false }
-        let!(:cde) { create :hmis_custom_data_element, data_element_definition: cded, owner: spouse_enrollment, data_source: ds1, value_string: 'bar' }
-
-        it 'returns the form for non-HoH' do
+        it 'returns the default form for non-HoH' do
           expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
         end
       end
@@ -428,5 +403,49 @@
         end
       end
     end
+
+    context 'PATH Status form' do
+      let(:legacy_expected_struct) do
+        have_attributes(
+          legacy: true,
+          definition: have_attributes(identifier: 'path_status'), # default form seeded by JsonForms.seed_all
+          data_collected_about: 'ALL_CLIENTS',
+        )
+      end
+
+      context 'when DateOfPATHStatus does not exist' do
+        it 'does not return PATH status form' do
+          expect(hoh_enrollment.occurrence_point_forms).to be_empty
+        end
+      end
+      context 'when DateOfPATHStatus exists' do
+        before(:each) { hoh_enrollment.update!(date_of_path_status: 3.weeks.ago) }
+        it 'returns the default PATH status form' do
+          expect(hoh_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
+        end
+      end
+    end
+
+    context 'Date of Engagement form' do
+      let(:legacy_expected_struct) do
+        have_attributes(
+          legacy: true,
+          definition: have_attributes(identifier: 'date_of_engagement'), # default form seeded by JsonForms.seed_all
+          data_collected_about: 'ALL_CLIENTS',
+        )
+      end
+
+      context 'when DateOfEngagement does not exist' do
+        it 'does not return PATH status form' do
+          expect(hoh_enrollment.occurrence_point_forms).to be_empty
+        end
+      end
+      context 'when DateOfEngagement exists' do
+        before(:each) { hoh_enrollment.update!(date_of_engagement: 3.weeks.ago) }
+        it 'returns the default PATH status form' do
+          expect(hoh_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
+        end
+      end
+    end
   end
 end

From c829c14ec7e4e19f2cd42028313435f6098f2f56 Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Mon, 3 Feb 2025 17:12:25 -0500
Subject: [PATCH 2/7] comments

---
 drivers/hmis/app/models/hmis/hud/enrollment.rb       | 2 +-
 drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hmis/app/models/hmis/hud/enrollment.rb b/drivers/hmis/app/models/hmis/hud/enrollment.rb
index 20c05582793..f234e1ebd95 100644
--- a/drivers/hmis/app/models/hmis/hud/enrollment.rb
+++ b/drivers/hmis/app/models/hmis/hud/enrollment.rb
@@ -363,7 +363,7 @@ def occurrence_point_forms
     # we still need to show it so that user can see the data and perform data correction.
     [
       # [<Enrollment field>, <default form identifier that should be used if field has any data>]
-      [:move_in_date, 'move_in_date'],
+      [:move_in_date, 'move_in_date'], # identifier matches file name of the default form, e.g. drivers/hmis/lib/form_data/default/occurrence_point_forms/move_in_date.json
       [:date_of_engagement, 'date_of_engagement'],
       [:date_of_path_status, 'path_status'],
     ].each do |(field, identifier)|
diff --git a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
index 1ab55d5386a..38399905419 100644
--- a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
+++ b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
@@ -436,13 +436,13 @@
       end
 
       context 'when DateOfEngagement does not exist' do
-        it 'does not return PATH status form' do
+        it 'does not return Date of engagement form' do
           expect(hoh_enrollment.occurrence_point_forms).to be_empty
         end
       end
       context 'when DateOfEngagement exists' do
         before(:each) { hoh_enrollment.update!(date_of_engagement: 3.weeks.ago) }
-        it 'returns the default PATH status form' do
+        it 'returns the default Date of engagement form' do
           expect(hoh_enrollment.occurrence_point_forms).to contain_exactly(legacy_expected_struct)
         end
       end

From 7f08f90beacee5f7c0ef2d8fee6e295f0bf60435 Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Tue, 4 Feb 2025 12:51:05 -0500
Subject: [PATCH 3/7] Move to class

---
 .../hmis/app/models/hmis/form/definition.rb   |   6 -
 .../form/occurrence_point_form_collection.rb  | 103 ++++++++++++++++++
 .../hmis/app/models/hmis/hud/enrollment.rb    |  43 +-------
 .../spec/models/hmis/hud/enrollment_spec.rb   |   1 +
 4 files changed, 105 insertions(+), 48 deletions(-)
 create mode 100644 drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb

diff --git a/drivers/hmis/app/models/hmis/form/definition.rb b/drivers/hmis/app/models/hmis/form/definition.rb
index 9cd0aa3d267..f94ae73468c 100644
--- a/drivers/hmis/app/models/hmis/form/definition.rb
+++ b/drivers/hmis/app/models/hmis/form/definition.rb
@@ -608,12 +608,6 @@ def updates_client_or_enrollment?
     link_id_item_hash.values.find { |item| ['ENROLLMENT', 'CLIENT'].include?(item.mapping&.record_type) }.present?
   end
 
-  def collects_enrollment_field?(field_name)
-    link_id_item_hash.values.find do |item|
-      item.mapping&.record_type == 'ENROLLMENT' && item.mapping&.field_name == field_name.to_s.camelize(:lower)
-    end.present?
-  end
-
   # Find and/or initialize CustomDataElementDefinitions that are collected by this form.
   # For application forms, we now rely on PublishFormDefinition to generate CDEDs, but
   # this is still used in test fixtures and in PublishExternalFormsJob.
diff --git a/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
new file mode 100644
index 00000000000..f20390f6377
--- /dev/null
+++ b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
@@ -0,0 +1,103 @@
+###
+# Copyright 2016 - 2025 Green River Data Analysis, LLC
+#
+# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
+###
+
+###
+# Hmis::Form::OccurrencePointFormCollection
+#
+# This class is responsible for determining which Occurrence Point forms to display on a given Enrollment in HMIS.
+# The Occurrence Point forms appear in the "Enrollment Details" card on the HMIS Enrollment dashboard.
+#
+# These forms collect data elements onto an Enrollment "at occurrence" (a.k.a. when they occur),
+# as opposed to data elements that are collected at a specific point in time (e.g. at intake, exit).
+###
+class Hmis::Form::OccurrencePointFormCollection
+  # Struct that backs Types::HmisSchema::OccurrencePointForm
+  OccurrencePointForm = Struct.new(:definition, :legacy, :data_collected_about, keyword_init: true)
+
+  def initialize(enrollment)
+    @enrollment = enrollment
+  end
+
+  def all
+    structs = active_forms
+    structs += legacy_forms(structs)
+    structs
+  end
+
+  def active_forms
+    # Get definitions for Occurrence Point forms, including inactive/retired (but excluding drafts)
+    # definitions = Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published_or_retired.latest_versions
+
+    # Filter to only those Occurrence Point Forms that are enabled for this
+    occurrence_point_definition_scope.map do |definition|
+      # Choose the most specific Instance that enables this FormDefinition for this Enrollment
+      best_instance = definition.instances.active.detect_best_instance_for_enrollment(enrollment: @enrollment)
+      # If there was no active instance, that means this Occurrence Point form is not enabled. Skip it.
+      next unless best_instance
+
+      create_form_struct(
+        definition: definition,
+        legacy: false, # not legacy, because there is an active Form Instance enabling it
+        data_collected_about: best_instance.data_collected_about,
+      )
+    end.compact
+  end
+
+  # Default Occurrence Point forms that collect HUD fields. The system should already enforce that
+  # these forms are enabled for the appropriate projects (e.g. Move-in Date collected on HoH in PH).
+  # This code ensures that for contexts when the form ISN'T enabled (e.g. Move-in Date on a Child),
+  # AND the Enrollment has a value for the primary field it collects (e.g. 'MoveInDate'), we still show the value and the form.
+  # This allows users to see the full set of HUD occurrence point data elements, and do data correction.
+  HUD_DEFAULT_FORMS = [
+    # Note: form_identifier matches the filename of the form, e.g. ../default/occurrence_point_forms/move_in_date.json
+    { form_identifier: :move_in_date, field_name: :move_in_date },
+    { form_identifier: :date_of_engagement, field_name: :date_of_engagement },
+    { form_identifier: :path_status, field_name: :date_of_path_status },
+  ].freeze
+
+  def legacy_forms(active_forms)
+    # Add legacy forms to ensure that HUD Data Elements are not hidden.
+    # In the event that an Enrollment has a MoveInDate, for example, but there is no active form that collects it,
+    # we still need to show it so that user can see the data and perform data correction.
+    HUD_DEFAULT_FORMS.map do |config|
+      form_identifier, field_name = config.values_at(:form_identifier, :field_name)
+      # this enrollment does not have this field (e.g. MoveInDate), skip
+      next unless @enrollment.send(field_name).present?
+      # this field is already collected by an active enable form, skip
+      next if active_forms.find { |s| collects_enrollment_field?(s.definition, field_name) }
+
+      definition = occurrence_point_definition_scope.find { |fd| fd.identifier == form_identifier.to_s && fd.managed_in_version_control? }
+      raise "Unexpected: #{field_name} present, but default form '#{form_identifier}' not found" unless definition
+
+      create_form_struct(definition: definition, legacy: true)
+    end.compact
+  end
+
+  private
+
+  def occurrence_point_definition_scope
+    @occurrence_point_definition_scope ||= Hmis::Form::Definition.
+      with_role(:OCCURRENCE_POINT).
+      published.
+      latest_versions
+  end
+
+  def create_form_struct(definition:, legacy:, data_collected_about: nil)
+    OccurrencePointForm.new(
+      definition: definition,
+      legacy: legacy,
+      data_collected_about: data_collected_about || 'ALL_CLIENTS',
+    )
+  end
+
+  # Check if the given FormDefinition collects the given field from the Enrollment.
+  # This is a bit hacky (transforming fieldname to graphql casing) but it works for the known fields (Move-in date, DOE, PATH).
+  def collects_enrollment_field?(definition, field_name)
+    definition.link_id_item_hash.values.find do |item|
+      item.mapping&.record_type == 'ENROLLMENT' && item.mapping&.field_name == field_name.to_s.camelize(:lower)
+    end.present?
+  end
+end
diff --git a/drivers/hmis/app/models/hmis/hud/enrollment.rb b/drivers/hmis/app/models/hmis/hud/enrollment.rb
index f234e1ebd95..c967a592888 100644
--- a/drivers/hmis/app/models/hmis/hud/enrollment.rb
+++ b/drivers/hmis/app/models/hmis/hud/enrollment.rb
@@ -342,48 +342,7 @@ def data_collection_features
   # Occurrence Point Forms that are enabled for this Enrollment.
   # Returns array of OpenStructs, which are resolved by the HmisSchema::OccurrencePointForm GQL type.
   def occurrence_point_forms
-    # Get definitions for Occurrence Point forms, including inactive/retired (but excluding drafts)
-    definitions = Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published_or_retired.latest_versions
-
-    # Filter to only those Occurrence Point Forms that are enabled
-    structs = definitions.map do |definition|
-      # Choose the most specific Instance that enables this FormDefinition for this Enrollment
-      best_instance = definition.instances.active.detect_best_instance_for_enrollment(enrollment: self)
-      next unless best_instance
-
-      OpenStruct.new(
-        legacy: false, # not legacy, because there is an active Form Instance enabling it
-        definition: definition,
-        data_collected_about: best_instance.data_collected_about || 'ALL_CLIENTS',
-      )
-    end.compact
-
-    # Add legacy forms to ensure that HUD Data Elements are not hidden.
-    # In the event that an Enrollment has a MoveInDate, for example, but there is no active form that collects it,
-    # we still need to show it so that user can see the data and perform data correction.
-    [
-      # [<Enrollment field>, <default form identifier that should be used if field has any data>]
-      [:move_in_date, 'move_in_date'], # identifier matches file name of the default form, e.g. drivers/hmis/lib/form_data/default/occurrence_point_forms/move_in_date.json
-      [:date_of_engagement, 'date_of_engagement'],
-      [:date_of_path_status, 'path_status'],
-    ].each do |(field, identifier)|
-      # field has no data on this enrollment, skip
-      next unless send(field).present?
-      # field is already collected by an enabled form, skip
-      next if structs.find { |s| s.definition.collects_enrollment_field?(field) }
-
-      # find default form to use
-      form = definitions.find { |fd| fd.identifier == identifier && fd.managed_in_version_control? }
-      raise "Unexpected: #{field} present, but default #{identifier} form not found" unless form
-
-      structs << OpenStruct.new(
-        legacy: true, # legacy because there is no Form Instance enabling this data collection
-        definition: form,
-        data_collected_about: 'ALL_CLIENTS',
-      )
-    end
-
-    structs
+    Hmis::Form::OccurrencePointFormCollection.new(self).all
   end
 
   def save_new_enrollment!
diff --git a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
index 38399905419..43eb7a92d0a 100644
--- a/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
+++ b/drivers/hmis/spec/models/hmis/hud/enrollment_spec.rb
@@ -351,6 +351,7 @@
             definition: definition,
             data_collected_about: 'ALL_CLIENTS',
           )
+          # binding.pry
           expect(hoh_enrollment.occurrence_point_forms).to contain_exactly(expected)
           expect(spouse_enrollment.occurrence_point_forms).to contain_exactly(expected)
         end

From 51ef313d46aab215d8cbd41ec42e059fed822602 Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Tue, 4 Feb 2025 13:19:21 -0500
Subject: [PATCH 4/7] Use for project too

---
 .../hmis_schema/occurrence_point_form.rb      |  2 +-
 .../form/occurrence_point_form_collection.rb  | 41 +++++++++++--------
 .../hmis/app/models/hmis/hud/enrollment.rb    |  2 +-
 drivers/hmis/app/models/hmis/hud/project.rb   | 12 +-----
 .../hmis/spec/models/hmis/hud/project_spec.rb | 13 +++---
 5 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/drivers/hmis/app/graphql/types/hmis_schema/occurrence_point_form.rb b/drivers/hmis/app/graphql/types/hmis_schema/occurrence_point_form.rb
index 6adad4a84a5..a937ff9ece3 100644
--- a/drivers/hmis/app/graphql/types/hmis_schema/occurrence_point_form.rb
+++ b/drivers/hmis/app/graphql/types/hmis_schema/occurrence_point_form.rb
@@ -14,7 +14,7 @@ class HmisSchema::OccurrencePointForm < Types::BaseObject
     # Form used for Viewing/Creating/Editing records
     field :definition, Types::Forms::FormDefinition, null: false, extras: [:parent]
 
-    # object is an OpenStruct, see Hmis::Hud::Enrollment occurrence_point_forms
+    # object is an OpenStruct, see Hmis::Form::OccurrencePointFormCollection
 
     def id(parent:)
       # Include project id (if present) so that instance is not cached for use across projects.
diff --git a/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
index f20390f6377..ff53ec0ae0f 100644
--- a/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
+++ b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
@@ -17,24 +17,38 @@ class Hmis::Form::OccurrencePointFormCollection
   # Struct that backs Types::HmisSchema::OccurrencePointForm
   OccurrencePointForm = Struct.new(:definition, :legacy, :data_collected_about, keyword_init: true)
 
-  def initialize(enrollment)
-    @enrollment = enrollment
+  # Occurrence Point forms to display on the Enrollment, including legacy forms to show existing data
+  def for_enrollment(enrollment)
+    structs = active_for_enrollment(enrollment)
+    structs += legacy_for_enrollment(enrollment, active_forms: structs)
+    structs
   end
 
-  def all
-    structs = active_forms
-    structs += legacy_forms(structs)
-    structs
+  # Occurrence Point forms that are enabled in the Project. This is only used for purposes of displaying Project configuration.
+  def for_project(project)
+    occurrence_point_definition_scope.map do |definition|
+      # Choose the most specific Instance that enables this FormDefinition for this Project
+      best_instance = definition.instances.active.order(updated_at: :desc).detect_best_instance_for_project(project: project)
+      next unless best_instance
+
+      create_form_struct(
+        definition: definition,
+        legacy: false, # not legacy, because there is an active Form Instance enabling it
+        data_collected_about: best_instance.data_collected_about,
+      )
+    end.compact
   end
 
-  def active_forms
+  private
+
+  def active_for_enrollment(enrollment)
     # Get definitions for Occurrence Point forms, including inactive/retired (but excluding drafts)
     # definitions = Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published_or_retired.latest_versions
 
     # Filter to only those Occurrence Point Forms that are enabled for this
     occurrence_point_definition_scope.map do |definition|
       # Choose the most specific Instance that enables this FormDefinition for this Enrollment
-      best_instance = definition.instances.active.detect_best_instance_for_enrollment(enrollment: @enrollment)
+      best_instance = definition.instances.active.order(updated_at: :desc).detect_best_instance_for_enrollment(enrollment: enrollment)
       # If there was no active instance, that means this Occurrence Point form is not enabled. Skip it.
       next unless best_instance
 
@@ -58,14 +72,14 @@ def active_forms
     { form_identifier: :path_status, field_name: :date_of_path_status },
   ].freeze
 
-  def legacy_forms(active_forms)
+  def legacy_for_enrollment(enrollment, active_forms:)
     # Add legacy forms to ensure that HUD Data Elements are not hidden.
     # In the event that an Enrollment has a MoveInDate, for example, but there is no active form that collects it,
     # we still need to show it so that user can see the data and perform data correction.
     HUD_DEFAULT_FORMS.map do |config|
       form_identifier, field_name = config.values_at(:form_identifier, :field_name)
       # this enrollment does not have this field (e.g. MoveInDate), skip
-      next unless @enrollment.send(field_name).present?
+      next unless enrollment.send(field_name).present?
       # this field is already collected by an active enable form, skip
       next if active_forms.find { |s| collects_enrollment_field?(s.definition, field_name) }
 
@@ -76,13 +90,8 @@ def legacy_forms(active_forms)
     end.compact
   end
 
-  private
-
   def occurrence_point_definition_scope
-    @occurrence_point_definition_scope ||= Hmis::Form::Definition.
-      with_role(:OCCURRENCE_POINT).
-      published.
-      latest_versions
+    @occurrence_point_definition_scope ||= Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published
   end
 
   def create_form_struct(definition:, legacy:, data_collected_about: nil)
diff --git a/drivers/hmis/app/models/hmis/hud/enrollment.rb b/drivers/hmis/app/models/hmis/hud/enrollment.rb
index c967a592888..6083bd3f4f7 100644
--- a/drivers/hmis/app/models/hmis/hud/enrollment.rb
+++ b/drivers/hmis/app/models/hmis/hud/enrollment.rb
@@ -342,7 +342,7 @@ def data_collection_features
   # Occurrence Point Forms that are enabled for this Enrollment.
   # Returns array of OpenStructs, which are resolved by the HmisSchema::OccurrencePointForm GQL type.
   def occurrence_point_forms
-    Hmis::Form::OccurrencePointFormCollection.new(self).all
+    Hmis::Form::OccurrencePointFormCollection.new.for_enrollment(self)
   end
 
   def save_new_enrollment!
diff --git a/drivers/hmis/app/models/hmis/hud/project.rb b/drivers/hmis/app/models/hmis/hud/project.rb
index 8b9f730e305..8427cd22944 100644
--- a/drivers/hmis/app/models/hmis/hud/project.rb
+++ b/drivers/hmis/app/models/hmis/hud/project.rb
@@ -295,17 +295,7 @@ def available_service_types
 
   # Occurrence Point Form Instances that are enabled for this project (e.g. Move In Date form)
   def occurrence_point_form_instances
-    # All instances for Occurrence Point forms
-    base_scope = Hmis::Form::Instance.with_role(:OCCURRENCE_POINT).active.published
-
-    # All possible form identifiers used for Occurrence Point collection
-    occurrence_point_identifiers = base_scope.pluck(:definition_identifier).uniq
-
-    # Choose the most specific instance for each definition identifier
-    occurrence_point_identifiers.map do |identifier|
-      scope = base_scope.where(definition_identifier: identifier).order(updated_at: :desc)
-      scope.detect_best_instance_for_project(project: self)
-    end.compact
+    Hmis::Form::OccurrencePointFormCollection.new.for_project(self)
   end
 
   def uniq_coc_codes
diff --git a/drivers/hmis/spec/models/hmis/hud/project_spec.rb b/drivers/hmis/spec/models/hmis/hud/project_spec.rb
index 0dbfe7b02cd..80138635e75 100644
--- a/drivers/hmis/spec/models/hmis/hud/project_spec.rb
+++ b/drivers/hmis/spec/models/hmis/hud/project_spec.rb
@@ -209,13 +209,16 @@ def selected_instances
     end
 
     it 'returns most specific instance per definition identifier' do
-      mid_ptype = create(:hmis_form_instance, role: role, entity: nil, project_type: 13, definition_identifier: 'move_in_date')
-      mid_project = create(:hmis_form_instance, role: role, entity: project, definition_identifier: mid_ptype.definition_identifier)
+      mid_ptype = create(:hmis_form_instance, role: role, entity: nil, project_type: 13, definition_identifier: 'move_in_date', data_collected_about: 'HOH')
+      mid_project = create(:hmis_form_instance, role: role, entity: project, definition_identifier: mid_ptype.definition_identifier, data_collected_about: 'HOH_AND_ADULTS')
 
-      doe_default = create(:hmis_form_instance, role: role, entity: nil, definition_identifier: 'date_of_engagement')
-      doe_org = create(:hmis_form_instance, role: role, entity: project.organization, definition_identifier: doe_default.definition_identifier)
+      doe_default = create(:hmis_form_instance, role: role, entity: nil, definition_identifier: 'date_of_engagement', data_collected_about: 'ALL_CLIENTS')
+      doe_org = create(:hmis_form_instance, role: role, entity: project.organization, definition_identifier: doe_default.definition_identifier, data_collected_about: 'HOH')
 
-      expect(selected_instances).to contain_exactly(mid_project, doe_org)
+      expect(selected_instances).to contain_exactly(
+        have_attributes(definition: mid_project.definition, data_collected_about: mid_project.data_collected_about),
+        have_attributes(definition: doe_default.definition, data_collected_about: doe_org.data_collected_about),
+      )
     end
 
     it 'does not return draft forms, even for active instances' do

From c53fceaf7249e30d356c456b7acc6790c22a0fce Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Tue, 4 Feb 2025 15:49:48 -0500
Subject: [PATCH 5/7] Add admin unit picklist for reassigning type

---
 drivers/hmis/app/graphql/schema.graphql       |  9 +++-
 .../types/forms/enums/pick_list_type.rb       |  3 +-
 .../graphql/types/forms/pick_list_option.rb   | 26 ++++++++--
 .../hmis/app/models/hmis/hud/enrollment.rb    |  1 +
 .../hmis/spec/requests/hmis/pick_list_spec.rb | 50 +++++++++++++------
 5 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/drivers/hmis/app/graphql/schema.graphql b/drivers/hmis/app/graphql/schema.graphql
index 74cd01eb2b8..2e367a03449 100644
--- a/drivers/hmis/app/graphql/schema.graphql
+++ b/drivers/hmis/app/graphql/schema.graphql
@@ -6656,6 +6656,12 @@ type PickListOption {
 }
 
 enum PickListType {
+  """
+  Units available for the given Enrollment at the given project. Includes all
+  available units at project even if they have a different type from what the
+  household is currently occupying.
+  """
+  ADMIN_AVAILABLE_UNITS_FOR_ENROLLMENT
   ALL_SERVICE_CATEGORIES
   ALL_SERVICE_TYPES
 
@@ -6680,7 +6686,8 @@ enum PickListType {
   AVAILABLE_SERVICE_TYPES
 
   """
-  Units available for the given household at the given project
+  Units available for the given Enrollment at the given project. List is limited
+  to units with the same unit type currently occupied by the household, if any.
   """
   AVAILABLE_UNITS_FOR_ENROLLMENT
 
diff --git a/drivers/hmis/app/graphql/types/forms/enums/pick_list_type.rb b/drivers/hmis/app/graphql/types/forms/enums/pick_list_type.rb
index bc3dda05be0..88c4d699a5b 100644
--- a/drivers/hmis/app/graphql/types/forms/enums/pick_list_type.rb
+++ b/drivers/hmis/app/graphql/types/forms/enums/pick_list_type.rb
@@ -27,7 +27,8 @@ class Forms::Enums::PickListType < Types::BaseEnum
     value 'ALL_UNIT_TYPES', 'All unit types.'
     value 'POSSIBLE_UNIT_TYPES_FOR_PROJECT', 'Unit types that are eligible to be added to project'
     value 'AVAILABLE_UNIT_TYPES', 'Unit types that have unoccupied units in the specified project'
-    value 'AVAILABLE_UNITS_FOR_ENROLLMENT', 'Units available for the given household at the given project'
+    value 'AVAILABLE_UNITS_FOR_ENROLLMENT', 'Units available for the given Enrollment at the given project. List is limited to units with the same unit type currently occupied by the household, if any.'
+    value 'ADMIN_AVAILABLE_UNITS_FOR_ENROLLMENT', 'Units available for the given Enrollment at the given project. Includes all available units at project even if they have a different type from what the household is currently occupying.'
     value 'ALL_SERVICE_TYPES'
     value 'ALL_SERVICE_CATEGORIES'
     value 'CUSTOM_SERVICE_CATEGORIES'
diff --git a/drivers/hmis/app/graphql/types/forms/pick_list_option.rb b/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
index 44fbd670c2a..3a6c41a842f 100644
--- a/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
+++ b/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
@@ -70,6 +70,8 @@ def self.options_for_type(pick_list_type, user:, project_id: nil, client_id: nil
         available_unit_types_for_project(project)
       when 'AVAILABLE_UNITS_FOR_ENROLLMENT'
         available_units_for_enrollment(project, household_id: household_id)
+      when 'ADMIN_AVAILABLE_UNITS_FOR_ENROLLMENT'
+        admin_available_units_for_enrollment(project, household_id: household_id)
       when 'OPEN_HOH_ENROLLMENTS_FOR_PROJECT'
         open_hoh_enrollments_for_project(project, user: user)
       when 'ENROLLMENTS_FOR_CLIENT'
@@ -474,7 +476,7 @@ def self.enrollments_for_client(client, user:)
       end
     end
 
-    def self.available_units_for_enrollment(project, household_id: nil)
+    def self.admin_available_units_for_enrollment(project, household_id: nil)
       return [] unless project
 
       # Eligible units are unoccupied units, PLUS units occupied by household members
@@ -487,10 +489,8 @@ def self.available_units_for_enrollment(project, household_id: nil)
         []
       end
 
-      unit_types_assigned_to_household = Hmis::Unit.where(id: hh_units).pluck(:unit_type_id).compact.uniq
       eligible_units = Hmis::Unit.where(id: unoccupied_units + hh_units)
-      # If some household members are assigned to units with unit types, then list should be limited to units of the same type.
-      eligible_units = eligible_units.where(unit_type_id: unit_types_assigned_to_household) if unit_types_assigned_to_household.any?
+
       eligible_units.preload(:unit_type).
         order(:unit_type_id, :id).
         map do |unit|
@@ -504,6 +504,24 @@ def self.available_units_for_enrollment(project, household_id: nil)
         end
     end
 
+    def self.available_units_for_enrollment(project, household_id: nil)
+      return [] unless project
+
+      # use picklist that includes all available units including units of other types
+      picklist = admin_available_units_for_enrollment(project, household_id: household_id)
+      return picklist unless household_id # no household, so no need to filter unit types
+
+      # drop units that have different types
+      hh_unit_type_ids = project.enrollments.where(household_id: household_id).map(&:current_unit_type).compact.map(&:id).uniq
+      return picklist unless hh_unit_type_ids.any? # household doesn't have a unit type, so no need for further filtering
+
+      # if the household has a unit type, exclude units that don't match
+      allowed_unit_type_unit_ids = project.units.where(unit_type_id: hh_unit_type_ids).pluck(:id)
+      picklist.filter do |option|
+        option[:code].in?(allowed_unit_type_unit_ids)
+      end
+    end
+
     def self.assessment_names_for_project(project)
       # It's a little odd to combine the "roles" (eg INTAKE) with the identifiers (eg housing_needs_assessment), but
       # we need to do that in order to get the desired behavior. The "Intake" option should show all Intakes,
diff --git a/drivers/hmis/app/models/hmis/hud/enrollment.rb b/drivers/hmis/app/models/hmis/hud/enrollment.rb
index 6083bd3f4f7..211efd07a32 100644
--- a/drivers/hmis/app/models/hmis/hud/enrollment.rb
+++ b/drivers/hmis/app/models/hmis/hud/enrollment.rb
@@ -82,6 +82,7 @@ class Hmis::Hud::Enrollment < Hmis::Hud::Base
   has_many :unit_occupancies, class_name: 'Hmis::UnitOccupancy', inverse_of: :enrollment, dependent: :destroy
   has_one :active_unit_occupancy, -> { active }, class_name: 'Hmis::UnitOccupancy', inverse_of: :enrollment
   has_one :current_unit, through: :active_unit_occupancy, class_name: 'Hmis::Unit', source: :unit
+  has_one :current_unit_type, through: :current_unit, class_name: 'Hmis::UnitType', source: :unit_type
 
   # Cached chronically homeless at entry
   has_one :ch_enrollment, class_name: 'Hmis::ChEnrollment', dependent: :destroy
diff --git a/drivers/hmis/spec/requests/hmis/pick_list_spec.rb b/drivers/hmis/spec/requests/hmis/pick_list_spec.rb
index 3c327787d04..9e5a55ba2c6 100644
--- a/drivers/hmis/spec/requests/hmis/pick_list_spec.rb
+++ b/drivers/hmis/spec/requests/hmis/pick_list_spec.rb
@@ -308,36 +308,56 @@ def picklist_option_codes(project)
     end
   end
 
-  describe 'AVAILABLE_UNITS_FOR_ENROLLMENT' do
+  describe 'unit picklists' do
     let!(:e1) { create :hmis_hud_enrollment, data_source: ds1, project: p1, client: c1 }
-    let!(:un1) { create :hmis_unit, project: p1 }
-    let!(:un2) { create :hmis_unit, project: p1 }
-    let!(:un3) { create :hmis_unit } # in another project
+    let!(:br1) { create :hmis_unit_type, description: '1 BR' }
+    let!(:br2) { create :hmis_unit_type, description: '2 BR' }
+
+    let!(:un1) { create :hmis_unit, project: p1, unit_type: br1 }
+    let!(:un2) { create :hmis_unit, project: p1, unit_type: br1 }
+    let!(:un3) { create :hmis_unit, project: p1, unit_type: br2 }
+
+    # cruft: units in other projects
+    let!(:un4) { create :hmis_unit, unit_type: br1 }
+    let!(:un5) { create :hmis_unit, unit_type: br2 }
+    let!(:un6) { create :hmis_unit }
 
     # assign e1 to un1
     let!(:uo1) { create :hmis_unit_occupancy, unit: un1, enrollment: e1, start_date: 1.week.ago }
 
-    def picklist_option_codes(project, household_id = nil)
+    def picklist_option_codes(project, picklist: 'AVAILABLE_UNITS_FOR_ENROLLMENT', household_id: nil)
       Types::Forms::PickListOption.options_for_type(
-        'AVAILABLE_UNITS_FOR_ENROLLMENT',
+        picklist,
         user: hmis_user,
         project_id: project.id,
         household_id: household_id,
       ).map { |opt| opt[:code] }
     end
 
-    it 'resolves available units for project' do
-      expect(picklist_option_codes(p1)).to contain_exactly(un2.id)
-    end
+    context 'AVAILABLE_UNITS_FOR_ENROLLMENT' do
+      it 'resolves available units for project' do
+        expect(picklist_option_codes(p1)).to contain_exactly(un2.id, un3.id)
+      end
+
+      it 'includes units that are currently occupied by the household, plus other units of the same type' do
+        result = picklist_option_codes(p1, household_id: e1.household_id)
+        expect(result).to contain_exactly(un1.id, un2.id)
+      end
 
-    it 'includes units that are currently occupied by the household' do
-      expect(picklist_option_codes(p1, e1.household_id)).to contain_exactly(un1.id, un2.id)
+      it 'if household unit doesn\'t have a type, includes all available units' do
+        un1.update!(unit_type: nil)
+        expect(picklist_option_codes(p1, household_id: e1.household_id)).to contain_exactly(un1.id, un2.id, un3.id)
+      end
     end
 
-    it 'if household is occupied by a unit that has a type, excludes other unit typoes from list' do
-      un1.unit_type = create(:hmis_unit_type)
-      un1.save!
-      expect(picklist_option_codes(p1, e1.household_id)).to contain_exactly(un1.id)
+    context 'ADMIN_AVAILABLE_UNITS_FOR_ENROLLMENT' do
+      it 'resolves available units for project' do
+        expect(picklist_option_codes(p1)).to contain_exactly(un2.id, un3.id)
+      end
+
+      it 'includes units with differing unit types' do
+        expect(picklist_option_codes(p1, picklist: 'ADMIN_AVAILABLE_UNITS_FOR_ENROLLMENT', household_id: e1.household_id)).to contain_exactly(un1.id, un2.id, un3.id)
+      end
     end
   end
 

From fe8f83060ecca2476852ff4d6270f979e41c50e6 Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Tue, 4 Feb 2025 17:12:53 -0500
Subject: [PATCH 6/7] Update comments

---
 .../models/hmis/form/occurrence_point_form_collection.rb | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
index ff53ec0ae0f..c8effed6b03 100644
--- a/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
+++ b/drivers/hmis/app/models/hmis/form/occurrence_point_form_collection.rb
@@ -33,19 +33,16 @@ def for_project(project)
 
       create_form_struct(
         definition: definition,
-        legacy: false, # not legacy, because there is an active Form Instance enabling it
         data_collected_about: best_instance.data_collected_about,
+        legacy: false, # not legacy, because there is an active Form Instance enabling it
       )
     end.compact
   end
 
   private
 
+  # Occurrence Point forms that are enabled for this Enrollment via an active form instance
   def active_for_enrollment(enrollment)
-    # Get definitions for Occurrence Point forms, including inactive/retired (but excluding drafts)
-    # definitions = Hmis::Form::Definition.with_role(:OCCURRENCE_POINT).published_or_retired.latest_versions
-
-    # Filter to only those Occurrence Point Forms that are enabled for this
     occurrence_point_definition_scope.map do |definition|
       # Choose the most specific Instance that enables this FormDefinition for this Enrollment
       best_instance = definition.instances.active.order(updated_at: :desc).detect_best_instance_for_enrollment(enrollment: enrollment)
@@ -54,8 +51,8 @@ def active_for_enrollment(enrollment)
 
       create_form_struct(
         definition: definition,
-        legacy: false, # not legacy, because there is an active Form Instance enabling it
         data_collected_about: best_instance.data_collected_about,
+        legacy: false, # not legacy, because there is an active Form Instance enabling it
       )
     end.compact
   end

From b38900352399bcf9c661b569a4f255af13f0990c Mon Sep 17 00:00:00 2001
From: Gig Ashton <gig@greenriver.org>
Date: Wed, 5 Feb 2025 07:56:35 -0500
Subject: [PATCH 7/7] Address feedback

---
 drivers/hmis/app/graphql/types/forms/pick_list_option.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hmis/app/graphql/types/forms/pick_list_option.rb b/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
index 3a6c41a842f..9cff15d2bd3 100644
--- a/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
+++ b/drivers/hmis/app/graphql/types/forms/pick_list_option.rb
@@ -513,10 +513,10 @@ def self.available_units_for_enrollment(project, household_id: nil)
 
       # drop units that have different types
       hh_unit_type_ids = project.enrollments.where(household_id: household_id).map(&:current_unit_type).compact.map(&:id).uniq
-      return picklist unless hh_unit_type_ids.any? # household doesn't have a unit type, so no need for further filtering
+      return picklist if hh_unit_type_ids.empty? # household doesn't have a unit type, so no need for further filtering
 
       # if the household has a unit type, exclude units that don't match
-      allowed_unit_type_unit_ids = project.units.where(unit_type_id: hh_unit_type_ids).pluck(:id)
+      allowed_unit_type_unit_ids = project.units.where(unit_type_id: hh_unit_type_ids).pluck(:id).to_set
       picklist.filter do |option|
         option[:code].in?(allowed_unit_type_unit_ids)
       end