Skip to content
This repository has been archived by the owner on Jun 24, 2020. It is now read-only.

prevLoanType is referenced, but is not defined. #661

Open
anselmbradford opened this issue May 18, 2016 · 3 comments
Open

prevLoanType is referenced, but is not defined. #661

anselmbradford opened this issue May 18, 2016 · 3 comments

Comments

@anselmbradford
Copy link
Member

In the past, prevLoanType was defined, so there was no problem when it was checked for a value. However, L:415 has since been removed, so it looks like prevLoanType is undefined and if ( prevLoanType === 'fha-hb' ) { will always be false.

@contolini and @mistergone you seem to have been working in this area, do you know where prevLoanType should be grabbing its value? Not pointing fingers—I can't find where it was actually removed.

@anselmbradford anselmbradford changed the title prevLoanType if referenced, but is not defined. prevLoanType is referenced, but is not defined. May 18, 2016
@mistergone
Copy link
Member

I don't know when or why it was removed, but I can give insight into why it existed.

I'm going off memory, but when a user changed from parameters which would result in a jumbo loan to parameters that would not, we wanted to make sure to "drop" them into the non-jumbo loan category that matched the jumbo loan category that they had just "left". So, the script would grab the current loan type as prevLoanType before it updated parameters so that it could be compared against later. That might be self-explanatory from the code, but I wanted to note it just as a matter of clarification. 😁

I haven't worked on the project in a long time (18th months?), so I don't know at all if that is still a desired behavior or if that line was taken out for some specific reason... but it seems to me if it were added to the beginning of processCounty() it would work again. In that case, it seems like maybe the code should be modified to use dropdown() because it might be more efficient, but I'm not 100% sure because I've mostly forgotten what all this stuff does. 😭

@anselmbradford
Copy link
Member Author

@mistergone Cool, thanks for the background! I think I'm missing an API key to test this change locally, but maybe I'll hit you on chat for it if someone else doesn't get to this.

@mthibos
Copy link
Contributor

mthibos commented May 23, 2016

@anselmbradford

Do you have capacity to fix this issue? I am happy to work with you on the details and testing to make sure it works.

I have been trying to hit up @virginiacc to fix a related bug (which I don't know, may have been made worse with this line getting removed) but I'm sure she's love it if you could fix it instead!

GHE/OAH/OAH-notes/issues/1125

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

No branches or pull requests

3 participants