cfpb / college-costs

⚠️ Deprecated: see note. ⚠️ A tool to help students weigh the costs and rewards of a college program.
https://www.consumerfinance.gov/paying-for-college2/understanding-your-financial-aid-offer/about-this-tool/
50 stars 27 forks source link

Fixes, Round 1 #227

Closed mistergone closed 8 years ago

mistergone commented 8 years ago

This PR fixes the following bugs:

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.766% when pulling 388e3c6b545c5aefe0909753e6cf541b5b02495a on fixes1 into 820b2c4a6aaf34e6dcb7a5735b1de4752ac940d2 on master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.766% when pulling f7690427795e23e3a0adf22b7ca84bb3a4a3a092 on fixes1 into 820b2c4a6aaf34e6dcb7a5735b1de4752ac940d2 on master.

marteki commented 8 years ago

All six bugs tested across certificate, undergraduate, and graduate programs. All appear to be working as intended!

There is weirdness with the Perkins loan section appearing for graduate programs (when they shouldn't be, because grad programs aren't eligible for them), but that might be a separately tracked bug.

I also noted that it would be better if we didn't just copy-paste and duplicate sections of code, when it's already available in the codebase to be called. After that change is made, :+1: for merge.

mistergone commented 8 years ago

Merging, filed an issue internally relating to the duplicated code of updateElement()

higs4281 commented 8 years ago

late to party, but it tested out for me 👍