console
console copied to clipboard
appwrite
🐛 Bug Report: Inconsistency in User Verification Status between Email and Phone Number
👟 Reproduction steps
- To trigger this bug, begin by accessing the Auth,
- Create a new user account and navigate to the user profile to verify the account.
- Start the verification process for both the email address and the phone number associated with the user account. Initially, verify the email address, which should proceed smoothly.
- Next, attempt to verify the phone number, Despite the successful verification of the email address, the system incorrectly indicates that the user is unverified when verifying the phone number.
- In an attempt to rectify this inconsistency, proceed to unverify the phone number. Unexpectedly, the system now displays an alert indicating that the user is verified, contradicting the initial indication of being unverified.
👍 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.
🎲 Appwrite version
Version 1.4.x
💻 Operating system
Windows
🧱 Your Environment
No response
👀 Have you spent some time to check if this issue has been raised before?
- [X] I checked and didn't find similar issue
🏢 Have you read the Code of Conduct?
- [X] I have read the Code of Conduct
https://github.com/appwrite/appwrite/assets/73753957/d393de11-ff8a-4ba9-aa97-0d8e5623f94c
@gurjeetsinghvirdee, thanks for raising this issue! 🙏🏼 Let me confirm with the team.
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"
FYI, the code that needs to be updated is here:
https://github.com/appwrite/console/blob/6748b1425f038bb8ec31ebbcb3284236d6ca4aa5/src/routes/(console)/project-%5Bproject%5D/auth/user-%5Buser%5D/updateStatus.svelte#L15-L54
Hi, assuming that the previous PR has some issues since it hasn't been merged so far. Can I take over this issue?
@pushkar707 Since it doesn't merge that doesn't mean the PR have issues, PRs been merged according to the priority level!
Hi, I'm working on this.
Our team was able to reproduce the bug:
"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."
This looked like a condition logic flaw to us. We then found the condition that was causing the flaw here
Which was missing a bang.
stnguyen90 mentioned the desired 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 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"
We implemented a solution that met these requirements by changing this code
To this code:
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'},
Our team then implemented the same solution to updateVerificationEmail() as well, as seen here
This solution can be tested just as gurjeetsinghvir shows in the video posted to this issue. 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.
Here is out PR: fix-1392
Did we approach this issue correctly? Shoutouts to my teammate @anthonythang1
@toukirkhan would love to get an update or status here, thank you :)
@MicahHeneveld , my apologies for the misunderstanding, Toukirkhan already mentioned he's working on it (and sent me a DM about it that his team is also working on it with him to prepare a PR). I just forgot to assign it on time. If Toukirkhan cancels or does not continue, we will review your PR next.
We also updated the contributor guide to be more clear on the proper steps before putting in a PR for next time, since there were 3 people working on a PR the same time. (not a bad thing, right?) :) I appreciate your efforts and hoping to see more from ya :)
We discussed this again internally and we want to avoid the awkward "'s" and "unverified" so the desired behavior is:
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 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
As a reminder, the code that needs to be updated is here:
https://github.com/appwrite/console/blob/4538dbb7efab8465cf9ba63282522bc686c70659/src/routes/(console)/project-%5Bregion%5D-%5Bproject%5D/auth/user-%5Buser%5D/updateStatus.svelte#L18-L61
We discussed this again internally and we want to avoid the awkward "'s" and "unverified" so the desired behavior is:
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 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
I've done the fix PR complied with the teams demand and was notified me that this issue is taking care of by internal team as of now, It has been some time but the bug remains. 3 days ago this issue assigned to someone and he still didn't finish it. Meantime I clarified with the team and did as they said and They are still waiting for the assignee to finish the task. There is no response from both of them. Assignee is taking like a month to finish this simple thing and not even responding. @stnguyen90 @tessamero consider my PR or Reference my PR to solve this. You guys think about it, unlike others I am constantly checking on Issues and PR related to Appwrite and spending my time to resolve issues as much as I can. @stnguyen90 even provided the code to be changed for them to do it faster but still no response.
[!NOTE] Its True that I didn't ask for Issue assign but I am the one who asked for the opinion on how to address the popup and we came to the conclusion of going with the current format meanwhile others didn't even took the initiative to do so.
We discussed this again internally and we want to avoid the awkward "'s" and "unverified" so the desired behavior is: 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 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
I've done the fix PR complied with the teams demand and was notified me that this issue is taking care of by internal team as of now, It has been some time but the bug remains. 3 days ago this issue assigned to someone and he still didn't finish it. Meantime I clarified with the team and did as they said and They are still waiting for the assignee to finish the task. There is no response from both of them. Assignee is taking like a month to finish this simple thing and not even responding. @stnguyen90 @tessamero consider my PR or Reference my PR to solve this. You guys think about it, unlike others I am constantly checking on Issues and PR related to Appwrite and spending my time to resolve issues as much as I can. @stnguyen90 even provided the code to be changed for them to do it faster but still no response.
Note
Its True that I didn't ask for Issue assign but I am the one who asked for the opinion on how to address the popup and we came to the conclusion of going with the current format meanwhile others didn't even took the initiative to do so.
Hey Joyal — I really appreciate you continuing to follow up on this and for all the time you’ve put into helping. As I mentioned earlier in Discord, Toukirkhan had already asked to work on this and confirmed with me directly, (and confusion may have arose from me being late to assigning the issue to him and that there hasn't been any follow ups on progress). I’ve asked him just now (via DM) to post an update on the status here so the thread doesn’t go quiet. (We updated our contributors guide also so this situation doesnt cause conflict again to be more fair to all contributors.
Your PR and feedback have still been helpful for shaping the discussion and clarifying the right approach and that impact is important. thank you!
PS - We have a new idea and we are potentially going to update the contributor guide to also require a weekly checkin for people who are assigned to issues, or it will be reassigned. hopefully this makes a big difference in the whole contribution process :) So, good things are happening!
@tessamero like I said in the dc, I wanted to see the bug removed. Appriciate that you checked with him and confirming he is still working on it. If he is improving beyond yours expectation then great news for me too.
hey there @Joyal-George-KJ, we were unable to make much progress on this issue. Please feel free to claim this one by requesting someone from The Appwrite team. Thanks!
Thanks for the update @toukirkhan.
I have reassigned the issue to @MicahHeneveld and assigned their PR to be reviewed by our engineering team.
Hi, I see this issue is still open. I’d love to work on it and submit a PR with the updated alert messages as per the team’s guidance. Could you please assign me if it’s available?
@Simransingh06 The assignee's PR is in Review and He is waiting for the merge of the PR. I think this is considered as Issue closed because his PR clears the Issue.