CRM-19621: Contribution confirm page does not display state/country

[jitendrapurohit]

• CRM-19621: Contribution "confirm" page does not display state/country

[eileenmcnaughton]

@seamuslee001 is saying that he is experiencing fatal errors which go away if he reverts this patch.

I would prefer to revert this out of the rc if we can't fix it in time - it feels like the error Seamus is reporting is quite serious compared to this being a fix for a minor issue.

I would also prefer to see the fix for this bug closer to the template since it's mostly about transforming data for display it probably shouldn't touch form parameters etc.

Great to see a test - although I don't know who actually checks to make sure the webtests are passing & how often :-(

[seamuslee001]

the error i am getting is this:

backTrace

#0 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Error.php(369): CRM_Core_Error::backtrace()
#1 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Utils/Type.php(428): CRM_Core_Error::fatal("One of parameters  (value: AU) is not of the type Integer")
#2 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/DAO.php(1367): CRM_Utils_Type::validate("AU", "Integer")
#3 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/DAO.php(1284): CRM_Core_DAO::composeQuery("\nSELECT civicrm_state_province.name name, civicrm_state_province.id id\n  FR...", (Array:1), TRUE)
#4 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/PseudoConstant.php(1532): CRM_Core_DAO::executeQuery("\nSELECT civicrm_state_province.name name, civicrm_state_province.id id\n  FR...", (Array:1))
#5 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/BAO/Location.php(417): CRM_Core_PseudoConstant::stateProvinceForCountry("AU")
#6 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Form.php(2248): CRM_Core_BAO_Location::getChainSelectValues((Array:1), "country", TRUE)
#7 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Form.php(883): CRM_Core_Form->preProcessChainSelectFields()
#8 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/QuickForm/Action/Display.php(111): CRM_Core_Form->toSmarty()
#9 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/QuickForm/Action/Display.php(99): CRM_Core_QuickForm_Action_Display->renderForm(Object(CRM_Contribute_Form_Contribution_Confirm))
#10 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/packages/HTML/QuickForm/Controller.php(203): CRM_Core_QuickForm_Action_Display->perform(Object(CRM_Contribute_Form_Contribution_Confirm), "display")
#11 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/packages/HTML/QuickForm/Page.php(103): HTML_QuickForm_Controller->handle(Object(CRM_Contribute_Form_Contribution_Confirm), "display")
#12 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Controller.php(351): HTML_QuickForm_Page->handle("display")
#13 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Invoke.php(310): CRM_Core_Controller->run((Array:3), NULL)
#14 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Invoke.php(84): CRM_Core_Invoke::runItem((Array:15))
#15 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/CRM/Core/Invoke.php(52): CRM_Core_Invoke::_invoke((Array:3))
#16 /var/local/www/election.greens.org.au/htdocs/profiles/sl_dev/modules/contrib/civicrm/drupal/civicrm.module(448): CRM_Core_Invoke::invoke((Array:3))
#17 [internal function](): civicrm_invoke("contribute", "transact")
#18 /var/local/www/election.greens.org.au/htdocs/includes/menu.inc(527): call_user_func_array("civicrm_invoke", (Array:2))
#19 /var/local/www/election.greens.org.au/htdocs/index.php(21): menu_execute_active_handler()
#20 {main}

[seamuslee001]

Just noting that I may have a little bit of capacity this week to work on a fix. However I should be able to test any fix that comes up. Just ping me if you folk think a fix is there

[eileenmcnaughton]

My general preference is to revert this & fix for 4.7.15 as the bug seems minor & that feels like a safe & low stress approach

[jitendrapurohit]

@seamuslee001 Done a little investigation on this and I'm slightly confused as what seems to be going wrong with the country element and how #9461 removes code on Confirm.php) provides a fix for the backtrace shown above (which doesn't actually navigates to this file).

• Does this backtrace pops up during a js call, i.e, after changing a select option for Country field? (I've tried this with core and custom country field and didn't got any).
• If not on AJAX call, on which page does this error occurs ? Is it replicable on the demo site?

Moreover, looking at the backtrace, it seems that a country iso code is being sent to get the list of states and not the country Id. I don't think this can be due to the Confirm.php changes made in this PR as the $controlValue is set from the element's value $controlField->getValue(); which should always be an id instead of an ISO (as it is not unique).

See - https://cloud.githubusercontent.com/assets/5929648/20698853/e3c252bc-b629-11e6-99ce-defe66265def.png

@eileenmcnaughton This is not done in the template due to a miss in the form parameter as the mapParams() function(where id to iso conversion was done before) was called only if the PP supports preApproval which keeps the country and state iso missing on the confirm page for the remaining processors.

Thanks!

[eileenmcnaughton]

@jitendrapurohit So, the thing that is worrying me about it is that shared logic is being moved out of the shared form onto one specific form - so do we know how that affects all the other forms that share mapParams - it's called from a bunch of places

[jitendrapurohit]

@eileenmcnaughton As I recall, this conversion was not always present in mapParams. It was actually moved from Confirm.php to mapParams() fixing some e-notices for country and state which inturn led to this ticket.
See 0b05b9a.

I've rechecked that issue while creating this PR and it didn't show any notice to me for different processors. So I think it won't be affecting any other places not known now.

[seamuslee001]

@jitendrapurohit Jitendra

I haven't been able to replicate on dmaster.demo.civicrm.org at all. However i just re-loaded my db and re-upgraded to the rc tar ball and failed.

I have hit this when trying to access the Confirm screen. I'm hitting it on page like https://contact-nsw-dev2.ubox.greens.org.au/civicrm/contribute/transact?_qf_Confirm_display=true&..... I too am unsure how this is working on dmaster but regularly fatalling for me.

[eileenmcnaughton]

In general my feeling is that if we have a reported issue that is new to this tarball & is fixable by reverting the patch, and the reported issue is more severe than the bug being fixed we should revert and fix in a more relaxed timeframe.

However, if one of the the 2 alternate PRs is acceptable & fixes the original bug without a bug being demonstrable by Seamus we can go with that.

[jitendrapurohit]

Sure, I'm currently checking the latest PR #9466 and will comment on it once it works fine for me.