Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(6)

Issue 52521002: party: Use country code as local prefix for phone numbers format (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 months, 3 weeks ago by pokoli
Modified:
1 month, 1 week ago
Reviewers:
dave, rietveld-bot, ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 14

Patch Set 2 : Fix ced's comments #

Patch Set 3 : Add addresses on depends and use scenario for testing #

Total comments: 8

Patch Set 4 : Use single call to phonenumbers.parse #

Patch Set 5 : Use single call also on validate and move country code test outside of try/except #

Total comments: 12

Patch Set 6 : Use a list of country codes and add parse_phonenumber method #

Total comments: 17

Patch Set 7 : Fix remarks #

Patch Set 8 : Use generator #

Total comments: 1

Patch Set 9 : remove _get from method name #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -17 lines) Patch
M CHANGELOG View 1 chunk +2 lines, -0 lines 1 comment Download
M contact_mechanism.py View 1 2 3 4 5 6 7 8 4 chunks +27 lines, -17 lines 0 comments Download
A tests/scenario_party_phone_number.rst View 1 2 1 chunk +41 lines, -0 lines 0 comments Download
M tests/test_party.py View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 37
pokoli
2 months, 3 weeks ago (2018-11-02 11:06:36 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
2 months, 3 weeks ago (2018-11-02 11:27:57 UTC) #2
ced
https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py#newcode133 contact_mechanism.py:133: def _get_country_code(self): It is a code for phone only ...
2 months, 2 weeks ago (2018-11-10 12:37:05 UTC) #3
pokoli
Fix ced's comments
2 months, 2 weeks ago (2018-11-10 16:12:09 UTC) #4
pokoli
https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py#newcode133 contact_mechanism.py:133: def _get_country_code(self): On 2018/11/10 12:37:05, ced wrote: > It ...
2 months, 2 weeks ago (2018-11-10 16:12:20 UTC) #5
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
2 months, 2 weeks ago (2018-11-10 16:43:03 UTC) #6
ced
https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/1/contact_mechanism.py#newcode146 contact_mechanism.py:146: for address in party.addresses: On 2018/11/10 16:12:20, pokoli wrote: ...
2 months, 1 week ago (2018-11-11 21:01:50 UTC) #7
pokoli
Add addresses on depends and use scenario for testing
2 months, 1 week ago (2018-11-11 22:11:05 UTC) #8
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
2 months, 1 week ago (2018-11-11 22:39:31 UTC) #9
ced
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 contact_mechanism.py:145: phonenumber = phonenumbers.parse(value, country_code) It should first try to ...
2 months ago (2018-11-19 20:59:00 UTC) #10
pokoli
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 contact_mechanism.py:145: phonenumber = phonenumbers.parse(value, country_code) On 2018/11/19 20:59:00, ced wrote: ...
2 months ago (2018-11-20 14:24:05 UTC) #11
ced
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode137 contact_mechanism.py:137: return address.country.code It should return all the countries. https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 ...
2 months ago (2018-11-21 08:32:47 UTC) #12
pokoli
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 contact_mechanism.py:145: phonenumber = phonenumbers.parse(value, country_code) On 2018/11/21 08:32:46, ced wrote: ...
2 months ago (2018-11-21 09:21:40 UTC) #13
ced
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 contact_mechanism.py:145: phonenumber = phonenumbers.parse(value, country_code) On 2018/11/21 09:21:40, pokoli wrote: ...
2 months ago (2018-11-21 10:22:13 UTC) #14
pokoli
Use single call to phonenumbers.parse
2 months ago (2018-11-21 11:29:51 UTC) #15
pokoli
Use single call also on validate and move country code test outside of try/except
2 months ago (2018-11-21 11:32:46 UTC) #16
pokoli
https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/40001/contact_mechanism.py#newcode145 contact_mechanism.py:145: phonenumber = phonenumbers.parse(value, country_code) On 2018/11/21 10:22:13, ced wrote: ...
2 months ago (2018-11-21 11:33:13 UTC) #17
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
2 months ago (2018-11-21 11:43:36 UTC) #18
dave
Please consider these minor alterations to the comments. https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py#newcode146 contact_mechanism.py:146: # ...
2 months ago (2018-11-21 11:47:53 UTC) #19
ced
https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py#newcode137 contact_mechanism.py:137: return address.country.code I think it should return all the ...
1 month, 4 weeks ago (2018-11-26 21:45:47 UTC) #20
pokoli
Use a list of country codes and add parse_phonenumber method
1 month, 2 weeks ago (2018-12-04 11:14:05 UTC) #21
pokoli
https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py#newcode143 contact_mechanism.py:143: if country_code and country_code not in SUPPORTED_REGIONS: On 2018/11/26 ...
1 month, 2 weeks ago (2018-12-04 11:15:00 UTC) #22
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
1 month, 2 weeks ago (2018-12-04 11:37:44 UTC) #23
ced
https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py#newcode143 contact_mechanism.py:143: if country_code and country_code not in SUPPORTED_REGIONS: On 2018/12/04 ...
1 month, 2 weeks ago (2018-12-04 11:42:18 UTC) #24
pokoli
Fix remarks
1 month, 2 weeks ago (2018-12-04 16:42:26 UTC) #25
pokoli
https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/80001/contact_mechanism.py#newcode143 contact_mechanism.py:143: if country_code and country_code not in SUPPORTED_REGIONS: On 2018/12/04 ...
1 month, 2 weeks ago (2018-12-04 16:43:18 UTC) #26
ced
https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py#newcode130 contact_mechanism.py:130: def _get_phone_country_codes(self): On 2018/12/04 16:43:18, pokoli wrote: > On ...
1 month, 2 weeks ago (2018-12-04 17:09:28 UTC) #27
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
1 month, 2 weeks ago (2018-12-04 17:14:23 UTC) #28
pokoli
Use generator
1 month, 2 weeks ago (2018-12-05 09:11:39 UTC) #29
pokoli
https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py#newcode130 contact_mechanism.py:130: def _get_phone_country_codes(self): On 2018/12/04 17:09:27, ced wrote: > On ...
1 month, 2 weeks ago (2018-12-05 09:12:09 UTC) #30
ced
https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py File contact_mechanism.py (right): https://codereview.tryton.org/52521002/diff/100001/contact_mechanism.py#newcode130 contact_mechanism.py:130: def _get_phone_country_codes(self): On 2018/12/05 09:12:08, pokoli wrote: > On ...
1 month, 2 weeks ago (2018-12-05 09:25:08 UTC) #31
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
1 month, 2 weeks ago (2018-12-05 09:34:39 UTC) #32
pokoli
remove _get from method name
1 month, 2 weeks ago (2018-12-05 12:12:51 UTC) #33
reviewbot
flake8 OK URL: https://codereview.tryton.org/52521002
1 month, 2 weeks ago (2018-12-05 12:16:05 UTC) #34
ced
Otherwise LGTM https://codereview.tryton.org/52521002/diff/160001/CHANGELOG File CHANGELOG (right): https://codereview.tryton.org/52521002/diff/160001/CHANGELOG#newcode1 CHANGELOG:1: * Use country code as local prefix ...
1 month, 2 weeks ago (2018-12-10 21:38:19 UTC) #35
rietveld-bot_tryton.org
New changeset cba01f8cd768 by Sergi Almacellas Abellana in branch 'default': Use country code as local ...
1 month, 1 week ago (2018-12-13 17:51:59 UTC) #36
rietveld-bot_tryton.org
1 month, 1 week ago (2018-12-13 17:52:05 UTC) #37
New changeset 65db6e646ce4 by Sergi Almacellas Abellana in branch 'default':
Use country code as local prefix for phone numbers format
https://hg.tryton.org/tryton-env/rev/65db6e646ce4
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 0147766