Phone Number Drupal 7 Port

Yes, will work on D7 port when the time permit. As usual, any contribution/patches are welcome =)

Hi ckng,

Recently, I’ve been working on phone number field module for Drupal 7 and it is an improved version of Phone CCK (microformat’s phone types (home, work, cell, fax, etc) support, RDFA support, themeable phone field, optimized code). I have tried to coordinate with Phone CCK maintainer to avoid duplicate work done in existing project (http://drupal.org/cvs-application/requirements) but unfortunately, Phone CCK doesn’t take any co-maintainer (http://drupal.org/node/853490). I’m starting to plan to redesign my module and your module’s design is exactly what is in my mind. It would be best to redesign the module I developed making your Phone Number module as my base design. First of all, if you permit I’m interested to work with you to improve your module and be a co-maintainer of your module for Drupal 7. I hope to hear from you soon and looking forward to join force with you.

Thanks,
Roland

Roland,

Feel free to submit patches for D7, we'll see how it goes and I'd love to have a co-maintainer =)

Hi ckng,

I'll start to work on porting your module to D7 and send you the patch as soon I have a working module. Thank you for quick reply and positive welcome for a co-maintainer :)

Hi ckng,

The Phone Number module for Drupal 7 is completed and working (please see the attachment). The following are the improvements done:
- support for hCard microformats (with phone type selection: Voice, Home, Msg, Work, Pref, Fax, Cell, Video, Pager, BBS, Modem, Car, ISDN, PCS)
- support for RDFA (with customizable rel value, selection of phone number subaddress type: telephone/fax's extension number (ext), ISDN subaddress (isub), fax's T33 subaddress (tsub), modem's parameters (type) and recommended parameters (rec). Ref: http://tools.ietf.org/html/rfc2806)
- optimized the code for performance
- Phone number field option as textfield or autocomplete
TODO:
- javascript porting
- simpletest porting

Please use CVS updated Drupal 7.

BTW, I admire on how you design your module. Clean and scalable. Looking forward to join force with you ckng :)

Thanks,
Roland

@arpeggio, thanks for the effort.

Could you submit a patch instead of a whole module? It will be easier to review.

Preferably new features should be on separate issue queue and added separately, to reduce review time, facilitate discussion and it may worth back porting to D6 version, making the process smoother, instead of too many things clutter inside single queue.

I'm breaking it to several new and existing queues:
#849006: Phone types (home, work, mobile, fax, etc) support - could probably go with the hCard improvement you suggested
#864640: Support for RDFa

- Phone number field option as autocomplete, could you elaborate more what it does? From where it will pull the data?

Not all codes are mine, I've referred to quite a number of CCK modules for implementation.
Will create the D7 branch soon.

Sure, the attachment is the basic Phone number module for Drupal 7 patch. The following links are the upgrade ladder to full features offered above:
http://drupal.org/node/865054 - autocomplete
http://drupal.org/node/849006 - hCard microformat
http://drupal.org/node/864640 - RDFA

+++ cck_phone.install	23 Jul 2010 11:58:48 -0000
@@ -21,27 +20,51 @@
+function _cck_phone_update_country_code_list() {
+  // This function is too expensive to call at hook_init() and ¶
+  // should be called only if there's a new country code added.

The reason it is in hook_init() is to cater user making changes to includes files. Putting it under hook_update, user has no mean of getting new file included. Unless we provide a method for user to refresh from within the admin area.

+++ cck_phone.module	26 Jul 2010 13:54:58 -0000
@@ -3,341 +3,321 @@
-define('CCK_PHONE_CC_MAX_LENGTH', 2);

Would prefer this is not remove, just in case there are changes and it is more meaningful in the code than just the number 2.

+++ cck_phone.module	26 Jul 2010 13:54:58 -0000
@@ -3,341 +3,321 @@
+  // We don't need to detect if the device is mobile. RDFA FOAF states:
+  // A phone, specified using fully qualified tel: URI scheme ¶
+  // (http://xmlns.com/foaf/spec/#term_phone)
+  $form['phone_as_link'] = array(
+    '#type'          => 'checkbox',
+    '#title'         => t('Display phone number as link. !foaf: A phone, specified using fully qualified tel: URI scheme', array('!foaf' => l('RDFA FOAF', 'http://xmlns.com/foaf/spec/#term_phone'))),
+    '#default_value' => $settings['phone_as_link'],
+    '#weight'        => 3,
+  );

EDIT: Are we always going to output as RDFa? Shouldn't it remain as an option for user where they only want plain number without link?

General comment on the patch, it should be as minimal changes as possible only to support Field API, no feature should be added or removed (noticed token support has been removed) and no name changes, yet. To help reduce mind cluttering with name mapping between D6 and D7. =)

