Title reflects the bug.

I am using all current modules, plus Phone-7.x-2.x-dev

Tried twice, from within Redhen-Contact and a custom ContentType.

Thanks for the effort, this module is looking great.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Elin Yordanov’s picture

I have also the same problem. I know that the 7.x-2.x is not production ready, but I would like to also help with the development. Any hints will be appreciated. I'll also look into the code but at the very first I don't know where to start :)

PatchRanger’s picture

Status: Active » Needs review
FileSize
2.08 KB

Here is the patch against 7.x-2.x.
The problem was in implicit dependency on rdf: it was assumed but not declared.
Patch fixes the issue.
Please review.
PS I've included mxwitkowski (Mark Witkowski) into patch credits as he is the sponsor of the patch: see http://www.freedomsponsors.org/core/issue/324/phone-extension-field-not-... .

mxwitkowski’s picture

I have applied the patch in #2 and unfortunately the issue persists. I had the rdf module installed, so that was in place already. I can confirm the following:
1. The field setting to allow extensions is checked in the field settings; the profile edit page for the user is showing an extension field.
2. The Drupal database for the element has the proper table field for the extension
3. When a user enters the extension and saves their profile, the database is not getting updated with the extension (but the main number and phone type changes are all working properly).

PatchRanger’s picture

Please check your dblog for any errors, warnings or even info: admin/reports/dblog .
You have all UI - but saving doesn't work. It looks like you don't have libphonenumber-for-php library properly installed. Please make sure that you've downloaded the library from Git as described in README.txt and placed it as sites/all/libraries/libphonenumber-for-php . Also visit admin/reports/status and check that there is no requirement error such as "Lib Phone Number" or similar. Sorry for this silly assumption if everything is ok :) As you understand, that are the steps that are "must done" in order to go further.
I've double-checked: in my case patched phone field with extension works in content type and in profile.
UPDATE: I've created .make-file to facilitate the process of installing dependencies: see #2069489: Create .make-file to ease installing dependencies and the library.

PatchRanger’s picture

Status: Needs review » Needs work

It needs tests to verify that it works.

PatchRanger’s picture

Status: Needs work » Needs review
FileSize
736 bytes
2.97 KB

I have finally got the issue: "Extension" is not being saved! I thought the problem is with the element called "number type". Sorry for that. mxwitkowski did a lot to explain me what is the problem, thanks.
I haven't created tests but managed to fix the issue, since the root of the problem is trivial: we just passed empty extension to the library. Please review the re-rolled patch.

Elin Yordanov’s picture

Hi PatchRanger, thanks for the spotting on the bug! I'll review your patch this week and give you a feedback.

Do you still think that the rdf module should be set as required?

Elin Yordanov’s picture

I've just tested it shortly. It saves the extension now successfully, however on the display it doesn't show it, yet. I'll try to debug it and give a feedback.

Ah, one point to note that I don't have rdf module enabled.

UPDATE: My apologies, I didn't check the option in the display settings to display the extension. Your patch simply works!

P.S.: Maybe you want to update the patch so that the RDF module is not required, since it works also without RDF!

Thanks a lot!

PatchRanger’s picture

Do you still think that the rdf module should be set as required?

Actually I don't, I've checked saving Phone extension and it works even without RDF - so here is the updated patch.

It saves the extension now successfully, however on the display it doesn't show it, yet.

What do you mean by "on the display"? I've checked admin/structure/types/manage/{content_type}/display - and everything looks like it should. Looking forward your additional debug feedback.
UPDATE: Ah, I see :)

mxwitkowski’s picture

Nice work! I have tested the patch and all is working fine for me.

I did discover a new issue when using views to display the phone number and extension. Seems the code to manage the phone number extension is blocking the display of the extension. I checked the view options for the field and there are no controls to enable or disable the extension. Interestingly, these display options do appear in the display options when showing the field in a content type though. In any case, this matter appears to warrant a separate issue in the queue.

Elin Yordanov’s picture

Status: Needs review » Reviewed & tested by the community

So I go ahead and set the status of the issue 'RTBC' :)

Elin Yordanov’s picture

@mxwitkowski, I've posted a workaround patch for this issue, but I want to get some focus on this patch to find out the real problem. For the others reading this issue, please check my post on the relative issue at: https://drupal.org/node/2079503#comment-7926093

Thanks