Update UTM mapping#2555
Conversation
WalkthroughCentralizes and standardizes UTM handling: replaces "medium" with "campaign", adds UTM helper utilities and license-aware enrichment in FrmAppHelper, replaces many hard-coded docs/upgrade links with admin_upgrade_link(), injects floating-links URLs into JS, and adds a min-version notice view. Changes
Sequence Diagram(s)sequenceDiagram
participant Controller as Controller/View
participant Helper as FrmAppHelper
participant UTM as UTM Normalizer
participant JS as Floating Links JS
rect rgb(245,245,255)
Note over Controller,Helper: Build tracked admin/docs/upgrade links
Controller->>Helper: admin_upgrade_link(args, path)
Helper->>UTM: adjust_legacy_utm_args(args)
UTM-->>Helper: normalized campaign/content/medium
Helper->>Helper: maybe_add_utm_license(query_args)
Helper-->>Controller: return URL (with utm params & anchor)
end
rect rgb(245,255,245)
Note over Controller,JS: Provide floating links to client
Controller->>Helper: build upgrade/docs URLs
Helper-->>JS: inject s11FloatingLinksData.upgradeUrl & documentationUrl
JS->>JS: config.js reads s11FloatingLinksData and uses URLs in UI
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (4)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
classes/controllers/FrmAppController.php (1)
437-442: Inconsistency confirmed: migrate'medium'to'campaign'at line 439.The review comment is accurate. Line 439 uses the legacy
'medium'key while identical array structures elsewhere in the file (line 583, lines 1338–1342) use the new'campaign'key. The migration is incomplete, and line 439 should be updated for consistency.Additionally, line 361 also uses the legacy
'medium'key in a similar context and should be migrated as well for completeness.
🧹 Nitpick comments (4)
classes/models/fields/FrmFieldCaptcha.php (1)
183-185: Fix is correct, but consider refactoring to useadd_query_argfor more robust URL construction.The string replacement correctly addresses the URL malformation issue when query parameters are conditionally appended. However, the
hcaptcha_api_urlmethod (lines 200-222) demonstrates a cleaner pattern usingadd_query_arg(line 210) for URL construction, which is more maintainable and less prone to edge cases.Consider refactoring the method to use
add_query_arg:protected function recaptcha_api_url( $frm_settings ) { - $api_js_url = 'https://www.google.com/recaptcha/api.js?'; + $api_js_url = 'https://www.google.com/recaptcha/api.js'; + $query_args = array(); if ( $this->allow_multiple( $frm_settings ) ) { - $api_js_url .= '&onload=frmRecaptcha&render=explicit'; + $query_args['onload'] = 'frmRecaptcha'; + $query_args['render'] = 'explicit'; } $lang = apply_filters( 'frm_recaptcha_lang', $frm_settings->re_lang, $this->field ); if ( $lang ) { - $api_js_url .= '&hl=' . $lang; + $query_args['hl'] = $lang; } - // Since this URL initially ends with ? and we never use add_query_arg, remove the extra - // & that appears immediately after the ? - $api_js_url = str_replace( '?&', '?', $api_js_url ); + if ( ! empty( $query_args ) ) { + $api_js_url = add_query_arg( $query_args, $api_js_url ); + } /** * @param string $api_js_url */ $api_js_url = apply_filters( 'frm_recaptcha_js_url', $api_js_url ); return $api_js_url; }classes/controllers/FrmFormsController.php (1)
321-333: Polish: set accurate @SInCE and consider naming/comment clarity
- Replace @SInCE x.x with the release version used in this PR (likely 6.25).
- Optional: add a short comment that we intentionally route through admin_upgrade_link for KB pages to inherit UTMs/affiliate handling.
No behavior change required.
Confirm that applying affiliate tracking to knowledgebase links is desired; if not, we can bypass make_affiliate_url for docs.
js/packages/floating-links/config.js (1)
57-57: Guard against undefined s11FloatingLinksData to prevent runtime errorsIf s11FloatingLinksData isn’t localized on a page, direct access throws a ReferenceError. Guard it and reuse the guarded object.
Apply this diff:
@@ -( wp => { +( wp => { @@ - // Determine the appropriate links and initialize the S11FloatingLinks class - frmFloatingLinksConfig.links = s11FloatingLinksData.proIsInstalled ? frmFloatingLinksConfig.proVersionLinks : frmFloatingLinksConfig.freeVersionLinks; + // Safely read localized data with fallbacks + const data = window.s11FloatingLinksData || { + upgradeUrl: '#', + documentationUrl: 'https://formidableforms.com/knowledgebase/', + proIsInstalled: false + }; @@ - url: s11FloatingLinksData.upgradeUrl, + url: data.upgradeUrl, @@ - url: s11FloatingLinksData.documentationUrl, + url: data.documentationUrl, @@ - url: s11FloatingLinksData.documentationUrl, + url: data.documentationUrl, @@ - frmFloatingLinksConfig.links = s11FloatingLinksData.proIsInstalled ? frmFloatingLinksConfig.proVersionLinks : frmFloatingLinksConfig.freeVersionLinks; + frmFloatingLinksConfig.links = data.proIsInstalled ? frmFloatingLinksConfig.proVersionLinks : frmFloatingLinksConfig.freeVersionLinks;Please confirm s11FloatingLinksData is always localized before this script loads; if so, the guard is a no-op but prevents hard-to-debug errors on edge screens.
Also applies to: 69-69, 81-81, 108-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
classes/controllers/FrmAddonsController.php(1 hunks)classes/controllers/FrmAppController.php(2 hunks)classes/controllers/FrmDashboardController.php(1 hunks)classes/controllers/FrmFormsController.php(3 hunks)classes/helpers/FrmAppHelper.php(3 hunks)classes/helpers/FrmDashboardHelper.php(1 hunks)classes/helpers/FrmEntriesListHelper.php(1 hunks)classes/models/fields/FrmFieldCaptcha.php(1 hunks)classes/views/addons/min-version-notice.php(1 hunks)classes/views/addons/upgrade_to_pro.php(0 hunks)classes/views/dashboard/templates/pro-features-list.php(1 hunks)classes/views/frm-entries/no_entries.php(1 hunks)classes/views/frm-settings/general.php(1 hunks)classes/views/frm-settings/license_box.php(2 hunks)classes/views/shared/admin-footer-links.php(2 hunks)classes/views/shared/views-info.php(2 hunks)js/packages/floating-links/config.js(3 hunks)
💤 Files with no reviewable changes (1)
- classes/views/addons/upgrade_to_pro.php
🧰 Additional context used
🧬 Code graph analysis (11)
classes/helpers/FrmEntriesListHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/views/frm-entries/no_entries.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/views/addons/min-version-notice.php (2)
classes/controllers/FrmAddonsController.php (2)
FrmAddonsController(6-1512)is_license_expired(475-493)classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/views/frm-settings/general.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/helpers/FrmAppHelper.php (1)
stubs.php (2)
FrmProAddonsController(128-163)get_readable_license_type(161-162)
classes/controllers/FrmDashboardController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/controllers/FrmAppController.php (1)
classes/helpers/FrmAppHelper.php (3)
FrmAppHelper(6-4664)pro_is_installed(355-357)admin_upgrade_link(122-165)
classes/views/shared/admin-footer-links.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/controllers/FrmFormsController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
classes/controllers/FrmAddonsController.php (1)
classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)maybe_add_missing_utm(215-238)
classes/views/shared/views-info.php (2)
classes/controllers/FrmAddonsController.php (1)
FrmAddonsController(6-1512)classes/helpers/FrmAppHelper.php (2)
FrmAppHelper(6-4664)admin_upgrade_link(122-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Run PHP Syntax inspection (8.3)
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
🔇 Additional comments (23)
classes/models/fields/FrmFieldCaptcha.php (1)
183-185: Inconsistency between PR objectives and actual changes.The PR objectives and AI summary describe updating UTM mapping and refactoring UTM parameters across the codebase. However, this specific change addresses a URL construction bug in the reCAPTCHA API URL and does not involve UTM parameters or campaign tracking.
classes/views/shared/views-info.php (2)
31-36: LGTM! Dynamic link generation implemented correctly.The demo link properly uses the new UTM parameter structure with
campaignandcontentkeys, and correctly callsFrmAppHelper::admin_upgrade_linkdirectly to generate the URL dynamically.
107-119: LGTM! Dynamic link generation implemented correctly.The learn more link properly uses the new UTM parameter structure with
campaignandcontentkeys, and correctly callsFrmAppHelper::admin_upgrade_linkdirectly to generate the URL dynamically.classes/views/addons/min-version-notice.php (3)
1-4: LGTM: Standard WordPress security check.The ABSPATH check correctly prevents direct file access.
6-7: LGTM: Proper WordPress admin banner structure.The banner uses appropriate classes and properly translates the message using
esc_html_e().
9-15: LGTM: Proper license check and UTM structure.The conditional logic correctly checks license expiration, and the UTM array properly uses the
campaignkey as per the PR's refactoring objectives.classes/helpers/FrmDashboardHelper.php (1)
217-219: UTM key migration looks goodSwitch to campaign/content is consistent with the new helpers and preserves existing behavior. LGTM.
classes/views/dashboard/templates/pro-features-list.php (1)
23-25: Consistent UTM usagecampaign/content keys align with admin_upgrade_link and maybe_add_missing_utm. Looks correct.
classes/controllers/FrmFormsController.php (1)
309-311: Good: centralized docs link generationUsing self::get_form_too_long_docs_url() avoids hardcoded UTMs and keeps tracking consistent. LGTM.
classes/helpers/FrmAppHelper.php (3)
116-165: Centralized upgrade/docs URL builder looks solid; verify utm_source changeUpdated admin_upgrade_link uses:
- utm_source=plugin (was previously different in parts of the codebase),
- utm_medium via get_utm_medium(),
- legacy UTM normalization and anchors.
LGTM. Please verify analytics/reporting expect utm_source='plugin' for these links.
200-207: Legacy arg shim is correctMapping medium -> campaign keeps older call sites working with the new UTM shape. LGTM.
1513-1515: Banner UTMs migrated correctlycampaign/content keys align with the new helper behavior. Looks good.
classes/controllers/FrmDashboardController.php (1)
259-259: LGTM! Good security practice.Wrapping the dynamically generated URL with
esc_url()is appropriate defensive coding when outputting URLs in HTML attributes, even when the source function is trusted.classes/views/frm-settings/license_box.php (1)
12-15: LGTM! UTM parameter naming updated correctly.The changes properly migrate from the legacy
mediumkey to the newcampaignkey structure, consistent with the PR's UTM mapping refactor.Also applies to: 28-31
classes/controllers/FrmAddonsController.php (1)
836-847: LGTM! Cleaner UTM handling approach.Replacing explicit query parameter construction with the centralized
maybe_add_missing_utmhelper improves maintainability and ensures consistent UTM handling across the codebase.classes/controllers/FrmAppController.php (3)
582-591: LGTM! Correct UTM structure.Lines 583-584 properly use the new
campaignkey structure, consistent with the PR's UTM mapping refactor.
1337-1350: LGTM! Proper dynamic URL generation for floating links.The floating links now use campaign-based UTMs and dynamically generate URLs via
admin_upgrade_link, which properly centralizes UTM handling and URL construction.
359-364: Review comment is valid but background compatibility mitigates the concern.The code at line 361 does use the legacy
'medium'key. However, verification shows thatFrmAppHelper::maybe_add_missing_utm()automatically callsadjust_legacy_utm_args()internally, which converts'medium'to'campaign'on the fly. This means the code functions correctly.That said, the review comment's suggestion to update this for consistency is reasonable. While the backward-compatible conversion works transparently, directly using the new
'campaign'key structure would be more maintainable and consistent with the PR's migration intent.The suggested diff is appropriate:
- $upgrade_link = FrmAppHelper::maybe_add_missing_utm( $upgrade_link, array( 'medium' => 'plugin-row' ) ); + $upgrade_link = FrmAppHelper::maybe_add_missing_utm( $upgrade_link, array( 'campaign' => 'plugin-row' ) );classes/helpers/FrmEntriesListHelper.php (1)
231-243: LGTM! Dynamic URL generation implemented correctly.The spam protection link now uses the new campaign-based UTM structure and dynamically generates the knowledgebase URL via
admin_upgrade_link, replacing the previous hardcoded approach.classes/views/frm-entries/no_entries.php (1)
35-44: LGTM! Dynamic documentation link implemented correctly.The documentation link now uses the new campaign-based UTM structure and dynamically generates the knowledgebase URL, making it consistent with the broader UTM refactor.
classes/views/shared/admin-footer-links.php (2)
10-22: LGTM! Upgrade link UTM structure updated correctly.The upgrade link now uses the new campaign-based UTM structure, and the fallback logic properly handles both Pro and Lite scenarios with consistent UTM parameters.
39-45: LGTM! Dynamic documentation link implemented correctly.The documentation link now uses the new campaign-based UTM structure and dynamically generates the knowledgebase URL via
admin_upgrade_link, replacing the previous hardcoded approach.classes/views/frm-settings/general.php (1)
99-109: LGTM! Dynamic GDPR documentation link implemented correctly.The GDPR documentation link now uses the new campaign-based UTM structure and dynamically generates the knowledgebase URL. The translation string is properly structured with placeholders for the link tags.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2555 +/- ##
============================================
+ Coverage 27.15% 27.19% +0.04%
- Complexity 8753 8766 +13
============================================
Files 143 143
Lines 29472 29518 +46
============================================
+ Hits 8002 8027 +25
- Misses 21470 21491 +21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
This update changes how our UTM attributes are mapped, based on how the marketing team wants them.
I've also changed a lot of hard coded URLs to go through our admin upgrade link function, many of which had no UTM attributes.