codeforamerica / balance

A text message system for checking one's EBT card balance (SNAP benefits and more)
MIT License
47 stars 37 forks source link

Make max recording time a state-specific parameter #269

Closed daguar closed 9 years ago

daguar commented 9 years ago

In our QA of transcriptions ( #227 ) we found that more than a few of our errors were caused by the recording ending too early, and so missing out on more being spoken on the audio file.

This Twilio parameter is set here: https://github.com/codeforamerica/balance/blob/master/app.rb#L95

Since this actually state-specific, we should make this a method of the StateHandler class, with:

  1. A default method in StateHandler::Base that returns the current value of 18
  2. A longer state-specific value per state when useful — at the very least we should increase the CA length to maybe 20 or 22

@esmithayer — If this interests you, feel free to assign yourself! It's a fairly chunked out piece of work. LMK if you have questions or want to pair to get set up.

hlprmnky commented 9 years ago

I found this on the Civic Tech Issue Finder, and I'd love to take a crack at it.

Update, I have a decent implementation and tests ready on a fork and just need to test it against staging, @daguar is that something I can get access for to do?

daguar commented 9 years ago

(Ugh, apparently this comment was stuck in AJAX hell; just restarted my browser and found the below did not go through, sorry @hlprmnky!)

Awesome, thanks @hlprmnky!

QAing this is a little tricky because it involves using some sensitive information (EBT numbers.) So if possible, I think the best process would be for you to open a PR with your work, I'll deploy it to our staging server and test.

hlprmnky commented 9 years ago

Cool, I opened PR #281 with my changes - I look forward to learning if they're what you were after!

daguar commented 9 years ago

Tested and good to go — this is closed by PR #281. Thanks so much Chris! :+1: :ship: :rocket: