-
Notifications
You must be signed in to change notification settings - Fork 13
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
FYST-526 Update refund taxes owed UI and add financial information to MD pdf/xml #5007
Changes from 36 commits
9ce7a6e
32d75ff
3166c36
f27a562
8e19444
c047de8
4dcf802
ad00864
82e10b0
75f7eec
9f2d157
5e37860
6ed6f77
8d67efc
2c0239a
09d242f
233092f
fa21585
526e07f
59268d4
d307903
836500d
f9c95ce
1548841
3952c13
bd4d521
a394bcc
62e0f95
1b5ba18
a59c336
34c7c01
9b3e7ed
f61bdea
d3608b4
f14a5e4
e9dc7c9
52575f1
56cccd1
f7479e1
1380324
6ba181f
b2219bd
8c565ba
32a3033
66c08cb
7fef8f3
0d68106
4d82db6
1778dde
a8d228c
8461769
fd6e3a1
0f98c19
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,29 @@ class MdTaxRefundForm < QuestionsForm | |
:routing_number, | ||
:account_number, | ||
:account_type, | ||
:bank_name, | ||
:account_holder_name | ||
:account_holder_first_name, | ||
:account_holder_middle_initial, | ||
:account_holder_last_name, | ||
:account_holder_suffix, | ||
:has_joint_account_holder, | ||
:joint_account_holder_first_name, | ||
:joint_account_holder_middle_initial, | ||
:joint_account_holder_last_name, | ||
:joint_account_holder_suffix, | ||
:bank_authorization_confirmed | ||
set_attributes_for :confirmation, :routing_number_confirmation, :account_number_confirmation | ||
|
||
validates :payment_or_deposit_type, presence: true | ||
|
||
with_options unless: -> { payment_or_deposit_type == "mail" } do | ||
validates :account_holder_name, presence: true | ||
SUFFIX_OPTIONS = %w[JR SR I II III IV V VI VII VIII IX X] | ||
|
||
validates :bank_name, presence: true | ||
with_options unless: -> { payment_or_deposit_type == "mail" } do | ||
validates :account_holder_first_name, presence: true | ||
validates :account_holder_last_name, presence: true | ||
validates :account_holder_first_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,15})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.first_name.invalid_format') }} | ||
validates :account_holder_last_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,31})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.last_name.invalid_format') }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ask Tiffany about following schema strictly? (allowing numbers in first/last name)
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. answered!
|
||
validates :account_holder_middle_initial, length: { maximum: 1 }, format: { with: /\A[A-Za-z]\z/.freeze, allow_blank: true } | ||
validates :account_holder_suffix, inclusion: { in: SUFFIX_OPTIONS }, allow_blank: true, if: -> { joint_account_holder_suffix.present? } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like these validations aren't tested? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! and they caught an error b/c i had |
||
validates :account_type, presence: true | ||
|
||
validates :account_number, presence: true, confirmation: true, length: { in: 5..17 }, numericality: true | ||
|
@@ -24,9 +37,20 @@ class MdTaxRefundForm < QuestionsForm | |
validates :routing_number, presence: true, confirmation: true, routing_number: true | ||
validates :routing_number_confirmation, presence: true | ||
|
||
validates :bank_authorization_confirmed, acceptance: { accept: 'yes', message: ->(_object, _data) { I18n.t("state_file.questions.md_tax_refund.md_bank_details.bank_authorization_confirmation_error") }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't see anything in the form spec about this field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed 32a3033 |
||
|
||
with_options if: -> { account_number.present? && routing_number.present? } do | ||
validate :bank_numbers_not_equal | ||
end | ||
|
||
with_options if: -> { has_joint_account_holder == 'yes' } do | ||
validates :joint_account_holder_first_name, presence: true | ||
validates :joint_account_holder_last_name, presence: true | ||
validates :joint_account_holder_first_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,15})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.first_name.invalid_format') }} | ||
validates :joint_account_holder_last_name, format: { with: /\A[a-zA-Z]{1}([A-Za-z\-\s']{0,137})\z/.freeze, message: ->(_object, _data) { I18n.t('errors.attributes.last_name.invalid_format') }} | ||
validates :joint_account_holder_middle_initial, length: { maximum: 1 }, format: { with: /\A[A-Za-z]\z/.freeze, allow_blank: true } | ||
validates :joint_account_holder_suffix, inclusion: { in: SUFFIX_OPTIONS }, allow_blank: true, if: -> { joint_account_holder_suffix.present? } | ||
end | ||
end | ||
|
||
def save | ||
|
@@ -35,7 +59,7 @@ def save | |
|
||
def self.existing_attributes(intake) | ||
attributes = super | ||
attributes.except(:routing_number, :account_number, :routing_number_confirmation, :account_number_confirmation, :bank_name) | ||
attributes.except(:routing_number, :account_number, :routing_number_confirmation, :account_number_confirmation) | ||
end | ||
|
||
private | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,6 +79,7 @@ def calculate | |
set_line(:MD502_LINE_26, :calculate_line_26) | ||
set_line(:MD502_LINE_27, :calculate_line_27) | ||
set_line(:MD502_LINE_40, :calculate_line_40) | ||
set_line(:MD502_AUTHORIZE_DIRECT_DEPOSIT, :calculate_authorize_direct_deposit) | ||
|
||
@md502cr.calculate | ||
@lines.transform_values(&:value) | ||
|
@@ -420,6 +421,10 @@ def calculate_line_40 | |
@intake.state_file1099_rs.sum { |item| item.state_tax_withheld_amount.round } | ||
end | ||
|
||
def calculate_authorize_direct_deposit | ||
@intake.bank_authorization_confirmed_yes? ? "X" : nil | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would love to discuss the following, probably on tuple: i know that the original intent of the calculators was for them to mainly hold values that are numbers or booleans used to calculate other lines. this doesn't look like it's used downstream anywhere so it could just be determined from the xml builder and left out of the calculator. either way i think i'd prefer not to have a calculator method/line set to the string "X" and instead return a boolean that can be converted to "X" in the xml builder (because the only reason for "X" is that it's required by an xml type). hopefully that makes sense! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, update: i went and looked around and i guess this calculator is littered with methods that return "X" lol. i can't think of a reason this functionally matters so idk if i'm going to recommend a refactor? again, we can discuss There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i asked em the same question when we were pairing on review. i was also curious because there's nothing being calculated here. em had said sometimes these methods are used elsewhere in the calculator for actual calculations. that doesn't seem to be the case here. 👀 i'm curious what y'all decide post tuple & discussion! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. upon discussion: let's make this return a boolean & in the XML set |
||
|
||
def filing_status_dependent? | ||
@filing_status == :dependent | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ def initialize(submission) | |
end | ||
|
||
def hash_for_pdf | ||
{ | ||
answers = { | ||
'Enter 1': @xml_document.at("Form502 Income FederalAdjustedGrossIncome")&.text, | ||
'Enter 1a': @xml_document.at("Form502 Income WagesSalariesAndTips")&.text, | ||
'Enter 1b': @xml_document.at("Form502 Income EarnedIncome")&.text, | ||
|
@@ -86,13 +86,24 @@ def hash_for_pdf | |
'Enter 3': @xml_document.at('Form502 Additions StateRetirementPickup')&.text, | ||
'Enter 6': @xml_document.at('Form502 Additions Total')&.text, | ||
'Enter 7': @xml_document.at('Form502 Additions FedAGIAndStateAdditions')&.text, | ||
"Enter 15": @xml_document.at('Form502 Subtractions Total')&.text, | ||
"Enter 16": @xml_document.at('Form502 Subtractions StateAdjustedGrossIncome')&.text, | ||
'Enter 15': @xml_document.at('Form502 Subtractions Total')&.text, | ||
'Enter 16': @xml_document.at('Form502 Subtractions StateAdjustedGrossIncome')&.text, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rubocop told me to make it single quotes 👮♂️ |
||
'Text Box 30': @xml_document.at('Form502 StateTaxComputation StateIncomeTax')&.text, | ||
'Text Box 36': @xml_document.at('Form502 StateTaxComputation PovertyLevelCredit')&.text, | ||
'Text Box 40': @xml_document.at('Form502 StateTaxComputation TotalCredits')&.text, | ||
'Text Box 42': @xml_document.at('Form502 StateTaxComputation StateTaxAfterCredits')&.text, | ||
'Check Box 39': @xml_document.at('Form502 AuthToDirectDepositInd')&.text == 'X' ? 'Yes' : 'Off', | ||
'Text Box 95': full_names_of_bank_account_holders || "" | ||
} | ||
if @xml_document.at('RefundDirectDeposit').present? | ||
answers.merge!({ | ||
'Check Box 41': @xml_document.at('Checking')&.text == "X" ? 'Yes' : 'Off', | ||
'Check Box 42': @xml_document.at('Savings')&.text == "X" ? 'Yes' : 'Off', | ||
'Text Box 93': @xml_document.at('RoutingTransitNumber')&.text, | ||
'Text Box 94': @xml_document.at('BankAccountNumber')&.text, | ||
}) | ||
end | ||
answers | ||
end | ||
|
||
def spouse_ssn_if_mfs | ||
|
@@ -131,6 +142,24 @@ def generate_codes_for_502_su | |
applicable_codes | ||
end | ||
|
||
def full_names_of_bank_account_holders | ||
intake = @submission.data_source | ||
return nil unless intake.payment_or_deposit_type.to_sym == :direct_deposit | ||
|
||
if intake.has_joint_account_holder_yes? | ||
account_holder_full_name + " and " + account_holder_full_name(for_joint: true) | ||
else | ||
account_holder_full_name | ||
end | ||
end | ||
|
||
def account_holder_full_name(for_joint: false) | ||
attributes = %w[FirstName MiddleInitial LastName NameSuffix] | ||
account_holder_xmls = @xml_document.css('Form502 NameOnBankAccount') | ||
account_holder_xml = for_joint ? account_holder_xmls[1] : account_holder_xmls[0] | ||
attributes.map { |attr| account_holder_xml.at(attr)&.text }.filter_map(&:presence).join(" ") | ||
end | ||
|
||
def calculated_fields | ||
@calculated_fields ||= @submission.data_source.tax_calculator.calculate | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,6 @@ | |
# account_type :integer | ||
# armed_forces_member :integer default("unfilled"), not null | ||
# armed_forces_wages_amount :decimal(12, 2) | ||
# bank_name :string | ||
# charitable_cash_amount :decimal(12, 2) | ||
# charitable_contributions :integer default("unfilled"), not null | ||
# charitable_noncash_amount :decimal(12, 2) | ||
|
@@ -86,7 +85,7 @@ | |
# index_state_file_az_intakes_on_spouse_state_id_id (spouse_state_id_id) | ||
# | ||
class StateFileAzIntake < StateFileBaseIntake | ||
self.ignored_columns = %w[charitable_cash charitable_noncash household_excise_credit_claimed_amt tribal_wages armed_forces_wages] | ||
self.ignored_columns = %w[charitable_cash charitable_noncash household_excise_credit_claimed_amt tribal_wages armed_forces_wages bank_name df_data_import_failed] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the one we actually use/used is called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is a typo... 😓 and yes I was trying to clean up the unused column correctly |
||
encrypts :account_number, :routing_number, :raw_direct_file_data, :raw_direct_file_intake_data | ||
|
||
has_many :az322_contributions, dependent: :destroy | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
class StateFileBaseIntake < ApplicationRecord | ||
self.ignored_columns = [:df_data_import_failed] | ||
self.ignored_columns = [:df_data_import_failed, :bank_name] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if we add bank_name to the shared base intake then we don't need to add it for Az right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ^ i think so too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunately, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. refactor: b2219bd |
||
|
||
devise :lockable, :timeoutable, :trackable | ||
|
||
|
@@ -371,7 +371,6 @@ def self.opted_out_state_file_intakes(email) | |
def sanitize_bank_details | ||
if (payment_or_deposit_type || "").to_sym != :direct_deposit | ||
self.account_type = "unfilled" | ||
self.bank_name = nil | ||
self.routing_number = nil | ||
self.account_number = nil | ||
self.withdraw_amount = nil | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry a little confused about what this is doing, would you mind explaining?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea sorry, this is unclear.. basically the
#joint-account-holder-questions
element gets thismargin: 0
styling which makes it try to be centered within the container unintentionally. Is there a different way I could achieve this without making it be confusing? I thoughtsame-card-as-question
made sense at the time of implementation, but i guess maybe the difference here is just simply "not-centered"? Orinner-question
another element with same styling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anisharamnani here's the clean up 1380324