Add Match Route Eleven as Clone of Provider Only Route #820

dtgreiner · 2024-06-24T12:48:58Z

Please squash merge this PR

Description

Adds Match Route Eleven as a clone of the Provider Only route but with its own version of notifications, contacts, routes

Type of change

New feature (adds functionality)

Checklist before requesting review

I have performed a self-review of my code
I have run the code that is being changed under ideal conditions, and it doesn't fail
My code includes comments and/or descriptive variable names to help other engineers understand the intent (or not applicable)
My code follows the style guidelines of this project (rubocop)
I have updated the documentation (or not applicable)
If it's not obvious how to test this change, I have provided testing instructions in this PR or the related issue

eanders · 2024-06-24T13:42:03Z

app/models/match_decisions/eleven/eleven_confirm_hsa_accepts_client_decline_dnd_staff.rb

+
+      def decline_confirmed
+        match.rejected!
+        # TODO maybe rerun the matching engine for that vacancy and client


I think we can remove this:

Suggested change

# TODO maybe rerun the matching engine for that vacancy and client

eanders · 2024-06-24T13:43:44Z

app/models/match_decisions/eleven/eleven_confirm_hsa_accepts_client_decline_dnd_staff.rb

+module MatchDecisions::Eleven
+  class ElevenConfirmHsaAcceptsClientDeclineDndStaff < ::MatchDecisions::Base
+    def to_partial_path
+      'match_decisions/eleven_confirm_hsa_accepts_client_decline_dnd_staff'


How much would it break things if we started putting the decisions into a folder?

Suggested change

'match_decisions/eleven_confirm_hsa_accepts_client_decline_dnd_staff'

'match_decisions/eleven/confirm_hsa_accepts_client_decline_dnd_staff'

Seems like that would help tracking these down in the future for duplication etc.

From an initial test, it doen't look like it is breaking anything, but I'll do a bit more to get some more thorough results before including in in the next commit.

eanders · 2024-06-24T13:50:34Z

app/models/match_decisions/eleven/eleven_hsa_acknowledges_receipt.rb

+
+    def accessible_by? contact
+      contact.user_can_act_on_behalf_of_match_contacts? ||
+      contact.in?(match.housing_subsidy_admin_contacts)


We seem to have cleaned up contact_actor_type in some decisions. Can you take a look at how we're using it in app/models/match_decisions/approve_match_housing_subsidy_admin.rb? I think we want this to be:

contact.in?(match.send(contact_actor_type))

and

def contact_actor_type nil end

to be:

def contact_actor_type :housing_subsidy_admin_contacts end

Looks like most (all?) of these decisions could be cleaned up that way.

app/models/match_routes/eleven.rb

eanders · 2024-06-24T13:54:04Z

app/models/match_routes/eleven.rb

+    end
+
+    def contact_label_for(contact_type)
+      case contact_type


I wonder if this should:

Suggested change

case contact_type

case contact_type&.to_sym

eanders · 2024-06-24T13:57:37Z

app/views/notifications_mailer/eleven_match_initiation_for_hsa.text.haml

+\
+The CAS has generated a new match recommendation.  
+\
+Please acknowledge receipt of this match and review the associated HSA contacts.


This should have a translation for HSA

eanders · 2024-06-24T13:58:26Z

app/views/notifications_mailer/hsa_decision_client.text.haml

@@ -2,7 +2,7 @@ Hello #{@contact.full_name},
 \
 = "A #{Translation.translate('Housing Subsidy Administrator')} has made a decision on a housing opportunity for which the client below applied."
 \
-Decision: #{@match.hsa_accepts_client_decision.status_label}
+Decision: #{@match.eleven_hsa_accepts_client_decision.status_label}


This looks like a mistake, since this is probably the original file for the provider only route

eanders · 2024-06-24T13:58:32Z

app/views/notifications_mailer/hsa_decision_hsp.text.haml

@@ -2,7 +2,7 @@ Hello #{@contact.full_name},
 \
 = "A #{Translation.translate('Housing Subsidy Administrator')} has made a decision on a housing opportunity for which the client below applied."
 \
-Decision: #{@match.hsa_accepts_client_decision.status_label}
+Decision: #{@match.eleven_hsa_accepts_client_decision.status_label}


This looks like a mistake, since this is probably the original file for the provider only route

eanders · 2024-06-24T13:58:38Z

app/views/notifications_mailer/hsa_decision_shelter_agency.text.haml

