-
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-1882] Update ID disability logic to only ask questions about filers who fall within 62-65 age range #5664
Changes from 3 commits
5f107e5
5658d84
680659e
b570cd9
10cc7ac
1d4af2d
c515486
5e3dc55
75306ab
de3d054
65f3e6a
d935736
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 |
---|---|---|
|
@@ -3,11 +3,24 @@ class IdDisabilityForm < QuestionsForm | |
set_attributes_for :intake, :primary_disabled, :spouse_disabled | ||
|
||
attr_accessor :mfj_disability | ||
validates_presence_of :mfj_disability, if: -> { intake.filing_status_mfj?} | ||
validates :primary_disabled, inclusion: { in: %w[yes no], message: :blank }, unless: -> { intake.filing_status_mfj? } | ||
validates_presence_of :mfj_disability, if: -> { intake.filing_status_mfj? && intake.all_filers_between_62_and_65_years_old? } | ||
validates :primary_disabled, inclusion: { in: %w[yes no], message: :blank }, if: -> { should_check_primary_disabled? } | ||
validates :spouse_disabled, inclusion: { in: %w[yes no], message: :blank }, if: -> { should_check_spouse_disabled? } | ||
|
||
def save | ||
def should_check_primary_disabled? | ||
if intake.filing_status_mfj? | ||
!intake.all_filers_between_62_and_65_years_old? && intake.primary_between_62_and_65_years_old? | ||
else | ||
intake.primary_between_62_and_65_years_old? | ||
end | ||
end | ||
|
||
def should_check_spouse_disabled? | ||
intake.filing_status_mfj? && !intake.all_filers_between_62_and_65_years_old? && intake.spouse_between_62_and_65_years_old? | ||
end | ||
|
||
def save | ||
if intake.filing_status_mfj? && intake.all_filers_between_62_and_65_years_old? | ||
case mfj_disability | ||
when "primary" | ||
@intake.update(primary_disabled: "yes", spouse_disabled: "no") | ||
|
@@ -28,12 +41,12 @@ def save | |
private | ||
|
||
def clean_up_followups | ||
primary_followups = @intake.filer_1099_rs(:primary).map(&:state_specific_followup).compact | ||
if primary_disabled == "no" || %w[spouse none].include?(mfj_disability) | ||
primary_followups = @intake.filer_1099_rs(:primary).map(&:state_specific_followup).compact | ||
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 more for my own understanding, but why do you delete the followups? 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 due to: If the user goes through the flow first for the 62-65 year old filer, answer "yes" for disability, then answer "eligible_income_source_yes?" for that eligible 1099R, but then comes back via the review page & then answers "no" for disabled. Without the clean up, we will add up all the income for 1099Rs, even though the filer may have changed their answer to I suppose another way to handle it is to:
Pros of current approach:
Cons of current approach:
Another unrelated refactor I'm looking at is using This way we can clean up the unnecessary data and clear up the logic in the calculations to mirror what's happening in the flow. What do you think? Happy to create a chore & clean up 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. quick followup here #5665 while my mind is on it but no need to get it through review or anything, unless we feel like we have loads of time to run through this quickly before release. |
||
primary_followups.each(&:destroy) | ||
end | ||
|
||
if @intake.filing_status_mfj? && %w[primary none].include?(mfj_disability) | ||
if @intake.filing_status_mfj? && (spouse_disabled == "no" || %w[primary none].include?(mfj_disability)) | ||
spouse_followups = @intake.filer_1099_rs(:spouse).map(&:state_specific_followup).compact | ||
spouse_followups.each(&:destroy) | ||
end | ||
|
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.
i thought about moving these to the intake level, but feel like the names could be a bit misleading/confusing and the use case for these methods are really limited to this presence check.