-
Notifications
You must be signed in to change notification settings - Fork 148
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: error not showing after post form submission #1510
base: develop
Are you sure you want to change the base?
fix: error not showing after post form submission #1510
Conversation
WalkthroughThis pull request modifies the error handling in the Changes
Possibly related PRs
Suggested labels
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
assets/js/frontend-form.js (2)
480-494
: Consider adding defensive checks for the error response structure.While the fix addresses the immediate issue, consider adding null checks to handle cases where
res.data
orres.data.error
might be undefined:if ( form.find('.g-recaptcha').length > 0 ) { grecaptcha.reset(); } Swal.fire({ - html: res.data.error, + html: res.data && res.data.error ? res.data.error : 'An error occurred during form submission', icon: 'warning', showCancelButton: false, confirmButtonColor: '#d54e21', confirmButtonText: 'OK', customClass: { cancelButton: 'btn btn-danger', } });
430-504
: Consider adding unit tests for the form submission error handling.The
formSubmit
method handles critical form submission logic. To ensure reliability, consider adding unit tests that cover:
- Success and error response handling
- Various response data structures
- Edge cases like undefined/malformed responses
- Integration with SweetAlert2 dialogs
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/js/frontend-form.min.js
is excluded by!**/*.min.js
📒 Files selected for processing (2)
assets/js/frontend-form.js
(1 hunks)languages/wp-user-frontend.pot
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- languages/wp-user-frontend.pot
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run PHPCS inspection
🔇 Additional comments (1)
assets/js/frontend-form.js (1)
484-493
: LGTM! The error handling fix looks correct.The change from
res.error
tores.data.error
properly accesses the error message from the server response structure. This fix ensures that validation errors are correctly displayed to users after form submission.
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.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
includes/Frontend/Frontend_Form.php (2)
122-122
: Replace shorthand list syntax for PHP 7.0 compatibilityThe shorthand list syntax
[...]
is not supported in PHP 7.0 or earlier. This needs to be updated to maintain backward compatibility.Apply this change:
-[ $post_vars, $taxonomy_vars, $meta_vars ] = $this->get_input_fields( $this->form_fields ); +list( $post_vars, $taxonomy_vars, $meta_vars ) = $this->get_input_fields( $this->form_fields );
38-38
: Remove error silencing and add proper error handlingUsing the
@
operator to silence errors is discouraged. Instead, implement proper error handling.Apply this change:
-@header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) ); +if (!headers_sent()) { + header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) ); +}includes/Ajax/Frontend_Form_Ajax.php (1)
38-38
: Remove error silencing and add proper error handlingUsing the
@
operator to silence errors is discouraged. Implement proper error handling instead.Apply this change:
-@header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) ); +if (!headers_sent()) { + header( 'Content-Type: application/json; charset=' . get_option( 'blog_charset' ) ); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/js/frontend-form.min.js
is excluded by!**/*.min.js
📒 Files selected for processing (4)
.gitignore
(1 hunks)includes/Ajax/Frontend_Form_Ajax.php
(2 hunks)includes/Frontend/Frontend_Form.php
(1 hunks)languages/wp-user-frontend.pot
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- languages/wp-user-frontend.pot
🧰 Additional context used
🪛 GitHub Actions: Inspections
includes/Frontend/Frontend_Form.php
[warning] 1-1: Silencing errors is strongly discouraged. Use proper error checking instead.
[error] 1-1: The shorthand list syntax '[]' to destructure arrays is not available in PHP 7.0 or earlier.
includes/Ajax/Frontend_Form_Ajax.php
[warning] 1-1: Silencing errors is strongly discouraged. Use proper error checking instead. Found: @Header( 'Content-Type: application/json; charset=' ...
🔇 Additional comments (2)
includes/Ajax/Frontend_Form_Ajax.php (2)
512-519
: Consistent error handling in user registrationThe change to
wp_send_json_error
maintains consistency in error handling. The response includes a redirect URL for login, which is a good UX practice.
339-339
: Consistent error handling with wp_send_json_errorThe change aligns the error handling with the rest of the codebase by using
wp_send_json_error
. However, ensure that the frontend code is updated to handle the modified response structure.Let's verify the frontend error handling:
|
||
wp_redirect( $response['redirect_to'] ); | ||
wpuf_clear_buffer(); | ||
wp_send_json( $response ); | ||
wp_send_json_error( $response ); |
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.
💡 Verification agent
🧩 Analysis chain
Breaking change: Response structure modified
The change from wp_send_json
to wp_send_json_error
modifies the response structure. Clients expecting the response data at the root level will need to access it via the data
property instead. This change requires corresponding updates in the frontend code that handles the response.
Let's verify if the frontend code has been updated to handle this change:
🏁 Script executed:
#!/bin/bash
# Search for JavaScript code that handles the response from publish_guest_post
rg -A 5 "publish_guest_post|formSubmit|wpuf_form_submit" --type js
Length of output: 22151
Based on our review, there’s a breaking change on the server side: using wp_send_json_error()
returns an object of the form
{
success: false,
data: { … }
}
instead of just a raw response at the root level. This means that any client code handling the response must now look for the result under the data
property. Our initial search of the frontend JS (in assets/js/frontend-form.js
) did not reveal any clear handling of the updated structure (for example, checks looking for response.data
when processing error responses).
Please verify that the JavaScript handling of the AJAX response (especially in the formSubmit
function in assets/js/frontend-form.js
) has been updated to expect the payload inside the data
property. Any code that was previously accessing the response directly (e.g. via response
instead of response.data
) will need updating to avoid breaking error handling downstream.
Actionable items:
- Review and update the AJAX success/error handling in
assets/js/frontend-form.js
so that error responses properly extract the error details fromresponse.data
. - Test the guest post submission flow to ensure errors are handled correctly after this backend change.
custom validation checking using
wpuf_add_post_validate
hook is not showing properly.Summary by CodeRabbit
Bug Fixes
Chores
.gitignore
entries to include additional files and directories.