Add log level threshold for Email Editor #60255
Conversation
Testing GuidelinesHi @eason9487 @woocommerce/developer-advocacy, Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
4ceefcf to
3828071
Compare
📝 WalkthroughWalkthroughThis change refines the logging system for the WooCommerce Email Editor by introducing a configurable severity threshold, improving log file path handling based on the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Logger
participant WooCommerceLogger
Client->>Logger: log(level, message, context)
Logger->>Logger: should_handle(level)?
alt Level >= threshold
Logger->>WooCommerceLogger: log(level, message, context)
else Level < threshold
Logger-->>Client: (No action)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/php/email-editor/src/Engine/Logger/class-default-email-editor-logger.php (1)
39-49: Enhanced WP_DEBUG_LOG handling looks good with a minor security suggestion.The improved logic properly handles different types of
WP_DEBUG_LOGvalues and is more explicit about the behavior in each case. This aligns well with the updated documentation.Consider adding path validation for the string case to guard against potentially dangerous inputs:
} elseif ( is_string( WP_DEBUG_LOG ) && ! empty( WP_DEBUG_LOG ) ) { - $this->log_file = WP_DEBUG_LOG; + // Validate the path to prevent potential security issues + $sanitized_path = realpath( dirname( WP_DEBUG_LOG ) ); + if ( $sanitized_path && is_writable( $sanitized_path ) ) { + $this->log_file = WP_DEBUG_LOG; + } else { + $this->log_file = ''; + }This would ensure the directory exists and is writable before attempting to use the custom path.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/features/email/email-editor-logging.md(1 hunks)packages/php/email-editor/README.md(2 hunks)packages/php/email-editor/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor(1 hunks)packages/php/email-editor/src/Engine/Logger/class-default-email-editor-logger.php(1 hunks)plugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor(1 hunks)plugins/woocommerce/src/Internal/EmailEditor/Logger.php(11 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
packages/php/email-editor/src/Engine/Logger/class-default-email-editor-logger.phpplugins/woocommerce/src/Internal/EmailEditor/Logger.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit Configuration File
**/*.{php,js,ts,jsx,tsx}: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
packages/php/email-editor/src/Engine/Logger/class-default-email-editor-logger.phpplugins/woocommerce/src/Internal/EmailEditor/Logger.php
🧠 Learnings (7)
📓 Common learnings
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Learnt from: triple0t
PR: woocommerce/woocommerce#59186
File: packages/js/email-editor/src/store/initial-state.ts:9-10
Timestamp: 2025-06-26T12:13:32.062Z
Learning: In WooCommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.ts), the current_post_id and current_post_type from window.WooCommerceEmailEditor are required parameters that should cause explicit errors if missing, rather than using fallback values or optional chaining. The design preference is to fail fast when critical initialization data is unavailable.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-07-21T05:22:46.426Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
Learnt from: NeosinneR
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-26T14:25:08.421Z
Learning: In WooCommerce transactional emails, product images have historically had display issues due to lazy loading attributes being applied, which email clients cannot process since they don't execute JavaScript. This issue existed in both old and new email templates, but became more visible with the new email template system. The fix involves preventing lazy loading on attachment images specifically during email generation by adding skip classes and data attributes.
Learnt from: prettyboymp
PR: woocommerce/woocommerce#59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:60-83
Timestamp: 2025-06-26T12:45:40.709Z
Learning: WooCommerce uses WordPress versioning conventions where minor versions in X.Y.Z format are constrained to 0-9 (Y cannot exceed 9). This means version increment logic should reset minor to 0 and increment major when minor reaches 9, rather than allowing two-digit minor versions like 9.10 or 9.11.
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#60250
File: .github/workflows/release-compile-changelog.yml:165-166
Timestamp: 2025-08-07T10:34:27.702Z
Learning: In WooCommerce release workflows, the VERSION variable used in changelog generation comes from the `validate-selected-branch-version` step, which extracts the version from `plugins/woocommerce/woocommerce.php` using `grep -oP '(?<=Version: )(.+)'` and validates it matches the input version. This ensures the version is always in the correct format by the time it reaches the changelog generation step, making additional error handling for version format validation unnecessary.
📚 Learning: in woocommerce core repository, changelog entries for all prs live in `plugins/woocommerce/changelog...
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
Applied to files:
packages/php/email-editor/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editorplugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editorpackages/php/email-editor/README.md
📚 Learning: in woocommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.t...
Learnt from: triple0t
PR: woocommerce/woocommerce#59186
File: packages/js/email-editor/src/store/initial-state.ts:9-10
Timestamp: 2025-06-26T12:13:32.062Z
Learning: In WooCommerce email editor store initialization (packages/js/email-editor/src/store/initial-state.ts), the current_post_id and current_post_type from window.WooCommerceEmailEditor are required parameters that should cause explicit errors if missing, rather than using fallback values or optional chaining. The design preference is to fail fast when critical initialization data is unavailable.
Applied to files:
packages/php/email-editor/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editordocs/features/email/email-editor-logging.mdplugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editorplugins/woocommerce/src/Internal/EmailEditor/Logger.php
📚 Learning: in woocommerce release workflows, the version variable used in changelog generation comes from the `...
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#60250
File: .github/workflows/release-compile-changelog.yml:165-166
Timestamp: 2025-08-07T10:34:27.702Z
Learning: In WooCommerce release workflows, the VERSION variable used in changelog generation comes from the `validate-selected-branch-version` step, which extracts the version from `plugins/woocommerce/woocommerce.php` using `grep -oP '(?<=Version: )(.+)'` and validates it matches the input version. This ensures the version is always in the correct format by the time it reaches the changelog generation step, making additional error handling for version format validation unnecessary.
Applied to files:
packages/php/email-editor/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editorplugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor
📚 Learning: in woocommerce transactional emails, product images have historically had display issues due to lazy...
Learnt from: NeosinneR
PR: woocommerce/woocommerce#0
File: :0-0
Timestamp: 2025-06-26T14:25:08.421Z
Learning: In WooCommerce transactional emails, product images have historically had display issues due to lazy loading attributes being applied, which email clients cannot process since they don't execute JavaScript. This issue existed in both old and new email templates, but became more visible with the new email template system. The fix involves preventing lazy loading on attachment images specifically during email generation by adding skip classes and data attributes.
Applied to files:
docs/features/email/email-editor-logging.mdplugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor
📚 Learning: woocommerce uses wordpress versioning conventions where minor versions in x.y.z format are constrain...
Learnt from: prettyboymp
PR: woocommerce/woocommerce#59048
File: .github/workflows/cherry-pick-milestoned-prs.yml:60-83
Timestamp: 2025-06-26T12:45:40.709Z
Learning: WooCommerce uses WordPress versioning conventions where minor versions in X.Y.Z format are constrained to 0-9 (Y cannot exceed 9). This means version increment logic should reset minor to 0 and increment major when minor reaches 9, rather than allowing two-digit minor versions like 9.10 or 9.11.
Applied to files:
plugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor
📚 Learning: the core/spacer block in the woocommerce email editor intentionally does not have a dedicated render...
Learnt from: lysyjan
PR: woocommerce/woocommerce#59070
File: packages/php/email-editor/src/Integrations/Core/class-initializer.php:103-141
Timestamp: 2025-06-23T16:55:58.246Z
Learning: The core/spacer block in the WooCommerce email editor intentionally does not have a dedicated renderer class and is handled by the fallback renderer instead of having an explicit render_email_callback assigned in the switch statement.
Applied to files:
plugins/woocommerce/src/Internal/EmailEditor/Logger.php
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (8)
plugins/woocommerce/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor (1)
1-4: LGTM! Changelog entry follows WooCommerce conventions.The changelog entry is properly formatted and accurately describes the addition of the custom log level threshold feature for Email Editor logging.
packages/php/email-editor/changelog/wooplug-5003-review-and-refine-logging-in-the-email-editor (1)
1-4: LGTM! Appropriate changelog entry for the logging fix.The changelog accurately describes the fix to respect custom log file paths defined in
WP_DEBUG_LOG. The "fix" type is appropriate for this behavioral improvement.packages/php/email-editor/README.md (2)
129-133: Excellent documentation of WP_DEBUG_LOG behavior.The detailed explanation of how different
WP_DEBUG_LOGvalues affect logging behavior is clear and helpful for developers. This aligns well with the improved constructor logic in the logger class.
155-155: Good catch on the method name correction.Correcting the method name from
set_delegate_logger()toset_logger()ensures accuracy in the documentation.docs/features/email/email-editor-logging.md (1)
1-18: Excellent documentation for the new logging threshold feature.This documentation clearly explains the severity threshold concept and provides a practical code example. The reference to the main WooCommerce logging documentation is helpful for developers who need more detailed information about log levels.
plugins/woocommerce/src/Internal/EmailEditor/Logger.php (3)
13-13: LGTM! Good use of WooCommerce constants.Using
WC_Log_Levelsconstants improves type safety and consistency across the logging system.
64-64: Excellent refactoring to centralize logging logic.Converting all log level methods to delegate to the centralized
log()method with proper constants is a great improvement. This eliminates code duplication, ensures consistent threshold handling, and improves maintainability while preserving backwards compatibility.Also applies to: 75-75, 86-86, 97-97, 108-108, 119-119, 130-130, 141-141
153-155: LGTM! Clean implementation of threshold filtering.The conditional logging based on
should_handle()correctly implements the threshold filtering while maintaining backwards compatibility. The logic is clear and efficient.
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
Submission Review Guidelines:
Changes proposed in this Pull Request:
WC_Log_Levels::WARNINGas default.WP_DEBUG_LOGwhen specified.Closes WOOPLUG-5003: Review and Refine Logging in the Email Editor.
Screenshots or screen recordings:
N/A
How to test the changes in this Pull Request:
WC_Log_Levels::WARNINGis logged.woocommerce_email_editor_logging_thresholdfilter, e.g.:DebugandInfo(since it's higher level compare toWC_Log_Levels::WARNING) entries from the email editor.Testing that has already taken place:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment
Add custom log level threshold for Email Editor logging set to
WC_Log_Levels::WARNINGas default.