Skip to content

Fix: Gate consent banner on gdpr_enabled setting#143

Merged
parhumm merged 1 commit intodevelopmentfrom
fix/issue-140-consent-banner-regression
Mar 9, 2026
Merged

Fix: Gate consent banner on gdpr_enabled setting#143
parhumm merged 1 commit intodevelopmentfrom
fix/issue-140-consent-banner-regression

Conversation

@ParhamTehrani
Copy link
Copy Markdown
Contributor

@ParhamTehrani ParhamTehrani commented Mar 9, 2026

Close #140 - Consent banner was showing even when GDPR Compliance Mode was disabled because the check only validated use_slimstat_banner without checking gdpr_enabled.

Now the banner only renders when BOTH conditions are met:

  • gdpr_enabled is 'on'
  • use_slimstat_banner is 'on'

Describe your changes

...

Submission Review Guidelines:

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • Will this be part of a product update? If yes, please write one phrase about this update.
  • I have reviewed my code for security best practices.
  • Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.
  • My code follows the style guidelines of this project
  • I have updated the change-log in CHANGELOG.md.

Type of change

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Fix: Gate consent banner on gdpr_enabled setting
82c3483 
Resolves #140 - Consent banner was showing even when GDPR Compliance
Mode was disabled because the check only validated use_slimstat_banner
without checking gdpr_enabled.

Now the banner only renders when BOTH conditions are met:
- gdpr_enabled is 'on'
- use_slimstat_banner is 'on'
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 9, 2026

Warning

Rate limit exceeded

@ParhamTehrani has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7c5c0745-b859-42a1-9331-dc4ef0beaab2

📥 Commits

Reviewing files that changed from the base of the PR and between a756e42 and 82c3483.

📒 Files selected for processing (1)
  • wp-slimstat.php
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/issue-140-consent-banner-regression

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@parhumm
Copy link
Copy Markdown
Contributor

parhumm commented Mar 9, 2026 •
edited
Loading

Review notes:

  • The logic fix looks correct: banner hooks are now gated by both gdpr_enabled and use_slimstat_banner.
  • Issue (non-blocking): there is no regression test covering the disabled-GDPR path. Please add/update a test that asserts no banner assets/markup are enqueued/rendered when gdpr_enabled is off, even if use_slimstat_banner is on.

No other blocking concerns found in this diff.

@parhumm
Copy link
Copy Markdown
Contributor

parhumm commented Mar 9, 2026

Human QA Checklist - PR #143

Scope: Verify the consent banner is shown only when both conditions are true:

  • gdpr_enabled = on
  • use_slimstat_banner = on

Environment

  • Test on latest WordPress supported by plugin
  • SlimStat activated, caches cleared
  • Test in logged-out frontend and login screen (/wp-login.php)
  • Test with a clean browser session (incognito/private mode)

Functional Matrix

  • Case A: gdpr_enabled=on, use_slimstat_banner=on -> banner is displayed
  • Case B: gdpr_enabled=off, use_slimstat_banner=on -> banner is not displayed
  • Case C: gdpr_enabled=on, use_slimstat_banner=off -> banner is not displayed
  • Case D: gdpr_enabled=off, use_slimstat_banner=off -> banner is not displayed

Frontend Verification (when banner should show)

  • Banner appears on normal frontend pages
  • Banner appears on login page (/wp-login.php)
  • Banner CSS is loaded only when banner should be active
  • Banner markup renders once (no duplicate banner blocks)
  • Accept/Decline buttons are visible and clickable

Negative Verification (when banner should NOT show)

  • No banner markup is rendered in page footer
  • No banner CSS is enqueued
  • No JS errors related to missing banner elements

Regression Checks

  • Tracking script still loads according to existing tracking settings
  • WP Consent API flow still works when consent_integration=wp_consent_api
  • Saving settings in admin keeps use_slimstat_banner and integration behavior consistent
  • No PHP warnings/notices/fatal errors in debug log during page load

Optional Compatibility Spot Check

  • Verify behavior on multisite (if applicable)
  • Verify behavior with object/page cache enabled

QA Result

  • PASS
  • FAIL (include steps to reproduce + screenshots)

@parhumm parhumm merged commit ca09fe6 into development Mar 9, 2026
1 check passed
@parhumm parhumm deleted the fix/issue-140-consent-banner-regression branch March 9, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ParhamTehrani @parhumm