Proposal for 3.x-dev snapshot: Generic hook architecture, ITU support & phone types

Basically I did the following:

- Made one field type and moved the different phone number types as options to the field settings
- Invented 3 new hooks:
- hook_phone_number_info(): Labels of formatter in option field settings widget
- hook_phone_number_settings(): Additional form settings of a phone number type
- hook_phone_number_validation(): Error message if validation fails
- Moved includes to include directory
- Moved simpletest to test directory

A phone number type (language) can now implement these three hooks while the first is mandatory. It should
be easier to write phone number types, f. e. South Africa, to it.

Attachment	Size
phone-6.x-2.x-dev.zip	26.5 KB

Thanks for your work. I don't have time to do a full review so hoping someone else can, but I wanted to say that I really support your efforts to modularise this module. It desperately needs functionality split out into .inc files to make it easier for more National Numbering Plans to be added.

Thanks for the work. With a bit of clearing the phone_module and uploading the contents of your file - phone-6.x-2.x-dev.zip I am getting something on the screen.

the first though is an error message I am reporting here as it might help in the development of this great work. (Warning - I have to be clear I have no idea about developing modules or the code used in them.):

When I enter a telephone number now in the User Profile in the phone field such as 34 950123456 I do not get an error message - it should be +34 950123456.
When I enter 34 95012345123456
When I enter 3495012345123456 the error message is - "3495012345123456" is not a valid international phone number
International phone numbers should contain a country code prefixed by a plus sign followed by the local number. - fine, but there are also too many digits - 16.
When I enter 34 95012345123456 the error message is - the same as above and the fact that there are 16 digits (default left at 15) is not recognised.
When I enter +34 95012345123456 the error message is - "+34 95012345123456" is not a valid international phone number
International phone numbers should contain a country code prefixed by a plus sign followed by the local number. It should just tell me there are two many digits now??
When I enter +34 9501234512345 - there is no error message - fine there is a plus a valid two digit country code and just 15 digits.
When I enter +349501234512345 - there is an error message - "+349501234512345" is not a valid international phone number
International phone numbers should contain a country code prefixed by a plus sign followed by the local number. fine - I left the space out
When I enter 34 9501234512345 - there is no error message - though there should be something telling us that + is needed
When I enter 999 9501234512345 - there is an error message - "999 9501234512345" is not a valid international phone number
International phone numbers should contain a country code prefixed by a plus sign followed by the local number. fine - but there are 16 digits not 15.
When I enter 999 950123451234 - there is an error message - wrong there should be a reference to the missing +.

Well, I hope this helps - any other way I could help - let me know - and thanks for the work

Good luck

I think I have overcome the problems from #3. The error message came from some dirty form settings saving.

The test should be fine now. @tryitonce: adding some formalized simple validation tests in /tests/phone.inc.test would be
very helpful. You just have to copy and insert a function like this in the class/file:

and modify the function name and its arguemnts: the first argument is the input, the second the expected output. Write FALSE if the input should not be acceptable.

Done:
- Removed hook_phone_number_validation() and added error message to hook_phone_number_info()
- Various simplifications and cleaning in phone.module
- Phone types (Office, Home, Mobile, Other) support (in widget as select box and as additional information in the view theme)
- Added various test functions

ToDos (any volunteers?):
- Multiple ITU number support realized by slashes('/')
- Update script from version 2.x

Attachment	Size
phone-6.x-3.x-dev.zip	31.97 KB

Hey guys, thanks for the dev version of this module, I really needed phone number types.

However, I discovered a problem. I'm using a singe phone field with unlimited instances (so you can attach multiple phone numbers to one node). By default, when creating a new node it opens 2 phone fields for me. If I only use one, it still saves the other field, with and empty phone number, but the default type "office". I tried adding another value for type "null" but even then it still saves the extra, empty phone number, now with type "null".

I'll keep working on a solution to this problem, I feel close to one, but just wanted to make you aware of it.

Thanks.

Sorry,

found the issue I mentioned here (http://drupal.org/node/275843#comment-1689888)

Always check issues before posting! :/

thanks gnindl , danke,

for the quick response - unfortunately I had to put it a bit to the back of the task list and shall hopefully come round to it in a week or so.

great - help - I might to get a bit more advice from you as I am not a programmer and my code knowledge is limited.

gnindl, I wish you'd post patches rather than .zip files. It's much harder to review what you're doing.

Have you looked at #407844: Refactor country metadata into a single function? It might clean some stuff up ahead of your patch.

@gnindl, could you please roll patch who kill phone types feature or make it optional?

I definitely concur that moving towards having ONE "phone" type field and not this crazy load of extra junk in the cck field type selectors needs to happen ASAP, as a matter or urgency. It's madness as it stands and I don't want to use this right now because I'm so confident it will change.

I'm probably leaving this out and using a text field for now - what are the likely ramifications on the data if I do that and want to move later? Or is it likely that if I carry on and use phone.module, DB updates will be incorporated that will cater for each individual case no matter what?

@drewish, I propose here a reengineering and reorganisation of the module architecture, there would be too many patches to review.

I can't work with the current 2.x versions because I just want ONE field type and ONE widget type called 'Phone' . The phone number validation (ITU, Austrilian etc.) is configured in the settings and they are plugged in via hooks. Further I need typing for phone numbers, like 'office', 'fax', 'mobile' etc.

The purpose of this suggestion is not a patch but discussing hooks I've described in #1.

subscribe

Good work.

I've separately worked on remodularisation in ticket #613606: refactoring and simplification suggesition, adding what could be described as sub-module auto-discovery, and reflection.

More work is needed with the architecture of this module. For example function phone_field_settings($op, $field) creates and handles help entries and special fields for certain country selections (currently int and ca). This functionality should be moved to the respective country sub-modules.

I'm attaching here the diff of the changes I've made to version 6.x-2.9 of phone.module

Attachment	Size
phone.module.diff	17.49 KB

Good luck, guys!

I gave the patches up for now and develop on my custom code base. The module maintainer doesn't seem to be interested in such a rewrite/refactoring although it looks common sense to me.

Desparetely waiting for a new "Better Phone" module or Field API of Drupal 7.

it seems like this module has been abandoned so if someone is interested in taking it over they should start that process.

The module has not been abandonned, some new languages continue to be added from time to time

Well, yes, some new countries were recently added in phone 6.x-2.11, but I think the re-factoring suggested by many here is good and needed.

By the way, why does release 6.x-2.11 has only "some fix" in the release notes, thierry_gd, when in fact new countries have been added. this is not a trivial addition? Can you be more verbose about it?