Add basic validation - #128
Merged
Merged
Conversation
This has the effect of disabling the paypal buttons until basic validate passes. The goal here is to prevent the scenario where someone clicks on the paypal button, authorizes the payment and then has to re-authorize due to the form submission not being accepted. We REALLY don't want to take their authorization until the form is correct. Note that there is an api we could use (ContributionPage.validate ['action' => 'create', 'id' => x] This attempt #98 was against the previous version of paypal smartbuttons but it demonstrates the api I couldn't find any evidence of being able to listen to when a form is valid - posts like this jquery-validation/jquery-validation#1996 suggest the method I used. The easiest way to test this is by submitting the form as an anonymous user without entering an email - you should not be able to launch the paypal form and there should be red writing next to the field. Paypal documentation on it https://developer.paypal.com/docs/checkout/integration-features/validation/?mark=validat#synchronous-validation Note the async mode is BAD - it launches the modal & does the check while loading. Avoid. Perhaps we could only render the paypal button when valid - like they suggest - but then switching back & forth with valid & invalid gets hard. Am on the fence about hiding Unfortunately without a core patch you will ALSO get an alert. I am proposing a core patch along side this which will suppress it in core. Really I think it would be best to put the message down by the paypal button (possibly hiding it) but because of the need to call it at the start it needs to be less nasty.
eileenmcnaughton
added a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Jul 22, 2019
This means that scripts can suppress the nasty js alert message. This isn't my ultimate preference - per long discussion here eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor#128 I think being able to output a summary of messages near, or in place of, the paypal checkout button makes more sense - open to better options @colemanw
eileenmcnaughton
added a commit
to eileenmcnaughton/civicrm-core
that referenced
this pull request
Jul 25, 2019
…ages It's really really ugly & as discussed without it a) extensions can still opt to display the errors themselves and b) the inline errors still appear. Note this will have no real affect on existing code - the ugliness of this has been a blocker to getting into front end validation & at this stage only the omnipay extension is trying to work in this space. eileenmcnaughton/nz.co.fuzion.omnipaymultiprocessor#128 I think ideally with Omnipay it DOES makes sense to display messages near the checkout button - which might be a future step
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This has the effect of disabling the paypal buttons until basic validate passes. The goal
here is to prevent the scenario where someone clicks on the paypal button, authorizes the payment
and then has to re-authorize due to the form submission not being accepted. We REALLY don't want
to take their authorization until the form is correct. Note that there is
an api we could use (ContributionPage.validate ['action' => 'create', 'id' => x]
This attempt #98 was against
the previous version of paypal smartbuttons but it demonstrates the api
I couldn't find any evidence of being able to listen to when a form is valid - posts like this
jquery-validation/jquery-validation#1996
suggest the method I used.
The easiest way to test this is by submitting the form as an anonymous user without
entering an email - you should not be able to launch the paypal form and there should be red writing next to
the field.
Paypal documentation on it
https://developer.paypal.com/docs/checkout/integration-features/validation/?mark=validat#synchronous-validation
Note the async mode is BAD - it launches the modal & does the check while loading. Avoid. Perhaps we could only render
the paypal button when valid - like they suggest - but then switching back & forth with valid & invalid gets hard.
Am on the fence about hiding
Unfortunately without a core patch you will ALSO get an alert. I am proposing a core patch along side this
which will suppress it in core. Really I think it would be best to put the message down by the paypal button
(possibly hiding it) but because of the need to call it at the start it needs to be less nasty.