Skip to content

Conversation

@spivurno
Copy link
Contributor

Context

⛑️ Ticket(s): https://secure.helpscout.net/link/to/conversation

Summary

Added support for a base64 field modifier.

@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a new "base64" field modifier that returns Base64-encoded field values and relaxes modifier-name parsing to allow digits in modifier names (letters, digits, and underscores accepted).

Changes

Cohort / File(s) Summary
Base64 modifier implementation & modifier parsing enhancement
gravity-forms/gw-advanced-merge-tags.php
Added a base64 case in handle_field_modifiers to return Base64-encoded values. Updated the parse_modifiers regex to allow digits in modifier names (now permits letters, digits, and underscores).

Sequence Diagram(s)

sequenceDiagram
    participant Template as Template / Merge Tag
    participant Parser as parse_modifiers()
    participant Handler as handle_field_modifiers()
    participant Field as Field Value

    Template->>Parser: provide merge tag with modifiers
    Parser-->>Handler: parsed modifiers list (e.g., ["base64"])
    Handler->>Field: request raw field value
    Field-->>Handler: returns raw value
    alt modifier == "base64"
        Handler->>Handler: apply base64_encode(raw value)
        Handler-->>Template: return base64-encoded value
    else other/none
        Handler-->>Template: return processed or raw value
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check the updated regex in parse_modifiers for unintended matches or edge cases.
  • Verify base64 handling for binary, empty, and multibyte string inputs and ensure encoding usage is appropriate.

Possibly related PRs

Suggested reviewers

  • saifsultanc

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding support for a base64 field merge tag modifier, which aligns with the code changes.
Description check ✅ Passed The description includes a Context section with a ticket reference and a Summary section explaining the change, following the template structure.

📜 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 dd4dae5 and 1ff3f40.

📒 Files selected for processing (1)
  • gravity-forms/gw-advanced-merge-tags.php (3 hunks)

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.

@github-actions
Copy link

github-actions bot commented Nov 21, 2025

Fails
🚫

Commit message 'Added support for base64 field merge tag modifier.' does match the correct format. See our Snippet Library Commit Messages Guidelines in Notion.

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against dd4dae5

@spivurno spivurno changed the title Added support for base64 field merge tag modifier. gw-advanced-merge-tags.php: Added support for base64 field merge tag modifier. Nov 21, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
gravity-forms/gw-advanced-merge-tags.php (1)

5-21: Consider documenting the new base64 modifier.

The file header documents other supported features and modifiers. Adding the base64 modifier to this list would improve discoverability and maintain consistency with existing documentation patterns.

Example addition:

  *   + HTML fields
  *      {HTML:3}
  *      {all_fields:allowHtmlFields}
+ *   + base64 modifier
+ *      {Field:1:base64}
  *
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 80fa6c4 and dd4dae5.

📒 Files selected for processing (1)
  • gravity-forms/gw-advanced-merge-tags.php (2 hunks)
🔇 Additional comments (1)
gravity-forms/gw-advanced-merge-tags.php (1)

533-533: LGTM! Regex change correctly enables digit support.

The addition of 0-9 to the character class is necessary for parsing modifier names containing digits, such as 'base64'. Without this change, the parser would fail to recognize the new modifier.

Comment on lines +515 to +517
case 'base64':
return base64_encode( $value );
break;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove unreachable break statement.

The break on line 517 is unreachable since line 516 returns. This is consistent with other cases in this switch statement (e.g., lines 467, 469, 471) that return without a subsequent break.

Apply this diff:

 				case 'base64':
 					return base64_encode( $value );
-					break;
📝 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.

Suggested change
case 'base64':
return base64_encode( $value );
break;
case 'base64':
return base64_encode( $value );
🤖 Prompt for AI Agents
In gravity-forms/gw-advanced-merge-tags.php around lines 515 to 517, remove the
unreachable `break;` after the `return base64_encode( $value );` in the 'base64'
switch case — simply delete the `break;` line so the case mirrors other
return-only cases and avoids dead code.

@spivurno spivurno merged commit df43d15 into master Nov 21, 2025
1 of 3 checks passed
@spivurno spivurno deleted the dave/gw-advanced-merge-tags/base64-modifier branch November 21, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants