feat: verification email style update#137
Conversation
|
Warning Review limit reached
More reviews will be available in 17 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughEmail verification template redesigned with updated layout, messaging, and copy. New test suite validates the mailable renders correctly with expected verification text and links present while profile-edit sections are excluded. ChangesEmail Verification Template Redesign
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-137/ This page is automatically updated on each push to this PR. |
romanetar
force-pushed
the
feature/verification-email-update
branch
from
4d8e6ce to
0c179ce
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
resources/views/emails/auth/email_verification_request_fn.blade.php (2)
24-24: ⚡ Quick winConsider escaping application name from config.
While
Config::get('app.app_name')is typically admin-controlled, using escaped output{{ }}follows the principle of defense in depth and is the Laravel convention for displaying config values in templates.♻️ Proposed refactor
- <p style="font-family: 'Open Sans', Helvetica, Arial, sans-serif; font-size: 13px; color: `#5F5E5A`; margin: 20px 0 0; line-height: 1.6;">Didn't sign up for an {!! Config::get('app.app_name') !!} account? You can ignore this email — your account will be inactive without verification.</p> + <p style="font-family: 'Open Sans', Helvetica, Arial, sans-serif; font-size: 13px; color: `#5F5E5A`; margin: 20px 0 0; line-height: 1.6;">Didn't sign up for an {{ Config::get('app.app_name') }} account? You can ignore this email — your account will be inactive without verification.</p>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/views/emails/auth/email_verification_request_fn.blade.php` at line 24, Replace the unescaped Blade echo in the email template — currently using {!! Config::get('app.app_name') !!} in email_verification_request_fn.blade.php — with a safe/escaped output (e.g., use {{ Config::get('app.app_name') }} or the e() helper) so the application name is HTML-escaped; update any other occurrences in this view to follow the same convention.
29-29: ⚡ Quick winEscape help email address from config.
The help email from config is output unescaped in both the
mailto:href and display text. Use escaped syntax{{ }}for defense in depth, consistent with Laravel conventions.♻️ Proposed refactor
- Questions? <a href="mailto:{!! Config::get('app.help_email') !!}" style="color: `#0C64C8`; text-decoration: none;">{!! Config::get('app.help_email') !!}</a> + Questions? <a href="mailto:{{ Config::get('app.help_email') }}" style="color: `#0C64C8`; text-decoration: none;">{{ Config::get('app.help_email') }}</a>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/views/emails/auth/email_verification_request_fn.blade.php` at line 29, The help email is rendered unescaped in the anchor href and text; update the Blade template (email_verification_request_fn.blade.php) to escape the config value by replacing the unescaped {!! Config::get('app.help_email') !!} usages with the escaped Blade echo {{ Config::get('app.help_email') }} in both the mailto="..." and the link body so the email is HTML-escaped for defense-in-depth.tests/unit/EmailVerificationViewTest.php (2)
56-64: ⚡ Quick winConsider verifying that app name config is rendered.
The template injects
Config::get('app.app_name')at line 24 of the Blade file, but the test doesn't verify this value appears in the rendered HTML. Adding an assertion would improve coverage of the template's dynamic content.🧪 Optional enhancement
$this->assertStringContainsString('Verify email', $html); $this->assertStringContainsString('paste this link', $html); + $this->assertStringContainsString(Config::get('app.app_name'), $html); // _fn footer shows tenant support team, not an arbitrary help email🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/EmailVerificationViewTest.php` around lines 56 - 64, The Blade template injects Config::get('app.app_name') but the test in EmailVerificationViewTest does not assert it; update the test to fetch the expected app name (e.g., using Config::get('app.app_name') or config('app.app_name')) and add an assertion like $this->assertStringContainsString($expectedAppName, $html) (place it alongside the other $this->assertStringContainsString checks that use $html) so the rendered app name is verified.
25-30: ⚡ Quick winDon’t call
parent::prepareForTests()here—DB seeding isn’t used
Intests/unit/EmailVerificationViewTest.php,BrowserKitTestCase::prepareForTests()runs Doctrine migrations + seedsTestSeeder, butUserEmailVerificationRequestand its Blade templates only readConfigvalues and use the constructor inputs ($useris mocked), with no DB/model lookups found. Skipping the parent setup looks intentional and is consistent with the already-presentMail::fake()/Queue::fake().If the email/template later starts depending on DB-backed seeded data, update the test to call
parent::prepareForTests()(or seed only what’s needed).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/EmailVerificationViewTest.php` around lines 25 - 30, Remove any call to parent::prepareForTests() from the EmailVerificationViewTest::prepareForTests() method so the BrowserKitTestCase::prepareForTests() migrations/seeding are not run; keep the existing Queue::fake() and Mail::fake() calls and only call parent::prepareForTests() (or seed specific data) later if the email/template actually requires DB-backed data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/views/emails/auth/email_verification_request_fn.blade.php`:
- Line 19: The verification URL is currently output unescaped using the Blade
raw echo tag {!! $verification_link !!}, which can permit HTML injection; update
the email template to use Blade's escaped echo syntax (replace {!!
$verification_link !!} with {{ $verification_link }}) so the $verification_link
value is HTML-escaped when rendered and cannot inject markup.
- Line 8: Update the H1 heading in the email template to remove the trailing
period: locate the <h1> element containing "Confirm your email." in
email_verification_request_fn.blade.php and change the text to "Confirm your
email" (no period) so the heading follows standard UI copy conventions.
- Line 14: The anchor tag currently outputs the verification URL unescaped using
{!! $verification_link !!}; change it to use Blade's escaped output (e.g., {{
$verification_link }} or e($verification_link)) in the href so the link is
HTML-escaped and prevents potential injection if the link construction changes
upstream; update the href in the anchor in
email_verification_request_fn.blade.php accordingly.
- Line 9: The template outputs $user_email unescaped causing an XSS risk; update
the email_verification_request_fn.blade.php template so the displayed address is
HTML-escaped instead of raw HTML by replacing the unescaped interpolation of
$user_email (the {!! $user_email !!} usage) with Blade's escaped output (e.g.,
{{ $user_email }} or e($user_email)) so any HTML/JS in the value is encoded
before rendering.
In `@tests/unit/EmailVerificationViewTest.php`:
- Line 62: In EmailVerificationViewTest, the footer help email isn't being
asserted: set a known value with Config::set('app.help_email',
'support@fntech.example.com') at the start of the test and then add an assertion
that the rendered HTML contains that value (e.g.
assertStringContainsString('support@fntech.example.com', $html)) so the test
verifies Config::get('app.help_email') is actually rendered rather than only
checking for "Questions?". Ensure the new assertion is placed after the view is
rendered and before test teardown.
---
Nitpick comments:
In `@resources/views/emails/auth/email_verification_request_fn.blade.php`:
- Line 24: Replace the unescaped Blade echo in the email template — currently
using {!! Config::get('app.app_name') !!} in
email_verification_request_fn.blade.php — with a safe/escaped output (e.g., use
{{ Config::get('app.app_name') }} or the e() helper) so the application name is
HTML-escaped; update any other occurrences in this view to follow the same
convention.
- Line 29: The help email is rendered unescaped in the anchor href and text;
update the Blade template (email_verification_request_fn.blade.php) to escape
the config value by replacing the unescaped {!! Config::get('app.help_email')
!!} usages with the escaped Blade echo {{ Config::get('app.help_email') }} in
both the mailto="..." and the link body so the email is HTML-escaped for
defense-in-depth.
In `@tests/unit/EmailVerificationViewTest.php`:
- Around line 56-64: The Blade template injects Config::get('app.app_name') but
the test in EmailVerificationViewTest does not assert it; update the test to
fetch the expected app name (e.g., using Config::get('app.app_name') or
config('app.app_name')) and add an assertion like
$this->assertStringContainsString($expectedAppName, $html) (place it alongside
the other $this->assertStringContainsString checks that use $html) so the
rendered app name is verified.
- Around line 25-30: Remove any call to parent::prepareForTests() from the
EmailVerificationViewTest::prepareForTests() method so the
BrowserKitTestCase::prepareForTests() migrations/seeding are not run; keep the
existing Queue::fake() and Mail::fake() calls and only call
parent::prepareForTests() (or seed specific data) later if the email/template
actually requires DB-backed data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db379881-50c8-4acb-a535-62bcd2c87e6a
📒 Files selected for processing (2)
resources/views/emails/auth/email_verification_request_fn.blade.phptests/unit/EmailVerificationViewTest.php
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-137/ This page is automatically updated on each push to this PR. |
romanetar
force-pushed
the
feature/verification-email-update
branch
from
0c179ce to
d07947c
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-137/ This page is automatically updated on each push to this PR. |
Signed-off-by: romanetar <roman_ag@hotmail.com>
romanetar
force-pushed
the
feature/verification-email-update
branch
from
d07947c to
fbf8869
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-137/ This page is automatically updated on each push to this PR. |
1 participant
ref https://app.clickup.com/t/86b9txecd
Summary by CodeRabbit
New Features
Tests