You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[$1000] [HOLD for payment 2023-06-21] [HOLD for payment 2023-06-19] Web - New Group - Wrong user details page opens on clicking at display names #20583
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Identify the pull request that introduced this issue and revert it.
Please re-state the problem that we are trying to solve in this issue.
Wrong user details page opens on clicking at display names
What is the root cause of that problem?
The root cause of this problem is that, the accountIDs and the participant emails are not matched in correct order.
This is the screen shot of accountIDs and their emails in ReportScreen
And here are the details of those users -
If we pay close attention for first two users in the first picture IDs are interchanged, That's why opening the user details were taking on wrong page.
This is the issue video -
What changes do you think we should make in order to solve the problem?
Instead of using index we can create the object, that will map the participant emails to their accountID
We can use personalDetails for that, this will ensure we are navigating with proper accountID each time.
What alternative solutions did you explore? (Optional)
I think I may use @BhuvaneshPatil solution but implement it myself!
I'm checking if personalDetails will always have the accountID, if that's not the case we can also search for the participant's actual index searching the email from displayNamesWithTooltips in participants. I like the proposed solution much better than doing this but it's an alternative : )
Using personalDetails works fine when accounts have a Display Name but not so much for SMS based accounts, I'm thinking of a way to make that work : ) (it has to do with the login param and how each object/list is generating theirs)
Using the participants list has the same problem where the domain is stripped and we need that to find the account btw
I don't want to simply use an include() to check if the email matches so I'm still investigating
Edit: Oh I think I can just use addSMSDomainIfPhoneNumber with the tooltip content!
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
[@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
[@aimane-chnaif] Determine if we should create a regression test for this bug.
[@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
This bug was introduced while implementing feature of new profile page.
But actually I found that our solution was temporary. (original solution was right but only after migration completed)
The logic on this page (ReportWelcomeText) was refactored again in accountID migration PR.
As the issue caused during migration, no need regression test.
Activity
OSBotify commentedon Jun 12, 2023
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
StagingDeployCashdeploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:melvin-bot commentedon Jun 12, 2023
Triggered auto assignment to @PauloGasparSv (
Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.BhuvaneshPatil commentedon Jun 12, 2023
Proposal
Please re-state the problem that we are trying to solve in this issue.
Wrong user details page opens on clicking at display names
What is the root cause of that problem?
The root cause of this problem is that, the accountIDs and the participant emails are not matched in correct order.

This is the screen shot of accountIDs and their emails in
ReportScreenAnd here are the details of those users -

If we pay close attention for first two users in the first picture IDs are interchanged, That's why opening the user details were taking on wrong page.
This is the issue video -
7.New.Expensify.Mozilla.Firefox.2023-06-12.19-00-08.mp4
Here we are using index to navigate -
App/src/components/ReportWelcomeText.js
Lines 94 to 112 in 297e5aa
What changes do you think we should make in order to solve the problem?
Instead of using index we can create the object, that will map the participant emails to their accountID
We can use
personalDetailsfor that, this will ensure we are navigating with proper accountID each time.What alternative solutions did you explore? (Optional)
None
PauloGasparSv commentedon Jun 12, 2023
I think I may use @BhuvaneshPatil solution but implement it myself!
I'm checking if
personalDetailswill always have theaccountID, if that's not the case we can also search for the participant's actual index searching the email fromdisplayNamesWithTooltipsinparticipants. I like the proposed solution much better than doing this but it's an alternative : )PauloGasparSv commentedon Jun 12, 2023
Note: Also test what happens if an account has a
displayNameand sms/phoneNumber accountsPauloGasparSv commentedon Jun 12, 2023
Using
personalDetailsworks fine when accounts have a Display Name but not so much for SMS based accounts, I'm thinking of a way to make that work : ) (it has to do with theloginparam and how each object/list is generating theirs)Using the
participantslist has the same problem where the domain is stripped and we need that to find the account btwI don't want to simply use an
include()to check if the email matches so I'm still investigatingEdit: Oh I think I can just use
addSMSDomainIfPhoneNumberwith the tooltip content!31 remaining items
melvin-bot commentedon Jun 21, 2023
Current assignee @aimane-chnaif is eligible for the External assigner, not assigning anyone new.
johncschuster commentedon Jun 21, 2023
@BhuvaneshPatil and @aimane-chnaif, can you apply to the Upwork job I just created above?
BhuvaneshPatil commentedon Jun 21, 2023
Applied
johncschuster commentedon Jun 22, 2023
Thanks! @aimane-chnaif, I've invited you on Upwork as well. Can you accept that invite when you get the chance?
Puneet-here commentedon Jun 22, 2023
@johncschuster, I have also applied to the job. I am reporter on this.
johncschuster commentedon Jun 23, 2023
Perfect! Thanks, @Puneet-here!
johncschuster commentedon Jun 23, 2023
I've sent offers to you both, @aimane-chnaif and @Puneet-here. Please ping me when you've accepted!
Puneet-here commentedon Jun 23, 2023
@johncschuster accepted, thanks!
PauloGasparSv commentedon Jun 26, 2023
This is not overdue but we still need to fill the bugzero checklist so bump @aimane-chnaif : )
aimane-chnaif commentedon Jun 26, 2023
This bug was introduced while implementing feature of new profile page.
But actually I found that our solution was temporary. (original solution was right but only after migration completed)
The logic on this page (ReportWelcomeText) was refactored again in accountID migration PR.
As the issue caused during migration, no need regression test.
johncschuster commentedon Jun 26, 2023
Payment has been issued to everyone 🎉
johncschuster commentedon Jun 27, 2023
I think we can close this one out