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
[HOLD for payment 2023-06-29] [HOLD for payment 2023-06-28] Name is missing in LHN tooltip #20801
Please re-state the problem that we are trying to solve in this issue.
When hovering over avatars in LHN, display name is not displayed in the tooltio
What is the root cause of that problem?
The issue is caused by the incorrect implementation of the ReportUtils.getAccountIDForLogin method that probably left while working on switching to using accountID as keys in the personalDetails login.
What changes do you think we should make in order to solve the problem?
* Gets the accountID for a login by looking in the ONYXKEYS.PERSONAL_DETAILS Onyx key (stored in the local variable, allPersonalDetails). If it doesn't exist in Onyx,
What alternative solutions did you explore? (Optional)
n/a
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Actually, it must be '0' because lodashKeys returns an array of strings, so the result of lodashFind will also be a string - the fallback type should be consistent here.
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.29-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-06-28. 🎊
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty
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:
[@Beamanator] The PR that introduced the bug has been identified. Link to the PR:
[@Beamanator] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@Beamanator] 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:
[@burczu] Determine if we should create a regression test for this bug.
[@burczu] 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.
[@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
changed the title [-][HOLD for payment 2023-06-28] Name is missing in LHN tooltip[/-][+][HOLD for payment 2023-06-29] [HOLD for payment 2023-06-28] Name is missing in LHN tooltip[/+]on Jun 22, 2023
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.30-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-06-29. 🎊
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty
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:
[@Beamanator] The PR that introduced the bug has been identified. Link to the PR:
[@Beamanator] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@Beamanator] 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:
[@burczu] Determine if we should create a regression test for this bug.
[@burczu] 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.
[@Christinadobrzyn] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
Activity
melvin-bot commentedon Jun 15, 2023
Triggered auto assignment to @Christinadobrzyn (
Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.melvin-bot commentedon Jun 15, 2023
Bug0 Triage Checklist (Main S/O)
Platformsin OP are ✅)burczu commentedon Jun 15, 2023
Hi I'm Bartek from Callstack - expert contributor group - I can work on this issue.
burczu commentedon Jun 15, 2023
Proposal
Please re-state the problem that we are trying to solve in this issue.
When hovering over avatars in LHN, display name is not displayed in the tooltio
What is the root cause of that problem?
The issue is caused by the incorrect implementation of the
ReportUtils.getAccountIDForLoginmethod that probably left while working on switching to usingaccountIDas keys in thepersonalDetailslogin.What changes do you think we should make in order to solve the problem?
The root cause is here:
App/src/libs/ReportUtils.js
Lines 824 to 832 in d396430
As we can see, it tries to access the
personalDetailsobject by theloginas the key. It should be changed as below:What alternative solutions did you explore? (Optional)
n/a
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
Beamanator commentedon Jun 15, 2023
@burczu yeppp good catch 👍 Though I think at the end of the 3rd line we want to default to
0, not'0'Though you did that in the return, so it's probably fine either way 👍
burczu commentedon Jun 15, 2023
Actually, it must be
'0'becauselodashKeysreturns an array of strings, so the result oflodashFindwill also be a string - the fallback type should be consistent here.burczu commentedon Jun 15, 2023
PR is ready for review!
melvin-bot commentedon Jun 15, 2023
Job added to Upwork: https://www.upwork.com/jobs/~0137b40d29cb6cb326
14 remaining items
melvin-bot commentedon Jun 19, 2023
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.
If a regression has occurred and you are the assigned CM follow the instructions here.
If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.
melvin-bot commentedon Jun 19, 2023
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.
If a regression has occurred and you are the assigned CM follow the instructions here.
If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.
[-]Name is missing in LHN tooltip[/-][+][HOLD for payment 2023-06-28] Name is missing in LHN tooltip[/+]melvin-bot commentedon Jun 21, 2023
Reviewinglabel has been removed, please complete the "BugZero Checklist".melvin-bot commentedon Jun 21, 2023
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.29-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-06-28. 🎊
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
melvin-bot commentedon Jun 21, 2023
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:
[-][HOLD for payment 2023-06-28] Name is missing in LHN tooltip[/-][+][HOLD for payment 2023-06-29] [HOLD for payment 2023-06-28] Name is missing in LHN tooltip[/+]melvin-bot commentedon Jun 22, 2023
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.30-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-06-29. 🎊
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
melvin-bot commentedon Jun 22, 2023
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: