-
Notifications
You must be signed in to change notification settings - Fork 185
fix-1392-Inconsistency-in-User-Verification-Status #2112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix-1392-Inconsistency-in-User-Verification-Status #2112
Conversation
👍 Expected behavior When the user hits Verify Phone, the alert should confirm that the user is verified. When the user hits Unverify Phone, the alert should confirm that the user is unverified. 👎 Actual Behavior When attempting to verify the phone number, an alert incorrectly states that the user is unverified. However, when attempting to unverify the phone, the system incorrectly indicates that the user is verified. Followed stnguyen90's guidelines: We want to update the alerts to be specific to say: XYZ's email has been verified XYZ's email has been unverified XYZ's phone has been verified XYZ's phone has been unverified Note: The "s" should only be there if the name does not end with s. For example, if the user's name was "Charles", it should be "Charles' email has been verified" Changed these lines: message: `${$user.name || $user.email || $user.phone || 'The account'} has been ${ !$user.emailVerification ? 'unverified' : 'verified' }`, message: `${$user.name || $user.email || $user.phone || 'The account'} has been ${ $user.phoneVerification ? 'unverified' : 'verified' }`, To these lines: message: `${$user.name || $user.email || $user.phone || 'The account'}${( $user.name || $user.email || $user.phone || 'The account' ).endsWith('s') ? "'" : "'s"} email has been ${!$user.emailVerification ? 'unverified' : 'verified'}`, message: `${$user.name || $user.email || $user.phone || 'The account'}${( $user.name || $user.email || $user.phone || 'The account' ).endsWith('s') ? "'" : "'s"} phone has been ${!$user.phoneVerification ? 'unverified' : 'verified'}`, Current behavior: XYZ's email has been verified XYZ's email has been unverified XYZ's phone has been verified XYZ's phone has been unverified OR (If the user's name ends in a s) QRS' email has been verified QRS' email has been unverified QRS' phone has been verified QRS' phone has been unverified
@stnguyen90 @ItzNotABug PR review request. A PR was submitted and reviewed by another contributor after mine was submitted. I don't know if you still want to review this one. |
That was me that submitted the PR. Sorry I didn't see your PR under the issue so I thought you forgot/left the issue. I did the PR similar to yours and they said I had to make some changes and after I made the changes they said someone else was working on it. @MicahHeneveld you should check out my PR just to be clear on what they wanted in case yours is next. I added another Issues fix in the same PR too. You can ignore that! Note I didn't saw that an Issue has to be assigned before you proceed with fix in the contribution guideline that's why I made the fix. |
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
Hi, I'm working alongisde @MicahHeneveld! We improved the PR to fit the updated standards: We want to update the alerts to be specific to say: if name exists and isn't empty: The email for XYZ has been verified if name does not exist or is empty: The email has been verified Here's what we changed:
To this:
The code snippet is the same for the email and phone verification with some modifications to the variable names. This change allows the system to read if a name is provided and acts accordingly. |
Hey 👋, Please run the formatter via |
The preview deployment is ready. 🟢 Open Preview | Open Build Logs Last updated at: 2025-08-20 19:22:59 CET |
The preview deployment is ready. 🟢 Open Preview | Open Build Logs Last updated at: 2025-08-20 19:23:00 CET |
Why do we need to change/add |
I apologize, the docker-compose yaml file was created when I ran pnpm format and the linter test. I had committed and pushed everything because I didn't see the docker-compose file was added. I should've checked the staged files before committing. I went ahead and pushed again without the docker-compose.yaml file. The PR should be ready to merge now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't quite match requirements:
if name exists and isn't empty:
The email for XYZ has been verified
The email for XYZ is no longer verified
The phone for XYZ has been verified
The phone for XYZ is no longer verified
if name does not exist or is empty:
The email has been verified
The email is no longer verified
The phone has been verified
The phone is no longer verified
WalkthroughThe PR updates notification messages in src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte for email and phone verification status changes. Messages now conditionally include the user’s name when present and use “has been verified” for verified and “is no longer verified” for unverified states. A blank line was added after await invalidate(Dependencies.USER) in the updated functions. No other logic related to toggling verification, invalidation, notification types, or analytics was changed. Assessment against linked issues
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
I made the changes. I apologize for the delayed response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte (1)
27-31
: Reduce duplication with a tiny helper for message composition.Keeps copy consistent and future-proof if wording changes.
Example helper (place in this script block):
function verificationMessage(kind: 'email' | 'phone', name: string, next: boolean) { const subject = name ? `The ${kind} for ${name}` : `The ${kind}`; return `${subject} ${next ? 'has been verified' : 'is no longer verified'}`; }Usage:
const displayName = $user.name?.trim() || ''; const nextEmail = !$user.emailVerification; addNotification({ message: verificationMessage('email', displayName, nextEmail), type: 'success' });Also applies to: 52-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte
(2 hunks)
message: `${ | ||
$user.name && $user.name.trim() !== '' | ||
? `The email for ${$user.name} ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | ||
: `The email ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | ||
}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Stabilize message against timing/race; base it on the intended new state, not the reactive store.
Right after invalidate(), $user
may still momentarily reflect the previous value. Build the message from a local nextEmail
computed before/with the API call to avoid inverted alerts.
Apply this diff to simplify and de-duplicate the string logic:
- message: `${
- $user.name && $user.name.trim() !== ''
- ? `The email for ${$user.name} ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}`
- : `The email ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}`
- }`,
+ message: `${displayName ? `The email for ${displayName}` : 'The email'} ${nextEmail ? 'has been verified' : 'is no longer verified'}`,
Add these lines inside updateVerificationEmail()
(outside the changed hunk) to define and use stable values:
const nextEmail = !$user.emailVerification;
const displayName = $user.name?.trim() || '';
await sdk.forProject(page.params.region, page.params.project).users.updateEmailVerification($user.$id, nextEmail);
Optionally confirm copy choice: do we prefer “is no longer verified” or “has been unverified” for consistency with button text?
🏁 Script executed:
#!/bin/bash
# Locate and inspect the updateStatus.svelte component to verify message construction and API call timing
file=$(fd -e svelte updateStatus.svelte)
sed -n '1,80p' "$file"
Length of output: 1076
🏁 Script executed:
#!/bin/bash
# Focus on the auth user updateStatus component
FILE=$(fd -e svelte updateStatus.svelte | grep '/auth/user-')
echo "Inspecting $FILE"
grep -n 'updateVerificationEmail' "$FILE" || true
echo "--- beginning of file ---"
sed -n '1,200p' "$FILE"
Length of output: 8976
Stabilize and simplify the success message in updateVerificationEmail by computing nextEmail
and displayName
before the API call and using them in addNotification
; apply the same pattern to updateVerificationPhone
.
--- a/src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte
@@ async function updateVerificationEmail() {
- await sdk
- .forProject(page.params.region, page.params.project)
- .users.updateEmailVerification($user.$id, !$user.emailVerification);
+ const nextEmail = !$user.emailVerification;
+ const displayName = $user.name?.trim() || '';
+ await sdk
+ .forProject(page.params.region, page.params.project)
+ .users.updateEmailVerification($user.$id, nextEmail);
@@
- addNotification({
- message: `${
- $user.name && $user.name.trim() !== ''
- ? `The email for ${$user.name} ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}`
- : `The email ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}`
- }`,
- type: 'success'
- });
+ addNotification({
+ message: `${displayName ? `The email for ${displayName}` : 'The email'} ${nextEmail ? 'has been verified' : 'is no longer verified'}`,
+ type: 'success'
+ });
Apply an equivalent change in updateVerificationPhone()
using const nextPhone = !$user.phoneVerification
and displayName
before its API call.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
message: `${ | |
$user.name && $user.name.trim() !== '' | |
? `The email for ${$user.name} ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | |
: `The email ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | |
}`, | |
--- a/src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte | |
@@ async function updateVerificationEmail() { | |
- await sdk | |
- .forProject(page.params.region, page.params.project) | |
const nextEmail = !$user.emailVerification; | |
const displayName = $user.name?.trim() || ''; | |
await sdk | |
.forProject(page.params.region, page.params.project) | |
.users.updateEmailVerification($user.$id, nextEmail); | |
@@ | |
- addNotification({ | |
- message: `${ | |
- $user.name && $user.name.trim() !== '' | |
- ? `The email for ${$user.name} ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | |
- : `The email ${!$user.emailVerification ? 'is no longer verified' : 'has been verified'}` | |
- }`, | |
- type: 'success' | |
addNotification({ | |
message: `${displayName ? `The email for ${displayName}` : 'The email'} ${ | |
nextEmail ? 'has been verified' : 'is no longer verified' | |
}`, | |
type: 'success' | |
}); |
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte
around lines 27 to 31, the success notification builds the message inline using
$user values after the API call; compute const nextEmail =
!$user.emailVerification and const displayName = $user.name && $user.name.trim()
!== '' ? $user.name : $user.email before making the updateVerificationEmail API
call, then pass those precomputed values into addNotification (use nextEmail to
decide "has been verified" vs "is no longer verified" and displayName for the
name/email shown). Apply the same pattern in updateVerificationPhone(): define
const nextPhone = !$user.phoneVerification and reuse a precomputed displayName
before the phone API call and in addNotification.
message: `${ | ||
$user.name && $user.name.trim() !== '' | ||
? `The phone for ${$user.name} ${!$user.phoneVerification ? 'is no longer verified' : 'has been verified'}` | ||
: `The phone ${!$user.phoneVerification ? 'is no longer verified' : 'has been verified'}` | ||
}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Mirror the same robustness for phone verification messages.
Use a stable nextPhone
rather than reading from $user
after invalidation to prevent occasional mismatches.
Apply this diff:
- message: `${
- $user.name && $user.name.trim() !== ''
- ? `The phone for ${$user.name} ${!$user.phoneVerification ? 'is no longer verified' : 'has been verified'}`
- : `The phone ${!$user.phoneVerification ? 'is no longer verified' : 'has been verified'}`
- }`,
+ message: `${displayName ? `The phone for ${displayName}` : 'The phone'} ${nextPhone ? 'has been verified' : 'is no longer verified'}`,
Add these lines inside updateVerificationPhone()
(outside the changed hunk):
const nextPhone = !$user.phoneVerification;
const displayName = $user.name?.trim() || '';
await sdk.forProject(page.params.region, page.params.project).users.updatePhoneVerification($user.$id, nextPhone);
🏁 Script executed:
#!/bin/bash
# Locate updateStatus.svelte
file=$(fd updateStatus.svelte -t f)
echo "File found: $file"
# Show context around the message snippet
rg -nC3 'message:' "$file"
# Search for updateVerificationPhone usage
rg -nC2 'updateVerificationPhone' "$file"
# Search for displayName and nextPhone declarations
rg -nE 'displayName|nextPhone' -nC2 "$file"
Length of output: 3264
🏁 Script executed:
#!/bin/bash
# Target the auth user updateStatus.svelte file
file=$(fd -e svelte updateStatus.svelte | grep 'auth/user' | head -n1)
echo "Using file: $file"
# Show the message block context
rg -nC3 'message:' "$file"
# Locate updateVerificationPhone function definition and calls
rg -nC3 'updateVerificationPhone' "$file"
# Check for existing displayName or nextPhone variables
rg -nC3 'displayName' "$file"
rg -nC3 'nextPhone' "$file"
Length of output: 4017
🏁 Script executed:
#!/bin/bash
# Find the auth user updateStatus.svelte file
file=$(fd -e svelte updateStatus.svelte | grep 'auth/user' | head -n1)
echo "Inspecting: $file"
# Print the first 200 lines to view updateVerificationPhone and related context
sed -n '1,200p' "$file"
Length of output: 8576
Use local nextPhone
and displayName
in updateVerificationPhone
– Inside async function updateVerificationPhone()
(in src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte), add at the top:
const nextPhone = !$user.phoneVerification;
const displayName = $user.name?.trim() || '';
– Replace the SDK call to use nextPhone
:
- await sdk
- .forProject(page.params.region, page.params.project)
- .users.updatePhoneVerification($user.$id, !$user.phoneVerification);
+ await sdk
+ .forProject(page.params.region, page.params.project)
+ .users.updatePhoneVerification($user.$id, nextPhone);
– Update the success notification to:
addNotification({
message: `${displayName ? `The phone for ${displayName}` : 'The phone'} ${nextPhone ? 'has been verified' : 'is no longer verified'}`,
type: 'success'
});
This ensures the message and API call consistently use the intended new verification state.
🤖 Prompt for AI Agents
In
src/routes/(console)/project-[region]-[project]/auth/user-[user]/updateStatus.svelte
around lines 52 to 56, the updateVerificationPhone function should use local
variables for the intended new verification state and display name so the API
call and success notification are consistent; add at the top of that function:
const nextPhone = !$user.phoneVerification; const displayName =
$user.name?.trim() || ''; then change the SDK call to use nextPhone (not
toggling or reading $user again) and update the addNotification message to use
displayName and nextPhone so the text reads either "The phone for {displayName}
has been verified" / "is no longer verified" or "The phone has been verified" /
"is no longer verified" accordingly.
What does this PR do?
Fixes #1392
👍 Expected behavior
When the user hits Verify Phone, the alert should confirm that the user is verified. When the user hits Unverify Phone, the alert should confirm that the user is unverified. 👎 Actual Behavior
When attempting to verify the phone number, an alert incorrectly states that the user is unverified. However, when attempting to unverify the phone, the system incorrectly indicates that the user is verified.
Followed stnguyen90's guidelines:
We want to update the alerts to be specific to say:
XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified
Note:
The "s" should only be there if the name does not end with s. For example, if the user's name was "Charles", it should be "Charles' email has been verified"
Changed these lines:
message:
${$user.name || $user.email || $user.phone || 'The account'} has been ${ !$user.emailVerification ? 'unverified' : 'verified' }
,message:
${$user.name || $user.email || $user.phone || 'The account'} has been ${ $user.phoneVerification ? 'unverified' : 'verified' }
,To these lines:
message:
${$user.name || $user.email || $user.phone || 'The account'}${( $user.name || $user.email || $user.phone || 'The account' ).endsWith('s') ? "'" : "'s"} email has been ${!$user.emailVerification ? 'unverified' : 'verified'}
,Current behavior:
XYZ's email has been verified
XYZ's email has been unverified
XYZ's phone has been verified
XYZ's phone has been unverified
OR (If the user's name ends in a s)
QRS' email has been verified
QRS' email has been unverified
QRS' phone has been verified
QRS' phone has been unverified
Test Plan
Under a project, select Auth. In Auth, select or create a user that has both an email and a phone number. Select the 'Verify account' option and then verify the phone number and email. The correct alerts will be displayed.
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit