fix(drip-email): Embed product trial logo inline in trial emails (backport #7576) - #7579
Merged
Merged
Conversation
The product_trial_email template received the Product Trial logo but never rendered it, so trial drip emails showed no logo. Render it beside the title via <img embed=...> so Frappe attaches it as an inline CID image (no remote fetch, no "Load images"), with a clean fallback when a trial has no logo. (cherry picked from commit 039b2f5)
The product_trial_email template is shared with suspension emails, where logo is a full URL but the inline image is registered under the relative filename (logo_name). Embed logo_name so both callers resolve, and pass logo_name plus the inline image from the drip path too. (cherry picked from commit 7341b63)
Use frappe.utils.get_url on the Product Trial logo so the img src is an absolute URL that resolves in the email client, instead of a relative path or a CID that must be registered per caller. (cherry picked from commit 53e7227)
A remote src does not render when the client blocks external images, so revert to embedding the logo as an inline CID image via the registered filename (logo_name). Add a test that the drip send registers the logo as an inline image. (cherry picked from commit d78a1ed)
Use frappe's get_filecontent_from_path, which resolves the logo path and enforces directory containment, instead of opening it directly. Give the test a unique logo filename and cover its whole lifetime with cleanup so it cannot clobber or leak a public file. (cherry picked from commit cc2f7c6)
Collaborator
|
🎉 This PR is included in version 0.136.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7579 +/- ##
===========================================
- Coverage 86.44% 62.33% -24.11%
===========================================
Files 157 1064 +907
Lines 29487 103779 +74292
Branches 1909 1907 -2
===========================================
+ Hits 25489 64687 +39198
- Misses 3942 39035 +35093
- Partials 56 57 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trial drip emails passed the Product Trial logo to the
product_trial_emailtemplate but never rendered it, so no logo showed. Render it beside the title as an inline CID image via<img embed="{{ logo_name }}">, so it displays even when the client blocks external images (no "Load images"). Falls back cleanly when a trial has no logo, and also fixes the same latent mismatch in the suspension email.Note: logos should be PNG — Gmail/Outlook/Apple Mail don't render SVG even inline (Frappe CRM's trial already has a PNG attached).
This is an automatic backport of pull request #7576 done by Mergify.