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.

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

Fix Payroll Tax + Income Tax buttons on difference table #648

Merged
merged 3 commits into from
Sep 18, 2017

Conversation

GoFroggyRun
Copy link
Contributor

This PR fixes the bug reported in #644.

Before this PR, when payroll tax is ON and income tax is OFF, the difference table will show combined taxes; when payroll tax is ON and income tax is ON, the difference table will show payroll taxes. Both cases are incorrect.

The fix in this PR assigns proper data to each button so that:

Pay_taxes is ON    Inc_taxes is ON    ===>  Combined taxes are shown
Pay_taxes is ON    Inc_taxes is OFF   ===>  Payroll taxes are shown
Pay_taxes is OFF   Inc_taxes is ON    ===>  Income taxes are shown
Pay_taxes is OFF   Inc_taxes is OFF   ===>  Nothing is shown

This PR also changes the two buttons' names from "Payroll" and "Income" to "Payroll Tax" and "Income Tax", as requested in #644 .

However, turning OFF the two buttons, namely "Payroll Tax" and "Income Tax", are still allowed. I have not come up with a fix for this.

@brittainhard Could you review?

cc @martinholmer @MattHJensen

@martinholmer
Copy link
Contributor

@GoFroggyRun said:

However, turning OFF the two buttons, namely "Payroll Tax" and "Income Tax", are still allowed. I have not come up with a fix for this.

What you describe as the new behavior (showing no results when both the buttons are off) sounds just fine with me. I wouldn't spend any more time on trying to "come up with a fix for this".

Thanks for the quick attention to this bug report. I'm looking forward to the next version of TaxBrain.
You and @hdoupe seem to be doing great on fixing things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants