Fix/add data removing option#1251
Open
sapayth wants to merge 13 commits intoweDevsOfficial:developfrom
Open
Conversation
sapayth
added
needs: dev review
reset Tools > Uninstall Settings anyway upon plugin deletion
FaraziF
added
QA Approved
Contributor
|
Issue: Scenario: |
Member
Author
Seems ok from my end. Would you please test again? |
Contributor
Still exists, when you select all the options and then Delete the Plugin |
sapayth
added
dev: on-progress
and removed
QA Approved
Contributor
|
@sapayth the issue still exists, selecting all the settings given in PR and then uninstalling plugin > takes away admin's ability to access site Screenshot for reference: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new uninstall settings UI to admin settings, a Tools admin page with form import/export and maintenance actions, and an uninstall script that conditionally deletes plugin tables, posts/pages, and options based on the stored uninstall settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
admin/class-tools.php (3)
210-223: Useelseifinstead ofelse if.WordPress coding standards prefer
elseifoverelse if.🧹 Proposed fix
public function export_forms( $form_type, $export_type, $form_ids ) { if ( $export_type === 'all' ) { static::export_to_json( $form_type ); - } else if ( $export_type === 'selected' ) { + } elseif ( $export_type === 'selected' ) { if ( empty( $form_ids ) ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin/class-tools.php` around lines 210 - 223, In the export_forms method, replace the "else if" conditional with the WordPress-preferred "elseif" form to conform to coding standards; locate the function export_forms and change the "else if ( $export_type === 'selected' )" to "elseif ( $export_type === 'selected' )" (keeping the same block logic and calls to static::export_to_json and the empty($form_ids) check) so only the conditional syntax is updated.
287-295: WPCS style violations informetted_meta_key_value().Multiple coding standard issues:
- Reserved keyword
arrayas parameter name- Extra space in empty array declaration
- Missing spaces around array key
Also note: method name has a typo — "formetted" should be "formatted".
🧹 Proposed fixes
- public function formetted_meta_key_value( $array ) { - $result = [ ]; + public function formatted_meta_key_value( $data ) { + $result = []; foreach ( $array as $key => $val ) { - $result[$key] = $val[0]; + $result[ $key ] = $val[0]; } return $result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin/class-tools.php` around lines 287 - 295, Rename the method formetted_meta_key_value to formatted_meta_key_value, change the parameter from the reserved name $array to something like $meta (or $items), normalize the empty array declaration to $result = []; and apply WPCS spacing for array keys/indices (e.g. $result[ $key ] = $val[0];). Update any callers of formetted_meta_key_value to the new method name and new parameter name as needed.
231-278: Multiple WPCS violations and potential timezone issues.Several issues flagged by the pipeline:
- Line 235:
date()should begmdate()to avoid timezone issues- Line 238, 247: Missing space after
!- Line 268:
json_encode()should bewp_json_encode()♻️ Proposed fixes
public static function export_to_json( $post_type, $post_ids = [] ) { $formatted_data = []; $ids = []; $blogname = strtolower( str_replace( ' ', '-', get_option( 'blogname' ) ) ); - $date = date( 'Y-m-d' ); + $date = gmdate( 'Y-m-d' ); $json_name = $blogname . '-wpuf-' . $post_type . '-' . $date; // Namming the filename will be generated. - if ( !empty( $post_ids ) ) { + if ( ! empty( $post_ids ) ) { foreach ( $post_ids as $key => $value ) { array_push( $ids, $value ); } } $args = [ 'post_status' => 'publish', 'post_type' => $post_type, - 'post__in' => ( !empty( $ids ) ) ? $ids : '', + 'post__in' => ( ! empty( $ids ) ) ? $ids : '', ]; // ... existing code ... - $json_file = json_encode( $formatted_data ); // Encode data into json data + $json_file = wp_json_encode( $formatted_data ); // Encode data into json data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@admin/class-tools.php` around lines 231 - 278, Change date() to gmdate() when building $date in export_to_json to avoid timezone issues (update the $date assignment in export_to_json), ensure there is a space after the negation operator in conditionals like if ( ! empty( $post_ids ) ) and the ternary for 'post__in' (use ! empty( $ids ) ), and replace json_encode( $formatted_data ) with wp_json_encode( $formatted_data ) when assigning $json_file; these edits are all inside the export_to_json method and reference $date, the if ( ! empty( $post_ids ) ) check, the 'post__in' => ( ! empty( $ids ) ) ? $ids : '' expression, and the $json_file = ... line.uninstall.php (2)
54-73: Remove unusedget_all_page_ids()method.This private method is never called. Page deletion on line 33 uses direct SQL instead. Remove dead code to improve maintainability.
♻️ Proposed fix
- /** - * Delete all WordPress page ids as an array - * - * `@since` WPUF - * - * `@return` void - */ - private function get_all_page_ids( $post_type = 'page' ) { - $page_ids = []; - $pages = get_posts( - [ - 'post_type' => $post_type, - 'numberposts' => - 1, - 'no_found_rows' => true, - 'update_post_meta_cache' => false, - 'update_post_term_cache' => false, - ] - ); - - if ( $pages ) { - foreach ( $pages as $page ) { - $page_ids[] = $page->ID; - } - } - - return $page_ids; - } -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@uninstall.php` around lines 54 - 73, Remove the dead private method get_all_page_ids( $post_type = 'page' ) and its entire body from uninstall.php because page deletion is handled elsewhere via direct SQL; search the codebase for any references to get_all_page_ids and remove or update them if found to avoid undefined calls, then run tests/lint to confirm no usages remain.
134-141: Non-existent tables in get_tables() list.Per
includes/Installer.php, onlywpuf_transactionandwpuf_subscriberstables are created. Thewpuf_messageandwpuf_activitytables don't exist in the codebase. WhileDROP TABLE IF EXISTShandles this gracefully, you should remove references to non-existent tables to avoid confusion.🧹 Proposed cleanup
private function get_tables() { return [ 'wpuf_transaction', 'wpuf_subscribers', - 'wpuf_message', - 'wpuf_activity', ]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@uninstall.php` around lines 134 - 141, The get_tables() method currently lists non-existent tables (wpuf_message, wpuf_activity); update the get_tables() function to only return the actual created tables (wpuf_transaction and wpuf_subscribers) as defined in includes/Installer.php, removing wpuf_message and wpuf_activity to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@admin/class-tools.php`:
- Around line 129-132: The sprintf() call in the else branch is creating a
string but discarding it (same bug as in list_forms); update the else branch so
the message is actually output—either replace sprintf() with printf() or wrap it
with echo (e.g., echo sprintf(...)) so the '<p>Sorry you have no form to
export</p>' is rendered; locate the sprintf call in class-tools.php and modify
it accordingly.
- Around line 115-119: Remove the stray literal '";' after the PHP echo in the
option element inside the foreach loop; locate the foreach ( $forms as $form )
block and the line that renders <option value="<?php echo esc_attr( $form->ID );
?>"><?php echo esc_attr( $form->post_title ); ?></option>"; and delete the
trailing '";' so only the option markup and PHP echo remain.
- Around line 270-277: The header() calls are sent after echoing $json_file
which can trigger "headers already sent" warnings; move the header(
'Content-Type: ...' ) and header( "Content-Disposition: attachment;
filename=$json_name.json" ) lines to before the echo $json_file (keep ob_clean()
before the headers if you want to clear buffers), ensure $json_name is safely
escaped for the filename, then echo $json_file and exit(); so headers are always
sent prior to any output.
- Around line 65-67: The sprintf() call in the else branch is returning a string
that is never output, so users never see the "no forms" message; update the else
branch in admin/class-tools.php (the block containing sprintf( '<p>%s</p>', __(
'Sorry you have no form to export', 'wp-user-frontend' ) );) to actually output
the string—e.g., use echo sprintf(...) or replace with printf(...) (and ensure
proper escaping if needed) so the message is rendered to the user.
In `@uninstall.php`:
- Around line 32-34: The DELETE query under the
uninstall_settings['delete_pages'] check only matches post_content that starts
with the shortcode; update the SQL in the $wpdb->query call that targets
{$wpdb->posts} (the DELETE WHERE post_type = 'page' AND post_content LIKE ...)
to use a contains pattern (change LIKE '[wpuf%' to LIKE '%[wpuf%') so pages that
contain WPUF shortcodes anywhere in the content are matched; verify this change
in the uninstall_settings / delete_pages flow to ensure it aligns with the
intended deletion policy.
- Around line 36-38: The current DELETE uses a substring pattern "%wpuf_%" which
matches anywhere; change it to a prefix match so only options that start with
the wpuf_ prefix are removed: update the $wpdb->query call that references
$wpdb->options and option_name to use a LIKE pattern that matches from the start
(e.g., "wpuf\_%" with an appropriate SQL ESCAPE or escaped backslash in the PHP
string) instead of "%wpuf_%", ensuring you still target only options beginning
with the wpuf_ prefix when checking uninstall_settings['delete_settings'].
---
Nitpick comments:
In `@admin/class-tools.php`:
- Around line 210-223: In the export_forms method, replace the "else if"
conditional with the WordPress-preferred "elseif" form to conform to coding
standards; locate the function export_forms and change the "else if (
$export_type === 'selected' )" to "elseif ( $export_type === 'selected' )"
(keeping the same block logic and calls to static::export_to_json and the
empty($form_ids) check) so only the conditional syntax is updated.
- Around line 287-295: Rename the method formetted_meta_key_value to
formatted_meta_key_value, change the parameter from the reserved name $array to
something like $meta (or $items), normalize the empty array declaration to
$result = []; and apply WPCS spacing for array keys/indices (e.g. $result[ $key
] = $val[0];). Update any callers of formetted_meta_key_value to the new method
name and new parameter name as needed.
- Around line 231-278: Change date() to gmdate() when building $date in
export_to_json to avoid timezone issues (update the $date assignment in
export_to_json), ensure there is a space after the negation operator in
conditionals like if ( ! empty( $post_ids ) ) and the ternary for 'post__in'
(use ! empty( $ids ) ), and replace json_encode( $formatted_data ) with
wp_json_encode( $formatted_data ) when assigning $json_file; these edits are all
inside the export_to_json method and reference $date, the if ( ! empty(
$post_ids ) ) check, the 'post__in' => ( ! empty( $ids ) ) ? $ids : ''
expression, and the $json_file = ... line.
In `@uninstall.php`:
- Around line 54-73: Remove the dead private method get_all_page_ids( $post_type
= 'page' ) and its entire body from uninstall.php because page deletion is
handled elsewhere via direct SQL; search the codebase for any references to
get_all_page_ids and remove or update them if found to avoid undefined calls,
then run tests/lint to confirm no usages remain.
- Around line 134-141: The get_tables() method currently lists non-existent
tables (wpuf_message, wpuf_activity); update the get_tables() function to only
return the actual created tables (wpuf_transaction and wpuf_subscribers) as
defined in includes/Installer.php, removing wpuf_message and wpuf_activity to
avoid confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc2f1c19-d9ce-4022-8add-4898aca215d4
📒 Files selected for processing (3)
admin/class-admin-settings.phpadmin/class-tools.phpuninstall.php
Comment on lines
+65
to
+67
| } else { | ||
| sprintf( '<p>%s</p>', __( 'Sorry you have no form to export', 'wp-user-frontend' ) ); | ||
| } |
There was a problem hiding this comment.
Bug: sprintf() output is discarded.
The sprintf() return value is not echoed, so the "no forms" message is never displayed to users.
🐛 Proposed fix
<?php
} else {
- sprintf( '<p>%s</p>', __( 'Sorry you have no form to export', 'wp-user-frontend' ) );
+ printf( '<p>%s</p>', esc_html__( 'Sorry you have no form to export', 'wp-user-frontend' ) );
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin/class-tools.php` around lines 65 - 67, The sprintf() call in the else
branch is returning a string that is never output, so users never see the "no
forms" message; update the else branch in admin/class-tools.php (the block
containing sprintf( '<p>%s</p>', __( 'Sorry you have no form to export',
'wp-user-frontend' ) );) to actually output the string—e.g., use echo
sprintf(...) or replace with printf(...) (and ensure proper escaping if needed)
so the message is rendered to the user.
Comment on lines
+115
to
+119
| <select class="formlist" name="form_ids[]" multiple="multiple"> | ||
| <?php foreach ( $forms as $form ) { ?> | ||
| <option value="<?php echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title ); ?></option>"; | ||
| <?php } ?> | ||
| </select> |
There was a problem hiding this comment.
HTML syntax error: stray "; will be rendered in output.
Line 117 has extra "; after the closing PHP tag that will appear as literal text in the select dropdown.
🐛 Proposed fix
<select class="formlist" name="form_ids[]" multiple="multiple">
<?php foreach ( $forms as $form ) { ?>
- <option value="<?php echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title ); ?></option>";
+ <option value="<?php echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title ); ?></option>
<?php } ?>
</select>📝 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
| <select class="formlist" name="form_ids[]" multiple="multiple"> | |
| <?php foreach ( $forms as $form ) { ?> | |
| <option value="<?php echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title ); ?></option>"; | |
| <?php } ?> | |
| </select> | |
| <select class="formlist" name="form_ids[]" multiple="multiple"> | |
| <?php foreach ( $forms as $form ) { ?> | |
| <option value="<?php echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title ); ?></option> | |
| <?php } ?> | |
| </select> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin/class-tools.php` around lines 115 - 119, Remove the stray literal '";'
after the PHP echo in the option element inside the foreach loop; locate the
foreach ( $forms as $form ) block and the line that renders <option value="<?php
echo esc_attr( $form->ID ); ?>"><?php echo esc_attr( $form->post_title );
?></option>"; and delete the trailing '";' so only the option markup and PHP
echo remain.
Comment on lines
+129
to
+132
| <?php | ||
| } else { | ||
| sprintf( '<p>%s</p>', __( 'Sorry you have no form to export', 'wp-user-frontend' ) ); | ||
| } |
There was a problem hiding this comment.
Bug: sprintf() output is discarded (same as list_forms).
🐛 Proposed fix
} else {
- sprintf( '<p>%s</p>', __( 'Sorry you have no form to export', 'wp-user-frontend' ) );
+ printf( '<p>%s</p>', esc_html__( 'Sorry you have no form to export', 'wp-user-frontend' ) );
}
}📝 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
| <?php | |
| } else { | |
| sprintf( '<p>%s</p>', __( 'Sorry you have no form to export', 'wp-user-frontend' ) ); | |
| } | |
| <?php | |
| } else { | |
| printf( '<p>%s</p>', esc_html__( 'Sorry you have no form to export', 'wp-user-frontend' ) ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin/class-tools.php` around lines 129 - 132, The sprintf() call in the else
branch is creating a string but discarding it (same bug as in list_forms);
update the else branch so the message is actually output—either replace
sprintf() with printf() or wrap it with echo (e.g., echo sprintf(...)) so the
'<p>Sorry you have no form to export</p>' is rendered; locate the sprintf call
in class-tools.php and modify it accordingly.
Comment on lines
+270
to
+277
| ob_clean(); | ||
|
|
||
| echo $json_file; // phpcs:ignore WordPress.XSS.EscapeOutput.OutputNotEscaped | ||
|
|
||
| header( 'Content-Type: text/json; charset=' . get_option( 'blog_charset' ) ); | ||
| header( "Content-Disposition: attachment; filename=$json_name.json" ); | ||
|
|
||
| exit(); |
There was a problem hiding this comment.
Send headers before output to prevent "headers already sent" errors.
The header() calls on lines 274-275 occur after echo on line 272. This will cause a PHP warning if output buffering is disabled, and potentially corrupt the downloaded file.
🐛 Proposed fix — move headers before output
$json_file = wp_json_encode( $formatted_data );
ob_clean();
+ header( 'Content-Type: text/json; charset=' . get_option( 'blog_charset' ) );
+ header( "Content-Disposition: attachment; filename=$json_name.json" );
+
echo $json_file; // phpcs:ignore WordPress.XSS.EscapeOutput.OutputNotEscaped
- header( 'Content-Type: text/json; charset=' . get_option( 'blog_charset' ) );
- header( "Content-Disposition: attachment; filename=$json_name.json" );
-
exit();
}📝 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
| ob_clean(); | |
| echo $json_file; // phpcs:ignore WordPress.XSS.EscapeOutput.OutputNotEscaped | |
| header( 'Content-Type: text/json; charset=' . get_option( 'blog_charset' ) ); | |
| header( "Content-Disposition: attachment; filename=$json_name.json" ); | |
| exit(); | |
| ob_clean(); | |
| header( 'Content-Type: text/json; charset=' . get_option( 'blog_charset' ) ); | |
| header( "Content-Disposition: attachment; filename=$json_name.json" ); | |
| echo $json_file; // phpcs:ignore WordPress.XSS.EscapeOutput.OutputNotEscaped | |
| exit(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@admin/class-tools.php` around lines 270 - 277, The header() calls are sent
after echoing $json_file which can trigger "headers already sent" warnings; move
the header( 'Content-Type: ...' ) and header( "Content-Disposition: attachment;
filename=$json_name.json" ) lines to before the echo $json_file (keep ob_clean()
before the headers if you want to clear buffers), ensure $json_name is safely
escaped for the filename, then echo $json_file and exit(); so headers are always
sent prior to any output.
Comment on lines
+36
to
+38
| if ( ! empty( $uninstall_settings['delete_settings'] ) ) { | ||
| $wpdb->query( "DELETE FROM {$wpdb->options} WHERE option_name LIKE '%wpuf_%'" ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all option names used by this plugin to verify naming conventions
rg -n "get_option\s*\(\s*['\"]" --type php | grep -i wpuf | head -30
rg -n "update_option\s*\(\s*['\"]" --type php | grep -i wpuf | head -30
rg -n "delete_option\s*\(\s*['\"]" --type php | grep -i wpuf | head -30Repository: weDevsOfficial/wp-user-frontend
Length of output: 7647
🏁 Script executed:
cat -n uninstall.phpRepository: weDevsOfficial/wp-user-frontend
Length of output: 4610
Use prefix matching instead of substring matching for better precision.
The pattern '%wpuf_%' is unnecessarily broad. Since all WPUF options use the wpuf_ prefix consistently, use a more precise prefix match:
Suggested change
if ( ! empty( $uninstall_settings['delete_settings'] ) ) {
- $wpdb->query( "DELETE FROM {$wpdb->options} WHERE option_name LIKE '%wpuf_%'" );
+ $wpdb->query( "DELETE FROM {$wpdb->options} WHERE option_name LIKE 'wpuf_%'" );
}📝 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 ( ! empty( $uninstall_settings['delete_settings'] ) ) { | |
| $wpdb->query( "DELETE FROM {$wpdb->options} WHERE option_name LIKE '%wpuf_%'" ); | |
| } | |
| if ( ! empty( $uninstall_settings['delete_settings'] ) ) { | |
| $wpdb->query( "DELETE FROM {$wpdb->options} WHERE option_name LIKE 'wpuf_%'" ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@uninstall.php` around lines 36 - 38, The current DELETE uses a substring
pattern "%wpuf_%" which matches anywhere; change it to a prefix match so only
options that start with the wpuf_ prefix are removed: update the $wpdb->query
call that references $wpdb->options and option_name to use a LIKE pattern that
matches from the start (e.g., "wpuf\_%" with an appropriate SQL ESCAPE or
escaped backslash in the PHP string) instead of "%wpuf_%", ensuring you still
target only options beginning with the wpuf_ prefix when checking
uninstall_settings['delete_settings'].
Replace direct DB deletion of pages with a new delete_pages() method that selects pages containing WPUF shortcodes and removes them via wp_delete_post() to ensure proper cleanup. Remove the old get_all_page_ids helper and tidy up uninstall flows (normalize WP_Query args, remove extra blank lines). Keep existing settings, options cleanup, and table drops intact; maintain postmeta cleanup after form deletions.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
In User Frontend > Tools a new setting is introduced to delete some data upon plugin deletion. When you delete the WP User Frontend version, it will also delete all the data related to WP User Frontend Pro as well. This won't happen when the plugins are deactivated.
Settings that will be removed are:
all options containing wpuf_
DB tables that will be removed are:
All pages containing wpuf shortcode
Summary by CodeRabbit