[PM-25240] Send Access OTP email in MJML format #6411
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6411 +/- ##
==========================================
+ Coverage 50.99% 51.82% +0.83%
==========================================
Files 1885 1870 -15
Lines 83249 82401 -848
Branches 7360 7273 -87
==========================================
+ Hits 42450 42707 +257
+ Misses 39186 38064 -1122
- Partials 1613 1630 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ate-otp-email-mjml
…com/bitwarden/server into auth/pm-25240/update-otp-email-mjml
| </mj-style> | ||
|
|
||
| <!-- Responsive icon visibility --> | ||
| <mj-style> |
There was a problem hiding this comment.
So is this file used in all emails, or will other emails have to add this same css selector to get the responsive behavior?
There was a problem hiding this comment.
If a mjml tempate wishes to use the styling in the head.mjml template, then it would need to be referenced like this:
<mjml>
<mj-head>
<mj-include path="../components/head.mjml" />
</mj-head>
...
</mjml>Then the class would be applied to tags using the css-class attribute. In mj-bw-hero.js we use it to hide the logo in the blue header area.
<mj-image
src="${this.getAttribute("img-src")}"
alt=""
width="140px"
height="140px"
padding="0px"
css-class="hide-small-img" /> <!-- here -->There was a problem hiding this comment.
So the hide-small-img class wouldn't do anything if the template doesn't include the head.mjml template, right? I guess I'm trying to think if having the styling separate from the class usage could cause sneaky bugs if the styling isn't included
There was a problem hiding this comment.
I think my recommendation would be to always include the head.mjml as a matter of practice. Which should avoid that issue? Not sure if that's the best approach or not.
There was a problem hiding this comment.
If it's expected to always be included then that seems fine to me. Can we document that somewhere?
Added information about build script definitions for MJML.
…ate-otp-email-mjml
src/Core/MailTemplates/Handlebars/Auth/SendAccessEmailOtpEmailv2.html.hbs
Show resolved
Hide resolved
| This code can only be used once and expires in {{Expiry}} minutes. After that you'll need to verify your email again. | ||
|
|
||
| Bitwarden Send transmits sensitive, temporary information to others easily and securely. Learn more about Bitwarden Send or sign up to try it today. | ||
| {{/BasicTextLayout}} No newline at end of file |
There was a problem hiding this comment.
⛏️ Same as the other .hbs file comment re: newline.
5 participants
🎟️ Tracking
PM-25240
📔 Objective
This work supports the building of
*.html.hbsartifacts for the Send Access OTP email using theMJMLtemplating library.MJML templating is not standard in our code base yet, a feature flag is added in the
SendEmailOtpRequestValidator.Since the
HandlebarsMailServiceis registered as aSingletonwe cannot inject theFeatureServicesince theFeatureServiceis registered asScoped. This means we had to update the call site of the email rather than in theHandlebarsMailService.📸 Screenshots
Responsive email
Full email
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes