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.

Sign up for GitHub

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

FFS-2110: Update flow to read/write indexing data to CbvApplicant #438

Merged
merged 16 commits into from
Feb 12, 2025
Merged

FFS-2110: Update flow to read/write indexing data to CbvApplicant #438

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
72f714b
Rename CbvClient -> CbvApplicant
tdooner Feb 5, 2025
bb55e9d
Write CbvApplicant fields directly from Caseworker form
tdooner Feb 6, 2025
be4b100
Use `accepts_nested_attributes_for` in CbvFlow/CbvFlowInvitation
tdooner Feb 6, 2025
7e17957
Add data redaction for CbvApplicant
tdooner Feb 6, 2025
2fa708f
Convert final accessings of indexing data over to `cbv_applicant`
tdooner Feb 6, 2025
2a00e03
Merge remote-tracking branch 'origin/main' into td/ffs-2110-update-fl…
tdooner Feb 6, 2025
3c6c91b
Fix API error translation
tdooner Feb 6, 2025
6516d3e
Track `cbv_applicant_id` as attribute on all events
tdooner Feb 6, 2025
aec6fa0
Fix flaky rake tests by always loading Rake tasks once
tdooner Feb 6, 2025
a50ffd5
Merge remote-tracking branch 'origin/main' into td/ffs-2110-update-fl…
tdooner Feb 7, 2025
579a785
Prevent duplicate CbvApplicant creation in Api::InvitationsController
tdooner Feb 7, 2025
3871ef1
Remove redact_applicants method in DataRetentionService
tdooner Feb 7, 2025
a1b67b8
Update invitation form for NYC and MA as well
tdooner Feb 7, 2025
436a2a1
Fix crash on /summary page when an identity job fails
tdooner Feb 7, 2025
8d7b98f
Merge remote-tracking branch 'origin/main' into td/ffs-2110-update-fl…
tdooner Feb 12, 2025
fefe2b7
Merge remote-tracking branch 'origin/main' into td/ffs-2110-update-fl…
tdooner Feb 12, 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
61 changes: 43 additions & 18 deletions app/app/controllers/api/invitations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ class Api::InvitationsController < ApplicationController
before_action :authenticate

def create
@cbv_flow_invitation = CbvInvitationService.new(event_logger).invite(cbv_flow_invitation_params, @current_user, delivery_method: nil)
@cbv_flow_invitation = CbvInvitationService.new(event_logger)
.invite(cbv_flow_invitation_params, @current_user, delivery_method: nil)

if @cbv_flow_invitation.errors.any?
return render json: @cbv_flow_invitation.errors, status: :unprocessable_entity
errors = @cbv_flow_invitation.errors
if errors.any?
return render json: errors_to_json(errors), status: :unprocessable_entity
end

@cbv_client = CbvClient.create_from_invitation(@cbv_flow_invitation)

render json: {
url: @cbv_flow_invitation.to_url,
expiration_date: @cbv_flow_invitation.expires_at,
Expand All @@ -21,31 +21,56 @@ def create

# can these be inferred from the model?
def cbv_flow_invitation_params
if (applicant_attributes = params.delete(:agency_partner_metadata))
params[:cbv_applicant_attributes] = applicant_attributes.merge(site_id: params[:site_id])
end
params[:email_address] = @current_user.email

params.permit(
:first_name,
:middle_name,
:language,
:last_name,
:client_id_number,
:case_number,
:email_address,
:snap_application_date,
:agency_id_number,
:beacon_id,
:site_id,
:user_id,
:site_id
cbv_applicant_attributes: [
:first_name,
:middle_name,
:last_name,
:site_id,
:client_id_number,
:case_number,
:snap_application_date,
:agency_id_number,
:beacon_id
]
)
end

def site_id
cbv_flow_invitation_params[:site_id]
end

private

def authenticate
authenticate_or_request_with_http_token do |token, options|
@current_user = User.find_by_access_token(token)
end
end

def errors_to_json(errors)
# Generates a Hash of attribute => error_message and translates the
# internal names of objects (cbv_applicant) to the external names
# (agency_partner_metadata)
errors.map do |error|
next if error.attribute == :cbv_applicant

error_message = error.message

case error
when ActiveModel::NestedError
prefix, attribute_name = error.attribute.to_s.split(".")
prefix = "agency_partner_metadata" if prefix == "cbv_applicant"

[ "#{prefix}.#{attribute_name}", error_message ]
else
[ error.attribute, error_message ]
end
end.compact.to_h
end
end
2 changes: 2 additions & 0 deletions app/app/controllers/api/pinwheel_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def user_action
base_attributes = {
timestamp: Time.now.to_i,
cbv_flow_id: @cbv_flow.id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
}
event_name = user_action_params[:event_name]
Expand Down Expand Up @@ -87,6 +88,7 @@ def token_params
def track_event
event_logger.track("ApplicantBeganLinkingEmployer", request, {
cbv_flow_id: @cbv_flow.id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
response_type: token_params[:response_type]
})
Expand Down
54 changes: 16 additions & 38 deletions app/app/controllers/caseworker/cbv_flow_invitations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,24 @@ def new
end

def create
invitation_params = base_params.merge(site_specific_params)
# handle errors from the mail service
begin
@cbv_flow_invitation = CbvInvitationService.new(event_logger).invite(
invitation_params,
invitation_params.deep_merge(site_id: site_id, cbv_applicant_attributes: { site_id: site_id }),
current_user,
delivery_method: :email
)
rescue => e
Rails.logger.error("Error inviting applicant: #{e.message}")
flash[:alert] = t(".invite_failed",
email_address: cbv_flow_invitation_params[:email_address],
email_address: invitation_params[:email_address],
error_message: e.message)
return redirect_to caseworker_dashboard_path(site_id: params[:site_id])
end

if @cbv_flow_invitation.errors.any?
@cbv_flow_invitation.errors.delete(:cbv_applicant)
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. During an error scenario, because it's a separate model, we have to delete it. I'm surprised there isn't a way to configure this as part of validations? Could you say:

accepts_nested_attributes_for :asdf, :reject_if => :something_bad

Copy link
Contributor Author

@tdooner tdooner Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is only an issue because we iterate over the whole @cbv_flow_invitation.errors hash to generate the error alert message at the top of the page (e.g. "5 Errors: [list of errors]").

With cbv_applicant in the .errors hash, the count one-too-many and there is an error message for cbv_applicant that isn't meant to be user-facing.


error_count = @cbv_flow_invitation.errors.size
error_header = "#{helpers.pluralize(error_count, 'error')} occurred"

Expand All @@ -44,11 +45,8 @@ def create
return render :new, status: :unprocessable_entity
end

# hydrate the cbv_client with the invitation if there are no cbv_flow_invitation errors
@cbv_client = CbvClient.create_from_invitation(@cbv_flow_invitation)

flash[:slim_alert] = {
message: t(".invite_success", email_address: cbv_flow_invitation_params[:email_address]),
message: t(".invite_success", email_address: invitation_params[:email_address]),
type: "success"
}
redirect_to caseworker_dashboard_path(site_id: params[:site_id])
Expand All @@ -69,40 +67,20 @@ def ensure_valid_params!
end
end

def base_params
cbv_flow_invitation_params.slice(
:first_name,
:middle_name,
:language,
:last_name,
:email_address,
:snap_application_date,
).merge(site_id: site_id)
end

def site_specific_params
case site_id
when "ma"
cbv_flow_invitation_params.slice(:agency_id_number, :beacon_id)
when "nyc"
cbv_flow_invitation_params.slice(:client_id_number, :case_number)
else
{}
end
end

def cbv_flow_invitation_params
def invitation_params
params.fetch(:cbv_flow_invitation, {}).permit(
:first_name,
:middle_name,
:language,
:last_name,
:client_id_number,
:case_number,
:email_address,
:snap_application_date,
:agency_id_number,
:beacon_id,
cbv_applicant_attributes: [
:first_name,
:middle_name,
:last_name,
:client_id_number,
:case_number,
:snap_application_date,
:agency_id_number,
:beacon_id
]
)
end

Expand Down
2 changes: 2 additions & 0 deletions app/app/controllers/cbv/base_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ def track_timeout_event
def track_expired_event(invitation)
event_logger.track("ApplicantLinkExpired", request, {
invitation_id: invitation.id,
cbv_applicant_id: invitation.cbv_applicant_id,
timestamp: Time.now.to_i
})
rescue => ex
Expand All @@ -121,6 +122,7 @@ def track_invitation_clicked_event(invitation, cbv_flow)
timestamp: Time.now.to_i,
invitation_id: invitation.id,
cbv_flow_id: cbv_flow.id,
cbv_applicant_id: cbv_flow.cbv_applicant_id,
site_id: cbv_flow.site_id,
seconds_since_invitation: (Time.now - invitation.created_at).to_i
})
Expand Down
4 changes: 4 additions & 0 deletions app/app/controllers/cbv/employer_searches_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ def fetch_employers(query = "")
def track_clicked_popular_payroll_providers_event
event_logger.track("ApplicantClickedPopularPayrollProviders", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand All @@ -46,6 +47,7 @@ def track_clicked_popular_payroll_providers_event
def track_clicked_popular_app_employers_event
event_logger.track("ApplicantClickedPopularAppEmployers", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand All @@ -58,6 +60,7 @@ def track_accessed_search_event

event_logger.track("ApplicantAccessedSearchPage", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand All @@ -70,6 +73,7 @@ def track_applicant_searched_event

event_logger.track("ApplicantSearchedForEmployer", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
num_results: @employers.length,
Expand Down
2 changes: 2 additions & 0 deletions app/app/controllers/cbv/entries_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ def show
event_logger.track("ApplicantViewedAgreement", request, {
timestamp: Time.now.to_i,
site_id: @cbv_flow.site_id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand All @@ -13,6 +14,7 @@ def create
event_logger.track("ApplicantAgreed", request, {
timestamp: Time.now.to_i,
site_id: @cbv_flow.site_id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand Down
1 change: 1 addition & 0 deletions app/app/controllers/cbv/missing_results_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def show
def track_missing_results_event
event_logger.track("ApplicantAccessedMissingResultsPage", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand Down
2 changes: 2 additions & 0 deletions app/app/controllers/cbv/payment_details_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ def track_viewed_event
return if @pinwheel_account.nil?

event_logger.track("ApplicantViewedPaymentDetails", request, {
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
pinwheel_account_id: @pinwheel_account.id,
Expand All @@ -142,6 +143,7 @@ def track_saved_event
comment_data = @cbv_flow.additional_information[params[:user][:account_id]]

event_logger.track("ApplicantSavedPaymentDetails", request, {
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
additional_information_length: comment_data ? comment_data["comment"].length : 0
Expand Down
1 change: 1 addition & 0 deletions app/app/controllers/cbv/successes_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def show
def track_accessed_success_event
event_logger.track("ApplicantAccessedSuccessPage", request, {
timestamp: Time.now.to_i,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id
})
Expand Down
21 changes: 12 additions & 9 deletions app/app/controllers/cbv/summaries_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ def show
event_logger.track("ApplicantDownloadedIncomePDF", request, {
timestamp: Time.now.to_i,
site_id: @cbv_flow.site_id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
locale: I18n.locale
Expand Down Expand Up @@ -101,7 +102,7 @@ def transmit_to_caseworker
time_now = Time.now
beginning_date = (Date.parse(@payments_beginning_at).strftime("%b") rescue @payments_beginning_at)
ending_date = (Date.parse(@payments_ending_at).strftime("%b%Y") rescue @payments_ending_at)
@file_name = "IncomeReport_#{@cbv_flow.cbv_flow_invitation.agency_id_number}_" \
@file_name = "IncomeReport_#{@cbv_flow.cbv_applicant.agency_id_number}_" \
"#{beginning_date}-#{ending_date}_" \
"Conf#{@cbv_flow.confirmation_code}_" \
"#{time_now.strftime('%Y%m%d%H%M%S')}"
Expand Down Expand Up @@ -166,16 +167,16 @@ def generate_csv
pinwheel_account = PinwheelAccount.find_by(cbv_flow_id: @cbv_flow.id)

data = {
client_id: @cbv_flow.cbv_flow_invitation.agency_id_number,
first_name: @cbv_flow.cbv_flow_invitation.first_name,
last_name: @cbv_flow.cbv_flow_invitation.last_name,
middle_name: @cbv_flow.cbv_flow_invitation.middle_name,
client_id: @cbv_flow.cbv_applicant.agency_id_number,
first_name: @cbv_flow.cbv_applicant.first_name,
last_name: @cbv_flow.cbv_applicant.last_name,
middle_name: @cbv_flow.cbv_applicant.middle_name,
client_email_address: @cbv_flow.cbv_flow_invitation.email_address,
beacon_userid: @cbv_flow.cbv_flow_invitation.beacon_id,
app_date: @cbv_flow.cbv_flow_invitation.snap_application_date.strftime("%m/%d/%Y"),
beacon_userid: @cbv_flow.cbv_applicant.beacon_id,
app_date: @cbv_flow.cbv_applicant.snap_application_date.strftime("%m/%d/%Y"),
report_date_created: pinwheel_account.created_at.strftime("%m/%d/%Y"),
report_date_start: @cbv_flow.cbv_flow_invitation.paystubs_query_begins_at.strftime("%m/%d/%Y"),
report_date_end: @cbv_flow.cbv_flow_invitation.snap_application_date.strftime("%m/%d/%Y"),
report_date_start: @cbv_flow.cbv_applicant.paystubs_query_begins_at.strftime("%m/%d/%Y"),
report_date_end: @cbv_flow.cbv_applicant.snap_application_date.strftime("%m/%d/%Y"),
confirmation_code: @cbv_flow.confirmation_code,
consent_timestamp: @cbv_flow.consented_to_authorized_use_at.strftime("%m/%d/%Y %H:%M:%S"),
pdf_filename: "#{@file_name}.pdf",
Expand All @@ -191,6 +192,7 @@ def track_transmitted_event(cbv_flow, payments)
event_logger.track("ApplicantSharedIncomeSummary", request, {
timestamp: Time.now.to_i,
site_id: cbv_flow.site_id,
cbv_applicant_id: cbv_flow.cbv_applicant_id,
cbv_flow_id: cbv_flow.id,
invitation_id: cbv_flow.cbv_flow_invitation_id,
account_count: payments.map { |p| p[:account_id] }.uniq.count,
Expand All @@ -209,6 +211,7 @@ def track_accessed_income_summary_event(cbv_flow, payments)
timestamp: Time.now.to_i,
site_id: cbv_flow.site_id,
cbv_flow_id: cbv_flow.id,
cbv_applicant_id: cbv_flow.cbv_applicant_id,
invitation_id: cbv_flow.cbv_flow_invitation_id,
account_count: payments.map { |p| p.account_id }.uniq.count,
paystub_count: payments.count,
Expand Down
23 changes: 13 additions & 10 deletions app/app/controllers/help_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@ def show

cbv_flow = session[:cbv_flow_id] ? CbvFlow.find_by(id: session[:cbv_flow_id]) : nil

event_logger.track("ApplicantViewedHelpTopic", request, {
topic: @help_topic,
cbv_flow_id: session[:cbv_flow_id],
invitation_id: cbv_flow&.cbv_flow_invitation_id,
site_id: current_site&.id,
flow_started_seconds_ago: cbv_flow ? (Time.now - cbv_flow.created_at).to_i : nil,
language: I18n.locale
}.compact)
rescue => ex
Rails.logger.error "Unable to track event (ApplicantViewedHelpTopic): #{ex}"
begin
event_logger.track("ApplicantViewedHelpTopic", request, {
topic: @help_topic,
cbv_applicant_id: cbv_flow&.cbv_applicant_id,
cbv_flow_id: session[:cbv_flow_id],
invitation_id: cbv_flow&.cbv_flow_invitation_id,
site_id: current_site&.id,
flow_started_seconds_ago: cbv_flow ? (Time.now - cbv_flow.created_at).to_i : nil,
language: I18n.locale
})
rescue => ex
Rails.logger.error "Unable to track event (ApplicantViewedHelpTopic): #{ex}"
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait, I don't understand why. Is it bad form to use that approach to rescues (on the method)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rescue on a method is fine in general, but I didn't like it in this case because it might accidentally rescue exceptions for the other parts of the method that aren't related to event sending. It would make debugging the situation harder because we wouldn't get the actual exception reported to NewRelic.

end

private
Expand Down
2 changes: 2 additions & 0 deletions app/app/controllers/webhooks/pinwheel/events_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def authorize_webhook

def track_account_synced_event(cbv_flow, pinwheel_account)
event_logger.track("ApplicantFinishedPinwheelSync", request, {
cbv_applicant_id: cbv_flow.cbv_applicant_id,
cbv_flow_id: cbv_flow.id,
invitation_id: cbv_flow.cbv_flow_invitation_id,
identity_success: pinwheel_account.job_succeeded?("identity"),
Expand All @@ -77,6 +78,7 @@ def track_account_synced_event(cbv_flow, pinwheel_account)

def track_account_created_event(cbv_flow, platform_name)
event_logger.track("ApplicantCreatedPinwheelAccount", request, {
cbv_applicant_id: cbv_flow.cbv_applicant_id,
cbv_flow_id: cbv_flow.id,
invitation_id: cbv_flow.cbv_flow_invitation_id,
platform_name: platform_name
Expand Down
Loading