Merge pull request #5001 from greenriver/release-145

Release 145

.github/dependencies.txt

@@ -35,6 +35,7 @@ libxslt-dev
 nodejs
 npm
 nss
+poppler-utils
 postgis
 postgresql
 postgresql-dev

.github/workflows/build_images.yml

@@ -41,7 +41,7 @@ jobs:
             trivy_skip_files: /app/config/key.pem,/app/docker/sftp/ssh_host_ed25519_key,/app/docker/sftp/ssh_host_rsa_key,/app/spec/fixtures/files/health/secret.key
             build-args: |
               BUILD_TAG=3.1.6-alpine3.20
-              BUNDLER_VERSION=2.5.17
+              BUNDLER_VERSION=2.5.23
             tags: |
               type=sha,prefix=githash-
               type=ref,event=branch,prefix=branch-

.github/workflows/container_setup/action.yml

@@ -58,7 +58,7 @@ runs:
     - name: Install gems
       shell: bash
       run: |
-        gem install bundler --version=2.5.17
+        gem install bundler --version=2.5.23
         bundle config set --local without 'production staging'
         bundle install --jobs 10 --retry 3
 

Gemfile

@@ -175,7 +175,7 @@ gem 'rubyXL', require: false
 gem 'soundex', require: false # for HMIS 6.11 + exports that use SHA-256 of soundex
 
 # PDF Exports
-gem 'combine_pdf'
+gem 'pdfunite'
 gem 'grover'
 
 gem 'whenever', require: false

Gemfile.lock

@@ -713,6 +713,7 @@ GEM
     parser (3.3.5.0)
       ast (~> 2.4.1)
       racc
+    pdfunite (0.6.0)
     pg (1.5.6)
     pghero (3.6.1)
       activerecord (>= 6.1)
@@ -987,7 +988,7 @@ GEM
       net-ssh (>= 2.8.0)
     ssrf_filter (1.0.8)
     stackprof (0.2.26)
-    stringio (3.1.1)
+    stringio (3.1.2)
     strong_migrations (1.8.0)
       activerecord (>= 5.2)
     sync (0.5.0)
@@ -1137,7 +1138,6 @@ DEPENDENCIES
   census_api!
   charlock_holmes
   coffee-rails
-  combine_pdf
   composite_primary_keys!
   cuprite
   curb
@@ -1215,6 +1215,7 @@ DEPENDENCIES
   paper_trail
   parallel
   paranoia (~> 2.0)
+  pdfunite
   pg
   pg_fixtures!
   pghero

app/controllers/admin/talentlms_controller.rb

@@ -59,6 +59,7 @@ def site_config_scope
     def site_config_params
       params.require(:grda_warehouse_config).permit(
         :number_lms_courses_required,
+        :default_lms_email_to_warehouse_email,
       )
     end
 
@@ -67,6 +68,7 @@ def config_params
         :subdomain,
         :api_key,
         :create_new_accounts,
+        :allow_automatic_redirect_to_course,
       )
     end
   end

app/controllers/concerns/health_careplan.rb

@@ -62,7 +62,7 @@ def careplan_pdf_coversheet
         wait_until: 'networkidle0',
         print_background: true,
       }
-      CombinePDF.parse(Grover.new(wrap_in_html(coversheet), **grover_options).to_pdf, allow_optional_content: true)
+      PdfGenerator.merge_inline_pdfs(Grover.new(wrap_in_html(coversheet), **grover_options).to_pdf)
     end
 
     def careplan_pdf_full
@@ -99,7 +99,7 @@ def careplan_pdf_full
         wait_until: 'networkidle0',
         print_background: true,
       }
-      CombinePDF.parse(Grover.new(wrap_in_html(full_careplan), **grover_options).to_pdf, allow_optional_content: true)
+      PdfGenerator.merge_inline_pdfs(Grover.new(wrap_in_html(full_careplan), **grover_options).to_pdf)
     end
 
     def careplan_pdf_pctp
@@ -138,7 +138,7 @@ def careplan_pdf_pctp
         print_background: true,
       }
 
-      CombinePDF.parse(Grover.new(wrap_in_html(pctp), **grover_options).to_pdf, allow_optional_content: true)
+      PdfGenerator.merge_inline_pdfs(Grover.new(wrap_in_html(pctp), **grover_options).to_pdf)
     end
 
     private def wrap_in_html(content)
@@ -154,7 +154,7 @@ def careplan_combine_pdf_object
       # If we already have a document with a signature, use that to try and avoid massive duplication
       if (health_file_id = @careplan.most_appropriate_pdf_id)
         if (health_file = Health::HealthFile.find(health_file_id))
