GHOP #165: International phone numbers for CCK

Thomas_Zahreddin - January 20, 2008 - 21:35
Project:Phone (CCK)
Version:HEAD
Component:Code
Category:task
Priority:normal
Assigned:ezyang
Status:patch (code needs work)
Description

Task description / Requirements:
Create an abstraction of the existing filters for phone numbers, so that the filter (to be created) works for all country-codes, therefor:
+ add a configuration option for a default country code,
+ The new filter should validate the number from the user-input against the rules of ITU: validate the country code if started with a "+" sign
+ add a default country-code if the user didn't typed it in, delete therefor a leading 0
+ add a configuration option for the maximum number of digits a phone number is allowed to have (default 20; ITU recommondation 15 + Country-code)
+ test the length of the phone numbers (digits only) against the configured length, while ignoring all brackets, dots, dashes etc. form the user - input,

Approach:
Install drupal, cck and phone
create a input-type with a phone nuber, test the limits of the existing phone-module (not all country-codes can be used)
take the php-Editor / IDE of your choice
Learn how the existing filters are build (fr, us, ca, it, ru)
Create the configuration for the filter
Create the generic filter for all phone-numbers
Integrate the configuration: the filter and the existing phone-module

Versions:
phone is now only for D5 aviable, so the students / you will have to use D5 with the actual developer release of the phone module

Deliverable(s)
a new filter for phone (Ready for review)
a patch of phone.module to integrate the new filter (Ready for review)
at least one page of user documentation: Who defines country-codes? Where are they officiealy documented? What is the recommendation for the length of phone numbers? What signs are used to format a phone-number ?
The used standards should be described in the readme.txt, the appropriate places in the source-code and appropriate links should also placed on the configuration page.

Resources
http://ftp.drupal.org/files/projects/drupal-5.6.tar.gz
http://drupal.org/project/cck
IRC: #drupal
The phone modul: http://drupal.org/project/phone
http://en.wikipedia.org/wiki/List_of_country_calling_codes
RegEx - Tester: http://sourceforge.net/projects/kodos/
official list of country codes: http://www.itu.int/publ/T-SP-E.164D-2007/en
Number-format and -length:
http://www.antelope.org.uk/numbering/World_numbering_developments.pdf see section 2.1.1
a link to the general resources: http://code.google.com/p/google-highly-open-participation-drupal/wiki/Fu...

Maintainer for the module phone: Thierry Guégan; http://drupal.org/user/43798
(this task is on the to-do list of the module)

Primary contact
Thomas_Zahreddin http://drupal.org/user/118299
mailto:ghop@it-arts.org

#1

aclight - January 20, 2008 - 22:08
Project:Google Highly Open Participation Contest (GHOP)» Phone (CCK)
Version:» HEAD
Component:GHOP Task» Code

Moving this to the phone.module issue queue.

The official task associated with this issue can be found at:
http://code.google.com/p/google-highly-open-participation-drupal/issues/...

#2

ezyang - January 21, 2008 - 23:09
Assigned to:Anonymous» ezyang
Status:active» patch (code needs review)

Here's the full implementation of the task.

A few notes about some of the stipulations in the task that might raise some questions:

validate the country code if started with a "+" sign

Our validation of country code is restricted to checking if it is three digits or less. While a list of valid country codes could have been added to the patch with very little effort, I believe this is a bad idea: country codes can be quite volatile and it makes little sense to date the module.

delete therefor a leading 0

The convention of 0 as a trunk code is not uniform throughout all countries; where 7 or 8 has been explicitly set as a single digit, it has been removed too.

add a configuration option for the maximum number of digits a phone
number is allowed to have (default 20; ITU recommondation 15 + Country-code)

The stipulation here is plain wrong. The ITU says that international phone numbers SHALL NOT exceed 15 digits, so it's considerably lower than 20. I believe that this should not be configurable, either, due to the fact that length of numbers are inextricably linked to digit analysis by routers. I have set 15 and will stick by this decision.

test the length of the phone numbers (digits only) against the configured length, while ignoring all brackets, dots, dashes etc. form the user - input,

Validation is considerably more lenient than comparable modules. For example, +1 [732] (234) (333) is considered a valid phone number, which normalizes to +1 732 234 333. Because of the re-formatting capabilities of the Phone module, I believe this is reasonable behavior.

Please review! There are also some miscellaneous refactorings/coding style changes. I recommend that all files in Phone be run through coder_format. Oh, and did I mention, there are unit tests! :-)

AttachmentSize
phone-1.patch16.51 KB

#3

ezyang - January 21, 2008 - 23:35

Updated patch that fixes some formatting style problems.

AttachmentSize
phone-2.patch16.59 KB

#4

bdragon - January 21, 2008 - 23:50

Visually inspected patch. Excellent patch.

Other than noting the TODO regarding matching brackets, and a couple of patch bands with slightly suspect indentation (see code block), I cannot find any problems visually.

-  || $countrycode == 'ru') {
+  || $countrycode == 'ru'
+    || $countrycode == 'int') {

I don't have a test site handy for this, so someone else needs to check that it runs as intended, but I say it's good to go with a quick functional review by someone.

#5

ezyang - January 21, 2008 - 23:57

BDragon: The suspect indentation already existed in the module. I can post another patch with all the formatting normalized, but that would make it difficult to visually inspect the patch and see what functional changes were made.

#6

bdragon - January 22, 2008 - 00:17

I was only referring to the two extra spaces before the || on the second + line. The rest looks good.

#7

ezyang - January 22, 2008 - 00:18

Yeah. That's because there's tabs (crowd gasps) in the source file.

#8

ezyang - January 22, 2008 - 03:52

Updated patch, with friendly error reporting, and checking for foreign characters.

AttachmentSize
phone-3.patch19.32 KB

#9

ezyang - January 22, 2008 - 04:27

Update patch with extra documentation.

AttachmentSize
phone-4.patch20.37 KB

#10

ezyang - January 22, 2008 - 04:44

Updated patch... with even more documentation!

AttachmentSize
phone-5.patch20.82 KB

#11

aclight - January 22, 2008 - 04:52
Status:patch (code needs review)» patch (reviewed & tested by the community)

I gave this a pretty thorough testing and couldn't find any problems with it. It seems like the standard itself is a bit vague and at times nonspecific, but I think ezyang did his best given the standard. He even threw in some simple tests to make testing future changes to the code more reliable. This is great work.

#12

wad - April 15, 2008 - 04:55
Status:patch (reviewed & tested by the community)» patch (code needs work)

@ezyang #2 - The limit of 15 digits is a strong recommendation, not a mandate. The Australian National Numbering Plan is at least one NNP where the ITU-T recommendation is ignored, allowing a maximum numbering length (excluding the country code) of 15 digits.

 
 

Drupal is a registered trademark of Dries Buytaert.