You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by je...@apache.org on 2015/08/13 20:07:04 UTC
[37/50] allura git commit: [#7960] instead of assuming 10-digit ph#
is US, just provide better error help
[#7960] instead of assuming 10-digit ph# is US, just provide better error help
Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/2f144a9d
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/2f144a9d
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/2f144a9d
Branch: refs/heads/ib/7922
Commit: 2f144a9dc3b11e8987815200dd66c93976643c18
Parents: 9653816
Author: Dave Brondsema <db...@slashdotmedia.com>
Authored: Wed Aug 12 22:16:55 2015 +0000
Committer: Igor Bondarenko <je...@gmail.com>
Committed: Thu Aug 13 15:09:11 2015 +0000
----------------------------------------------------------------------
Allura/allura/lib/phone/nexmo.py | 23 +++++++++++++++-----
Allura/allura/lib/utils.py | 3 ---
.../tests/functional/test_neighborhood.py | 2 +-
Allura/allura/tests/test_utils.py | 2 +-
Allura/allura/tests/unit/phone/test_nexmo.py | 15 +++++++++++++
5 files changed, 34 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/allura/blob/2f144a9d/Allura/allura/lib/phone/nexmo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/phone/nexmo.py b/Allura/allura/lib/phone/nexmo.py
index 3b28c71..dfb1bf0 100644
--- a/Allura/allura/lib/phone/nexmo.py
+++ b/Allura/allura/lib/phone/nexmo.py
@@ -17,9 +17,12 @@
import logging
from urlparse import urljoin
-
+import cgi
import json
+
import requests
+import jinja2
+
from allura.lib.phone import PhoneService
from allura.lib.utils import phone_number_hash
@@ -46,10 +49,18 @@ class NexmoPhoneService(PhoneService):
}
return dict(params, **common)
- def error(self, code=None, msg=None):
- allowed_codes = ['3', '10', '15', '16', '17']
+ def error(self, code=None, msg=None, number=''):
+ allowed_codes = ['3', '10', '15', '16', '17'] # https://docs.nexmo.com/index.php/verify/search#verify_return_code
if code is None or str(code) not in allowed_codes:
msg = 'Failed sending request to Nexmo'
+ if str(code) == '3' and msg.endswith(' number'):
+ msg = jinja2.Markup(
+ '{}{}{}'.format(
+ cgi.escape(msg), # escape it just in case Nexmo sent some HTML we don't want through
+ '<br>Make sure you include the country code (see examples above)',
+ '. For US numbers, you must include <code>1-</code> before the area code.' if len(number) == 10 else '',
+ ))
+
return {'status': 'error', 'error': msg}
def ok(self, **params):
@@ -64,10 +75,10 @@ class NexmoPhoneService(PhoneService):
log_params = dict(params, api_key='...', api_secret='...')
if 'number' in log_params:
log_params['number'] = phone_number_hash(log_params['number'])
- params = json.dumps(params, sort_keys=True)
+ post_params = json.dumps(params, sort_keys=True)
log.info('PhoneService (nexmo) request: %s %s', url, log_params)
try:
- resp = requests.post(url, data=params, headers=headers)
+ resp = requests.post(url, data=post_params, headers=headers)
log.info('PhoneService (nexmo) response: %s', resp.content)
resp = resp.json()
except Exception:
@@ -75,7 +86,7 @@ class NexmoPhoneService(PhoneService):
return self.error()
if resp.get('status') == '0':
return self.ok(request_id=resp.get('request_id'))
- return self.error(code=resp.get('status'), msg=resp.get('error_text'))
+ return self.error(code=resp.get('status'), msg=resp.get('error_text'), number=params.get('number',''))
def verify(self, number):
url = urljoin(self.BASE_URL, 'verify')
http://git-wip-us.apache.org/repos/asf/allura/blob/2f144a9d/Allura/allura/lib/utils.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index b00d7c1..eb273d2 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -607,9 +607,6 @@ class DateJSONEncoder(json.JSONEncoder):
def clean_phone_number(number):
pattern = re.compile('\W+')
number = pattern.sub('', number)
- if len(number) == 10:
- # assume US phone
- number = u'1' + number
return number
http://git-wip-us.apache.org/repos/asf/allura/blob/2f144a9d/Allura/allura/tests/functional/test_neighborhood.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/functional/test_neighborhood.py b/Allura/allura/tests/functional/test_neighborhood.py
index f87e9ea..9359efc 100644
--- a/Allura/allura/tests/functional/test_neighborhood.py
+++ b/Allura/allura/tests/functional/test_neighborhood.py
@@ -984,7 +984,7 @@ class TestPhoneVerificationOnProjectRegistration(TestController):
with h.push_config(config, **{'project.verify_phone': 'true'}):
phone_service.verify.return_value = {
'request_id': 'request-id', 'status': 'ok'}
- r = self.app.get('/p/verify_phone', {'number': '555-444-3333'})
+ r = self.app.get('/p/verify_phone', {'number': '1-555-444-3333'})
phone_service.verify.assert_called_once_with('15554443333')
assert_equal(r.json, {'status': 'ok'})
rid = r.session.get('phone_verification.request_id')
http://git-wip-us.apache.org/repos/asf/allura/blob/2f144a9d/Allura/allura/tests/test_utils.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/test_utils.py b/Allura/allura/tests/test_utils.py
index 39d61b3..dd67ded 100644
--- a/Allura/allura/tests/test_utils.py
+++ b/Allura/allura/tests/test_utils.py
@@ -317,7 +317,7 @@ def test_clean_phone_number():
clean = utils.clean_phone_number
assert_equal(clean('123456789'), '123456789')
assert_equal(clean('+123 456:789'), '123456789')
- assert_equal(clean('555-555-5555'), '15555555555')
+ assert_equal(clean('555-555-5555'), '5555555555')
assert_equal(clean('1-555-555-5555'), '15555555555')
http://git-wip-us.apache.org/repos/asf/allura/blob/2f144a9d/Allura/allura/tests/unit/phone/test_nexmo.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/phone/test_nexmo.py b/Allura/allura/tests/unit/phone/test_nexmo.py
index 0d9c419..640aab6 100644
--- a/Allura/allura/tests/unit/phone/test_nexmo.py
+++ b/Allura/allura/tests/unit/phone/test_nexmo.py
@@ -18,6 +18,7 @@
import json
from mock import patch
from datadiff.tools import assert_equal
+from nose.tools import assert_in, assert_not_in
from allura.lib.phone.nexmo import NexmoPhoneService
@@ -52,6 +53,20 @@ class TestPhoneService(object):
expected = {'status': 'error', 'error': 'text'}
assert_equal(expected, res)
+ # invalid format, possibly US
+ res = self.phone.error(code='3', msg='Invalid value for parameter: number', number='8005551234')
+ assert_equal(res['status'], 'error')
+ assert_in('Invalid value for parameter: number', res['error'])
+ assert_in('country code', res['error'])
+ assert_in('US', res['error'])
+
+ # invalid format, not US
+ res = self.phone.error(code='3', msg='Invalid value for parameter: number', number='738005551234')
+ assert_equal(res['status'], 'error')
+ assert_in('Invalid value for parameter: number', res['error'])
+ assert_in('country code', res['error'])
+ assert_not_in('US', res['error'])
+
def test_ok(self):
res = self.phone.ok(request_id='123', other='smth')
expected = {'status': 'ok', 'request_id': '123', 'other': 'smth'}