Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move event payment to the confirmation page #24781

Merged
merged 3 commits into from
Jun 2, 2023

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Oct 20, 2022

Overview

For multiple event registration workflow payment details must be entered before you add the additional participants and before you know the total amount.
This is a very confusing workflow and causes big problems for payment processors such as Stripe which need to authorize the payment and present card security measures (eg. SCA / 3d secure).
It has also been reported by many as a source of confusion because it's not "normal" to put your card details in before completing all details.

This provides the option to move the payment processor onto the "Confirm" page for events.

Before

Payment details must be entered on first page before you add additional participants. Amount is not known.

After

Payment details are entered on the confirmation page when you can see all participants / fee selections and the final/total amount.

image

Technical Details

Comments

Updated to put payment details section at the top of confirmation page.

@civibot
Copy link

civibot bot commented Oct 20, 2022

(Standard links)

@civibot civibot bot added the master label Oct 20, 2022
@mattwire mattwire force-pushed the eventmultiplepaymentend branch from 72b9d59 to 269d1f3 Compare October 20, 2022 10:55
@JKingsnorth
Copy link
Contributor

+1 for this concept [=

@guyiac
Copy link
Contributor

guyiac commented Oct 20, 2022

@mattwire I have a client who is dealing with this right now who might be willing to kick in. What is your estimate for number of hours required and a delivery date if you get funding?

@mattwire
Copy link
Contributor Author

@guyiac I have no idea if this might throw up problems elsewhere (ie. with other payment workflows) but this patch seems to work now for multiple participant registrations. So I'd suggest if you have a client that wants to test then you do that and let me know if there are any issues. To get this into core we need to verify that it doesn't break anything else and look at if we need to add/change any tests. Also need to see if it will work with multiple payment processor selection.

@Guydn
Copy link

Guydn commented Oct 20, 2022

I saw the post on the Sprint channel (I attended). I could sponsor a part of the development.

@larssandergreen
Copy link
Contributor

Thanks for doing this!

I gave it a test run with IATS: The billing fields are not shown, but you get a bunch of validation errors saying that each of them is required.

I also gave it a test with IATS & Pay Later: Payment method selection not show, but you get a validation error saying to please select a payment method.

@marcusjwilson
Copy link

Thanks for looking into this @mattwire - Obviously this is quite a big issue for quite a few folk.

My understanding is that the "Confirmation" screen for front end event registration is optional in CiviCRM currently. Would this change mean that paid registrations would now always require the Confirmation screen as the last step of the registration process?

@mattwire
Copy link
Contributor Author

Hi @marcusjwilson the confirmation page is always required for event pages. It is only for contribution pages that it can be disabled (unless you use an extension).

@Guydn
Copy link

Guydn commented Oct 24, 2022

I have activated the "skip event confirm page" https://github.com/pradpnayak/skipeventconfirmpage/blob/master/README.md
Because users thought they were done before the confirm page, and forgot to confirm (especially on their phone).

@JKingsnorth
Copy link
Contributor

@mattwire - Thinking around this. Could this also help prevent Stripe card testing attacks, because the Stripe element will not be on the first page? So could it be rolled out to contribution pages somehow?

@marcusjwilson
Copy link

@mattwire We've been testing your patch locally with two participants registered with different amounts per registration. It seems to work well - the Stripe processor output appears on the Confirmation screen as expected, with 3D authentication where relevant. Payment works and registrations are recorded to CiviCRM.

The only usage issue we saw in our testing is that you still see the payment form on the first screen after you select multiple participants. You actually are not required to fill in the details but, if you do, that is of course useless because you are asked again on the last screen. So maybe a check can be added on the first screen to hide credit card input when we have multiple registrants?

Thanks
Marcus

@pradpnayak
Copy link
Contributor

I feel Stripe should use a transparent page concept to take only payment details like how Omnipay extension does.

Main >> Confirm >> Payment >> [Payment Gateway] >> Thank You.

The payment page will have only the credit card details field when submit either it redirects to the Payment gateway or process the params on the server side and redirects to Thank you page.

I understand once the user hits confirm page all the necessary entries are created in the database. So using the above method will end up creating payments in Civi before taking card details. Alternatively, we can open a dialog box to enter card details when the user hits confirm the page and before the form is submitted.

Either the payment page should be implemented in Civi or in Stripe extension.

We have lots of clients with the Confirmation page disabled for Multi-participant because lots of users go to the confirmation page and do not submit the form thinking their registration is confirmed. So probably adding the card details on the Confirmation page will cause a similar issue for us.

@marcusjwilson
Copy link

We have lots of clients with the Confirmation page disabled for Multi-participant because lots of users go to the confirmation page and do not submit the form thinking their registration is confirmed. So probably adding the card details on the Confirmation page will cause a similar issue for us.

@pradpnayak - We had a similar probably with users forgetting to click to confirm on the final Confirmation screen - but we think it was because they had already entered their payment details on Step 1 of the event registration process. If the payment is not taken until this last screen, then I think the issue of abandoning the registration on the Confirmation should be resolved? Or maybe you could only turn off the Confirmation page conditionally for Free events, where payment is not necesssary

@mattwire mattwire force-pushed the eventmultiplepaymentend branch 2 times, most recently from 33a7642 to 1836a7d Compare November 24, 2022 16:09
@mattwire mattwire changed the title Proof of Concept: Move payment to the end for multiple participant payment Proof of Concept: Move event payment to the confirmation page Nov 24, 2022
@mattwire mattwire force-pushed the eventmultiplepaymentend branch 7 times, most recently from e9ef826 to bb4da34 Compare November 24, 2022 20:55
@mattwire
Copy link
Contributor Author

This has now been updated to support multiple payment processors and can be enabled/disabled via an experimental setting on the Administer->CiviEvent->Event component settings.

@mattwire mattwire force-pushed the eventmultiplepaymentend branch from bb4da34 to 31e404d Compare November 24, 2022 22:03
@mattwire mattwire force-pushed the eventmultiplepaymentend branch from 31e404d to 5184977 Compare January 9, 2023 21:07
@mattwire mattwire marked this pull request as ready for review January 9, 2023 21:07
@mattwire mattwire changed the title Proof of Concept: Move event payment to the confirmation page Move event payment to the confirmation page Jan 9, 2023
@gibsonoliver
Copy link

Tested with live payments (Stripe) and works well including with 3d Secure. My only comment would be that the card details section on the confirm page might be better at the top of the page. With a few participant signups and questions its takes a lot of scrolling down the thank you page to get to the card payment section.

@aydun
Copy link
Contributor

aydun commented Feb 3, 2023

Test this please

@mark-rodgers
Copy link

Tested on CiviCRM 5.60.0 and this now allows us to enable multi-participant registration with Stripe. Hoping this PR is merged soon! 🎉

@GreysonStalcup
Copy link
Contributor

After applying this patch on CiviCRM v5.60.0, the issue with multiple registrations is resolved. However, using CiviDiscount with this patch causes BillingBlock.tpl to throw an error. I managed to fix this issue by adding an additional check to the if blocks causing the issues (PR #26064 ).

TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in include() (line 7 of /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%2F/2F3/2F376CCE%%BillingBlock.tpl.php)
#0 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1914): include()
#1 /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%C2/C29/C29BF079%%BillingBlockWrapper.tpl.php(8): Smarty->_smarty_include()
#2 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1914): include('...')
#3 /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%4F/4FB/4FBAC7AC%%Register.tpl.php(174): Smarty->_smarty_include()
#4 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1914): include('...')
#5 /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%0C/0CB/0CBEC124%%default.tpl.php(19): Smarty->_smarty_include()
#6 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1914): include('...')
#7 /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%F7/F77/F77C7890%%CMSPrint.tpl.php(61): Smarty->_smarty_include()
#8 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1914): include('...')
#9 /var/www/website.com/web/sites/default/files/civicrm/templates_c/en_US/%%2B/2BD/2BD99720%%drupal8.tpl.php(6): Smarty->_smarty_include()
#10 /var/www/website.com/vendor/civicrm/civicrm-packages/Smarty/Smarty.class.php(1273): include('...')
#11 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/Smarty.php(192): Smarty->fetch()
#12 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/QuickForm/Action/Display.php(117): CRM_Core_Smarty->fetch()
#13 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/QuickForm/Action/Display.php(83): CRM_Core_QuickForm_Action_Display->renderForm()
#14 /var/www/website.com/vendor/civicrm/civicrm-packages/HTML/QuickForm/Controller.php(203): CRM_Core_QuickForm_Action_Display->perform()
#15 /var/www/website.com/vendor/civicrm/civicrm-packages/HTML/QuickForm/Page.php(103): HTML_QuickForm_Controller->handle()
#16 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/QuickForm/Action/Reload.php(53): HTML_QuickForm_Page->handle()
#17 /var/www/website.com/vendor/civicrm/civicrm-packages/HTML/QuickForm/Controller.php(203): CRM_Core_QuickForm_Action_Reload->perform()
#18 /var/www/website.com/vendor/civicrm/civicrm-packages/HTML/QuickForm/Page.php(103): HTML_QuickForm_Controller->handle()
#19 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/Controller.php(355): HTML_QuickForm_Page->handle()
#20 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(319): CRM_Core_Controller->run()
#21 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(69): CRM_Core_Invoke::runItem()
#22 /var/www/website.com/vendor/civicrm/civicrm-core/CRM/Core/Invoke.php(36): CRM_Core_Invoke::_invoke()
#23 /var/www/website.com/web/modules/contrib/civicrm/src/Civicrm.php(88): CRM_Core_Invoke::invoke()
#24 /var/www/website.com/web/modules/contrib/civicrm/src/Controller/CivicrmController.php(80): Drupal\civicrm\Civicrm->invoke()
#25 [internal function]: Drupal\civicrm\Controller\CivicrmController->main()
#26 /var/www/website.com/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array()
#27 /var/www/website.com/web/core/lib/Drupal/Core/Render/Renderer.php(580): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#28 /var/www/website.com/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext()
#29 /var/www/website.com/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext()
#30 /var/www/website.com/vendor/symfony/http-kernel/HttpKernel.php(169): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#31 /var/www/website.com/vendor/symfony/http-kernel/HttpKernel.php(81): Symfony\Component\HttpKernel\HttpKernel->handleRaw()
#32 /var/www/website.com/web/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle()
#33 /var/www/website.com/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle()
#34 /var/www/website.com/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle()
#35 /var/www/website.com/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass()
#36 /var/www/website.com/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\page_cache\StackMiddleware\PageCache->handle()
#37 /var/www/website.com/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle()
#38 /var/www/website.com/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle()
#39 /var/www/website.com/web/core/lib/Drupal/Core/DrupalKernel.php(718): Stack\StackedHttpKernel->handle()
#40 /var/www/website.com/web/index.php(19): Drupal\Core\DrupalKernel->handle()
#41 {main}

colemanw added a commit that referenced this pull request Apr 17, 2023
Update BillingBlock.tpl - Error with CiviDiscount + Patch Issue #24781
@Upperholme
Copy link
Contributor

Upperholme commented Apr 18, 2023

It looks to be working for me - once I'd enabled it in the CiviEvent Component settings. Comments: the card details block is right at the bottom of the Confirm page, which does not provide by default any information or hint that this is the page where you need to enter card details - so some thinking about user guidance will be welcome to improve the UX, and it is under a heading that by default is labelled "Billing Name and Address" which is misleading, but apart from that it makes a lot more sense to have things working this way. Looking forward to seeing this in a release very soon. Thanks.

@Upperholme
Copy link
Contributor

A further comment: At the top of this thread it states that "This provides a working proof of concept (for the multiple event participant workflow) which puts the payment processor onto the "Confirm" page if multiple participants were selected." In my case at least it is putting the payment processor onto the Confirm page regardless of whether I select multiple participants.

@mattwire mattwire force-pushed the eventmultiplepaymentend branch from 5184977 to 8b429ff Compare April 21, 2023 09:28
@marcusjwilson
Copy link

@mattwire

We have had some issues with this patch on a Production site today. With the settings enabled to move the Stripe payment to the Confirmation screen, users aren't able to progress past step 1 of the Civi booking process for the event (keep getting redirected to step 1). We have this running on Civi v5.58.1 and Stripe v6.7.14, Payments Shared v1.2.11

Would it help to update anything further at this stage? I can send you steps to recreate if that's useful.

Ta!
Marcus

@mattwire
Copy link
Contributor Author

mattwire commented May 9, 2023

Payment details moved to the top of the form and PR description updated per feedback

@mattwire mattwire force-pushed the eventmultiplepaymentend branch from 8b429ff to eaeb1fe Compare May 9, 2023 13:36
@aydun
Copy link
Contributor

aydun commented Jun 1, 2023

@mattwire Are there any comments above not addressed, or do we think this is good to merge?

@mattwire
Copy link
Contributor Author

mattwire commented Jun 1, 2023

@aydun It's in use on a few production sites and I think I've addressed all comments. So in my opinion good to merge

@aydun
Copy link
Contributor

aydun commented Jun 1, 2023

Test this please

@Upperholme
Copy link
Contributor

I have this working successfully on a production site running 5.56.0. Keen to see it merged and released.

@aydun
Copy link
Contributor

aydun commented Jun 2, 2023

Test failures look unrelated

Test this please

@aydun
Copy link
Contributor

aydun commented Jun 2, 2023

Ok, we've got multiple good reports of this in production and the new behaviour requires an opt-in per event. Let's merge ...

@aydun aydun merged commit 02e6d99 into civicrm:master Jun 2, 2023
@aydun
Copy link
Contributor

aydun commented Jun 2, 2023

Thanks for all the effort on this @mattwire and others - this is one of the 'features' of Civi that is intuitively wrong: no-one wants to put their card details in before knowing the total amount. Hopefully we can change this to the default behaviour before too long.

@mattwire mattwire deleted the eventmultiplepaymentend branch June 2, 2023 17:41
@mattwire
Copy link
Contributor Author

mattwire commented Jun 2, 2023

👍🏻

@larssandergreen
Copy link
Contributor

This is great!
I've done a little testing and found a few issues and a possible improvement:

  1. Issue: Not working with pay later disabled, at least with IATS and Test Processor (and pay later messages are displayed when they shouldn't be if pay later is enabled).
  2. PHP Notice on dmaster and validation missing: PR Fix payment validation for Payment on Confirm #26430.
  3. I think it makes sense to show the amounts and total you are paying for above the place where you enter your payment details. PR Show Event Info at bottom of Registration Confirm & Fee Info above Payment #26431.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.