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

give_require_billing_address filter is always set to false #249

Closed
pryley opened this issue Sep 15, 2015 · 8 comments
Closed

give_require_billing_address filter is always set to false #249

pryley opened this issue Sep 15, 2015 · 8 comments
Milestone

Comments

@pryley
Copy link
Contributor

pryley commented Sep 15, 2015

The give_require_billing_address filter (/includes/process-purchase.php:275) is true by default, but will always be set to false because of give_manual_no_cc_validation().

Here is a alternate way to write the function that allows other functions to modify the filtered value:

/**
 * Manual Gateway does not need CC billing address validation, so remove it.
 *
 * @since 1.0
 *
 * @param bool $bool
 *
 * @return void
 */
function give_manual_no_cc_validation( $bool ) {
    if ( isset( $_POST['give-gateway'] ) && $_POST['give-gateway'] == 'manual' ) {
        $bool = false;
    }

    return $bool;
}

add_filter( 'give_require_billing_address', 'give_manual_no_cc_validation' );

Since the default filtered value is true, you will need to add a similar function to any gateways that do not require billing address validation (i.e. PayPal Standard, Offline, etc.).


There is also an issue with the Braintree gateway plugin where form validation is not performed via ajax, making it impossible to use the single Form Payment Field Reveal Upon Click / Modal Window Upon Click options successfully.

I mention this here because I am sending you an email with a preliminary fix for the Braintree plugin, but it is not complete without being able to also validate the billing address fields.

pryley added a commit to geminilabs/Give that referenced this issue Sep 15, 2015
Allow to modify the `give_require_billing_address` filtered value.
@pryley pryley mentioned this issue Sep 15, 2015
@DevinWalker
Copy link
Member

Hmm, I'm going to dig into this one. Seems alarming off the bat.

@DevinWalker
Copy link
Member

@pryley how about we simply remove give_manual_no_cc_validation() it seems entirely unnecessary?

@DevinWalker
Copy link
Member

I'm trying to wrap my head around this...

I have removed give_manual_no_cc_validation() but now currently PayPal and Manual (Test) are broken because of the true value returned here: https://github.com/WordImpress/Give/blob/master/includes/process-purchase.php#L281

Perhaps we can write a function similar to give_is_cc_verify_enabled() that checks whether or not the current gateway has CC fields enabled then return true/false dynamically depending on the chosen gateway.

@DevinWalker DevinWalker added this to the 1.3 Milestone milestone Sep 16, 2015
pryley added a commit to geminilabs/Give that referenced this issue Sep 16, 2015
- Require billing address validation if $_POST contains the 'billing_country' field.
- Verify that the offline form customization option is enabled when checking whether or not to show the billing fields.
@DevinWalker
Copy link
Member

@pryley we're getting close here. The only issue now that I can see is with give_is_field_required here https://github.com/WordImpress/Give/blob/master/includes/forms/functions.php#L206 when there's no $_POST available the fields required labels don't display properly.

@pryley
Copy link
Contributor Author

pryley commented Sep 16, 2015

Currently gateways determine whether or not they include the CC fields by including a gateway specific hook ('give_' . $payment_mode . '_cc_form'). This hook allows you to override the default give_cc_form hook which displays the CC fields.

Since the form ID is passed to these hooks, you could pass it through to the give_field_is_required() which in turn can send it to give_require_billing_address(). If the $form_id exists, then address fields are required by default, if not, they aren't.

And of course gateways can override through the give_require_billing_address hook.

@DevinWalker
Copy link
Member

I've almost got nearly what you described working. Give me a sec to commit

@DevinWalker
Copy link
Member

@pryley if you have a moment can you check out what I did here: 15cf87d

It seems to be functioning nicely upon initial testing. Here's the branch: https://github.com/WordImpress/Give/tree/issue/249

@pryley
Copy link
Contributor Author

pryley commented Sep 16, 2015

@DevinWalker Oh I didn't see this, I also just made a commit. :)
I'm heading to bed now so I'll have a look tomorrow.

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

No branches or pull requests

2 participants