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

Gyr1 608/remove adopted and buyer #5208

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+7 −18
Open

Gyr1 608/remove adopted and buyer #5208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
905e597
Removed the HomebuyerCredit and the Adoption credit controllers
powersurge360 Dec 13, 2024
c110bd6
Set show to be false for extra insurance they won't pop up
powersurge360 Dec 13, 2024
72e8ea2
Correct single filer spec
powersurge360 Dec 13, 2024
4550d00
Correcting tests
powersurge360 Dec 13, 2024
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
6 changes: 2 additions & 4 deletions app/controllers/questions/adopted_child_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ module Questions
class AdoptedChildController < QuestionsController
include AuthenticatedClientConcern

def self.show?(intake)
intake.had_dependents_yes?
end
def self.show?(intake) = false
Copy link
Contributor

Choose a reason for hiding this comment

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

these two controllers seem to be intentionally not removed, but rather changing show to false. what is the logic behind that? why not delete them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about this yesterday and I think I'm leaning towards just disabling because I am reluctant to remove too much on my first ticket on unfamiliar code that isn't well socialized among the team. If you feel really strongly that I should fully remove controllers, remove tests, remove forms, and remove views I might be persuaded, but I currently am preferring a light touch.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good question here. I normally love-love-love tossing out code, but the light touch might be better here, for a couple reasions: this feels like an unusual situation: a bunch of last-minute changes that could, conceivably, be reversed again in next year's form -- or, heaven forbid, even reversed in some sort of 11th-hour update in the next few weeks (which is unlikely but not impossible). and also, keeping the per-ticket surface area of change small will help us meet the tight deadline we have. that said, i am open to discussing further 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks to you both for explaining!


layout "yes_no_question"
end
end
end
4 changes: 1 addition & 3 deletions app/controllers/questions/homebuyer_credit_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ class HomebuyerCreditController < QuestionsController

layout "yes_no_question"

def self.show?(intake)
intake.ever_owned_home_yes?
end
def self.show?(intake) = false

private

Expand Down
2 changes: 0 additions & 2 deletions app/lib/navigation/gyr_question_navigation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class GyrQuestionNavigation

# Dependent related questions
Questions::DependentCareController,
Questions::AdoptedChildController,

# Student questions
Questions::StudentController,
Expand Down Expand Up @@ -130,7 +129,6 @@ class GyrQuestionNavigation
Questions::EverOwnedHomeController,
Questions::SoldHomeController,
Questions::MortgageInterestController,
Questions::HomebuyerCreditController,

# Miscellaneous
Questions::DisasterLossController,
Expand Down
7 changes: 4 additions & 3 deletions spec/controllers/questions/homebuyer_credit_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
require "rails_helper"

RSpec.describe Questions::HomebuyerCreditController do
describe ".show?" do
it_behaves_like :a_show_method_dependent_on_ever_owned_home
end
# This controller was removed for TY 2024
# describe ".show?" do
# it_behaves_like :a_show_method_dependent_on_ever_owned_home
# end
end

4 changes: 0 additions & 4 deletions spec/features/web_intake/new_joint_filers_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,6 @@ def intake_up_to_documents
expect(page).to have_selector("h1", text: "In #{current_tax_year}, did you or your spouse pay any child or dependent care expenses?")
end
click_on "Yes"
screenshot_after do
expect(page).to have_selector("h1", text: "In #{current_tax_year}, did you or your spouse adopt a child?")
end
click_on "Yes"

# Student questions
screenshot_after do
Expand Down
2 changes: 0 additions & 2 deletions spec/features/web_intake/new_single_filer_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ def intake_after_triage_up_to_documents(intake)
click_on "No"
expect(page).to have_selector("h1", text: "In #{current_tax_year}, did you pay any mortgage interest?")
click_on "No"
expect(page).to have_selector("h1", text: "Did you receive the First Time Homebuyer Credit in 2008?")
click_on "Yes"

# Miscellaneous
expect(intake.reload.current_step).to end_with("/questions/disaster-loss")
Expand Down
Loading