@@ -2,7 +2,7 @@ Hello #{@contact.full_name},
 \
 = "A #{Translation.translate('Housing Subsidy Administrator')} has made a decision on a housing opportunity for which the client below applied."
 \
-Decision: #{@match.hsa_accepts_client_decision.status_label}
+Decision: #{@match.eleven_hsa_accepts_client_decision.status_label}


This looks like a mistake, since this is probably the original file for the provider only route

eanders · 2024-06-24T13:58:42Z

app/views/notifications_mailer/hsa_decision_ssp.text.haml

@@ -2,7 +2,7 @@ Hello #{@contact.full_name},
 \
 = "A #{Translation.translate('Housing Subsidy Administrator')} has made a decision on a housing opportunity for which the client below applied."
 \
-Decision: #{@match.hsa_accepts_client_decision.status_label}
+Decision: #{@match.eleven_hsa_accepts_client_decision.status_label}


This looks like a mistake, since this is probably the original file for the provider only route

you are correct, this and the others you've noted should not be here. I'm flagging them to be revereted in the next commit.

eanders · 2024-06-24T14:00:22Z

db/migrate/20240621125508_match_route_eleven_translations.rb

+class MatchRouteElevenTranslations < ActiveRecord::Migration[7.0]
+  def up
+    translations_eleven.each do |k, v|
+      Translation.create!(
+        key: k,
+        text: v,
+      )
+    end
+  end
+
+  def down
+    Translation.where(key: translations_eleven.keys).destroy_all
+  end
+
+  private
+
+  def translations_eleven = {
+    'Match Route Eleven' => 'Match Route Eleven',
+    'SSP Eleven' => 'SSP',
+    'Shelter Agency Eleven' => 'Shelter Agency',
+    'HSA Eleven' => 'HSA',
+    'Housing Search Provider Eleven' => 'Housing Search Provider',
+    'Housing Subsidy Administrator Eleven' => 'Housing Subsidy Administrator',
+    'Stabilization Service Provider Eleven' => 'Stabilization Services Provider',
+    'Stabilization Services Provider Eleven' => 'Stabilization Services Provider',
+    'CoC Eleven' => 'DND',
+  }
+end


Hmm, I could be wrong, and maybe we want some default translations in there, but I think you can leave this off and the app will add them via a cron job and/or if you visit something that needs it. (It would just add the untranslated value though, so maybe something like this is useful.)

From my understanding the app will automatically add these transalations. I'll remove this file and we can add it back in if needed. I saw a previous match route setting the default translations via script so included it here based on that knowledge.

eanders

Thanks @dtgreiner, it generally looks really good.
I added a couple thoughts on cleaning things up a bit, let me know if you have questions.

eanders

Thanks @dtgreiner, looks great!

dtgreiner added 3 commits June 24, 2024 08:44

Add Match Route Eleven as Clone of Provider Only Route

442e9e2

rubocop errors

a82f1c5

more rubocop fixes

9d8cd98

dtgreiner requested a review from eanders June 24, 2024 13:06

eanders reviewed Jun 24, 2024

View reviewed changes

app/models/match_routes/eleven.rb Show resolved Hide resolved

eanders reviewed Jun 24, 2024

View reviewed changes

eanders requested changes Jun 24, 2024

View reviewed changes

dtgreiner added 2 commits June 24, 2024 13:49

Updates from code review

4cc7208

remove default translations

c238dad

eanders changed the base branch from release-59 to release-60 June 24, 2024 19:18

eanders approved these changes Jun 24, 2024

View reviewed changes

eanders merged commit 4187930 into release-60 Jun 24, 2024
16 checks passed

eanders deleted the dg/housing_navigation_rework_route_create_route_eleven-6123 branch June 24, 2024 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Match Route Eleven as Clone of Provider Only Route #820

Add Match Route Eleven as Clone of Provider Only Route #820

dtgreiner commented Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

dtgreiner Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

eanders Jun 24, 2024

dtgreiner Jun 24, 2024

eanders Jun 24, 2024

dtgreiner Jun 24, 2024

eanders left a comment

eanders left a comment

	'match_decisions/eleven_confirm_hsa_accepts_client_decline_dnd_staff'
	'match_decisions/eleven/confirm_hsa_accepts_client_decline_dnd_staff'

Add Match Route Eleven as Clone of Provider Only Route #820

Add Match Route Eleven as Clone of Provider Only Route #820

Conversation

dtgreiner commented Jun 24, 2024

Please squash merge this PR

Description

Type of change

Checklist before requesting review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eanders left a comment

Choose a reason for hiding this comment

eanders left a comment

Choose a reason for hiding this comment