-
Notifications
You must be signed in to change notification settings - Fork 11
Payment issue 70 #90
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
Payment issue 70 #90
Conversation
assets/wpuf/js/frontend-form.js
Outdated
| }; | ||
|
|
||
| $.fn.extend({ | ||
| /** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spacing of /**
Should look like this:
/**
* Short description. (use period)
*/There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the method we're duplicating, both on the frontend and backend? If so, maybe a @todo comment mentioning this and we should fix it one day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
includes/class-ajax.php
Outdated
| $post_data = wp_unslash( $_POST ); | ||
| if ( isset( $post_data['form_data'] ) ) { | ||
| parse_str( sanitize_text_field( wp_unslash( $post_data['form_data'] ) ), $form_data ); | ||
| parse_str( sanitize_text_field( wp_unslash( $post_data['form_data'] ) ), $form_data ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for the extra space added.
includes/class-ajax.php
Outdated
| } | ||
|
|
||
| // Fire a hook for integration | ||
| //Fire a hook for integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a mahesh change. But from what I see it was to make sure the entry is created/not created depending on the settings prior to any redirects after the form is submitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this talking about the space after // , this has been updated with the most recent commit.
includes/class-ajax.php
Outdated
| 'form_id' => $form_id, | ||
| 'page_id' => $page_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why were the spaces removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, added them back so that they line up
@bwmarkle Code provided from mahesh prior to departure that fixed the transaction table issue. Also included emogrifier updates. Made sure that the code does not disrupt previous/pending merges with regards to weSerialize and PHP8 fixes (emogrifier regex update).
To test broken state:
SCRIPT_DEBUG must be enabled to use the non-minified file.
From the master branch:
Create a payment form and proceed through the payment process.

** Note fix to frontend-form.js is added (weSerialize) or the form would not submit. **
email: elanad@inmotionhosting.com
card: 4242 4242 4242 4242
exp date: 04/24
cvc: 424
name: 424242424
zip: 90245
The transaction table id shows as zero instead of the entry id created in the entries table.
To Test Fix:
SCRIPT_DEBUG must be enabled to use the non-minified file.
From the Payment issue 70 branch:
Use the form created above and submit the entry and payment.
You will see that the transaction table id matches the entry id created