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

Fix for Issue/249 #262

Merged
merged 5 commits into from
Sep 17, 2015
Merged

Fix for Issue/249 #262

merged 5 commits into from
Sep 17, 2015

Conversation

DevinWalker
Copy link
Member

@mathetos @pryley check out this PR to fix #249

I've tested and it's working well with the gateways I've tested (braintree, offline, paypal standard, manual/test)

@DevinWalker DevinWalker self-assigned this Sep 16, 2015
@DevinWalker DevinWalker added this to the 1.3 Milestone milestone Sep 16, 2015
@mathetos
Copy link
Member

Just tested with Stripe (test keys, but Give in Live Mode) and it failed. Here's the error:

Warning: Missing argument 1 for give_purchase_form_required_fields(), called in ...\wp-content\plugins\give\includes\process-purchase.php on line 598 and defined in ...\wp-content\plugins\give\includes\process-purchase.php on line 274

@DevinWalker
Copy link
Member Author

@mathetos can you check now?

@mathetos
Copy link
Member

Looks good now. Used a test credit card number and when Give was LIVE it declined the card, and when I put it in TEST the donation went through perfectly.

@DevinWalker
Copy link
Member Author

@mathetos thanks for checking stripe. The issue we're looking for is whether gateways that do NOT require billing fields (address/zip/country) that they do not incorrectly respond back with validation errors saying these fields are required (when they aren't even output).

We need to confirm:

  • PayPal Standard works*
  • Offline Works*
  • Stripe Works
  • Manual / Test works
  • PayPal Pro works
  • Braintree works
  • Authorize.net works
  • Dwolla works*

* doesn't required billing fields

@mathetos
Copy link
Member

Updated list:

  • PayPal Standard works*
  • Offline Works*
  • Stripe Works
  • Manual / Test works
  • PayPal Pro works
  • Braintree works
  • Authorize.net works
  • Dwolla works*

@DevinWalker
Copy link
Member Author

Confirmed Dwolla & PPP

@DevinWalker
Copy link
Member Author

Seems pretty solid. I'd like to hear from @pryley

@DevinWalker
Copy link
Member Author

Confirmed Authorize just now too

@pryley
Copy link
Contributor

pryley commented Sep 17, 2015

It looks good to me.

You could also make $form_id optional in give_field_is_required() as it seems to be only the billing address fields that use it (?). That would mean getting the $payment_mode in give_require_billing_address() and then a check for only if form_id !== false...or something like that.

@DevinWalker
Copy link
Member Author

Thinking future-wise I think it'll be useful to pass the $form_id var for form-specific validation.

DevinWalker pushed a commit that referenced this pull request Sep 17, 2015
@DevinWalker DevinWalker merged commit ac9dd9d into master Sep 17, 2015
@DevinWalker DevinWalker deleted the issue/249 branch October 28, 2015 19:29
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

Successfully merging this pull request may close these issues.

None yet

3 participants