CRM-20598 Phone ext in profile edit mode is messing up with address

[samuelsov]

cf. https://issues.civicrm.org/jira/browse/CRM-20598

[seamuslee001]

Jenkins re test this please

[seamuslee001]

Jenkins re test this please

[eileenmcnaughton]

test this please

[eileenmcnaughton]

Note this is something I would be uncomfortable merging without a test

[samuelsov]

@eileenmcnaughton Ok, i guess you mean you want me to add a test. It could be an excellent exercise for me :)

[eileenmcnaughton]

what a good idea :-)

[samuelsov]

It was kind of tricky to reproduce using php...
It seemed like an easy to reproduce problem but it's a bit of an edge case... we need to have a few conditions realized at the same time :

• it's only when using the profile to update a contact and not when creating a contact
• the user must be logged in (or we need a checksum) so that it will blank empty values

// If a user has logged in, or accessed via a checksum
// Then deliberately 'blanking' a value in the profile should remove it from their record

• phone field must be after the address field so that phone extension will add a new empty address block at the end (otherwise the correct address will be recreated after being deleted)

Anyway, the test is there and gives an error without the patch and no error with the patch which is what we want i guess.

[samuelsov]

As a side note, here is an abstract of the data array we get without the patch after formatProfileContactParams (2 addresses but only one in the profile because the phone extension has leaked in the address) :

[address] => Array
        (
            [1] => Array
                (
                    [location_type_id] => 1
                    [is_primary] => 1
                    [street_address] => Saint Helier St
                    [country_id] => 1228
                )

            [2] => Array
                (
                    [location_type_id] => 1
                    [is_primary] => 1
                )

        )

[eileenmcnaughton]

test this please

[samuelsov]

I'm not sure to understand how those failing tests are related to the PR but look like it is because other PR seems to pass the test correctly. I will have to reproduce this locally (off-topic, my testing environment is broken so it will take longer than i though... it always takes longer)

[mlutfy]

jenkins, test this please

[eileenmcnaughton]

test this please

[mlutfy]

For reference:

CRM_Contribute_Form_ContributionTest::testSubmitCreditCardWithEmailReceipt
Failed asserting that 0 matches expected 1.

/home/jenkins/buildkit/build/core-10377-7appn/sites/all/modules/civicrm/tests/phpunit/CRM/Contribute/Form/ContributionTest.php:316
/home/jenkins/buildkit/build/core-10377-7appn/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:184
/home/jenkins/buildkit/bin/phpunit4:545

[eileenmcnaughton]

My money is the test fail is an e-notice & the true fail is being hidden because of an issue with the test - which #11798 should fix

[seamuslee001]

test this please

[eileenmcnaughton]

closing in favour of #11978 which has a couple of minor tweaks and, importantly, is not on the last page :-)