-          return CombinePDF.parse(health_file.content, allow_optional_content: true)
+          return PdfGenerator.merge_inline_pdfs(health_file.content)
         end
       end
 
@@ -168,7 +168,7 @@ def careplan_combine_pdf_object
       end
       @cha = @patient.comprehensive_health_assessments.recent.first
 
-      pdf = CombinePDF.new
+      pdf = []
 
       pdf << careplan_pdf_coversheet
 
@@ -188,10 +188,10 @@ def careplan_combine_pdf_object
 
       pdf << careplan_pdf_full
 
-      pdf << CombinePDF.parse(@careplan.health_file.content, allow_optional_content: true) if @careplan.health_file.present?
-      pdf << CombinePDF.parse(@cha.health_file.content, allow_optional_content: true) if @cha.present? && @cha.health_file.present? && @cha.health_file.content_type == 'application/pdf'
-      pdf << CombinePDF.parse(@form.health_file.content, allow_optional_content: true) if @form.present? && @form.is_a?(Health::SelfSufficiencyMatrixForm) && @form.health_file.present?
-      pdf
+      pdf << PdfGenerator.merge_inline_pdfs(@careplan.health_file.content) if @careplan.health_file.present?
+      pdf << PdfGenerator.merge_inline_pdfs(@cha.health_file.content) if @cha.present? && @cha.health_file.present? && @cha.health_file.content_type == 'application/pdf'
+      pdf << PdfGenerator.merge_inline_pdfs(@form.health_file.content) if @form.present? && @form.is_a?(Health::SelfSufficiencyMatrixForm) && @form.health_file.present?
+      PdfGenerator.merge_inline_pdfs(pdf)
     end
   end
 end

app/controllers/health/aco_signature_requests_controller.rb

@@ -42,7 +42,7 @@ def edit
     def download_careplan
       pdf = careplan_combine_pdf_object
       file_name = 'care_plan'
-      send_data pdf.to_pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
+      send_data pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
     end
 
     # Build and send a PCP signing request and team member based on the values submitted

app/controllers/health/careplans_controller.rb

@@ -58,7 +58,7 @@ def index
     def show
       pdf = careplan_combine_pdf_object
       file_name = "care_plan_#{@careplan.updated_at.to_fs(:db)}"
-      send_data pdf.to_pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
+      send_data pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
     end
 
     def edit
@@ -119,15 +119,15 @@ def update
     def coversheet
       pdf = careplan_pdf_coversheet
       file_name = "care_plan_coversheet_#{@careplan.updated_at.to_fs(:db)}"
-      send_data pdf.to_pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
+      send_data pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
     end
 
     def pctp
       @document = 'pctp'
       pdf = careplan_pdf_coversheet
       pdf << careplan_pdf_pctp
       file_name = "care_plan_pctp_#{@careplan.updated_at.to_fs(:db)}"
-      send_data pdf.to_pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
+      send_data pdf, filename: "#{file_name}.pdf", type: 'application/pdf'
     end
 
     def form_url

app/controllers/user_training_controller.rb

@@ -61,27 +61,26 @@ def index
             end
             course_url = lms.course_url(config, course_id, redirect_url, logout_talentlms_url)
 
-            course_redirects << course_url
+            course_redirects << { course: course, url: course_url }
             configs_with_required_courses << config
           end
         end
 
         account_exists_in_all_configs = config_logins.values.all?(true)
-        number_configs_with_courses_to_complete = configs_with_required_courses.uniq.count
 
-        # If we only have one config with trainings required, send the user directly to the training portal
-        if course_redirects.present? && number_configs_with_courses_to_complete == 1
+        # If the user only has one required training course to complete, and that course's config
+        # allows automatic redirects, send them directly to the training portal
+        if course_redirects.present? && course_redirects.count == 1 && course_redirects.first[:course].config.allow_automatic_redirect_to_course
           # redirect to the course training
-          redirect_to course_redirects.first, allow_other_host: true
+          redirect_to course_redirects.first[:url], allow_other_host: true
           return
-        # If the user has an account in all configs and has no trainings left to complete, allow them to navigate the warehouse
+        # If the user has an active account in all configs and has no trainings left to complete, allow them to navigate the warehouse
         elsif account_exists_in_all_configs && course_redirects.blank?
           # All trainings are completed and the user has an account in all training configs
           redirect_to after_sign_in_path_for(current_user)
           return
         end
-        # At least one config requires an account to be created for this user or multiple configs have been
-        # identified as requiring trainings. Send the user to the captive portal for additional training options.
+        # For all other cases, send the user to the captive portal
         render 'required_trainings'
       rescue RuntimeError => e
         @message = e.message

app/helpers/application_helper.rb

@@ -492,4 +492,14 @@ def bracket_small_population(value, mask: true)
       bracket.first
     end
   end