Powered by Dreditor.

The Open questions section in http://groups.drupal.org/node/16597 already answered my question on RDFa, it should remain as an option where user can turn it off.

The patch need to be cleaned up, there are not related files got included from tortoise.
Please run through Coder as well.

The reason it is in hook_init() is to cater user making changes to includes files. Putting it under hook_update, user has no mean of getting new file included. Unless we provide a method for user to refresh from within the admin area.

hook_init() is always executed every time a page loads, it will be a performance concern. We don't need to execute this more than once if a new include file is added. Drupal provides hook_update_N() where usually a database update/alter matters are executed here. My suggestion is when a new country support include file is added create a new instance of hook_update_N() incrementing the N. Since variable_set('cck_phone_custom_cc', $countrycodes); is also a database matter which is updating the variable table. Example if we have a new include file support for Australia we can add this to our install file:

function cck_phone_update_7001() {
  _cck_phone_update_country_code_list();
}

and if we have a new include file support for China we can add this to our install file:

function cck_phone_update_7002() {
  _cck_phone_update_country_code_list();
}

etc...

After a user download and updates his Phone number module because a new country support is added, with this hook he would received a notification that he needs to update his database. That would update his custom country code list and it just executed only once versus in hook_init() executed very often.

Would prefer this is not remove, just in case there are changes and it is more meaningful in the code than just the number 2.

Sure we will not remove it.

Are we always going to output as RDFa? Shouldn't it remain as an option for user where they only want plain number without link?

I'm also thinking that although by default RDF is enabled in Drupal 7, but still a user has the option to turn off this feature. It would be best to make it as optional to user.

I'm sorry I forgot to add the Token support in TODO list.

ckng, can I apply to be your co-maintainer of your Phone number module for Drupal 7? :)

hook_init is not run on cached pages, and if your server is correctly implemented with disk caching, it has almost no different as reading from memory, so the impact may not as great as you think. Putting it inside update will create unnecessary mess in the long run. I'm not concerned when user updating the module, new includes file will not get loaded once the module is installed, such as when you are doing development on Phone number.

An alternative implementation would be a button on admin page to reload the includes files on demand, which IMHO not needed.

The attachment is the cleanup of the Phone number module and ran through Coder.

Your patch cannot be applied.

You should familiar yourself with Drupal Coding standards and configure your editor to conform to that to avoid generating too much noise in your patch. Wording changes in comment etc should be avoided.

I've profiled the hook_init(), it only took 1.5K microseconds on an low end machine with op code caching only, I think it is safe to say it is not that expensive.

Will try to come out with a cleaner patch hopefully before next week. As for the co-maintainer, it has to be earned, I hope you can understand that.

Yes of course I'm familiar with Drupal Coding standards. Anyway, thank you for the tip. My suggestion, don't assume that all users prefer their cache system is on. 1.5K microseconds or 1.5 milliseconds might be negligible to some people but still it contributes to site's performance drawback and one will surely prefer none at all. Yes ckng I completely understand :) I'm sorry if I didn't passed to your criteria in selecting a co-maintainer at least I tried. Anyway, for me I just feel obligated to contribute back to Drupal because it does wonderful things to my website :)

Any ideas why I am getting this err?

Drupal7a7

Phone Number
7.x-1.x-dev
The phone module allows administrators to define a CCK field type for phone numbers.
This version is not compatible with Drupal 7.x and should be replaced.
Requires: Content (missing)

