Skip to content

Commit

Permalink
Merge branch 'release-148' into martha-7188-auto-exit-hoh-bug
Browse files Browse the repository at this point in the history
  • Loading branch information
martha authored Jan 15, 2025
2 parents 77f1e42 + 20069e8 commit ef24316
Show file tree
Hide file tree
Showing 57 changed files with 3,267 additions and 97 deletions.
2 changes: 2 additions & 0 deletions .github/dependencies.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ libxext
libxml2-dev
libxrender
libxslt-dev
linux-headers
nodejs
npm
nss
Expand All @@ -48,4 +49,5 @@ ttf-droid
ttf-freefont
ttf-liberation
tzdata
yaml-dev
yarn
31 changes: 26 additions & 5 deletions app/controllers/admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ def update
User.transaction do
@user.skip_reconfirmation!
# Associations don't play well with acts_as_paranoid, so manually clean up user ACLs
@user.user_group_members.where.not(user_group_id: assigned_user_group_ids).destroy_all
@user.user_group_members.where.not(user_group_id: assigned_user_group_ids).destroy_all unless changing_to_acls?

# TODO: START_ACL remove when ACL transition complete
# Associations don't play well with acts_as_paranoid, so manually clean up user roles
if ! @user.using_acls?
if ! user_using_or_changing_to_acls?
@user.user_roles.where.not(role_id: user_params[:legacy_role_ids]&.select(&:present?)).destroy_all
@user.access_groups.not_system.
where.not(id: user_params[:access_group_ids]&.select(&:present?)).each do |g|
Expand All @@ -98,12 +98,21 @@ def update
end
# END_ACL
@user.disable_2fa! if user_params[:otp_required_for_login] == 'false'
@user.update!(user_params)

# The User Group data is not captured for update when using the Role-Based view. This means it will not be included
# in the params when switching from Role-Based permissions to ACLs. In order to prevent wiping out any existing
# user_group_id data, we need to ignore this param when changing to an ACL based permissions.
# The reverse is true for the access_group_ids field.
params_to_update = user_params
params_to_update = params_to_update.except(:user_group_ids) if changing_to_acls?
params_to_update = params_to_update.except(:access_group_ids) if changing_to_role_based?
@user.update!(params_to_update)

# if we have a user to copy user groups from, add them
copy_user_groups if @user.using_acls?
copy_user_groups if user_using_or_changing_to_acls?
# TODO: START_ACL remove when ACL transition complete
# Restore any health roles we previously had
if ! @user.using_acls?
if ! user_using_or_changing_to_acls?
@user.legacy_roles = (@user.legacy_roles + existing_health_roles).uniq
@user.set_viewables viewable_params
end
Expand Down Expand Up @@ -148,6 +157,18 @@ def title_for_index
'User List'
end

private def changing_to_acls?
params[:user][:permission_context] == 'acls' && @user.permission_context != params[:user][:permission_context]
end

private def changing_to_role_based?
params[:user][:permission_context] == 'role_based' && @user.permission_context != params[:user][:permission_context]
end

private def user_using_or_changing_to_acls?
@user.using_acls? || changing_to_acls?
end

private def adding_admin?
@adding_admin ||= begin
adding_admin = false
Expand Down
5 changes: 3 additions & 2 deletions app/jobs/importing/hud_zip/fetch_and_import_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ def perform(klass:, options:)
raise "Unknown import class: #{klass}; You must add it to the list of known classes in FetchAndImportJob" unless safe_klass.present?

data_source_id = options[:data_source_id]
lock_obtained = GrdaWarehouse::DataSource.with_advisory_lock(advisory_lock_name(data_source_id), timeout_seconds: 60) do
lock_obtained = nil
GrdaWarehouse::DataSource.with_advisory_lock(advisory_lock_name(data_source_id), timeout_seconds: 60) do
safe_klass.constantize.new(**options).import!
# To prevent re-running when called against the same files if run more than once in a day, yield true
true
lock_obtained = true
end

