Fee education utm updates#2733
Conversation
WalkthroughThe pull request updates tip link attribution across three helper files. Changes rename a parameter from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @square/helpers/FrmSquareLiteAppHelper.php:
- Line 55: The docblock in the FrmSquareLiteAppHelper class incorrectly
references "Stripe fees"; update the comment to say "Square fees" instead so the
documentation matches the payment gateway here (look for the docblock above the
FrmSquareLiteAppHelper class or the method that contains the line "Add education
about Stripe fees" and change "Stripe" to "Square").
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/helpers/FrmTipsHelper.phpsquare/helpers/FrmSquareLiteAppHelper.phpstripe/helpers/FrmStrpLiteAppHelper.php
🧰 Additional context used
🧬 Code graph analysis (2)
stripe/helpers/FrmStrpLiteAppHelper.php (1)
square/helpers/FrmSquareLiteAppHelper.php (1)
fee_education(62-87)
square/helpers/FrmSquareLiteAppHelper.php (1)
stripe/helpers/FrmStrpLiteAppHelper.php (1)
fee_education(106-131)
⏰ 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). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP 6.9
- GitHub Check: PHP 8 tests in WP 6.9
🔇 Additional comments (5)
classes/helpers/FrmTipsHelper.php (1)
51-52: LGTM! Clean migration to campaign-based attribution.The change correctly maintains backward compatibility by checking for both
mediumandcampaignkeys before defaulting tocampaign = 'tip'. This ensures existing code using 'medium' continues to work while new code adopts the 'campaign' convention.stripe/helpers/FrmStrpLiteAppHelper.php (2)
122-123: LGTM! Link structure correctly implements new UTM convention.The restructured link data with
'campaign' => 'stripe-fee'and'content' => $contentproperly aligns with the UTM parameter migration and standard naming conventions.
101-106: Parameter rename is safe—no named arguments used in any call site.All calls to
fee_education()use positional arguments only:
FrmStrpLiteAppHelper::fee_education( 'stripe-action-tip', $gateway )FrmStrpLiteAppHelper::fee_education( 'stripe-global-settings-tip' )FrmSquareLiteAppHelper::fee_education( 'square-action-tip', $gateway )FrmSquareLiteAppHelper::fee_education( 'square-global-settings-tip' )The parameter rename from
$mediumto$contentand updated documentation correctly reflect the restructured UTM attribution model with no breaking changes.square/helpers/FrmSquareLiteAppHelper.php (2)
57-62: LGTM! Parameter rename consistent with Stripe helper.The parameter rename from
$mediumto$contentwith updated documentation mirrors the changes in the Stripe helper and correctly reflects the new UTM structure.
78-79: LGTM! Link structure correctly updated.The link data structure with
'campaign' => 'square-fee'and'content' => $contentis consistent with the Stripe helper changes and properly implements the new UTM parameter convention.
| @@ -54,12 +54,12 @@ public static function active_mode() { | |||
| /** | |||
| * Add education about Stripe fees. | |||
There was a problem hiding this comment.
Fix docblock: references wrong payment gateway.
The docblock comment mentions "Stripe fees" but this is the Square helper class and should reference "Square fees" instead.
📝 Proposed fix
- * Add education about Stripe fees.
+ * Add education about Square fees.📝 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.
| * Add education about Stripe fees. | |
| * Add education about Square fees. |
🤖 Prompt for AI Agents
In @square/helpers/FrmSquareLiteAppHelper.php at line 55, The docblock in the
FrmSquareLiteAppHelper class incorrectly references "Stripe fees"; update the
comment to say "Square fees" instead so the documentation matches the payment
gateway here (look for the docblock above the FrmSquareLiteAppHelper class or
the method that contains the line "Add education about Stripe fees" and change
"Stripe" to "Square").
The logic for campaign vs content was backwards after I tried to make the parameters more unique.