Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Join Household mutation #5047

Open
wants to merge 43 commits into
base: release-151
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
b514eba
Fix erroneous deletion
martha Jan 6, 2025
fff4369
Revert "Fix erroneous deletion"
martha Jan 6, 2025
c14a4f4
Fix spec tests for table actions (#5012)
martha Jan 7, 2025
0aee665
Sketch join households mutation
martha Jan 9, 2025
8fece60
Keep conflicting enrollment validation on backend
martha Jan 15, 2025
940df46
Return both receiving and donor household
martha Jan 16, 2025
3524490
Fix bugs and add spec
martha Jan 17, 2025
68a8b98
Revert unused schema updates
martha Jan 17, 2025
adf3cac
Add system tests
martha Jan 21, 2025
eca913f
Add unit update logic
martha Jan 21, 2025
427d529
Add missing default
martha Jan 21, 2025
a6bad7a
Add event history table
martha Jan 21, 2025
bc87e46
Fix test failures (hopefully)
martha Jan 22, 2025
cefa537
Restructure and add comments
martha Jan 22, 2025
22082a7
Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh
martha Jan 22, 2025
1a4e3c9
Merge branch 'release-149' into deploy/6761-table-actions
martha Jan 22, 2025
7b00cfb
Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh
martha Jan 22, 2025
171fc5d
Avoid n+1 on household, hopefully
martha Jan 22, 2025
d4887ee
Renames for consistency and compatibilitY
martha Jan 22, 2025
1cba85f
Move enrollment snapshot method somewhere common
martha Jan 22, 2025
590f66f
Revert system test rerun
martha Jan 24, 2025
7d91c36
Fix aria labels after refactor
martha Jan 27, 2025
ba6cc8f
Add permission restriction
martha Jan 27, 2025
a0787be
Update system tests for Tables (revert changes) (#5071)
martha Jan 31, 2025
0d05ac1
Merge branch 'release-150' into deploy/6761-table-actions
gigxz Jan 31, 2025
d4fa2e3
Address PR feedback on db schema and migration (wip)
martha Feb 3, 2025
e5aaed9
Merge branch 'deploy/6761-table-actions' into martha/5752-join-hh
martha Feb 3, 2025
f767ad6
Only resolve donor enrollment, not hh (fe changes still incomplete)
martha Feb 3, 2025
a36a1a8
Update PR from feedback on mutation (still wip)
martha Feb 3, 2025
456ba1e
Clarify household snapshot code
martha Feb 4, 2025
a093ead
Simplify unit logic
martha Feb 4, 2025
a3ef6d6
Use snake instead of camel case for json
martha Feb 4, 2025
aa58608
Avoid multiple saves of enrollment
martha Feb 4, 2025
b9461a3
Add test on enrollment validator (wip)
martha Feb 4, 2025
60c31b6
Remove uniq constraint on index
martha Feb 4, 2025
a2f874e
Expand tests
martha Feb 5, 2025
239e750
Update spec tests
martha Feb 5, 2025
358a029
Return whole household
martha Feb 5, 2025
7f337a8
Cleanup and fix tests
martha Feb 5, 2025
f2e9e4f
Merge branch 'release-151' into martha/5752-join-hh
martha Feb 6, 2025
128e6b3
Merge branch 'release-151' into martha/5752-join-hh
martha Feb 6, 2025
e6661b2
Use CleanBaseMutation
martha Feb 6, 2025
d8eb511
Move household event creation logic into the HouseholdEvent class
martha Feb 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/models/grda_warehouse/utility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def self.modifier(model)
GrdaWarehouse::Hud::Client,
GrdaWarehouse::Hud::Project,
GrdaWarehouse::Hud::User,
GrdaWarehouse::Hud::Enrollment,
Copy link
Contributor

@gigxz gigxz Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this added in order to cascade deletion of hmis_household_events when deleting an enrollment?
it's not needed for the spec test to pass locally for me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes -- that's weird, I did need it at some point to get the spec tests to pass. but anyway, I will remove it per #5047 (comment).

GrdaWarehouse::ServiceHistoryEnrollment,
ActiveStorage::Attachment,
ActiveStorage::Blob,
Expand Down
6 changes: 6 additions & 0 deletions bin/run_hmis_system_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ cd "$ORIGINAL_CWD"
# skip okta if it's set in our local env
unset HMIS_OKTA_CLIENT_ID
unset OKTA_DOMAIN
# TODO @MARTHA - REVERT
martha marked this conversation as resolved.
Show resolved Hide resolved
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*
RUN_SYSTEM_TESTS=true RAILS_ENV=test CAPYBARA_APP_HOST="http://$HOSTNAME:5173" rspec drivers/hmis/spec/system/hmis/*

TEST_EXIT_CODE=$?
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright 2016 - 2024 Green River Data Analysis, LLC
#
# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
#

class AddHmisHouseholdEventsTable < ActiveRecord::Migration[7.0]
def change
create_table :hmis_household_events do |t|
t.string :household_id, index: true # hmis_households is a view, not a table
Copy link
Contributor

@gigxz gigxz Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the HUD HouseholdID string? That's what I would have expected, but am a little confused by your comment about the View.

I believe this should be NOT NULL as well.

I'm also not totally sure if it's useful to index the HouseholdID alone, since we'd always be looking it up by Data Source ID as well. So maybe an index on data_source_id,household_id.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just go ahead and use HouseholdID casing for this column for consistency. I worry that household_id is ambiguous because we may later add a household table, which then would have a primary key, but we're not referencing that here.

I'd say our convention right now is:

  • EnrollmentID likely contains the HUD Enrollment "key"
  • enrollment_id likely contains a primary key reference to the Enrollment table

So it may be confusing to add household_id that contains a HUD key.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sounds good. I'll remove the confusing comment; I just meant for it to indicate why we are not adding a fk relationship (referencing this migration when we added staff assignments).

t.references :data_source, null: false
t.string :event_type, null: false
t.jsonb :event_details
t.references :user, null: false
t.timestamps
end

# Create a join table since Postgres can't put fk constraints on array elements
create_table :hmis_household_event_enrollments, id: false do |t|
t.references :hmis_household_event, null: false, foreign_key: { to_table: :hmis_household_events }, index: { name: 'idx_hmis_household_event_enrollments_on_event_id' }
t.references :enrollment, null: false, foreign_key: { to_table: :Enrollment }, index: true # this is the ID of the enrollment record: Enrollment.id not Enrollment.EnrollmentID
end

# Add a composite unique index to prevent duplicate associations
add_index :hmis_household_event_enrollments,
[:hmis_household_event_id, :enrollment_id],
unique: true,
name: 'idx_hmis_household_event_enrollments_on_event_and_enrollment'
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this complexity is worth it just for fk constraints on the enrollment references.

For other similar tales (client merge histories, client merge audits) we don't have fk constraints.
The primary goal is to record events as they happen, even if the related records (Enrollments) are deleted or changed. Because of that, I think the fk constraints might not be necessary because the historical accuracy of the audit trail is more important than referential integrity.

For example if an Enrollment get's hard-deleted, we probably want the hmis_household_events that it was a part of to remain. We also don't want the soft-deletion of an enrollment to cascade and delete the household events that it was a part of.

end
333,616 changes: 26,683 additions & 306,933 deletions db/warehouse_structure.sql

Large diffs are not rendered by default.

110 changes: 110 additions & 0 deletions drivers/hmis/app/graphql/mutations/join_households.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
###
# Copyright 2016 - 2025 Green River Data Analysis, LLC
#
# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
###

module Mutations
class JoinHouseholds < BaseMutation
argument :receiving_household_id, ID, required: true
argument :joining_enrollment_inputs, [Types::HmisSchema::JoiningEnrollmentInput], required: true

field :receiving_household, Types::HmisSchema::Household, null: false
field :donor_household, Types::HmisSchema::Household, null: true # Will be null if there are no remaining members
# todo @martha - possible that there will be multiple households with same ID? mutation should require project_id?
def resolve(receiving_household_id:, joining_enrollment_inputs:)
receiving_enrollments = Hmis::Hud::Enrollment.
where(household_id: receiving_household_id).
viewable_by(current_user).
includes(:project)
access_denied! if receiving_enrollments.empty?

receiving_household = receiving_enrollments.first.household

receiving_project = receiving_enrollments.map(&:project).uniq.sole
access_denied! unless current_permission?(permission: :can_split_households, entity: receiving_project)

# The mutation input is a list, so convert it to a hashmap for ease of access
map_enrollment_id_to_relationship = joining_enrollment_inputs.map { |e| [e.enrollment_id, e.relationship_to_hoh] }.to_h

joining_enrollment_ids = map_enrollment_id_to_relationship.keys
joining_enrollments = Hmis::Hud::Enrollment.
viewable_by(current_user).
where(id: joining_enrollment_ids)
access_denied! unless joining_enrollments.count == joining_enrollment_inputs.count

raise 'Cannot merge enrollments from another project' unless joining_enrollments.pluck(:project_pk).uniq.sole == receiving_project.id

donor_household = joining_enrollments.map(&:household).uniq.sole
remaining_enrollments = Hmis::Hud::Enrollment.
where(household_id: donor_household.household_id).
where.not(id: joining_enrollment_ids)
remaining_hoh = remaining_enrollments.any? { |enrollment| enrollment.relationship_to_hoh == 1 }
raise 'This operation would leave behind a household with no HoH, which is not allowed' unless remaining_enrollments.empty? || remaining_hoh

# If the receiving household is assigned to a unit, re-assign the joining enrollments to the same unit.
# This works for the present-tense, but could be improved to better accommodate past data correction.
units = receiving_enrollments.map { |enrollment| enrollment.active_unit_occupancy&.unit }.compact.uniq

receiving_before_state = snapshot(receiving_enrollments)
donor_before_state = snapshot([*joining_enrollments, *remaining_enrollments])

Hmis::Hud::Enrollment.transaction do
joining_enrollments.each_with_index do |enrollment, index|
enrollment.update!(
household_id: receiving_household_id,
relationship_to_hoh: map_enrollment_id_to_relationship[enrollment.id.to_s],
)

# Whether or not the receiving household has a unit assignment, clear the joining enrollment's current unit assignment
enrollment.active_unit_occupancy&.assign_attributes(occupancy_period_attributes: { end_date: Date.current })

unless units.empty?
unit = units[index % units.length] # If the receiving household has multiple units, pick any, deterministically
enrollment.assign_unit(unit: unit, start_date: Date.current, user: current_user)
end

enrollment.save!
end

receiving_household.reload
donor_household.reload if remaining_enrollments.any?

joining_event = Hmis::HouseholdEvent.new
joining_event.user = current_user
joining_event.household = receiving_household
joining_event.event_type = Hmis::HouseholdEvent::JOIN
joining_event.event_details = {
'donorHouseholdId': donor_household.household_id,
'before': receiving_before_state,
'after': snapshot(receiving_household.enrollments),
}

leaving_event = Hmis::HouseholdEvent.new
leaving_event.user = current_user
leaving_event.household = donor_household
leaving_event.event_type = Hmis::HouseholdEvent::SPLIT
leaving_event.event_details = {
'receivingHouseholdId': receiving_household_id,
'before': donor_before_state,
'after': snapshot(remaining_enrollments),
}
Hmis::HouseholdEvent.import!([joining_event, leaving_event])
end

{
receiving_household: receiving_household,
donor_household: remaining_enrollments.any? ? donor_household : nil,
}
end

private def snapshot(enrollments)
enrollments.map do |enrollment|
{
'enrollmentId': enrollment.id,
'relationshipToHoh': enrollment.relationship_to_hoh,
}
end
end
end
end
37 changes: 37 additions & 0 deletions drivers/hmis/app/graphql/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -5628,6 +5628,36 @@ Represents untyped JSON
"""
scalar JSON

"""
Autogenerated input type of JoinHouseholds
"""
input JoinHouseholdsInput {
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
joiningEnrollmentInputs: [JoiningEnrollmentInput!]!
receivingHouseholdId: ID!
}

"""
Autogenerated return type of JoinHouseholds.
"""
type JoinHouseholdsPayload {
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
donorHousehold: Household
errors: [ValidationError!]!
receivingHousehold: Household!
}

input JoiningEnrollmentInput {
enrollmentId: ID!
relationshipToHoh: RelationshipToHoH!
}

"""
Arbitrary JSON Type
"""
Expand Down Expand Up @@ -6218,6 +6248,12 @@ type Mutation {
"""
input: DeleteUnitsInput!
): DeleteUnitsPayload
joinHouseholds(
"""
Parameters for JoinHouseholds
"""
input: JoinHouseholdsInput!
): JoinHouseholdsPayload
mergeClients(
"""
Parameters for MergeClients
Expand Down Expand Up @@ -8668,6 +8704,7 @@ type ProjectAccess {
canManageIncomingReferrals: Boolean!
canManageOutgoingReferrals: Boolean!
canManageUnits: Boolean!
canSplitHouseholds: Boolean!
canViewDob: Boolean!
canViewEnrollmentDetails: Boolean!
canViewFullSsn: Boolean!
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
###
# Copyright 2016 - 2025 Green River Data Analysis, LLC
#
# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
###

module Types
class HmisSchema::JoiningEnrollmentInput < BaseInputObject
argument :enrollment_id, ID, required: true
argument :relationship_to_hoh, Types::HmisSchema::Enums::Hud::RelationshipToHoH, required: true
end
end
2 changes: 2 additions & 0 deletions drivers/hmis/app/graphql/types/hmis_schema/mutation_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class HmisSchema::MutationType < Types::BaseObject
field :merge_clients, mutation: Mutations::MergeClients
field :bulk_merge_clients, mutation: Mutations::BulkMergeClients

field :join_households, mutation: Mutations::JoinHouseholds

field :create_form_definition, mutation: Mutations::CreateFormDefinition
field :update_form_definition, mutation: Mutations::UpdateFormDefinition
field :delete_form_definition, mutation: Mutations::DeleteFormDefinition
Expand Down
1 change: 1 addition & 0 deletions drivers/hmis/app/graphql/types/hmis_schema/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def self.authorized?(object, ctx)
can :manage_outgoing_referrals
can :manage_denied_referrals
can :manage_external_form_submissions
can :split_households
end
field :unit_types, [Types::HmisSchema::UnitTypeCapacity], null: false
field :has_units, Boolean, null: false
Expand Down
19 changes: 19 additions & 0 deletions drivers/hmis/app/models/hmis/household_event.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright 2016 - 2024 Green River Data Analysis, LLC
#
# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
#
class Hmis::HouseholdEvent < Hmis::HmisBase
JOIN = 'join'.freeze
SPLIT = 'split'.freeze
EVENT_TYPES = [JOIN, SPLIT].freeze # In the future we may add other events, like add and remove

belongs_to :user, class_name: 'Hmis::User'
belongs_to :household, class_name: 'Hmis::Hud::Household', primary_key: [:data_source_id, :HouseholdID], foreign_key: [:data_source_id, :household_id], inverse_of: :events
has_and_belongs_to_many :enrollments,
class_name: 'Hmis::Hud::Enrollment',
join_table: 'hmis_household_event_enrollments',
association_foreign_key: 'enrollment_id',
foreign_key: 'hmis_household_event_id'

validates :event_type, inclusion: { in: EVENT_TYPES, message: '%{value} is not a valid event type' }
end
1 change: 1 addition & 0 deletions drivers/hmis/app/models/hmis/hud/household.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class Hmis::Hud::Household < Hmis::Hud::Base
has_many :current_units, through: :enrollments
has_many :custom_assessments, through: :enrollments
has_many :staff_assignments, class_name: 'Hmis::StaffAssignment', primary_key: [:data_source_id, :HouseholdID], foreign_key: [:data_source_id, :household_id]
has_many :events, class_name: 'Hmis::HouseholdEvent', primary_key: [:data_source_id, :HouseholdID], foreign_key: [:data_source_id, :household_id]
alias_attribute :household_id, :HouseholdID

replace_scope :viewable_by, ->(user) do
Expand Down
33 changes: 22 additions & 11 deletions drivers/hmis/app/models/hmis/hud/validators/enrollment_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,24 @@ def self.already_enrolled_full_message
'Client has another enrollment in this project that conflicts with this entry date.'
end

def self.find_conflict_severity(enrollment)
def self.find_conflicting_enrollment(enrollment)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there test coverage for the changes here?

It looks like all the Enrollment creation tests are jammed into the submit_form_spec which is big

describe 'SubmitForm for Enrollment creation' do
let(:definition) { Hmis::Form::Definition.find_by(role: :ENROLLMENT) }
let!(:c3) { create :hmis_hud_client_complete, data_source: ds1 }
let(:test_input) do
{
form_definition_id: definition.id,
**mock_form_values_for_definition(definition),
project_id: p2.id,
client_id: c3.id,
confirmed: false,
}
end
def merge_hud_values(input, *args)
input.merge(hud_values: input[:hud_values].merge(*args))
end
def expect_error_message(input, exact: true, **expected_error)
response, result = post_graphql(input: { input: input }) { mutation }
errors = result.dig('data', 'submitForm', 'errors')
aggregate_failures 'checking response' do
expect(response.status).to eq(200), result&.inspect
if exact
expect(errors).to contain_exactly(include(expected_error.stringify_keys))
else
expect(errors).to include(include(expected_error.stringify_keys))
end
end
end
it 'should error if adding second HoH to existing household' do
input = merge_hud_values(
test_input,
'householdId' => e1.household_id,
)
expect_error_message(input, fullMessage: Hmis::Hud::Validators::EnrollmentValidator.one_hoh_full_message)
end
it 'should error if creating household without hoh' do
input = merge_hud_values(
test_input,
'relationshipToHoH' => Types::HmisSchema::Enums::Hud::RelationshipToHoH.key_for(2),
)
expect_error_message(input, fullMessage: Hmis::Hud::Validators::EnrollmentValidator.first_member_hoh_full_message)
end
it 'should error if client already has an open enrollment in the household' do
e2 = create(:hmis_hud_enrollment, data_source: ds1, project: p1, entry_date: '2000-01-01', household_id: e1.household_id)
input = merge_hud_values(
test_input.merge(client_id: e2.client.id),
'householdId' => e1.household_id,
'relationshipToHoH' => Types::HmisSchema::Enums::Hud::RelationshipToHoH.key_for(2),
)
expect_error_message(input, exact: false, fullMessage: Hmis::Hud::Validators::EnrollmentValidator.duplicate_member_full_message)
end
it 'should not error if client has a closed enrollment in the household' do
e2 = create(:hmis_hud_enrollment, data_source: ds1, project: p1, entry_date: '2000-01-01', household_id: e1.household_id)
create(:hmis_hud_exit, enrollment: e2, client: e2.client, data_source: ds1, exit_date: '2001-01-01')
input = merge_hud_values(
test_input.merge(client_id: e2.client.id),
'householdId' => e1.household_id,
'relationshipToHoH' => Types::HmisSchema::Enums::Hud::RelationshipToHoH.key_for(2),
)
response, result = post_graphql(input: { input: input }) { mutation }
errors = result.dig('data', 'submitForm', 'errors')
expect(response.status).to eq(200), result&.inspect
expect(errors).to be_empty
household_size = result.dig('data', 'submitForm', 'record', 'householdSize')
expect(household_size).to eq(2) # household size is 2 even though it contains 3 enrollments
end
it 'should warn if client already enrolled' do
input = merge_hud_values(
test_input.merge(client_id: e1.client.id, project_id: e1.project.id),
)
expect_error_message(input, fullMessage: Hmis::Hud::Validators::EnrollmentValidator.already_enrolled_full_message)
end
it 'should error if entry date is in the future' do
input = merge_hud_values(
test_input,
'entryDate' => Date.tomorrow.strftime('%Y-%m-%d'),
)
expect_error_message(input, message: Hmis::Hud::Validators::EnrollmentValidator.future_message)
end
describe 'on project that has closed' do
before(:each) { p2.update!(operating_start_date: today + 1.day) }
it 'should be invalid' do
input = merge_hud_values(test_input, 'entryDate' => today.to_fs(:db))
expect_error_message(input, message: Hmis::Hud::Validators::BaseValidator.before_project_start_message(p2.operating_start_date))
end
end
describe 'on project that has not started' do
before(:each) { p2.update!(operating_end_date: today - 1.day) }
it 'should be invalid' do
input = merge_hud_values(test_input, 'entryDate' => today.to_fs(:db))
expect_error_message(input, message: Hmis::Hud::Validators::BaseValidator.after_project_end_message(p2.operating_end_date))
end
end
end
describe 'SubmitForm for Enrollment updates' do
let(:definition) { Hmis::Form::Definition.find_by(role: :ENROLLMENT) }
let(:e1) { create :hmis_hud_enrollment, data_source: ds1 }
let(:wip_e1) { create :hmis_hud_wip_enrollment, data_source: ds1 }
let(:test_input) do
{
form_definition_id: definition.id,
**mock_form_values_for_definition(definition),
project_id: p1.id,
client_id: c2.id,
confirmed: false,
}
end
it 'should save new enrollment as WIP' do
record, _errors = submit_form(test_input)
expect(record['inProgress']).to eq(true)
end
it 'should not change WIP status (WIP enrollment)' do
input = test_input.merge(record_id: wip_e1.id)
submit_form(input)
wip_e1.reload
expect(wip_e1.in_progress?).to eq(true)
end
it 'should not change WIP status (non-WIP enrollment)' do
input = test_input.merge(record_id: e1.id)
submit_form(input)
e1.reload
expect(e1.in_progress?).to eq(false)
end
it 're-submission should not create a second FormProcessor' do
input = test_input.merge(record_id: e1.id)
expect { submit_form(input) }.to change(Hmis::Form::FormProcessor, :count).by(1)
expect { submit_form(input) }.not_to change(Hmis::Form::FormProcessor, :count)
end
end
describe 'SubmitForm for Enrollment on project with ProjectAutoEnterConfig' do
let(:definition) { Hmis::Form::Definition.find_by(role: :ENROLLMENT) }
let!(:aec) { create :hmis_project_auto_enter_config, project: p1 }
let(:test_input) do
{
form_definition_id: definition.id,
**mock_form_values_for_definition(definition),
project_id: p1.id,
client_id: c1.id,
confirmed: false,
}
end
it 'should save new enrollment without WIP status' do
record, errors = submit_form(test_input)
expect(errors).to be_empty
enrollment = Hmis::Hud::Enrollment.find_by(id: record['id'])
expect(enrollment).to be_present
expect(enrollment.in_progress?).to eq(false)
expect(enrollment.intake_assessment).to be_present
expect(enrollment.intake_assessment.assessment_date).to eq(enrollment.entry_date)
expect(enrollment.intake_assessment.wip).to eq(false)
expect(enrollment.intake_assessment.form_processor).to be_present
end
end
describe 'SubmitForm for Create+Enroll' do
let(:definition) { Hmis::Form::Definition.find_by(role: :NEW_CLIENT_ENROLLMENT) }
let(:test_input) do
{
form_definition_id: definition.id,
**mock_form_values_for_definition(definition),
project_id: p2.id,
confirmed: true,
}
end
def merge_hud_values(input, *args)
input.merge(hud_values: input[:hud_values].merge(*args))
end
it 'creates client and enrollment' do
record, errors = submit_form(test_input)
expect(errors).to be_empty
expect(record).to be_present
enrollment = Hmis::Hud::Enrollment.find(record['id'])
expect(enrollment.client).to be_present
expect(enrollment.client.first_name).to eq('First')
end
it 'validates client (invalid field)' do
input = merge_hud_values(
test_input,
'Client.dobDataQuality' => 'INVALID',
)
_, errors = submit_form(input)
expect(errors).to contain_exactly(include({ 'attribute' => 'dobDataQuality', 'type' => 'invalid' }))
end
it 'validates client (invalid DOB)' do
input = merge_hud_values(
test_input,
'Client.dob' => '2200-01-01', # future dob is not valid
)
_, errors = submit_form(input)
expect(errors).to contain_exactly(include({ 'attribute' => 'dob', 'type' => 'out_of_range' }))
end
end

This seems like the right place to add assertions on the data that is returned:

it 'should warn if client already enrolled' do
input = merge_hud_values(
test_input.merge(client_id: e1.client.id, project_id: e1.project.id),
)
expect_error_message(input, fullMessage: Hmis::Hud::Validators::EnrollmentValidator.already_enrolled_full_message)
end

conflict_scope = Hmis::Hud::Enrollment.
where(personal_id: enrollment.personal_id, data_source_id: enrollment.data_source_id).
with_conflicting_dates(project: enrollment.project, range: enrollment.entry_date...enrollment.exit_date)

if enrollment.persisted?
# If the entry date is being changed on an EXISTING enrollment, and it overlaps with another one, it should be a warning
conflict_scope = conflict_scope.where.not(id: enrollment.id)
return :warning if conflict_scope.any?
return :warning, conflict_scope.first if conflict_scope.any?
Copy link
Contributor

@gigxz gigxz Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be selected deterministically, or return all of them. Since it's unlikely to have multiple overlapping enrollments, it's probably OK to pick one -- maybe by earliest entry date, so it's consistent with the below.

else
min_conflict_date = conflict_scope.minimum(:entry_date)
if min_conflict_date
# If the entry date is being set on a NEW enrollment, and the entry date is on or after the entry date of any conflicting enrollments, it should be an error.
return :error if enrollment.entry_date >= min_conflict_date

# if the entry date is being set on a NEW enrollment, and the entry date is before the entry date of any conflicting enrollments, it should be a warning
return :warning if enrollment.entry_date < min_conflict_date
# If the entry date is being set on a NEW enrollment,
conflicting_enrollment = conflict_scope.order(entry_date: :asc).first # pick the conflicting enrollment with the earliest entry date
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
conflicting_enrollment = conflict_scope.order(entry_date: :asc).first # pick the conflicting enrollment with the earliest entry date
conflicting_enrollment = conflict_scope.min_by(&:entry_date)

if conflicting_enrollment
# If the entry date is on or after the entry date of any conflicting enrollments, it should be an error
return :error, conflicting_enrollment if enrollment.entry_date >= conflicting_enrollment.entry_date

# If the entry date is before the entry date of any conflicting enrollments, it should be a warning
return :warning, conflicting_enrollment if enrollment.entry_date < conflicting_enrollment.entry_date
end
end
end
Expand All @@ -74,8 +75,18 @@ def self.validate_entry_date(enrollment, household_members: nil, options: {})
errors.add :entry_date, :out_of_range, message: after_exit_message(exit_date), **options if exit_date.present? && exit_date < entry_date
return errors.errors if errors.any?

conflict_severity = find_conflict_severity(enrollment)
errors.add(:entry_date, :out_of_range, severity: conflict_severity, full_message: already_enrolled_full_message) if conflict_severity
conflict_severity, conflicting_enrollment = find_conflicting_enrollment(enrollment)
if conflict_severity
errors.add(
:entry_date,
:out_of_range,
severity: conflict_severity,
full_message: already_enrolled_full_message,
data: {
conflictingEnrollmentId: conflicting_enrollment.id.to_s,
},
)
end

unless enrollment.head_of_household?
household_members ||= enrollment.household_members
Expand Down
Loading
Loading