Feature Request | Country/User based Targeting Option for All Types of Notifications - #153
Open
ShahrearMSf wants to merge 5 commits into
Open
Feature Request | Country/User based Targeting Option for All Types of Notifications#153ShahrearMSf wants to merge 5 commits into
ShahrearMSf wants to merge 5 commits into
Conversation
Move country and user-role targeting from the PressBar extension and NotificationBar type into a new core Targeting system. This makes the feature available globally for all notification types rather than being tied to the bar module. The Targeting class registers fields via nx_customize_fields filter and enforces them via nx_show_on_exclude filter, so it works for any notification regardless of which modules are active.
- Add a 'Logged-out Visitors (Guests)' option to the user-role targeting field. The enforcement in show_on_exclude() already honored 'guest', but it was never selectable, so logged-out visitors could only be excluded, never targeted alongside specific roles. - Use the targeting tooltip for the Targeting section (was the button tooltip).
…l calls Country targeting now applies to all types, so the ip-api.com lookup in nx_get_visitor_country_code() could fire on many more page loads. Since the free tier is rate-limited by the SERVER IP (shared across all visitors), an uncached call would hit 429s and wrongly hide country-targeted notifications, and each blocking call added latency to page rendering. - Request-level static memo (resolve once per page load, even with multiple country-targeted notifications) - Per-IP transient cache for successful lookups (default 12h, filterable via nx_visitor_country_cache_ttl); failures are not cached so they retry - Short-circuit via host/CDN geo headers (CF-IPCountry, GEOIP_COUNTRY_CODE, X-Country-Code) to skip the external call entirely where available - Bounded 3s timeout; only cache on HTTP 200 - Filters nx_visitor_country_code (override/mocking) and nx_visitor_country_api
ShahrearMSf
added a commit
that referenced
this pull request
Aug 3, 2026
…ree release
Pre-release integration of three NotificationX Free candidates onto nx-dev
(branched from nx-dev, which was even with master) so the combined result can be
built and tested before the individual PRs merge to master. No version bump.
Cards / candidates
------------------
- 81117 BitIntegrations: unify $module ('modules_bitintegrations') and title
across BitIntegrationsConversions / EmailSubscription / Reviews (and the
module list in Types/Conversions + OfferAnnouncement) so the builder
renders ONE "Bit Integrations" source section instead of two. Fix the
doc links (bit-integrations-notification-alert, configure-discount-alert)
- both resolve HTTP 200.
- 2318 (PR #152) Coupon Popup: themes 8/11/12/13 now apply their own default
content on a default/untouched publish - subtitle, coupon code, and the
theme-thirteen coupon tickets. Fixes the Popup type default_theme key,
drops conflicting field-level defaults, and fills empty theme fields via
save_post + preview_settings.
- 78650 (PR #153) Targeting: country + user-role targeting (incl. a Guest /
logged-out option) for ALL notification types, decoupled from the bar
into Core\Targeting. Adds visitor-country geo-lookup caching
(request-level memo + per-IP transient + CDN header shortcut) so the
ip-api.com lookup no longer runs on every notification load.
Merge
-----
Merge order: nx-dev -> 81117 -> 2318 -> 78650. All feature code lives in disjoint
files (BitIntegrations vs Popup vs Targeting/Core), so there were no code
conflicts - only the shared translation template collided (see below).
POT file conflict fix
---------------------
languages/notificationx.pot was the only merge conflict, because every branch
regenerates it. Resolved by regenerating a fresh union with `npm run pot`
(wp i18n make-pot) so the template contains all three features' strings.
Zip build (standard dev way; node per .nvmrc = 16)
--------------------------------------------------
- nvm use 16 (node v16.20.2, npm 8.19.4)
- rm -rf node_modules/@wordpress/compose/node_modules \
node_modules/@wordpress/scripts/node_modules
# required: fixes "TS2786: 'Pagination' cannot be used as a JSX component"
# (duplicate @types/react nested under @wordpress/*)
- npm run build && npm run bb && npm run cd && npm run pot
- package honoring .distignore. `wp dist-archive .` needs wp-cli >= 2.13 and the
build machine has 2.11, so packaging used the equivalent
rsync --exclude-from=.distignore (identical file selection).
- artifact: notificationx-nx-dev-3.2.12.zip (Free 3.2.12)
Testing
-------
Installed the built zip on local with Pro 3.1.5 active (Free-new + Pro compat).
0 fatals; all three features working; country targeting also verified live on a
Cloudflare host. No PR and no merge to master.
The country/user-role Targeting section was decoupled to Core\Targeting for all notification types, but a leftover CSS rule in _create_popup.scss hard-hid .nx-targeting for every type and only re-showed it for .press_bar - so in the builder it appeared on the Notification Bar only. Removed that bar-scoped rule so visibility is governed by the section's PHP rule (shown for every source except GDPR / Exit Intent). Rebuilt admin.css. Verified: Targeting now renders in Customize for Bar, Comments, Reviews, Announcement, Sales; correctly hidden for Cookie Notice (GDPR) + Exit Intent.
- show_on_exclude() fails open when the visitor country can't be resolved (geo failure/rate-limit, CLI, local) instead of hiding the notification for everyone; genuine country mismatches still hide. Normalise country values and treat 'ALL' as "no filter". - nx_get_visitor_country_code(): cache per-IP in the object-cache group (no wp_options rows on cached sites), negative-cache failures briefly, bail on empty IP before the API call, memo with array_key_exists, and return '' (never null) for unknown. - README: add External services disclosure for ip-api.com.
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
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.
Country- and user-role-based targeting previously lived inside the PressBar extension and was enforced only in the Notification Bar type — so it worked only for the bar, and only while the bar module was enabled. This PR decouples targeting into a shared core system so every notification type gets it, and closes the gaps needed to make it production-ready.
Feature Request | Country/User based Targeting Option for All Types of Notifications
What's in this PR
Core\Targeting)nx_customize_fieldsschema, so it appears for all types (excludes only GDPR + Exit Intent, mirroring the coreappearancerule).nx_show_on_excludefilter (FrontEnd::get_notifications_ids()), so it applies to any notification regardless of active modules.PressBar) and enforcement (NotificationBar).REST→Targeting::restResponse), fixing search for all types.guest, but it was never selectable, so logged-out visitors could only be excluded, never targeted alongside roles.ip-api.comlookup could fire on many more page loads. The free tier is rate-limited by the server IP (shared across all visitors), so an uncached call would hit 429s and wrongly hide country-targeted notifications, plus add blocking latency. Now:CF-IPCountry,GEOIP_COUNTRY_CODE,X-Country-Code) to skip the external call entirely where available,nx_visitor_country_code(override/mock) andnx_visitor_country_api.Files changed
includes/Core/Targeting.php(new) — fields + enforcement + REST handlerincludes/Core/REST.php— route country search by fieldincludes/Core/Helper.php— cachednx_get_visitor_country_code()includes/NotificationX.php— bootTargeting::get_instance()includes/Types/NotificationBar.php,includes/Extensions/PressBar/PressBar.php— remove bar-only targetinglanguages/notificationx.pot,vendor/composer/autoload_*.phpTesting
Free-only change (Pro needs none —
is_profields unlock via Pro's generic mechanism; verified no targeting code in Pro).[US]shows,[BD]hidesis_profield savesis_pro_active)php -l, fatalsNotes
cc @priyomukul bhai — please review before merge.