requeue_at(Time.current + WAIT_MINUTES.minutes, "Import of Data Source: #{data_source_id} already running...re-queuing job for #{WAIT_MINUTES} minutes from now") unless lock_obtained
Expand Down
4 changes: 3 additions & 1 deletion app/jobs/importing/hud_zip/hmis_auto_migrate_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ class HmisAutoMigrateJob < BaseJob
after_enqueue :enforce_max_attempts

def perform(upload_id:, data_source_id:, deidentified: false, allowed_projects: false)
lock_obtained = GrdaWarehouse::DataSource.with_advisory_lock(advisory_lock_name(data_source_id), timeout_seconds: 60) do
lock_obtained = nil
GrdaWarehouse::DataSource.with_advisory_lock(advisory_lock_name(data_source_id), timeout_seconds: 60) do
importer = Importers::HmisAutoMigrate::UploadedZip.new(
data_source_id: data_source_id,
upload_id: upload_id,
deidentified: deidentified,
allowed_projects: allowed_projects,
)
importer.import!
lock_obtained = true
end

# when this exits, it will remove the current job from the queue, so add a new one to replace it
Expand Down
4 changes: 3 additions & 1 deletion app/jobs/update_warehouse_clients_caches_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ class UpdateWarehouseClientsCachesJob < BaseJob

def perform(client_ids: [], include_cas_and_cohorts: false, skip_expensive_calculations: false)
# If any for this class are already running, requeue for a few minutes in the future
lock_obtained = GrdaWarehouse::WarehouseClientsProcessed.with_advisory_lock('UpdateWarehouseClientsCachesJob', timeout_seconds: 20) do
lock_obtained = nil
GrdaWarehouse::WarehouseClientsProcessed.with_advisory_lock('UpdateWarehouseClientsCachesJob', timeout_seconds: 20) do
GrdaWarehouse::WarehouseClientsProcessed.update_cached_counts(client_ids: client_ids, include_cas_and_cohorts: include_cas_and_cohorts, skip_expensive_calculations: skip_expensive_calculations)
lock_obtained = true
end

requeue_at(Time.current + WAIT_MINUTES.minutes, "UpdateWarehouseClientsCachesJob is already running...re-queuing job for #{WAIT_MINUTES} minutes from now") unless lock_obtained
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def models
IncomeBenefitsReport::Client,
MaYyaReport::Client,
HudSpmReport::Fy2023::SpmEnrollment,
HudSpmReport::Fy2024::SpmEnrollment,
GrdaWarehouse::AdHocClient,
CePerformance::Client,
GrdaWarehouse::ClientContact,
Expand Down
10 changes: 5 additions & 5 deletions app/models/grda_warehouse/utility.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ def self.clear!
tables << HudPathReport::Fy2020::PathClient if RailsDrivers.loaded.include?(:hud_path_report)
if RailsDrivers.loaded.include?(:hud_spm_report)
tables << HudSpmReport::Fy2020::SpmClient
tables << HudSpmReport::Fy2023::SpmEnrollment
tables << HudSpmReport::Fy2023::Episode
tables << HudSpmReport::Fy2023::BedNight
tables << HudSpmReport::Fy2023::EnrollmentLink
tables << HudSpmReport::Fy2023::Return
tables << HudSpmReport::Fy2024::SpmEnrollment
tables << HudSpmReport::Fy2024::Episode
tables << HudSpmReport::Fy2024::BedNight
tables << HudSpmReport::Fy2024::EnrollmentLink
tables << HudSpmReport::Fy2024::Return
end

if RailsDrivers.loaded.include?(:hud_data_quality_report)
Expand Down
2 changes: 1 addition & 1 deletion app/models/health/qualifying_activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def self.date_search(start_date, end_date)
end

def face_to_face?
mode_of_contact.to_sym.in?(face_to_face_modes)
mode_of_contact&.to_sym.in?(face_to_face_modes)
end

