Fix issue with line breaks when importing quiz outcome template#2473
Conversation
WalkthroughAdds a conditional normalization in import_xml_views to convert double-escaped CRLF sequences to single-escaped CRLF in action post contents during import, only when the post_type matches the action post type. No new branches or signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
classes/helpers/FrmXMLHelper.php (2)
929-932: Consider normalizing all newline variants and short-circuiting when unnecessaryRight now only double-escaped CRLF (
\\r\\n) is handled. In the wild, content can contain just LF (\\n) or just CR (\\r) depending on source editors/OS. Also, we can avoid work when no backslashes are present. Suggest:
- Handle
\\\\nand\\\\rin addition to\\\\r\\\\n.- Skip replace unless a backslash exists in content.
Proposed in-place change:
-// Fix issue with line breaks appearing in descriptions as "rn". -if ( $post['post_type'] === $form_action_type ) { - $post['post_content'] = str_replace( '\\\\r\\\\n', '\\r\\n', $post['post_content'] ); -} +// Normalize double-escaped line breaks in action JSON so they render correctly after import. +// Handles CRLF, LF, and CR. Skip if no backslashes are present. +if ( $post['post_type'] === $form_action_type && false !== strpos( $post['post_content'], '\\\\' ) ) { + $post['post_content'] = str_replace( + array( '\\\\r\\\\n', '\\\\n', '\\\\r' ), + array( '\\r\\n', '\\n', '\\r' ), + $post['post_content'] + ); +}
929-932: Suggest JSON round-trip for action post_content normalization (optional)Instead of only replacing
\\r\\nin XML imports, you can decode and re-encode valid JSON payloads to preserve all fields and escapes, falling back to string-based normalization when the content isn’t JSON. This keeps behavior unchanged becausemaybe_create_actionwill decode the JSON into an array andsave_settingswill re-encode it back to a string.• File:
classes/helpers/FrmXMLHelper.php
Lines: 929–932--- a/classes/helpers/FrmXMLHelper.php +++ b/classes/helpers/FrmXMLHelper.php @@ -929,7 +929,18 @@ - // Fix issue with line breaks appearing in descriptions as "rn". - if ( $post['post_type'] === $form_action_type ) { - $post['post_content'] = str_replace( '\\\\r\\\\n', '\\r\\n', $post['post_content'] ); - } + // Prefer JSON round-trip when valid; fallback to string normalization of control chars. + if ( $post['post_type'] === $form_action_type ) { + $maybe_decoded = FrmAppHelper::maybe_json_decode( $post['post_content'] ); + if ( is_array( $maybe_decoded ) ) { + // Re-encode to valid JSON string, preserving all other escapes + $post['post_content'] = FrmAppHelper::prepare_and_encode( $maybe_decoded ); + } else { + // Only normalize common escaped line breaks + $post['post_content'] = str_replace( + array( '\\\\r\\\\n', '\\\\n', '\\\\r' ), + array( '\\r\\n', '\\n', '\\r' ), + $post['post_content'] + ); + } + }• No other import paths currently normalize
\\nor\\rfor actions, so this adds comprehensive coverage.
•maybe_create_actioninclasses/models/FrmFormAction.phpdecodes the incoming JSON string into an array and then callssave_settings, which usesprepare_and_encodeto re-serialize it back to JSON—so the round-trip keeps existing behavior intact.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
classes/helpers/FrmXMLHelper.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). (7)
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: Run PHP Syntax inspection (8.3)
- GitHub Check: PHP 8 tests in WP trunk
🔇 Additional comments (1)
classes/helpers/FrmXMLHelper.php (1)
929-932: Targeted and safe fix for action post contentScoping the normalization to
$form_action_typeis appropriate and low-risk. Placing it afterswitch_form_idsbut before creation/update keeps the change local to the action payload and avoids impacting other post types. LGTM.
1 participant
Before
After
