enhance: repeat field enhancement #1389
Open
Conversation
This file contains 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
- Reactivity for field title, help etc fixed
Slight optimization when removing the fields
Rat01047
added
needs: testing
needs: developer feedback
WalkthroughThe changes introduced in the codebase enhance the handling of form fields, particularly focusing on the integration of repeat fields and improving the overall logic for field types. Modifications include expanding conditions to accommodate both 'column_field' and 'repeat_field', improving accessibility with dynamic IDs for input elements, and refining validation checks for data processing. These updates aim to provide a more robust and flexible form-building experience. Changes
Assessment against linked issues
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Outside diff range, codebase verification and nitpick comments (13)
includes/Ajax/Frontend_Form_Ajax.php (10)
324-326: Improve readability by using named parameters.Using named parameters in the method call improves readability and maintainability.
Apply this diff to use named parameters:
- $response = $this->send_mail_for_guest( - $charging_enabled, $post_id, $form_id, $is_update, $post_author, $meta_vars - ); + $response = $this->send_mail_for_guest( + charging_enabled: $charging_enabled, + post_id: $post_id, + form_id: $form_id, + is_update: $is_update, + post_author: $post_author, + meta_vars: $meta_vars + );
330-331: Improve error message clarity.Provide a more descriptive error message to help users understand what went wrong.
Apply this diff to improve the error message:
- $this->send_error( __( 'Something went wrong', 'wp-user-frontend' ) ); + $this->send_error( __( 'An unexpected error occurred while processing your request. Please try again.', 'wp-user-frontend' ) );
342-343: Use strict comparison for boolean values.Use strict comparison for boolean values to avoid unexpected behavior.
Apply this diff to use strict comparison:
- if ( isset( $this->form_settings['guest_post'] ) && $this->form_settings['guest_post'] === 'true' && $this->form_settings['guest_details'] === 'true' ) { + if ( isset( $this->form_settings['guest_post'] ) && $this->form_settings['guest_post'] === true && $this->form_settings['guest_details'] === true ) {
442-443: Fix typo in comment.Fix the typo in the comment to improve readability.
Apply this diff to fix the typo:
- $this->generate_auth_link(); // Translate tag %login% %registration% to login registartion url + $this->generate_auth_link(); // Translate tag %login% %registration% to login registration url
458-460: Remove commented-out code.Remove commented-out code to improve readability and maintainability.
Apply this diff to remove the commented-out code:
- // check_ajax_referer( 'wpuf_form_add' );
490-520: Improve readability by using named parameters.Using named parameters in the method call improves readability and maintainability.
Apply this diff to use named parameters:
- if ( ! empty( $multi_repeated ) ) { - $fields_meta = ! empty( $multi_repeated['sub_fields'] ) ? $multi_repeated['sub_fields'] : []; - $parent = ! empty( $multi_repeated['parent'] ) ? $multi_repeated['parent'] : ''; - - if ( ! empty( $parent ) && ! empty( $fields_meta ) ) { - // get how many rows previously saved - $row_num = get_post_meta( $post_id, $parent, true ); - - // delete previous repeat values - foreach ( $fields_meta as $sub_field ) { - for ( $i = 0; $i < $row_num; $i++ ) { - $tmp_meta = $parent . '_' . $i . '_' . $sub_field; - delete_post_meta( $post_id, $tmp_meta ); - delete_post_meta( $post_id, '_' . $tmp_meta ); - } - } - - $row_number = ! empty( $multi_repeated['row_number'] ) ? $multi_repeated['row_number'] : 0; - $sub_fields = ! empty( $multi_repeated['fields'] ) ? $multi_repeated['fields'] : []; - - // save new data - for ( $i = 0; $i < $row_number; $i++ ) { - foreach ( $sub_fields as $key => $value ) { - update_post_meta( $post_id, $key, $value ); - update_post_meta( $post_id, '_' . $key, uniqid( 'field_' ) ); - } - } - - update_post_meta( $post_id, $parent, $row_number ); - } - } + if ( ! empty( $multi_repeated ) ) { + $fields_meta = ! empty( $multi_repeated['sub_fields'] ) ? $multi_repeated['sub_fields'] : []; + $parent = ! empty( $multi_repeated['parent'] ) ? $multi_repeated['parent'] : ''; + + if ( ! empty( $parent ) && ! empty( $fields_meta ) ) { + // get how many rows previously saved + $row_num = get_post_meta( $post_id, $parent, true ); + + // delete previous repeat values + foreach ( $fields_meta as $sub_field ) { + for ( $i = 0; $i < $row_num; $i++ ) { + $tmp_meta = $parent . '_' . $i . '_' . $sub_field; + delete_post_meta( $post_id, $tmp_meta ); + delete_post_meta( $post_id, '_' . $tmp_meta ); + } + } + + $row_number = ! empty( $multi_repeated['row_number'] ) ? $multi_repeated['row_number'] : 0; + $sub_fields = ! empty( $multi_repeated['fields'] ) ? $multi_repeated['fields'] : []; + + // save new data + for ( $i = 0; $i < $row_number; $i++ ) { + foreach ( $sub_fields as $key => $value ) { + update_post_meta( $post_id, meta_key: $key, meta_value: $value ); + update_post_meta( $post_id, meta_key: '_' . $key, meta_value: uniqid( 'field_' ) ); + } + } + + update_post_meta( $post_id, meta_key: $parent, meta_value: $row_number ); + } + }
757-758: Remove commented-out code.Remove commented-out code to improve readability and maintainability.
Apply this diff to remove the commented-out code:
- // if user has a subscription pack
762-763: Use strict comparison for boolean values.Use strict comparison for boolean values to avoid unexpected behavior.
Apply this diff to use strict comparison:
- if ( ! empty( $user_wpuf_subscription_pack ) && isset( $user_wpuf_subscription_pack['_enable_post_expiration'] ) - && isset( $user_wpuf_subscription_pack['expire'] ) && strtotime( $user_wpuf_subscription_pack['expire'] ) >= time() ) { + if ( ! empty( $user_wpuf_subscription_pack ) && isset( $user_wpuf_subscription_pack['_enable_post_expiration'] ) + && isset( $user_wpuf_subscription_pack['expire'] ) && strtotime( $user_wpuf_subscription_pack['expire'] ) >= time() ) {
757-758: Remove commented-out code.Remove commented-out code to improve readability and maintainability.
Apply this diff to remove the commented-out code:
- // if user has a subscription pack
762-763: Use strict comparison for boolean values.Use strict comparison for boolean values to avoid unexpected behavior.
Apply this diff to use strict comparison:
- if ( ! empty( $user_wpuf_subscription_pack </blockquote></details> <details> <summary>assets/js/frontend-form.js (3)</summary><blockquote> `348-362`: **Consider reinitializing plugins or event handlers for the cloned field.** The `cloneRepeatField` method is correctly implemented. However, ensure that any necessary plugins or event handlers are reinitialized for the cloned field. --- `364-381`: **Consider reinitializing plugins or event handlers for the remaining fields.** The `removeRepeatField` method is correctly implemented. However, ensure that any necessary plugins or event handlers are reinitialized for the remaining fields. --- `383-411`: **Consider reinitializing plugins or event handlers for the updated fields.** The `setRowNumber` and `calculateFieldsName` methods are correctly implemented. However, ensure that any necessary plugins or event handlers are reinitialized for the updated fields. </blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 49a2c9f62fbea2cf5e9295380a8a5c20a49a4258 and 3680861a73b80455ad3a36f106ab75880e40912e. </details> <details> <summary>Files selected for processing (21)</summary> * admin/form-builder/assets/js/components/field-options/index.js (1 hunks) * admin/form-builder/assets/js/components/form-column_field/index.js (2 hunks) * admin/form-builder/assets/js/form-builder.js (1 hunks) * admin/form-builder/assets/js/mixins/global.js (1 hunks) * assets/css/admin.css (1 hunks) * assets/css/frontend-forms.css (2 hunks) * assets/js/frontend-form.js (2 hunks) * assets/js/wpuf-form-builder-components.js (3 hunks) * assets/js/wpuf-form-builder-mixins.js (1 hunks) * assets/js/wpuf-form-builder-wpuf-forms.js (1 hunks) * assets/js/wpuf-form-builder.js (1 hunks) * assets/less/admin.less (1 hunks) * assets/less/frontend-forms.less (2 hunks) * includes/Ajax/Admin_Form_Builder_Ajax.php (1 hunks) * includes/Ajax/Frontend_Form_Ajax.php (1 hunks) * includes/Fields/Form_Field_Checkbox.php (2 hunks) * includes/Fields/Form_Field_Radio.php (1 hunks) * includes/Fields/Form_Field_Textarea.php (1 hunks) * includes/Integrations/WPUF_ACF_Compatibility.php (1 hunks) * includes/class-frontend-render-form.php (7 hunks) * wpuf-functions.php (5 hunks) </details> <details> <summary>Files skipped from review due to trivial changes (4)</summary> * admin/form-builder/assets/js/components/form-column_field/index.js * includes/Ajax/Admin_Form_Builder_Ajax.php * includes/Fields/Form_Field_Radio.php * includes/Integrations/WPUF_ACF_Compatibility.php </details> <details> <summary>Additional context used</summary> <details> <summary>Biome</summary><blockquote> <details> <summary>admin/form-builder/assets/js/form-builder.js</summary><blockquote> [error] 137-137: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) --- [error] 142-142: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) </blockquote></details> <details> <summary>assets/js/wpuf-form-builder.js</summary><blockquote> [error] 137-137: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) --- [error] 142-142: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) </blockquote></details> </blockquote></details> </details> <details> <summary>Additional comments not posted (38)</summary><blockquote> <details> <summary>admin/form-builder/assets/js/mixins/global.js (1)</summary><blockquote> `61-62`: **LGTM!** The code change correctly expands the conditional logic to include 'repeat_field', enhancing the functionality by accommodating additional field types. </blockquote></details> <details> <summary>admin/form-builder/assets/js/components/field-options/index.js (1)</summary><blockquote> `36-37`: **LGTM!** The code change correctly expands the conditional logic to include 'repeat_field', enhancing the functionality by accommodating additional field types. </blockquote></details> <details> <summary>assets/js/wpuf-form-builder-wpuf-forms.js (1)</summary><blockquote> `31-31`: **LGTM!** The code change correctly expands the conditional logic to include 'repeat_field', enhancing the functionality by accommodating additional field types. </blockquote></details> <details> <summary>includes/Fields/Form_Field_Checkbox.php (2)</summary><blockquote> `49-49`: **LGTM!** The addition of the `id` attribute enhances accessibility and allows for better targeting of the checkbox in scripts and styles. --- `193-196`: **LGTM!** The refinements ensure that the data is appropriately processed before being exploded into an array, improving the robustness of the form rendering process. </blockquote></details> <details> <summary>includes/Fields/Form_Field_Textarea.php (1)</summary><blockquote> `167-169`: **LGTM!** The validation check ensures that only string data is processed further, preventing potential errors or unexpected behavior when non-string data types are passed to the method. </blockquote></details> <details> <summary>assets/js/wpuf-form-builder-mixins.js (1)</summary><blockquote> `168-169`: **LGTM!** The modification broadens the criteria under which the inner fields of a form field are processed, enhancing the functionality by allowing the code to handle both 'column_field' and 'repeat_field' in the same logical branch. </blockquote></details> <details> <summary>assets/less/admin.less (1)</summary><blockquote> `842-843`: **LGTM!** The changes ensure consistent styling of control buttons across different field types in the form preview. </blockquote></details> <details> <summary>assets/css/admin.css (1)</summary><blockquote> `648-649`: **LGTM!** The changes ensure consistent styling of control buttons across different field types in the form preview. </blockquote></details> <details> <summary>includes/class-frontend-render-form.php (10)</summary><blockquote> `508-510`: **Ignore section break and HTML input type.** The code correctly ignores section break and HTML input types, which are not relevant for processing input fields. --- `514-514`: **Handle 'repeat' input type.** The code now includes a check for 'repeat' input type along with 'column_field'. This ensures that repeat fields are processed correctly. --- `521-523`: **Mark repeat field child.** The code correctly marks repeat field children with an `is_repeat_child` property. This helps in identifying and processing repeat field children separately. --- `550-555`: **Separate post and custom fields.** The code correctly separates post and custom fields, ensuring that meta fields are processed separately. --- `714-715`: **Initialize repeat fields array.** The code initializes a `repeat_fields` array to capture both parent and child field data. This enhances the organization of repeated field submissions. --- `718-719`: **Initialize repeat field children array.** The code initializes a `repeat_field_childs` array to map child fields to their parent repeat fields. This helps in organizing and processing repeat field children. --- `720-734`: **Process repeat fields.** The code processes repeat fields by looping through each column and capturing the child fields. This ensures that repeat fields and their sub-fields are correctly identified and organized. --- `756-775`: **Handle repeat field children.** The code handles repeat field children by capturing their data and organizing it under the parent repeat field. This ensures that repeat field submissions are correctly processed and stored. --- `831-831`: **Handle checkbox field.** The code correctly handles checkbox fields by ensuring that the data is stored in the correct format. --- `856-856`: **Return meta fields, repeat fields, and files.** The code returns the `meta_key_value`, `repeat_fields`, and `files` arrays, ensuring that all processed data is correctly returned. </blockquote></details> <details> <summary>admin/form-builder/assets/js/form-builder.js (2)</summary><blockquote> `132-133`: **LGTM!** The condition correctly includes both 'column_field' and 'repeat_field' using a regular expression. --- `141-145`: **LGTM!** The check prevents modifications to the 'name' field of existing inner fields unless they are marked as new, ensuring data integrity. <details> <summary>Tools</summary> <details> <summary>Biome</summary><blockquote> [error] 142-142: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) </blockquote></details> </details> </blockquote></details> <details> <summary>assets/js/wpuf-form-builder.js (2)</summary><blockquote> `132-133`: **LGTM!** The condition correctly includes both 'column_field' and 'repeat_field' using a regular expression. --- `141-145`: **LGTM!** The check prevents modifications to the 'name' field of existing inner fields unless they are marked as new, ensuring data integrity. <details> <summary>Tools</summary> <details> <summary>Biome</summary><blockquote> [error] 142-142: Do not access Object.prototype method 'hasOwnProperty' from target object. It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty(). See MDN web docs for more details. (lint/suspicious/noPrototypeBuiltins) </blockquote></details> </details> </blockquote></details> <details> <summary>assets/js/wpuf-form-builder-components.js (2)</summary><blockquote> `454-455`: **LGTM!** The change correctly broadens the criteria to recognize both column and repeat fields, enhancing the flexibility of the component. --- `932-933`: **LGTM!** The renaming corrects a typographical error, ensuring consistency and clarity in the codebase. </blockquote></details> <details> <summary>assets/less/frontend-forms.less (4)</summary><blockquote> `565-565`: **LGTM!** The code changes are approved. --- `568-571`: **LGTM!** The code changes are approved. --- `574-574`: **LGTM!** The code changes are approved. --- `578-580`: **LGTM!** The code changes are approved. </blockquote></details> <details> <summary>assets/css/frontend-forms.css (6)</summary><blockquote> `507-507`: **LGTM!** The restructuring of the selector to apply styles to any element with the class `.wpuf-repeatable-field` enhances the flexibility of the styling. --- `510-513`: **LGTM!** The introduction of a flexbox layout for `.wpuf-repeatable-field.wpuf-field-columns` improves the alignment and spacing of child elements. --- `515-515`: **LGTM!** The changes ensure that `.wpuf-column-field-inner-columns` spans the full width of its container. --- `518-520`: **LGTM!** The changes apply `box-sizing: border-box` to all elements within `.wpuf-repeatable-field`, improving the sizing behavior of the fields. --- `521-524`: **LGTM!** The changes ensure that `.wpuf-repeater-buttons` has a fixed width and padding, improving the layout and spacing of repeater buttons. --- `525-529`: **LGTM!** The changes ensure that images and icons within `.wpuf-repeater-buttons` are correctly sized and aligned. </blockquote></details> <details> <summary>includes/Ajax/Frontend_Form_Ajax.php (1)</summary><blockquote> `431-433`: **Avoid using `extract` function.** Avoid using the `extract` function as it can lead to variable name collisions and security issues. Apply this diff to avoid using the `extract` function: ```diff - extract( shortcode_atts( [ 'id' => 0 ], $atts ) ); + $atts = shortcode_atts( [ 'id' => 0 ], $atts ); + $id = $atts['id'];Likely invalid or redundant comment.
assets/js/frontend-form.js (1)
21-29: LGTM!The event listeners for cloning and removing repeated fields are correctly added.
wpuf-functions.php (1)
1835-1837: LGTM!The code changes are approved.
| // check if the editing field belong to a column field | ||
| if (state.form_fields[i].template === 'column_field') { | ||
| // check if the editing field belong to a column field or repeat field | ||
| if (state.form_fields[i].template.match(/^(column|repeat)_field$/)) { | ||
| var innerColumnFields = state.form_fields[i].inner_fields; | ||
|
|
||
| for (const columnFields in innerColumnFields) { | ||
| if (innerColumnFields.hasOwnProperty(columnFields)) { |
There was a problem hiding this comment.
Use Object.hasOwn() instead of hasOwnProperty.
It's recommended to use Object.hasOwn() for better reliability. See MDN web docs for more details.
Apply this diff to replace hasOwnProperty with Object.hasOwn():
- if (innerColumnFields.hasOwnProperty(columnFields)) {
+ if (Object.hasOwn(innerColumnFields, columnFields)) {- if (payload.field_name === 'name' && ! innerColumnFields[columnFields][columnFieldIndex].hasOwnProperty('is_new') ) {
+ if (payload.field_name === 'name' && ! Object.hasOwn(innerColumnFields[columnFields][columnFieldIndex], 'is_new') ) {Also applies to: 142-142
Tools
Biome
[error] 137-137: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
| // check if the editing field belong to a column field | ||
| if (state.form_fields[i].template === 'column_field') { | ||
| // check if the editing field belong to a column field or repeat field | ||
| if (state.form_fields[i].template.match(/^(column|repeat)_field$/)) { | ||
| var innerColumnFields = state.form_fields[i].inner_fields; | ||
|
|
||
| for (const columnFields in innerColumnFields) { | ||
| if (innerColumnFields.hasOwnProperty(columnFields)) { |
There was a problem hiding this comment.
Use Object.hasOwn() instead of hasOwnProperty.
It's recommended to use Object.hasOwn() for better reliability. See MDN web docs for more details.
Apply this diff to replace hasOwnProperty with Object.hasOwn():
- if (innerColumnFields.hasOwnProperty(columnFields)) {
+ if (Object.hasOwn(innerColumnFields, columnFields)) {- if (payload.field_name === 'name' && ! innerColumnFields[columnFields][columnFieldIndex].hasOwnProperty('is_new') ) {
+ if (payload.field_name === 'name' && ! Object.hasOwn(innerColumnFields[columnFields][columnFieldIndex], 'is_new') ) {Also applies to: 142-142
Tools
Biome
[error] 137-137: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Comment on lines
+582
to
+583
| public function prepare_mail_body( $content, $user_id, $post_id ) { | ||
| $user = get_user_by( 'id', $user_id ); |
There was a problem hiding this comment.
Sanitize user and post data.
Sanitize user and post data before using them to ensure they are safe and free from malicious content.
Apply this diff to sanitize user and post data:
- $user = get_user_by( 'id', $user_id );
- $post = get_post( $post_id );
+ $user = sanitize_user( get_user_by( 'id', $user_id ) );
+ $post = sanitize_post( get_post( $post_id ) );Committable suggestion was skipped due to low confidence.
Comment on lines
+310
to
+311
| $url = $img->getAttribute( 'src' ); | ||
| $url = str_replace( [ '"', "'", '\\' ], '', $url ); |
There was a problem hiding this comment.
Sanitize URL before processing.
Sanitize the URL before processing it to ensure it is safe and free from malicious content.
Apply this diff to sanitize the URL:
- $url = str_replace( [ '"', "'", '\\' ], '', $url );
+ $url = esc_url_raw( str_replace( [ '"', "'", '\\' ], '', $url ) );Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $url = $img->getAttribute( 'src' ); | |
| $url = str_replace( [ '"', "'", '\\' ], '', $url ); | |
| $url = $img->getAttribute( 'src' ); | |
| $url = esc_url_raw( str_replace( [ '"', "'", '\\' ], '', $url ) ); |
Comment on lines
+627
to
+629
| // custom fields | ||
| preg_match_all( '/%custom_([\w-]*)\b%/', $content, $matches ); | ||
| [ $search, $replace ] = $matches; |
There was a problem hiding this comment.
Check for empty matches before destructuring.
Check if matches are not empty before destructuring to avoid potential errors.
Apply this diff to check for empty matches:
- preg_match_all( '/%custom_([\w-]*)\b%/', $content, $matches );
- [ $search, $replace ] = $matches;
+ preg_match_all( '/%custom_([\w-]*)\b%/', $content, $matches );
+ if ( ! empty( $matches ) ) {
+ [ $search, $replace ] = $matches;
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // custom fields | |
| preg_match_all( '/%custom_([\w-]*)\b%/', $content, $matches ); | |
| [ $search, $replace ] = $matches; | |
| // custom fields | |
| preg_match_all( '/%custom_([\w-]*)\b%/', $content, $matches ); | |
| if ( ! empty( $matches ) ) { | |
| [ $search, $replace ] = $matches; | |
| } |
Comment on lines
+357
to
+367
| if ( $user ) { | ||
| // $post_author = $user->ID; | ||
| wp_send_json( | ||
| [ | ||
| 'success' => false, | ||
| 'error' => __( "You already have an account in our site. Please login to continue.\n\nClicking 'OK' will redirect you to the login page and you will lose the form data.\nClick 'Cancel' to stay at this page.", 'wp-user-frontend' ), | ||
| 'type' => 'login', | ||
| 'redirect_to' => wp_login_url( get_permalink( $page_id ) ), | ||
| ] | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
Return a WP_Error object if user email already exists.
Return a WP_Error object instead of sending an error response directly to maintain consistency.
Apply this diff to return a WP_Error object:
- if ( $user ) {
- wp_send_json(
- [
- 'success' => false,
- 'error' => __( "You already have an account in our site. Please login to continue.\n\nClicking 'OK' will redirect you to the login page and you will lose the form data.\nClick 'Cancel' to stay at this page.", 'wp-user-frontend' ),
- 'type' => 'login',
- 'redirect_to' => wp_login_url( get_permalink( $page_id ) ),
- ]
- );
- }
+ if ( $user ) {
+ return new WP_Error(
+ 'email_exists',
+ __( "You already have an account in our site. Please login to continue.\n\nClicking 'OK' will redirect you to the login page and you will lose the form data.\nClick 'Cancel' to stay at this page.", 'wp-user-frontend' ),
+ [
+ 'type' => 'login',
+ 'redirect_to' => wp_login_url( get_permalink( $page_id ) ),
+ ]
+ );
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( $user ) { | |
| // $post_author = $user->ID; | |
| wp_send_json( | |
| [ | |
| 'success' => false, | |
| 'error' => __( "You already have an account in our site. Please login to continue.\n\nClicking 'OK' will redirect you to the login page and you will lose the form data.\nClick 'Cancel' to stay at this page.", 'wp-user-frontend' ), | |
| 'type' => 'login', | |
| 'redirect_to' => wp_login_url( get_permalink( $page_id ) ), | |
| ] | |
| ); | |
| } else { | |
| if ( $user ) { | |
| return new WP_Error( | |
| 'email_exists', | |
| __( "You already have an account in our site. Please login to continue.\n\nClicking 'OK' will redirect you to the login page and you will lose the form data.\nClick 'Cancel' to stay at this page.", 'wp-user-frontend' ), | |
| [ | |
| 'type' => 'login', | |
| 'redirect_to' => wp_login_url( get_permalink( $page_id ) ), | |
| ] | |
| ); | |
| } else { |
Comment on lines
+708
to
+710
| public function publish_guest_post() { | ||
| $post_msg = isset( $_GET['post_msg'] ) ? sanitize_text_field( wp_unslash( $_GET['post_msg'] ) ) : ''; | ||
| $pid = isset( $_GET['p_id'] ) ? sanitize_text_field( wp_unslash( $_GET['p_id'] ) ) : ''; |
There was a problem hiding this comment.
Sanitize and validate input parameters.
Sanitize and validate input parameters to ensure they are safe and valid.
Apply this diff to sanitize and validate input parameters:
- $post_msg = isset( $_GET['post_msg'] ) ? sanitize_text_field( wp_unslash( $_GET['post_msg'] ) ) : '';
- $pid = isset( $_GET['p_id'] ) ? sanitize_text_field( wp_unslash( $_GET['p_id'] ) ) : '';
- $fid = isset( $_GET['f_id'] ) ? sanitize_text_field( wp_unslash( $_GET['f_id'] ) ) : '';
+ $post_msg = isset( $_GET['post_msg'] ) ? sanitize_key( wp_unslash( $_GET['post_msg'] ) ) : '';
+ $pid = isset( $_GET['p_id'] ) ? sanitize_key( wp_unslash( $_GET['p_id'] ) ) : '';
+ $fid = isset( $_GET['f_id'] ) ? sanitize_key( wp_unslash( $_GET['f_id'] ) ) : '';Committable suggestion was skipped due to low confidence.
Comment on lines
+333
to
+334
| public function wpuf_get_post_user() { | ||
| $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_key( wp_unslash( $_REQUEST['_wpnonce'] ) ) : ''; |
There was a problem hiding this comment.
Sanitize and validate nonce.
Sanitize and validate the nonce to ensure it is safe and valid.
Apply this diff to sanitize and validate the nonce:
- $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_key( wp_unslash( $_REQUEST['_wpnonce'] ) ) : '';
+ $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ) : '';Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public function wpuf_get_post_user() { | |
| $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_key( wp_unslash( $_REQUEST['_wpnonce'] ) ) : ''; | |
| public function wpuf_get_post_user() { | |
| $nonce = isset( $_REQUEST['_wpnonce'] ) ? sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ) : ''; |
Comment on lines
+1775
to
+1811
| if ( 'repeat' === $field['input_type'] ) { | ||
| if ( ! isset( $field['multiple'] ) ) { | ||
| $field['multiple'] = ''; | ||
| } | ||
|
|
||
| // if old repeat field format | ||
| if ( empty( $field['inner_fields'] ) ) { | ||
| $old_id = $field['id']; | ||
| $old_meta = $field['name']; | ||
| $old_label = $field['label']; | ||
| $new_id = wpuf_form_field_id_generator(); | ||
| $field['template'] = 'text_field'; | ||
|
|
||
| // set the new compatible values | ||
| $field['id'] = $new_id; | ||
| $field['name'] = $old_meta . '_' . $new_id; | ||
| $field['label'] = ''; | ||
| $field['inner_fields']['column-1'] = [ $field ]; | ||
| $field['inner_fields']['column-2'] = []; | ||
| $field['inner_fields']['column-3'] = []; | ||
| $field['template'] = 'repeat_field'; | ||
| $field['columns'] = 1; | ||
| $field['min_column'] = 1; | ||
| $field['max_column'] = 3; | ||
| $field['column_space'] = 5; | ||
|
|
||
| $field['id'] = $old_id; | ||
| $field['label'] = $old_label; | ||
| $field['name'] = $old_meta; | ||
| } | ||
|
|
||
| // if old repeat field format | ||
| if ( empty( $field['inner_columns_size'] ) ) { | ||
| $field['inner_columns_size']['column-1'] = '100%'; | ||
| $field['inner_columns_size']['column-2'] = '100%'; | ||
| $field['inner_columns_size']['column-3'] = '100%'; | ||
| } |
There was a problem hiding this comment.
Ensure backward compatibility and improve readability.
The changes ensure backward compatibility for old repeat field formats. However, the nested conditionals and assignments make the code hard to read. Consider refactoring the code to improve readability.
Extract the backward compatibility logic into a separate function.
+function ensure_backward_compatibility($field) {
+ if (empty($field['inner_fields'])) {
+ $old_id = $field['id'];
+ $old_meta = $field['name'];
+ $old_label = $field['label'];
+ $new_id = wpuf_form_field_id_generator();
+ $field['template'] = 'text_field';
+ $field['id'] = $new_id;
+ $field['name'] = $old_meta . '_' . $new_id;
+ $field['label'] = '';
+ $field['inner_fields']['column-1'] = [$field];
+ $field['inner_fields']['column-2'] = [];
+ $field['inner_fields']['column-3'] = [];
+ $field['template'] = 'repeat_field';
+ $field['columns'] = 1;
+ $field['min_column'] = 1;
+ $field['max_column'] = 3;
+ $field['column_space'] = 5;
+ $field['id'] = $old_id;
+ $field['label'] = $old_label;
+ $field['name'] = $old_meta;
+ }
+ if (empty($field['inner_columns_size'])) {
+ $field['inner_columns_size']['column-1'] = '100%';
+ $field['inner_columns_size']['column-2'] = '100%';
+ $field['inner_columns_size']['column-3'] = '100%';
+ }
+ return $field;
+}
function wpuf_get_form_fields($form_id) {
// ... existing code ...
foreach ($fields as $key => $content) {
$field = (array) maybe_unserialize($content->post_content);
// ... existing code ...
if ('repeat' === $field['input_type']) {
if (!isset($field['multiple'])) {
$field['multiple'] = '';
}
$field = ensure_backward_compatibility($field);
}
// ... existing code ...
}
// ... existing code ...
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( 'repeat' === $field['input_type'] ) { | |
| if ( ! isset( $field['multiple'] ) ) { | |
| $field['multiple'] = ''; | |
| } | |
| // if old repeat field format | |
| if ( empty( $field['inner_fields'] ) ) { | |
| $old_id = $field['id']; | |
| $old_meta = $field['name']; | |
| $old_label = $field['label']; | |
| $new_id = wpuf_form_field_id_generator(); | |
| $field['template'] = 'text_field'; | |
| // set the new compatible values | |
| $field['id'] = $new_id; | |
| $field['name'] = $old_meta . '_' . $new_id; | |
| $field['label'] = ''; | |
| $field['inner_fields']['column-1'] = [ $field ]; | |
| $field['inner_fields']['column-2'] = []; | |
| $field['inner_fields']['column-3'] = []; | |
| $field['template'] = 'repeat_field'; | |
| $field['columns'] = 1; | |
| $field['min_column'] = 1; | |
| $field['max_column'] = 3; | |
| $field['column_space'] = 5; | |
| $field['id'] = $old_id; | |
| $field['label'] = $old_label; | |
| $field['name'] = $old_meta; | |
| } | |
| // if old repeat field format | |
| if ( empty( $field['inner_columns_size'] ) ) { | |
| $field['inner_columns_size']['column-1'] = '100%'; | |
| $field['inner_columns_size']['column-2'] = '100%'; | |
| $field['inner_columns_size']['column-3'] = '100%'; | |
| } | |
| function ensure_backward_compatibility($field) { | |
| if (empty($field['inner_fields'])) { | |
| $old_id = $field['id']; | |
| $old_meta = $field['name']; | |
| $old_label = $field['label']; | |
| $new_id = wpuf_form_field_id_generator(); | |
| $field['template'] = 'text_field'; | |
| $field['id'] = $new_id; | |
| $field['name'] = $old_meta . '_' . $new_id; | |
| $field['label'] = ''; | |
| $field['inner_fields']['column-1'] = [$field]; | |
| $field['inner_fields']['column-2'] = []; | |
| $field['inner_fields']['column-3'] = []; | |
| $field['template'] = 'repeat_field'; | |
| $field['columns'] = 1; | |
| $field['min_column'] = 1; | |
| $field['max_column'] = 3; | |
| $field['column_space'] = 5; | |
| $field['id'] = $old_id; | |
| $field['label'] = $old_label; | |
| $field['name'] = $old_meta; | |
| } | |
| if (empty($field['inner_columns_size'])) { | |
| $field['inner_columns_size']['column-1'] = '100%'; | |
| $field['inner_columns_size']['column-2'] = '100%'; | |
| $field['inner_columns_size']['column-3'] = '100%'; | |
| } | |
| return $field; | |
| } | |
| if ( 'repeat' === $field['input_type'] ) { | |
| if ( ! isset( $field['multiple'] ) ) { | |
| $field['multiple'] = ''; | |
| } | |
| $field = ensure_backward_compatibility($field); | |
| } |
Comment on lines
+921
to
+955
|
|
||
| // get repeat field inner fields | ||
| if ( 'repeat' === $attr['input_type'] ) { | ||
| $inner_fields = $attr['inner_fields']; | ||
| foreach ( $inner_fields as $column_key => $column_fields ) { | ||
| if ( ! empty( $column_fields ) ) { | ||
| // ignore section break and HTML input type | ||
| foreach ( $column_fields as $column_field_key => $column_field ) { | ||
| if ( isset( $column_field['show_in_post'] ) && 'yes' === $column_field['show_in_post'] ) { | ||
| $repeat_field_name = ! empty( $attr['name'] ) ? $attr['name'] : ''; | ||
| $repeat_rows = get_post_meta( $post->ID, $repeat_field_name, true ); | ||
|
|
||
| if ( ! empty( $repeat_rows ) ) { | ||
| for ( $index = 0; $index < $repeat_rows; $index++ ) { | ||
| $field_value = get_post_meta( $post->ID, $repeat_field_name . '_' . $index . '_' . $column_field['name'], true ); | ||
| $hide_label = ! empty( $column_field['hide_field_label'] ) ? $column_field['hide_field_label'] : 'no'; | ||
|
|
||
| if ( is_array( $field_value ) ) { | ||
| $field_value = implode( ', ', $field_value ); | ||
| } | ||
|
|
||
| $html .= '<li>'; | ||
|
|
||
| if ( 'no' === $hide_label ) { | ||
| $html .= '<label>' . $column_field['label'] . ': </label>'; | ||
| } | ||
|
|
||
| $html .= make_clickable( $field_value ) . '</li>'; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance readability and maintainability.
The nested loops and conditionals make the code hard to read and maintain. Consider refactoring the code to improve readability and maintainability.
Extract the inner loop into a separate function to handle the processing of repeat fields.
+function process_repeat_fields($post, $attr, $column_fields) {
+ $html = '';
+ foreach ($column_fields as $column_field_key => $column_field) {
+ if (isset($column_field['show_in_post']) && 'yes' === $column_field['show_in_post']) {
+ $repeat_field_name = !empty($attr['name']) ? $attr['name'] : '';
+ $repeat_rows = get_post_meta($post->ID, $repeat_field_name, true);
+ if (!empty($repeat_rows)) {
+ for ($index = 0; $index < $repeat_rows; $index++) {
+ $field_value = get_post_meta($post->ID, $repeat_field_name . '_' . $index . '_' . $column_field['name'], true);
+ $hide_label = !empty($column_field['hide_field_label']) ? $column_field['hide_field_label'] : 'no';
+ if (is_array($field_value)) {
+ $field_value = implode(', ', $field_value);
+ }
+ $html .= '<li>';
+ if ('no' === $hide_label) {
+ $html .= '<label>' . $column_field['label'] . ': </label>';
+ }
+ $html .= make_clickable($field_value) . '</li>';
+ }
+ }
+ }
+ }
+ return $html;
+}
function wpuf_show_custom_fields($content) {
global $post;
// ... existing code ...
if ($form_vars) {
foreach ($form_vars as $attr) {
// ... existing code ...
if ('repeat' === $attr['input_type']) {
$inner_fields = $attr['inner_fields'];
foreach ($inner_fields as $column_key => $column_fields) {
if (!empty($column_fields)) {
$html .= process_repeat_fields($post, $attr, $column_fields);
}
}
}
}
}
// ... existing code ...
}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // get repeat field inner fields | |
| if ( 'repeat' === $attr['input_type'] ) { | |
| $inner_fields = $attr['inner_fields']; | |
| foreach ( $inner_fields as $column_key => $column_fields ) { | |
| if ( ! empty( $column_fields ) ) { | |
| // ignore section break and HTML input type | |
| foreach ( $column_fields as $column_field_key => $column_field ) { | |
| if ( isset( $column_field['show_in_post'] ) && 'yes' === $column_field['show_in_post'] ) { | |
| $repeat_field_name = ! empty( $attr['name'] ) ? $attr['name'] : ''; | |
| $repeat_rows = get_post_meta( $post->ID, $repeat_field_name, true ); | |
| if ( ! empty( $repeat_rows ) ) { | |
| for ( $index = 0; $index < $repeat_rows; $index++ ) { | |
| $field_value = get_post_meta( $post->ID, $repeat_field_name . '_' . $index . '_' . $column_field['name'], true ); | |
| $hide_label = ! empty( $column_field['hide_field_label'] ) ? $column_field['hide_field_label'] : 'no'; | |
| if ( is_array( $field_value ) ) { | |
| $field_value = implode( ', ', $field_value ); | |
| } | |
| $html .= '<li>'; | |
| if ( 'no' === $hide_label ) { | |
| $html .= '<label>' . $column_field['label'] . ': </label>'; | |
| } | |
| $html .= make_clickable( $field_value ) . '</li>'; | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| function process_repeat_fields($post, $attr, $column_fields) { | |
| $html = ''; | |
| foreach ($column_fields as $column_field_key => $column_field) { | |
| if (isset($column_field['show_in_post']) && 'yes' === $column_field['show_in_post']) { | |
| $repeat_field_name = !empty($attr['name']) ? $attr['name'] : ''; | |
| $repeat_rows = get_post_meta($post->ID, $repeat_field_name, true); | |
| if (!empty($repeat_rows)) { | |
| for ($index = 0; $index < $repeat_rows; $index++) { | |
| $field_value = get_post_meta($post->ID, $repeat_field_name . '_' . $index . '_' . $column_field['name'], true); | |
| $hide_label = !empty($column_field['hide_field_label']) ? $column_field['hide_field_label'] : 'no'; | |
| if (is_array($field_value)) { | |
| $field_value = implode(', ', $field_value); | |
| } | |
| $html .= '<li>'; | |
| if ('no' === $hide_label) { | |
| $html .= '<label>' . $column_field['label'] . ': </label>'; | |
| } | |
| $html .= make_clickable($field_value) . '</li>'; | |
| } | |
| } | |
| } | |
| } | |
| return $html; | |
| } | |
| function wpuf_show_custom_fields($content) { | |
| global $post; | |
| // ... existing code ... | |
| if ($form_vars) { | |
| foreach ($form_vars as $attr) { | |
| // ... existing code ... | |
| if ('repeat' === $attr['input_type']) { | |
| $inner_fields = $attr['inner_fields']; | |
| foreach ($inner_fields as $column_key => $column_fields) { | |
| if (!empty($column_fields)) { | |
| $html .= process_repeat_fields($post, $attr, $column_fields); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| // ... existing code ... | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently, the repeating field is hardcoded as a text field. With this enhancement, we could make almost any field into a repeating field.
Related PR #531
resolves #93, resolves #1246, resolves #1240, resolves #1307
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores