Skip to content

Fix undefined array key warning in maybe_add_utm_license#2601

Closed
shervElmi wants to merge 1 commit into
masterfrom
fix/utm-license-undefined-key
Closed

Fix undefined array key warning in maybe_add_utm_license#2601
shervElmi wants to merge 1 commit into
masterfrom
fix/utm-license-undefined-key

Conversation

@shervElmi
Copy link
Copy Markdown
Contributor

Fixes an undefined array key warning when utm_medium is not present in the query args.

Demo

CleanShot 2025-11-26 at 17 08 51@2x CleanShot 2025-11-26 at 17 09 02@2x

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Nov 26, 2025

Walkthrough

A guard condition is added to the maybe_add_utm_license method in the FrmAppHelper class to verify that utm_medium exists before comparing it to 'pro', preventing utm_license from being set when utm_medium is not provided.

Changes

Cohort / File(s) Summary
Defensive guard addition
classes/helpers/FrmAppHelper.php
Adds isset($query_args['utm_medium']) check before accessing and comparing utm_medium value in maybe_add_utm_license method

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

  • The change is a straightforward guard clause that prevents potential undefined index access
  • Single file affected with minimal scope
  • Low risk defensive programming practice

Suggested labels

action: needs qa

Suggested reviewers

  • Crabcyborg

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a guard to prevent an undefined array key warning in the maybe_add_utm_license method.
Description check ✅ Passed The description is directly related to the changeset, explaining that it fixes an undefined array key warning when utm_medium is not present, with supporting demo screenshots.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/utm-license-undefined-key

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 006d247 and e528882.

📒 Files selected for processing (1)
  • classes/helpers/FrmAppHelper.php (1 hunks)
⏰ 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). (9)
  • GitHub Check: PHP 7.4 tests in WP trunk
  • GitHub Check: PHP 8 tests in WP trunk
  • GitHub Check: Cypress
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP trunk
  • GitHub Check: PHP 8 tests in WP trunk
  • GitHub Check: PHP 7.4 tests in WP trunk
  • GitHub Check: Cypress
  • GitHub Check: PHP 8 tests in WP trunk
🔇 Additional comments (1)
classes/helpers/FrmAppHelper.php (1)

176-176: LGTM! Correct fix for undefined array key warning.

The isset() check properly prevents an undefined array key warning when maybe_add_utm_license() is called from maybe_add_missing_utm() (line 235), where utm_medium may not exist in $query_args if it's already present in the URL. The guard condition is correctly placed first in the logic chain and follows PHP best practices.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

*/
private static function maybe_add_utm_license( $query_args ) {
if ( 'pro' === $query_args['utm_medium'] && is_callable( 'FrmProAddonsController::get_readable_license_type' ) ) {
if ( isset( $query_args['utm_medium'] ) && 'pro' === $query_args['utm_medium'] && is_callable( 'FrmProAddonsController::get_readable_license_type' ) ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shervElmi This doesn't look super ideal.

If this wasn't set, it must mean that the link already has a utm_medium. Ideally we would still check for that and still add a license, rather than do nothing.

@Crabcyborg
Copy link
Copy Markdown
Contributor

Since this issue is causing our unit tests to fail, I went ahead and made a new PR #2606

Closing this one.

@Crabcyborg Crabcyborg closed this Nov 27, 2025
@Crabcyborg Crabcyborg deleted the fix/utm-license-undefined-key branch November 27, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants