Fix custom Home URL parsing

[davialexandre]

Overview

The code used to parse a custom Home URL in the CRM_Core_BAO_Navigation::createNavigation() would throw an E_NOTICE if the URL did not contain a query string.

Also, URL fragments would be removed from the parsed URL.

Before

Custom Home URLs without a query string would result in an E_NOTICE.

Fragments in a custom URL would be removed.

After

No E_NOTICE is thrown when parsing a custom Home URL without a query string.

Fragments are not removed from the custom URL.

Technical Details

The reason for the E_NOTICE was the usage of explode() to extract the parts of the URL and then using list() to assign the parts to variables. When the query string is not present, explode() will return an array with one single element and the assignment will fail. This was fixed by using the parse_url() function to query for the relevant parts of the URL.

There was also a problem where the previous code would end up removing fragments from the URL. This has also been fixed using parse_url() to get the fragment.

Finally, some minor cleanup was done in 2e42cb1. The $contactId param and the $config variable were not used anywhere inside the method, so I removed them.

Comments

Since this a quite simple fix, I thought it would be ok to not create a Gitlab ticket first. If the ticket is necessary, please let me know and I'll create it.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

I just fixed a very similar problem in a different place. See #12702 #12876.
IMO the block of code beginning with if (!empty($url)) { needs to be extracted into a function so it can be reused here as you are basically duplicating that code here.

[davialexandre]

@colemanw I've extracted the duplicated code as suggested. I don't really like the name of the new extracted method, but I couldn't come up with anything better than that. Part of the reason, maybe, is because I don't fully understand why this processing is necessary in the first place. At first, I thought it was some kind of validation, but since I was not so sure I decided to go with the more generic processUrl(). If you have any suggestions for a better name I'd be happy to update it.

[colemanw]

Looks good.
What it's doing is turning relative urls like "civicrm/dashboard" into fully-formed urls like "https://example.com/wp-admin?q=civicrm/dashboard" (unless the url is fully formed already).

[davialexandre]

changed the method name from processUrl() to makeFullyFormedUrl()

[seamuslee001]

Jenkins re test this please

[eileenmcnaughton]

Test Result (1 failure / +1)
CRM_Member_Form_MembershipTest.testFinancialEntiriesOnCancelledContribution

seems unrelated - will retry

[eileenmcnaughton]

test this please