Form-level error for bad Program ID

[mistergone]

Bad program IDs (those which do not appear in our data) will now generate a form-level error, stopping the app dead in its tracks and instructing the user to contact their school.

Technical notes: This commit generalizes the previous financial-view.js method, badSchoolData, to a more all-purpose method, missingData. This method now works for both missing school and missing program data (that is, when the iped or pid we get from the URL is not found in our data).

I also removed some jQuery calls in properties of metricView which were causing mocha to fail tests. Mocha doesn't like jQuery, since jQuery requires a window. So if you call jQuery in any code that mocha checks (like the properties of the main Object in a file), mocha will scream and fail. However, mocha couldn't care less if you call jQuery inside of a method it doesn't run in its tests. Note: Most of our view files don't get unit tests (because views are more fitting for functional tests), but metric-view has some, so I figured we'd let them work.

Changes

• Refitted financialView.badSchoolData() into financialView.missingData()
• Changed DOM to work with new financialView.missingData() method
• Removed jQuery calls from metricView properties.

  Testing

• Unit tests work!
• Functional tests mostly do not work

  Review

• @martei @higs4281 - Pull it down, gulp, gulp, gulp, and check it out!
  • (Okay, just gulp once)
• Here's a link: bad program ID
• Here's another! bad school ID, still works!

  Checklist

• [x] Changes are limited to a single goal (no scope creep)
• [x] Code can be automatically merged (no conflicts)
• [x] Code follows the standards laid out in the front end playbook
• [x] Passes all existing automated tests
• [x] New functions include new tests
• [x] New functions are documented (with a description, list of inputs, and expected output)
• [x] Placeholder code is flagged
• [x] Visually tested in supported browsers and devices
• [x] Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)

  Animated GIF

[coveralls]

Coverage remained the same at 100.0% when pulling e097f4e89c24ffb2c1c5d053237e9437b4273f5d on no-program into ffa0acc11f4e670d62f11ec6f7904d38277297b9 on master.

[higs4281]

👍 works for me.

This doesn't need to hold up this PR, but two other error states need to be caught:

• if there's an illegal character (like < ) in a program ID
• if there's an illegal char in the offer ID.

If that happens, the Program or Offer ID dd will be blank and the data-warning attr will contain a message beginning with "Error:"

All the various ID errors and warnings (found at [internal documentation for this project]/Bad-values-and-notifications) show up at

$("div.verify_wrapper").find("div.verify_content").attr("data-warning");

[marteki]

@higs4281 I edited your comment above to remove the link to an internal server.

This works if you remove the program ID or delete a character. I agree with Bill that we need to account for illegal characters in the ID, and I'd say that would hold up this PR for me. And now in testing further, I see that I missed on a earlier PR review that we aren't throwing errors for illegal characters in the IPEDS code. 😞

[mistergone]

It's way easier to strip illegal characters and then try what's left. Then we still give the same error and it's much less code. Sound good?

[marteki]

You don't need to strip the characters.

The backend is determining the error before it hits the JS, and delivering a error response in the div that Bill indicated above.

[mistergone]

Okay, we're already stripping the URL query strings down. So, we want to deliver an error if there's an illegal character in the program ID even if stripping that character out gives a valid program ID?

[marteki]

Yes. Because the URL was malformed.

[higs4281]

@marteki

we aren't throwing errors for illegal characters in the IPEDS code.

We are indeed catching illegal chars, because we test that IPEDS codes are only integers. We don't need to test specially for bad chars like <

But that test returns the current warning for "we can't find this school" so we're covered.

[higs4281]

The back end doesn't touch the URL. It just delivers the page and warnings. No URL stripping

[higs4281]

If an offer ID has illegal chars it can't be used, period. It doesn't make sense to strip because then it won't match the hash from the school.

for both security and matching, OID must be a-f, A-F, 0-9

[higs4281]

@marteki

I edited your comment

Grazie!

[higs4281]

@mistergone I misread your stripping note. That could work, but there's a chance we'd end up with the wrong school or program.

But for offer ID I don't think that works.

[marteki]

@higs4281 Chuck and I discussed the importance of not messing with the URL as it was delivered to the page, since we're only serving as the delivery mechanism for what the school is providing to the student. That hadn't been communicated before, so confusion all around. 😞

[mistergone]

I pushed a commit that will check for the [data-warning] in the page, and deliver form-level errors if it finds them. ( @higs4281 - the way I did this is a little messy. Is it possible to deliver a [data-warning__warning-code] alongside the [data-warning] that would have easy-to-use values like school or noSchool and the like? Right now, I'm basically searching the string, which is not ideal. This can be an upgrade later, I just wanted to note it. )

This commit also introduces a missing offer form-level error, so please check that out! The language is the same as the others.

[marteki]

Very nice! 👍 for merge

And this doesn't have to hold up this PR (since I was going to revisit the warnings to tweak the styling anyway), but I have a content suggestion to change the wording of the first line to:

"The information your school provided has an incorrect [whatever] ID"

As the ID isn't always missing.

[coveralls]

Coverage remained the same at 100.0% when pulling cb0525d7d595a77dec9e7a2eccf246f92371c909 on no-program into d03c95bba1b3c79bb55d364a3b1a0eda6b29fa35 on master.

[kurzn]

For the wording, I'd keep it simple with "The information your school provided has an incorrect or missing ID."

[higs4281]

@mistergone error codes simplified in PR 258

[mistergone]

Since @higs4281 did the work, I'll wait for #258, then refactor this slightly, then ask people to re-check.

[mistergone]

I tweaked the functions to match the work @higs4281 did, so please double-check that this PR still works! ( @marteki @higs4281 )

[coveralls]

Coverage remained the same at 100.0% when pulling ee3b1bde2ec8920f748adc59b23e4c10983cf0cb on no-program into 5a96c45e94730f24d7603812882451a9341f2a30 on master.

[higs4281]

👍 💥

[marteki]

👍 from me

I'll fix the styling and change the content as Natalie recommended in another PR. Merging this one, as proposed internally by @mistergone.

[saintsoup52]

Sorry to be late to the party... but if there is no PID in the URL we shouldn't be showing this error.

This makes it impossible to demo this tool to schools that don't have program data in the system.

[saintsoup52]

Not to pile on but not be able to test non-EDMC schools because we don't have PIDs for other schools makes it impossible for me to test parts of the tool (e.g., do Perkins loans show up)

[higs4281]

We have to have a way to flag bad PIDs for settlement students.

We could concoct a different URL for such demo use, but this seems to be out of scope for settlement release.

[saintsoup52]

Bad PIDS are fine for showing an error... I am just saying when there is no PID we shouldn't be showing this error.

[higs4281]

For a settlement student, a missing PID is an error. how can we say we're giving a disclosure without the program?

[saintsoup52]

We have to future proof this for all possibilities. It is possible a school might have to agree to this tool with ever giving us program data.

e.g., The states of OR and NY are considering requiring their schools to use our tool, but they won't be giving us program data, they would just be using school level data.

[marteki]

Let's take this discussion to our internal repo, where @SerenaEstrella can chime in as product owner and @kurzn and Christie can speak to the scoping part of this.

[higs4281]

I think I have a quick solution.

I can deliver a workable but blank disclosure if the bare /offer/ URL is hit (with no "?" querystring)