fix(email-template): missing attribute #2487
Conversation
withdraw approval template product publish template
WalkthroughThe pull request includes two minor modifications: an update to the Changes
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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 (1)
includes/Emails/WithdrawApproved.php (1)
84-87: Consider adding defensive programming checks.While the implementation is solid, consider adding validation for edge cases:
- Null amount handling
- Negative amount validation
- Currency compatibility checks
Example implementation:
- $amount = wc_format_decimal( $withdraw->get_amount(), false, true ); + $raw_amount = $withdraw->get_amount(); + if ( $raw_amount === null || $raw_amount < 0 ) { + $amount = 0; + } else { + $amount = wc_format_decimal( $raw_amount, false, true ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/Emails/WithdrawApproved.php(1 hunks)templates/emails/plain/product-published.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- templates/emails/plain/product-published.php
🔇 Additional comments (2)
includes/Emails/WithdrawApproved.php (2)
84-84: LGTM! Proper use of WooCommerce's decimal formatting.
The wc_format_decimal() function is appropriately used with correct parameters to ensure consistent decimal formatting.
Let's verify the formatting works correctly across different currency configurations:
✅ Verification successful
Based on the search results, I can see that the codebase properly handles currency formatting across different configurations:
- The system respects WooCommerce's currency settings including decimal points (
woocommerce_price_num_decimals), currency position, and separators. - The
wc_format_decimal()function used in the withdrawal email is consistent with how currency amounts are formatted elsewhere in the codebase. - The codebase properly handles currency symbols and formatting through WooCommerce's standard functions.
✓ The decimal formatting implementation is correct and consistent with the codebase's currency handling
The wc_format_decimal() usage in WithdrawApproved.php aligns with WooCommerce's currency formatting standards and will work correctly across different currency configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any custom currency formatting that might affect this change
rg -A 3 "woocommerce_price_num_decimals|woocommerce_currency" --type php
Length of output: 12678
87-87: LGTM! Proper currency symbol application.
The chain of formatting (decimal formatting followed by currency symbol application) ensures proper display of the withdrawal amount.
Let's verify the currency symbol application:
✅ Verification successful
LGTM! Currency symbol implementation is correctly used.
The verification shows that the currency_symbol() method is properly implemented in includes/Emails/Manager.php and is consistently used across various email templates. The implementation:
- Uses WooCommerce's standard currency formatting
- Properly handles HTML entities for symbol display
- Is used consistently across all withdrawal and product-related emails
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any custom currency symbol implementations that might affect this
ast-grep --pattern 'class Email {
$$$
currency_symbol($$$) {
$$$
}
$$$
}'
Length of output: 87
Script:
#!/bin/bash
# Let's try a broader search for currency_symbol method and its implementations
rg -A 5 "currency_symbol" --type php
# Also search for Email class definition
rg -A 10 "class Email" --type php
# Look for any files that might contain currency symbol related code
fd -e php -x grep -l "currency"
Length of output: 22453
kzamanbd
added
Needs: Testing
mrabbani
added
👍 Dev Review Done
and removed
Needs: Dev Review
shashwatahalder01
added
QA In Progress
QA approved
withdraw approval template
product publish template
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Bug Fixes