@MehmetOrun,
Because the D7 branch has no D7 patch committed yet, it is same as D6 branch actually.

Attached is the initial D7 patch (against the D7 branch), for those who would like to help out testing.
NOTE that it is not properly tested yet, don't test with important data.

If you like to help with the code, please refrain from adding new feature, would like to get the base solid first and get other stuffs completed like api, simpletest, css, js.

Hi ckng,

I would like to test this module with D7.

Any new patches for D7_beta2? The above patch "d7_r2" errors out:

#patch cck_phone.module < cck_phone-d7_r2.patch
patching file cck_phone.module
Hunk #1 FAILED at 2.
1 out of 1 hunk FAILED -- saving rejects to file cck_phone.module.rej
patching file cck_phone.install
patching file cck_phone.js
patching file cck_phone.module
can't find file to patch at input line 1395
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: includes/API.php
|===================================================================
|RCS file: /cvs/drupal-contrib/contributions/modules/cck_phone/includes/API.php,v
|retrieving revision 1.3
|diff -u -p -r1.3 API.php
|--- includes/API.php   12 Jul 2010 09:54:52 -0000      1.3
|+++ includes/API.php   4 Oct 2010 04:07:54 -0000
--------------------------
File to patch:

And there are a bunch of other files that error out also because of no match.

@ealfert, it should be

# cd cck_phone
# patch -p0 < cck_phone-d7_r2.patch

@ckng

Thank you, the following works...

# patch -p0 < cck_phone-d7_r2.patch
patching file cck_phone.info
Hunk #1 succeeded at 2 with fuzz 2.
patching file cck_phone.install
patching file cck_phone.js
patching file cck_phone.module
patching file includes/API.php
patching file includes/phone.gb.inc
patching file includes/phone.my.inc
patching file includes/phone.us.inc
patching file tests/cck_phone.crud.test
patching file tests/cck_phone.crud_input.test
patching file tests/phone.gb.test
patching file tests/phone.my.test
patching file tests/phone.us.test

I was able to...
(1) modify a content type and add a phone field and select settings for widget
(2) view modified content type node
(3) select to edit node and the phone field shows up and lets me enter information.

But, when I click "save" button to save phone number to node, I get the following errors...

* Warning: Missing argument 5 for _cck_phone_process(), called in /var/www/drupal7/sites/all/modules/cck_phone/cck_phone.module on line 191 and defined in _cck_phone_process() (line 442 of /var/www/drupal7/sites/all/modules/cck_phone/cck_phone.module).
* Notice: Undefined index: default_value in _cck_phone_process() (line 444 of /var/www/drupal7/sites/all/modules/cck_phone/cck_phone.module).
* Notice: Undefined index: extension in _cck_phone_sanitize() (line 489 of /var/www/drupal7/sites/all/modules/cck_phone/cck_phone.module).
* Notice: Undefined index: und in _service_links_get_tags() (line 558 of /var/www/drupal7/sites/all/modules/service_links/service_links.module).
* Notice: Undefined index: und in _service_links_get_tags() (line 558 of /var/www/drupal7/sites/all/modules/service_links/service_links.module).

Thanks for this great module... from what I saw of the field setup, options it has, and features of widget, I really like it and would love to use this to store phone numbers instead of a basic text field.

@ealfert, thanks for testing it out.

Try this new patch. A lot of code clean up and should have reached a usable stage.

Note:
- if you've existing Phone Number field, remove the field, and
- reinstall of the module

@ckng

Thanks...making some progress.

Module installs correctly and I am able to add field to a content type.

Entering phone number form works well and clicking "save" on content does not give any errors, warnings, or notices.

But, displaying of the phone number is not working correctly. All that appears is:

Phone Number:
ext. 567

The other phone number parts do not appear, only the extension field.

Manually looking in the database table "field_data_field_phone", I see it added the record correctly and all fields are stored.

etid, bundle, deleted, entity_id, revision_id, language, delta, field_phone_number, field_phone_country_codes, field_phone_extension
1, destination, 0, 13, 158, und, 0, 9545551234, us, 567

