Conversation
|
@greptileai review por favor? |
Greptile SummaryThis PR improves email verification by fixing the bug where expired (but undeleted) pending requests blocked users from re-adding the same email, and adds a resend-verification feature with a 10-minute cooldown. Tests are included for the new resend and unlink-pending-email paths. One UX inconsistency worth addressing: for expired verification requests where the cooldown has also elapsed, the Resend button is disabled but labeled "Resend soon", implying a short wait will help — when in reality no wait will ever enable it (the server rejects resends for expired tokens via the Confidence Score: 5/5Safe to merge — the core logic is correct, all paths are scoped to the current user, and tests cover the new flows. All findings are P2 (UX label wording). The bug fixes are correct, resend cooldown is properly enforced, and the soft-delete approach consistently prevents the softlock. Tests were added for the new behaviour. app/javascript/pages/Users/Settings/Integrations.svelte — minor label wording for expired tokens. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (Browser)
participant S as SessionsController
participant M as EmailVerificationRequest
participant Mail as EmailVerificationMailer
U->>S: POST /auth/email/add {email}
S->>M: where(deleted_at: nil).exists?(email)
alt Already pending/verified
S-->>U: redirect — already pending
else
S->>M: create!(email, token, expires_at: +30min)
S->>Mail: deliver verification email
S-->>U: redirect — check your email
end
U->>S: POST /auth/email/resend_verification {email}
S->>M: valid.find_by(email) [scope: expires_at > now AND deleted_at IS NULL]
alt Not found (expired or deleted)
S-->>U: redirect — no pending verification
else Found
S->>M: resend_available?
alt Within 10-min cooldown
S-->>U: redirect — please wait N minutes
else Cooldown passed
S->>M: refresh_for_resend! (new token, expires_at: +30min)
S->>Mail: deliver verification email
S-->>U: redirect — verification email resent
end
end
U->>S: DELETE /auth/email/unlink {email}
alt Verified email exists
S->>M: find any associated verification request
S->>M: email_record.destroy!
S->>M: soft_delete! verification request (if any)
S-->>U: redirect — email unlinked
else Only pending request exists
S->>M: where(deleted_at: nil).find_by(email)
S->>M: soft_delete!
S-->>U: redirect — pending email removed
end
U->>S: GET /auth/token/:token
S->>M: valid.find_by(token)
S->>M: verify! (creates EmailAddress, soft_deletes request)
S-->>U: redirect — email verified
Prompt To Fix All With AIThis is a comment left during a code review.
Path: app/javascript/pages/Users/Settings/Integrations.svelte
Line: 225-229
Comment:
**Misleading button label for expired tokens**
When a verification request is expired and its cooldown has also elapsed, `resend_cooldown_seconds` is `0` (clamped), so `formatCooldown(0)` returns `""`, and the button falls back to `"Resend soon"`. This implies waiting will eventually enable the button, but `resend_email_verification` uses the `.valid` scope and will always reject expired tokens regardless of the cooldown. The user actually needs to **Remove** the request and re-add the address.
Consider showing `"Expired"` (or hiding the button entirely) when `email.expired` is true:
```suggestion
{email.can_resend
? "Resend"
: email.expired
? "Expired"
: formatCooldown(email.resend_cooldown_seconds) ||
"Resend soon"}
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (5): Last reviewed commit: "Merge branch 'main' into email-verificat..." | Re-trigger Greptile |
|
@greptileai I forgor to do ts again |
#1139 needs to be merged for tests to pass on this pr :pf: