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

Hide the graduation rate cohort paragraph if that data isn't available #230

Closed marteki closed 8 years ago

marteki commented 8 years ago

This PR does what its title states.

Additions

Pull down the branch, and gulp to rebuild the JS files.

This program had at least one person attend the school in a cohort, and reported appropriately. This program reported a null for the graduation cohort, so we'll hide that paragraph.

All JS unit tests should pass. Our NPM security test should pass. The bulk of our functional tests are still failing, but you can test the two new ones I wrote for this PR by changing the first word of lines 51 and 61 in dd-school-data-spec.js to be "fit" instead of "it", and then running: protractor test/functional/conf.js --specs test/functional/dd-school-data-spec.js

Review

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.766% when pulling 4c5cde0e9627d56965c8c616bb51dc77432ff193 on grad-cohort into e1ac35e9abbc737deb99fb58de7fed88834286e1 on master.

higs4281 commented 8 years ago

This handles the text as noted, but when both program and school fail to deliver a graduation rate (school gradRate == "None"), the bar graph goes blank. Would we want to hide the bar graph in that case?

example: Argosy Inland Empire

marteki commented 8 years ago

@higs4281 I'm tackling that with the program-data/school-data/neither switching work I'm doing. We have messaging to place on the graph; it'll be a separate PR.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.766% when pulling 1893792af7fdbc1776e716d8102280794d5765fa on grad-cohort into 8ebe08fffc31f6951d015b099481f546208314f0 on master.

higs4281 commented 8 years ago

👍 the great state of Kansas casts its single vote to merge.

anselmbradford commented 8 years ago

Works as documented 👍

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.0006%) to 99.774% when pulling a1b996e33d5806b9e7e2d2eb1aafe7119017555d on grad-cohort into 50d8b068acfd8aa3fed27a7306c0c375c5901c94 on master.

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 99.774% when pulling a0e76d9d2a4c8f6c9bf812de7f26d8b162d31c94 on grad-cohort into a20e2a4662151d1329f12a73f0744ec36310327b on master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.09%) to 99.864% when pulling 2f66276260d8446ec680e75613367fbda6254740 on grad-cohort into a20e2a4662151d1329f12a73f0744ec36310327b on master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.09%) to 99.864% when pulling 8b99c8256d613fc73cc41431bf9ad1f08b87ca9b on grad-cohort into a20e2a4662151d1329f12a73f0744ec36310327b on master.

marteki commented 8 years ago

Made all @anselmbradford's suggested changes that weren't related to the browser test refactor. Merging.