feat: add country-based data deletion for GDPR erasure (closes #42) +semver: minor#50
Merged
Merged
Conversation
Reviewer's GuideImplements GDPR-oriented visitor data erasure by country by wiring a new admin-post handler, DB utilities for country-based deletion and logging, and a UI panel on the visitors admin screen to trigger the operation and display feedback. Sequence diagram for GDPR delete-by-country admin workflowsequenceDiagram
actor Admin
participant VisitorsAdminScreen
participant Browser
participant WordPressAdminPost
participant IpQuery_Admin
participant IpQuery_DB
participant WPDB
participant WPDebugLog
Admin->>VisitorsAdminScreen: Open visitors.php
VisitorsAdminScreen->>IpQuery_DB: get_distinct_countries()
IpQuery_DB->>WPDB: SELECT DISTINCT country, country_code ...
WPDB-->>IpQuery_DB: Result rows
IpQuery_DB-->>VisitorsAdminScreen: Countries array
VisitorsAdminScreen-->>Admin: Render Delete by Country panel
Admin->>Browser: Select country and click Delete Records
Browser->>Browser: Show confirm dialog
Browser->>WordPressAdminPost: POST action=ipquery_delete_by_country
WordPressAdminPost->>IpQuery_Admin: handle_delete_by_country()
IpQuery_Admin->>IpQuery_Admin: check_admin_referer()
IpQuery_Admin->>IpQuery_Admin: current_user_can(manage_options)
IpQuery_Admin->>IpQuery_Admin: Sanitize and normalize country_code
IpQuery_Admin->>IpQuery_DB: delete_by_country(country_code)
IpQuery_DB->>WPDB: DELETE FROM table WHERE country_code = ?
WPDB-->>IpQuery_DB: rows_deleted or false
IpQuery_DB-->>IpQuery_Admin: rows_deleted or false
alt rows_deleted is not false
IpQuery_Admin->>IpQuery_DB: log_action(message)
IpQuery_DB->>WPDebugLog: error_log([IpQuery] message) (if WP_DEBUG_LOG)
end
IpQuery_Admin->>WordPressAdminPost: wp_safe_redirect(...country_deleted, country_code)
WordPressAdminPost-->>Browser: Redirect to visitors.php
Browser->>VisitorsAdminScreen: GET with country_deleted and country_code
VisitorsAdminScreen->>VisitorsAdminScreen: Render success notice
VisitorsAdminScreen-->>Admin: Show "%d record(s) deleted for country" notice
Class diagram for IpQuery_Admin and IpQuery_DB GDPR additionsclassDiagram
class IpQuery_Admin {
+init() void
+handle_settings_save() void
+enqueue_assets() void
+handle_delete_ip() void
+handle_delete_by_country() void
+handle_purge() void
+handle_manual_lookup() void
+ajax_chart_data() void
}
class IpQuery_DB {
+delete_ip(ip string) void
+delete_by_country(country_code string) "int|false"
+get_distinct_countries() array
+log_action(message string) void
}
IpQuery_Admin --> IpQuery_DB : uses delete_by_country
IpQuery_Admin --> IpQuery_DB : uses log_action
IpQuery_Admin --> IpQuery_DB : uses get_distinct_countries
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds a country-based bulk deletion feature: admin UI to pick a country, nonce-protected form that posts to a new admin handler which validates, calls DB deletion, logs the action, and redirects back with a success or error notice. ChangesCountry-Based Data Deletion
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin Browser
participant WP as WordPress Admin (PHP)
participant DB as Database (IpQuery table)
participant Log as WP Debug Log
Admin->>WP: POST /admin-post.php?action=ipquery_delete_by_country (nonce, country_code)
WP->>WP: verify_nonce() & current_user_can('manage_options')
alt invalid
WP-->>Admin: redirect back with no action
else valid
WP->>DB: IpQuery_DB::delete_by_country(country_code)
DB-->>WP: int deleted_count / false
alt deleted_count !== false
WP->>Log: IpQuery_DB::log_action("Deleted X records for COUNTRY")
WP-->>Admin: redirect to visitors?country_deleted=deleted_count&country_code=CODE
else failure
WP-->>Admin: redirect to visitors?country_delete_error=1
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Failed to generate code suggestions for PR |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
handle_delete_by_country(), the redirect always includescountry_deletedeven when$deleted === false, which makes a DB failure indistinguishable from a successful operation that deleted 0 rows; consider branching the redirect (or notice) to separately handle the error case. - The delete-by-country panel in
visitors.phpuses an inlinestyle="margin-top:24px;"; consider moving this into an existing CSS class or shared stylesheet to keep presentational concerns out of the view markup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handle_delete_by_country()`, the redirect always includes `country_deleted` even when `$deleted === false`, which makes a DB failure indistinguishable from a successful operation that deleted 0 rows; consider branching the redirect (or notice) to separately handle the error case.
- The delete-by-country panel in `visitors.php` uses an inline `style="margin-top:24px;"`; consider moving this into an existing CSS class or shared stylesheet to keep presentational concerns out of the view markup.
## Individual Comments
### Comment 1
<location path="includes/class-ipquery-admin.php" line_range="225-231" />
<code_context>
+ *
+ * @return void
+ */
+ public static function handle_delete_by_country(): void {
+ check_admin_referer( 'ipquery_delete_by_country' );
+ if ( ! current_user_can( 'manage_options' ) ) {
+ wp_die( esc_html__( 'Not allowed.', 'ipquery' ) );
+ }
+
+ $country_code = strtoupper(
+ sanitize_text_field( wp_unslash( $_POST['country_code'] ?? '' ) )
+ );
</code_context>
<issue_to_address>
**issue (bug_risk):** Uppercasing the country code before deletion may prevent matching existing rows.
`country_code` is already sourced from a DB-backed `<select>`, so changing it to uppercase here assumes all stored values are uppercase. If any existing or future rows use lowercase/mixed-case codes (e.g. `us`), the delete will silently affect 0 rows. Either drop `strtoupper()` for the DB operation (only uppercase for display), or ensure `country_code` is consistently normalized at write-time throughout the codebase.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/views/visitors.php`:
- Around line 30-34: Replace the hardcoded translatable string using "record(s)"
with a proper singular/plural localization via _n(); specifically, call _n( '1
record deleted for country: %s.', '%d records deleted for country: %s.', (int)
$_GET['country_deleted'], 'ipquery' ) (or similar) to choose the correct form
based on (int) $_GET['country_deleted'], then pass the resulting localized
string into printf/sprintf and substitute the count and the esc_html(
strtoupper( sanitize_text_field( wp_unslash( $_GET['country_code'] ?? '' ) ) )
); keep existing escaping and nonce-ignore comments and ensure the number
argument used for _n() is the same (int) $_GET['country_deleted'].
In `@includes/class-ipquery-admin.php`:
- Around line 254-259: The redirect currently casts $deleted to (int) which
converts a DB failure (false) into 0, losing the false-vs-0 distinction; update
the code that builds the 'country_deleted' query value (used in the
wp_safe_redirect/admin_url call in class-ipquery-admin.php) to preserve false by
explicitly checking $deleted === false and setting the param to a string like
'false' (otherwise cast to (int) or string for numeric counts), e.g. compute
$deleted_param = $deleted === false ? 'false' : (string) (int) $deleted and use
that instead of (int) $deleted so the UI can distinguish failure from zero rows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0b9ab1c-300d-46a4-9dc1-24ba8130b56f
📒 Files selected for processing (3)
admin/views/visitors.phpincludes/class-ipquery-admin.phpincludes/class-ipquery-db.php
…e inline style to CSS
Contributor
Author
|
Thanks for the detailed review! I've addressed all the feedback in the latest commit |
gstraccini
Bot
added
hacktoberfest
![gstraccini[bot]](https://avatars.githubusercontent.com/in/480132?s=60&v=4){kind=small}
gstraccini
Bot
added
the
🚦 awaiting triage
guibranco
changed the title
guibranco
approved these changes
May 5, 2026
Owner
|
Hi @Suraj80 👋, Thank you so much for your pull request! 🙌 I appreciate the time and effort you put into this contribution. Thanks again for your valuable contribution! 🚀 |
Owner
|
Hi @Suraj80 , Are you able to review and fix the issues for: Thanks in advance! |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@admin/views/visitors.php`:
- Around line 29-31: The code treats any value other than the literal 'false' as
a successful deletion and blindly casts $_GET['country_deleted'] to int into
$ipquery_deleted_count, allowing invalid values to display as "0" success;
change the logic around the $_GET['country_deleted'] check to validate it's a
non-negative integer (e.g. use ctype_digit or
filter_var($_GET['country_deleted'], FILTER_VALIDATE_INT,
["options"=>["min_range"=>0]])) before assigning $ipquery_deleted_count and
showing the success notice, and otherwise fall back to the error branch; keep
the existing sanitization for $ipquery_deleted_country (esc_html( strtoupper(
sanitize_text_field( wp_unslash( $_GET['country_code'] ?? '' ) ) ) )) but only
use it when the numeric validation passes; apply the same validation fix to the
similar handling that sets $ipquery_deleted_count at the other occurrence.
In `@includes/class-ipquery-admin.php`:
- Around line 231-238: The code currently sanitizes and uppercases $country_code
but does not strictly validate it before deletion; update the handler that
processes $country_code (the variable produced via wp_unslash and
sanitize_text_field) to enforce an ISO alpha-2 format check (e.g., match
^[A-Z]{2}$) and only proceed with the DB delete when the regex passes; if the
value is invalid, perform the same wp_safe_redirect to
admin_url('admin.php?page=ipquery-visitors') and exit to block forged/non-ISO
values.
- Around line 249-255: The log currently records the admin's user_login in
IpQuery_DB::log_action (via wp_get_current_user()->user_login); change it to a
non-PII identifier by using the numeric user ID instead (e.g., use
wp_get_current_user()->ID or get_current_user_id()) so the sprintf message
contains the user ID rather than the username while preserving the rest of the
message and parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 945555ea-a52e-4ad7-9f1e-3f35c596fe16
📒 Files selected for processing (3)
admin/views/visitors.phpassets/css/admin.cssincludes/class-ipquery-admin.php
✅ Files skipped from review due to trivial changes (1)
- assets/css/admin.css
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
guibranco
removed
the
🚦 awaiting triage
Improve code readability by cleaning up formatting, such as adding missing colons for type hints and removing trailing spaces. Condense SQL query assignments by moving the comment to the end, making it more concise and easier to read. These changes enhance overall code maintenance and readability without altering functionality.
|
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
3 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.
Closes #42
📑 Description
Adds country-based visitor data deletion to support GDPR right-to-erasure workflows.
Changes made across 3 files:
includes/class-ipquery-db.phpdelete_by_country(string $country_code): int|false— deletes all visitor rows matching a given ISO country codeget_distinct_countries(): array— returns distinct countries that have records, used to populate the dropdownlog_action(string $message): void— writes audit entries to the WordPress debug log whenWP_DEBUG_LOGis enabledincludes/class-ipquery-admin.phpadmin_post_ipquery_delete_by_countryhook ininit()handle_delete_by_country()— validates nonce, checks capability, callsIpQuery_DB::delete_by_country(), logs the action, and redirects with a success noticeadmin/views/visitors.phpcountry_deletedredirect paramrequiredattribute, and anonclickconfirmation dialogChecks
Does this introduce a breaking change?
Additional Information
IpQuery_DB::log_action()which only writes whenWP_DEBUG_LOGistrue, consistent with WordPress best practices. Note: existing handlers (handle_delete_ip,handle_purge) do not currently log — this PR introduces logging specifically to satisfy the acceptance criteria of issue [FEATURE] Add country-based data deletion support #42$deleted !== falsecheck distinguishes a real DB failure from a legitimate "0 rows deleted" resultSummary by Sourcery
Add support for deleting visitor records by country to help fulfill GDPR right-to-erasure requests.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Style