test(onboarding): add mailsac-code email verification strategy#3080
Merged
ygrishajev merged 1 commit intomainfrom Apr 20, 2026
Merged
test(onboarding): add mailsac-code email verification strategy#3080ygrishajev merged 1 commit intomainfrom
ygrishajev merged 1 commit intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdded a new email verification strategy "mailsac-code", extended the environment enum to accept it, implemented a Mailsac polling strategy that extracts 6-digit codes and types them into Playwright pages, and wired the strategy into the factory function. (≤50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3080 +/- ##
==========================================
- Coverage 60.93% 60.05% -0.89%
==========================================
Files 1023 983 -40
Lines 24491 23488 -1003
Branches 6060 5891 -169
==========================================
- Hits 14924 14105 -819
+ Misses 8355 8184 -171
+ Partials 1212 1199 -13
*This pull request uses carry forward flags. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts`:
- Around line 16-20: The code assumes input.context.pages()[0] exists which can
crash when no pages or wrong tab is active; change the page selection to guard
for empty/multiple pages by checking input.context.pages().length > 0 and
selecting an appropriate page (e.g., the first non-null page or input.page)
before using it, and if no page found throw or return a clear error; update the
usage sites (page variable, firstDigitInput via getByLabel("Verification code
digit 1"), and subsequent page.keyboard.type(code)) so they only run after the
null/empty check.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5341274e-3608-4bf7-bf37-259741004ca0
📒 Files selected for processing (3)
apps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/services/email-verification/index.tsapps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Add a new email verification strategy for the upcoming code-based verification flow. Polls Mailsac for the verification code email, extracts the 6-digit code from the body, and enters it in the UI. Activated via EMAIL_VERIFICATION_STRATEGY=mailsac-code env var.
ygrishajev
force-pushed
the
test/onboarding-mailsac-code-strategy
branch
from
d5d3901 to
dde9013
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts (1)
48-49: Avoid sleeping after the final polling attempt.Line 48 currently adds an extra 2s delay even after the last attempt fails.
Proposed fix
- await new Promise(resolve => setTimeout(resolve, pollIntervalMs)); + if (attempt < maxAttempts - 1) { + await new Promise(resolve => setTimeout(resolve, pollIntervalMs)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts` around lines 48 - 49, The loop currently always awaits "await new Promise(resolve => setTimeout(resolve, pollIntervalMs));" even after the final polling attempt, causing an unnecessary delay; change the logic in the polling function (the loop that uses pollIntervalMs and maxAttempts) to only sleep between attempts by conditioning the await on not being the last iteration (e.g., only await when currentAttempt < maxAttempts - 1) or move the sleep to the start of the next iteration so no sleep occurs after the final attempt.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts`:
- Around line 48-49: The loop currently always awaits "await new Promise(resolve
=> setTimeout(resolve, pollIntervalMs));" even after the final polling attempt,
causing an unnecessary delay; change the logic in the polling function (the loop
that uses pollIntervalMs and maxAttempts) to only sleep between attempts by
conditioning the await on not being the last iteration (e.g., only await when
currentAttempt < maxAttempts - 1) or move the sleep to the start of the next
iteration so no sleep occurs after the final attempt.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d45d3d5-e7e7-46aa-aa70-1332d6b75504
📒 Files selected for processing (3)
apps/deploy-web/tests/ui/fixture/test-env.config.tsapps/deploy-web/tests/ui/services/email-verification/index.tsapps/deploy-web/tests/ui/services/email-verification/mailsac-code.strategy.ts
✅ Files skipped from review due to trivial changes (1)
- apps/deploy-web/tests/ui/fixture/test-env.config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/deploy-web/tests/ui/services/email-verification/index.ts
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
1 participant
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.
Why
Part of CON-152
Prepares E2E test infrastructure for the upcoming code-based email verification flow (PR #3052). The current strategies only support link-based verification.
What
MailsacCodeVerificationStrategy— polls Mailsac for the verification code email, extracts the 6-digit code from the plain text body via/text/API endpoint, and enters it in the UI viapage.keyboard.type()mailsac-codeoption in the strategy factory and env config schemaEMAIL_VERIFICATION_STRATEGY=mailsac-codeenv var; existing strategies unchangedSummary by CodeRabbit