Server-side address validation #62
Conversation
hartsick
force-pushed
the
ch/smarty-streets
branch
6 times, most recently
from
5ae5451 to
3f089cd
Compare
hartsick
force-pushed
the
ch/smarty-streets
branch
from
3f089cd to
d917eb6
Compare
hartsick
changed the title
hartsick
force-pushed
the
ch/smarty-streets
branch
from
ff4d405 to
a169316
Compare
Also changes default json column value to match expeced format (hash from array).
If Smarty is returning errors (timeout, unavailable, bad requests, etc) for any reason, aggressively switch to just accepting any provided address. This flags in the database if the address was pre-validated or not so that we can retroactively address any submissions while the API was down. Keeps timeout and retry configuration in the environment so we can more easily adjust them if needed.
| puts err | ||
| return | ||
| # Send to New Relic |
There was a problem hiding this comment.
this comment is indication of where I was thinking about sending errors in future, though we could do this in the validator catch instead?
There was a problem hiding this comment.
I think here is better because we have the original problem. If we did it in the validator we'd want to be sure to wrap the original reason within the ServiceIssueError
| build_us_street_api_client | ||
| @client = SmartyStreets::ClientBuilder.new(credentials) | ||
| .with_licenses(["us-core-cloud"]) | ||
| .with_max_timeout(MAX_TIMEOUT) |
There was a problem hiding this comment.
oh, that's wonderful that it's built-in. you're double sure that the timeout is given in seconds, and not in milliseconds?
There was a problem hiding this comment.
Just double checked and since it passes to Net::HTTP, can confirm it's seconds
| puts err | ||
| return | ||
| # Send to New Relic |
There was a problem hiding this comment.
I think here is better because we have the original problem. If we did it in the validator we'd want to be sure to wrap the original reason within the ServiceIssueError
Sets up a first pass at validating the address on the server based on understood deliverability—wanted to get it up for review and merge.
This saves user input if it's determined to be deliverable based on match code, but also stores the whole returned result from the Smarty API as an encrypted JSON blob for reference if we need it in the future. This is different than what's currently outlined in #43 (but just confirmed this approach with Porta and Em!)
Particularly invite feedback on validation service design and test setup; the stubbing is kind of painful so suggests things could be improved.
My developer account credentials for backend and frontend are in the credentials.yml, so we should have whatever secrets we need for testing the client-side APIs, as well.
Will still need:
And ideally would have:
Progress on #57