+
+  # Provides a generic mechanism to show an action menu if there is more than one item, button, if only one
+  # Expects an array of objects called items in the following format
+  # [{ link_to: { path: '/hud_reports/aprs/new?filter%5Bactive_roi%5D=false...'}, icon: :copy, label: 'Clone report' }, { link_to: { path: '/hud_reports/aprs/111', method: :delete }, icon: :cross, label: 'Delete' }]
+  def action_menu_or_button(items:)
+    return if items.empty?
+    return render('/common/action_menu', items: items) if items.many?
+
+    render('/common/action_button', item: items.sole)
+  end
 end

app/jobs/health/process_enrollment_changes_job.rb

@@ -71,6 +71,8 @@ def perform(enrollment_id)
             disenroll_patient(transaction, referral, file_date)
             disenrolled_patients += 1
           end
+        rescue Health::MedicaidIdConflict
+          errors << conflict_message(transaction)
         end
 
         enrollment.changes.each do |transaction|
@@ -139,7 +141,6 @@ def perform(enrollment_id)
             update_patient_referrals(referral.patient, transaction)
             updated_patients += 1
           end
-
         rescue Health::MedicaidIdConflict
           # The conflict prevents us from knowing the audit action
           errors << conflict_message(transaction)

app/models/concerns/hud_concerns/client.rb

@@ -58,6 +58,9 @@ def race_multi
         end
         # Per the data standards, only look to RaceNone if we don't have a more specific response
         gm << self.RaceNone if gm.empty? && self.RaceNone.in?([8, 9, 99])
+        # set to '99' (Data not collected). This should only occur if the field is missing or we have data that doesn't
+        # match a known race code, including codes for Client doesn't know, Prefers not to answer, and Data not collected
+        gm << HudUtility2024.race_field_name_to_id[:RaceNone] if gm.empty?
       end
     end
 
@@ -70,6 +73,9 @@ def race_multi_include_race_none
         end
         # Always include RaceNone data
         gm << self.RaceNone if self.RaceNone.in?([8, 9, 99])
+        # set to '99' (Data not collected). This should only occur if the field is missing or we have data that doesn't
+        # match a known race code, including codes for Client doesn't know, Prefers not to answer, and Data not collected
+        gm << HudUtility2024.race_field_name_to_id[:RaceNone] if gm.empty?
       end
     end
 

app/models/concerns/hud_reports/incomes.rb

@@ -203,14 +203,22 @@ module HudReports::Incomes
           {
             'Adults with Missing Income Information' => a_t[:annual_assessment_expected].eq(true).
               and(a_t[:annual_assessment_in_window].eq(true)).
-              and(a_t["income_total_at_#{suffix}"].eq(99).
-                or(a_t["income_total_at_#{suffix}"].eq(nil))),
+              and(a_t["income_from_any_source_at_#{suffix}".to_sym].eq(nil).
+                or(a_t["income_from_any_source_at_#{suffix}"].eq(99)).
+                or(
+                  a_t["income_total_at_#{suffix}"].eq(nil).
+                  and(
+                    # Responses of 8 & 9 are expected to have total income nil.
+                    # These are filtered out to prevent duplicates. They are captured in a different row.
+                    a_t["income_from_any_source_at_#{suffix}"].not_in([8, 9]),
+                  ),
+                )),
           },
         )
       else
         responses.merge!(
           {
-            'Adults with Missing Income Information' => a_t["income_total_at_#{suffix}"].eq(99).
+            'Adults with Missing Income Information' => a_t["income_from_any_source_at_#{suffix}"].eq(99).
               or(a_t["income_total_at_#{suffix}"].eq(nil)),
           },
         )

app/models/concerns/hud_reports/start_to_move_in_question.rb

@@ -67,12 +67,15 @@ def start_to_move_in_lengths
       cond = lengths.fetch(label).and(a_t[:hoh_move_in_date].between(@report.start_date..@report.end_date))
       [label, cond]
     end
+    # This is the largest amount of days being reported on so we can set a limit in the total.
+    max_days_in_query = 730
 
+    # Make sure totals only include time to move in dates within the ranges being reported on
     ret.merge(
-      'Total (persons moved into housing)' => a_t[:hoh_move_in_date].between(@report.start_date..@report.end_date),
+      'Total (persons moved into housing)' => a_t[:hoh_move_in_date].between(@report.start_date..@report.end_date).and(a_t[:time_to_move_in].between(0..max_days_in_query)),
       'Average length of time to housing' => :average,
       'Persons who were exited without move-in' => a_t[:hoh_move_in_date].eq(nil),
-      'Total persons' => Arel.sql('1=1'),
+      'Total persons' => a_t[:time_to_move_in].between(0..max_days_in_query).or(a_t[:hoh_move_in_date].eq(nil)),
     ).freeze
   end
 end