# Return the string and the key so we can check either
Expand Down
1 change: 1 addition & 0 deletions docker/app/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ RUN apk update \
tzdata \
git \
bash \
linux-headers \
freetds-dev \
icu icu-dev \
curl libcurl curl-dev \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ def header_data
name: 'Housing Placements',
display_method: :number_with_delimiter,
},
{
id: 'days_to_obtain_housing',
icon: 'icon-house',
value: average_days_to_obtain_housing.round.abs,
name: 'Average Number of Days Between Referral and Housing Move-in',
display_method: :number_with_delimiter,
},
# {
# id: 'days_to_obtain_housing',
# icon: 'icon-house',
# value: average_days_to_obtain_housing.round.abs,
# name: 'Average Number of Days Between Referral and Housing Move-in',
# display_method: :number_with_delimiter,
# },
{
id: 'no_return',
icon: 'icon-clip-board-check',
Expand Down
4 changes: 3 additions & 1 deletion drivers/health_pctp/app/models/health_pctp/careplan.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ def editable?
sent_to_pcp_on.nil?
end

# NOTE: this needs to be updated before expires_on can be based on sent_to_pcp_on
# What are the consequences of changing the completed? calculation?
def completed?
patient_signed_on.present?
end
Expand Down Expand Up @@ -127,7 +129,7 @@ def current_goals_list
end

def expires_on
sent_to_pcp_on + 1.year
(sent_to_pcp_on.presence || patient_signed_on) + 1.year
end

def identifying_information
Expand Down
9 changes: 9 additions & 0 deletions drivers/hmis/app/models/hmis/form/definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,11 @@ def validate_form_values(form_values)
is_missing = value.nil? || (value.respond_to?(:empty?) && value.empty?)
is_data_not_collected = value == 'DATA_NOT_COLLECTED'
field_name = item.mapping&.field_name || item.mapping&.custom_field_key

numeric_validator.call(item, value)&.each do |error_message|
errors.add field_name || :base, message: error_message, **error_context
end

# Validate required status
if item.required && is_missing
errors.add field_name || :base, :required, **error_context
Expand Down Expand Up @@ -662,6 +667,10 @@ def self.infer_cded_field_type(item_type)
end
end

def numeric_validator
@numeric_validator ||= Hmis::Form::NumericInputValidator.new
end

# Helper for determining CustomDataElementDefinition attributes
def self.generate_cded_field_label(item)
label = item.readonly_text.presence || item.brief_text.presence || item.text.presence || item.link_id.humanize
Expand Down
53 changes: 53 additions & 0 deletions drivers/hmis/app/models/hmis/form/numeric_input_validator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
###
# Copyright 2016 - 2025 Green River Data Analysis, LLC
#
# License detail: https://github.com/greenriver/hmis-warehouse/blob/production/LICENSE.md
###

class Hmis::Form::NumericInputValidator
CURRENCY_RGX = /\A-?(?:[1-9]\d*|0)(?:\.\d{1,2})?\z/
INTEGER_RGX = /\A-?(?:[1-9]\d*|0)\z/

SPECIAL_VALUES = ['DATA_NOT_COLLECTED', '_HIDDEN'].to_set.freeze
SUPPORTED_TYPES = ['INTEGER', 'CURRENCY'].to_set.freeze

def call(item, value)
return [] if value.blank? || SPECIAL_VALUES.include?(value)
return [] unless item.type.in? SUPPORTED_TYPES

format_errors = validate_format(item, value.to_s.strip)
return format_errors if format_errors.any?

validate_bounds(item, value.to_d)
end

private

def validate_format(item, value)
case item.type
when 'INTEGER'
INTEGER_RGX.match?(value) ? [] : ['not a valid integer']
when 'CURRENCY'
CURRENCY_RGX.match?(value) ? [] : ['not a valid currency amount']
else
[]
end
end

def validate_bounds(item, value)
return [] if item.bounds.blank?

item.bounds.each_with_object([]) do |bound, errors|
next unless bound.severity == 'error'
# bound.value_number can be nil in the case where the bound is against a local constant or another question
next unless bound.value_number

case bound.type
when 'MAX'
errors << "must be less than or equal to #{bound.value_number}" if value > bound.value_number
when 'MIN'
errors << "must be greater than or equal to #{bound.value_number}" if value < bound.value_number
end
end
end
end
20 changes: 18 additions & 2 deletions drivers/hmis/app/models/hmis/hoh_change_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ def apply_changes!
# TODO(#6857) For now, we leave the old MID as-is IF we were unable to transfer it because the new HoH entered after move-in. We plan to adjust this pending guidance from HUD.
hhm.move_in_date = nil unless hhm.head_of_household? && hhm.move_in_date && !new_hoh_move_in_date

# Clear RelationshipToHoH on previous HoH
# Update RelationshipToHoH on previous HoH
if hhm.head_of_household?
hhm.relationship_to_ho_h = 99
hhm.relationship_to_ho_h = infer_relationship_to_new_hoh(new_hoh_enrollment)
# Move-in Address(es) from old HoH should transfer to new HoH. We only expect 1, but it's OK if there are more.
hhm.move_in_addresses.each { |addr| addr.update!(enrollment: new_hoh_enrollment) }
end
Expand Down Expand Up @@ -128,5 +128,21 @@ def self.move_in_date_not_transfered_msg(move_in_date)
def add_warning(full_message)
validation_errors.add(:enrollment, :informational, severity: :warning, full_message: full_message)
end

# infer which relationship the previous HoH should have to the new HoH
def infer_relationship_to_new_hoh(new_hoh)
case new_hoh.relationship_to_ho_h
when 3 # Spouse
3 # Spouse
when 4 # Other relative
4 # Other relative
when 5 # Unrelated household member
5 # Unrelated household member
when 2 # Child
4 # "Other relative" to indicate parent. This is unlikely, because child shouldn't become HoH in household with parent, but it's possible.
else
5 # Unrelated household member
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,14 @@ def income_source_attributes(amount_field, value)
case @hud_values['IncomeBenefit.incomeFromAnySource']
when 'YES'
# If overall income was 1 (yes), then this specific income field must be 1 or 0 (yes or no)
bool_attribute_value = amount_attribute_value&.positive? ? 1 : 0
case amount_attribute_value
when Integer, Float, nil
bool_attribute_value = amount_attribute_value&.positive? ? 1 : 0
else
# The frontend input is not expected to send numeric values here
Sentry.capture_message("Unexpected value \"#{amount_attribute_value}\" received for #{amount_attribute_name}")
bool_attribute_value = 0
end
when 'NO'
# If overall income was 0 (no), then this specific income field is 0 (no)
amount_attribute_value = nil
Expand Down
11 changes: 11 additions & 0 deletions drivers/hmis/lib/hmis_data_cleanup/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ def self.make_sole_member_hoh!
end
end

# Change any RelationshipToHoH values that are 99 (Data not collected) to 5 (Unrelated household member)
# 99 is not a valid option for RelationshipToHoH, and causes flags in the LSA. Reference issue #7127.
def self.fix_relationship_to_hoh_99s!
without_papertrail_or_timestamps do
rows_affected = Hmis::Hud::Enrollment.hmis.where(relationship_to_hoh: 99).
update_all(relationship_to_hoh: 5) # skips callbacks

Rails.logger.info "Set RelationshipToHoH 99=>5 on #{rows_affected} Enrollments"
end
end

# Fix any instances of enrollment-related records where the PersonalID does not match the Enrollment's PersonalIDs
def self.fix_incorrect_personal_id_references!(classes: nil, dry_run: false)
classes&.each do |klass|
Expand Down
Loading

0 comments on commit ef24316

Please sign in to comment.