If I check the "Manage Display" page of the view, I have three choices: "Global phone number (default)" and "Local phone number" and "Hidden".

If I switch it to "Local phone number", save, and then reload the view page containing the content, it still only shows...

Phone Number:
ext. 567

Since my website is not live yet, and your module successfully stores information, I will start using it to populate my data. The viewing of the data is not critical for me at this point in time. Thanks.

EDIT: Found another issue...

Every time you "Edit" a content page, it adds another phone number instance.

For example,

(1) I create a new content page and fill in the phone number field.
(2) I save the content page.
(3) I view the content page and it shows the phone number information (just the extension since that is all that is showing as I referenced above).
(4) I click "edit" to edit the content page.
(5) The phone number field now shows 2 entries for the phone number (it was duplicated).
(6) If I do not modify anything in the content page but click the "save" button, it now creates a second record for the phone number with the same information as the original phone record.
(7) If I view the view the content page, now the phone number information shows up twice.
(8) If I "edit" the content page again, it now duplicates the phone number information one more time and therefore shows 3 identical instances of the phone number on the data entry page.

The module seems to work ok for me in D7, except for one problem: if you edit an existing node, the default value for countrycode is displayed, not value that already was selected for that node, so you have to select it again.

update: fixed, see http://drupal.org/node/999406 comment #4

Can you post a Development Release so it can be more widely tested?

Subscribe

Subscribe. Yea a dev release would be great!

Hey ckng, can you please roll-up a dev release....

Just uploaded the most recent 7.x dev version. I see that it still requires the CCK content.module as a dependency.

Should the module be reconfigured to hook into the core field api instead? Having the content.module dependency seems unnecessary in Drupal 7.

UPDATE: The latest 7.x-2.x dev version of CCK doesn't even include content.module anymore. It is now called cck.module.

After patch 3 :-
Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in cck_phone_field_formatter_info() (line 247 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in cck_phone_field_formatter_info() (line 252 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module

Notice: Undefined index: #field_name in cck_phone_process() (line 669 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined variable: form in cck_phone_process() (line 670 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined index: #columns in cck_phone_process() (line 671 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined index: #delta in cck_phone_process() (line 672 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined index: #description in cck_phone_process() (line 679 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined index: #required in cck_phone_process() (line 680 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).
Notice: Undefined index: #value in cck_phone_process() (line 699 of /Applications/MAMP/htdocs/spic/sites/all/modules/cck_phone/cck_phone.module).

So my dumb comment in #29 was the non-patched 7.x dev version. Sorry about that. After patching with #22 (and setting US as my default country), I receive the follow errors after saving with the field populated:

* Notice: Undefined index: #item in us_formatter_default() (line 84 of /[path]/sites/all/modules/cck_phone/includes/phone.us.inc).
* Notice: Undefined variable: ext in us_formatter_default() (line 105 of /[path]/sites/all/modules/cck_phone/includes/phone.us.inc).

Also the settings page may need some sort of validation for unchecking "Show all country codes" and not selecting a country code. Currently you can save the field settings and will get this upon saving a node:

* Notice: Undefined index: country_codes in cck_phone_phone_number_validate() (line 777 of /[path]/sites/all/modules/cck_phone/cck_phone.module).
* Invalid country code "" submitted.

subscribing

subscribing

subscribing

Subscribing.

Would like to see this progressing and used as part of a D7 version of SMS Framework.

subscribing, please give a guess when it will be ready to use!

subscribing

Just installed the dev version and found it needs 'content' module. I'm guessing this means the port of cck_phone to D7 is something more like a D6 CCK than a D7 field.

I was expecting that porting CCK_Phone to D7 meant making it a Phone_Field. Am I wrong?

If it's a CCK in D7, could anyone explain how different this is to a 'native' D7 field.

Or am I completely confused? :)

@Dru-p I did the same thing in #29 above. Do not use the 7.x version. Instead patch the 6.x version with the patch in #22 above and you will no longer see the cck dependency.

any progress with the upgrade?

Changed title for easier viewing in Dashboard and issue queue for subscribers.

D7 dev has been created.
Please test it out and create New issue queue if you encounter any problem.