app/models/concerns/hud_reports/sub_populations_by_subsidy_type_question.rb

@@ -38,7 +38,8 @@ def sub_populations_by_subsidy_type_question(question:, members:, sub_pops: sub_
       end
       sheet.update_cell_members(
         cell: [cols[col_index], 13],
-        members: scope,
+        # We only want clients captured in the rows above. Filter the scope to clients with a valid subsidy type
+        members: scope.where(HudUtility2024.rental_subsidy_types.keys.include?(a_t[:exit_destination_subsidy_type])),
       )
     end
   end

app/models/grda_warehouse/auth_policies/destination_client_policy.rb

@@ -17,6 +17,9 @@ class GrdaWarehouse::AuthPolicies::DestinationClientPolicy < GrdaWarehouse::Auth
     method_name = :"#{permission}?"
     define_method(method_name) do
       client.source_clients.any? do |source_client|
+        # Skip sources that are also destinations. This shouldn't be necessary but avoids SystemStackError on bad data
+        next if source_client.destination?(strict: true)
+
         user.policy_for(source_client).send(method_name)
       end
     end
@@ -27,7 +30,7 @@ class GrdaWarehouse::AuthPolicies::DestinationClientPolicy < GrdaWarehouse::Auth
 
   def validate_resource!(arg)
     ensure_arg_type!(arg, GrdaWarehouse::Hud::Client)
-    raise ArgumentError 'Must be a destination client' unless arg.destination?
+    raise ArgumentError 'Must be a destination client' unless arg.destination?(strict: true)
   end
 
   def client

app/models/grda_warehouse/auth_policies/source_client_policy.rb

@@ -74,6 +74,8 @@ def client
     results
   end
 
+  BASIC_CLIENT_PII_PERMS = Set.new([:can_view_client_name, :can_view_client_photo, :can_view_full_dob]).freeze
+
   # Window data sources are a deprecated legacy client data sharing mechanic, replaced by a System Collection when using Access Controls-based permissions
   def add_legacy_data_source_permissions(results)
     # is this a user with legacy role-based perms?
@@ -83,7 +85,33 @@ def add_legacy_data_source_permissions(results)
     # is the client in a window data source?
     return unless context.legacy_window_data_source_ids.include?(client.data_source_id)
 
-    # check roi if the window config requires release (the "can_*_with_roi" permissions are not relevant in this case)
+    # Legacy visibility rules for client attributes:
+    # If a user has either 'can_view_clients' or 'can_search_all_clients' permission, AND they
+    # have another permission (like viewing names), then the user is granted that permission
+    # for ANY client in window data sources, bypassing ROI requirements.
+    #
+    # For example: A user with both 'can_view_clients' and 'can_view_name' permissions can
+    # see names of all clients in window data sources, regardless of the client's ROI.
+    #
+    # Historical context: This behavior comes from the legacy role-based system where client
+    # visibility was considered "global" if the user could access clients in either "search"
+    # or "view" contexts, but only for "window" data sources.
+    if legacy_permissions.include?(:can_view_clients)
+      # all the legacy perms apply to the client
+      results.merge(legacy_permissions)
+      # early return since there's no point in checking ROI
+      return
+    elsif legacy_permissions.include?(:can_search_all_clients)
+      # The can_search_all_clients confers a reduced set or permissions. This is more restricted
+      # than the historic permissions. This is okay since search has limited client details.
+      #
+      # Notes
+      # - The client search controller (ClientAccessControl::ClientsController) also requires can_search_window || can_use_strict_search. We aren't enforcing that here.
+      # - See the searchable_to method in the Client extension (drivers/client_access_control/extensions/grda_warehouse/hud/client_extension.rb) which includes all clients in the search scope if the user has the can_search_all_clients permission
+      results.merge(legacy_permissions & BASIC_CLIENT_PII_PERMS)
+    end
+
+    # check ROI if the window config requires release (the "can_*_with_roi" permissions are not relevant in this case)
     return if context.legacy_window_access_requires_release? && !roi_authorized?
 
     results.merge(legacy_permissions)

app/models/grda_warehouse/client_roi_authorization.rb

@@ -14,6 +14,8 @@ class ClientRoiAuthorization < GrdaWarehouseBase
     PARTIAL_STATUS = 'partial'.freeze
     FULL_STATUS = 'full'.freeze
 
+    scope :with_invalid_client, -> { left_outer_joins(:destination_client).where(c_t[:id].eq(nil)) }
+
     def active?(date: Date.current)
       case status
       when PARTIAL_STATUS, FULL_STATUS