-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[HOLD for overpayment refund] [$1000] Web - Duplicate particpant name and avatar at the top of the Money request message. #25819
Comments
Triggered auto assignment to @mallenexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Proposed solutionPlease re-state the problem that we are trying to solve in this issue.Web - Duplicate the participant's name and avatar at the top of the Money request message. What is the root cause of that problem?**Here is the explanation: When User A requests money then an IOU (XX) report So both avatars are visible on each side. The first avatar Now User B requests the money then The first avatar App/src/pages/home/report/ReportActionItemSingle.js Lines 116 to 126 in 329cac3
What changes do you think we should make in order to solve the problem?**To fix this issue we need to check if it's an iou report then use the associated report owner and manager ID similar to participants list page https://github.com/Expensify/App/blob/main/src/pages/ReportParticipantsPage.js#L61 Updated code will be like: If actor is same owner then user manage ID What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Web - Duplicate participant name and avatar at the top of the Money request message. What is the root cause of that problem?We're always getting the second user detail from App/src/pages/home/report/ReportActionItemSingle.js Lines 117 to 118 in 329cac3
So when the ownerAccountID is also the person who request the money, it will cause this issue, because now the secondaryUserDetails is also the currentUserDetails.
What changes do you think we should make in order to solve the problem?We should add a new condition to identify the secondaryUserId: const secondaryAccountId = props.iouReport.ownerAccountID === actorAccountID ? props.iouReport.managerID : props.iouReport.ownerAccountID; Then use it below: if (displayAllActors) {
const secondaryUserDetails = props.personalDetailsList[secondaryAccountId] || {};
const secondaryDisplayName = lodashGet(secondaryUserDetails, 'displayName', '');
displayName = `${primaryDisplayName} & ${secondaryDisplayName}`;
secondaryAvatar = {
source: UserUtils.getAvatar(secondaryUserDetails.avatar, secondaryAccountId),
type: CONST.ICON_TYPE_AVATAR,
name: secondaryDisplayName,
id: secondaryAccountId,
};
} What alternative solutions did you explore? (Optional) |
@mallenexpensify Eep! 4 days overdue now. Issues have feelings too... |
Job added to Upwork: https://www.upwork.com/jobs/~01fc2d76b91c3cdcd1 |
Current assignee @mallenexpensify is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan ( |
Was able to reproduce, seems like it'd be External |
@situchan can you please review @hungvu193 's proposal above? #25819 (comment) |
Triggered auto assignment to @adelekennedy ( |
This comment was marked as off-topic.
This comment was marked as off-topic.
@adelekennedy I'm off this week, can you please keep 👀 on this then I'll snag it back on Monday? Thx |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
Regression Test Proposal
Do we agree 👍 or 👎 |
@mallenexpensify Checklist complete, and a friendly bump on the payment! |
@hungvu193, @mallenexpensify, @jjcoffee, @hayata-suenaga Whoops! This issue is 2 days overdue. Let's get this updated quick! |
melvin, we're waiting for payment |
Issue reporter: @Ahmed-Abdella paid $250 via Upwork Regression Test @jjcoffee can you confirm there wasn't a regression from the PR for this issue? I see a comment above but not anything in the PR #27912 |
Thanks @mallenexpensify! I wasn't able to repro the regression that @situchan commented here when retesting with just our PR, so it must have come from some other change. |
Please check the root cause in #28554 (comment) |
@hayata-suenaga Ohh my apologies, I thought I wasn't able to repro with just our PR, but actually I misunderstood this comment as meaning that the behaviour was happening for manual requests too, but apparently it was only for distance + scans. Agree now that there was indeed a regression. Sorry for the confusion! |
Ok, I'm updating the above for payment to account for the regression and removing the urgency bonus (per CONTRIBUTING.md )
I've requested refunds of $1k from @hungvu193 and @jjcoffee , will leave this issue open til I confirm the funds have been refunded. |
Will need a few days before my account gets topped back up enough to issue the refund. Sorry about this mess! |
Melvin, we're waiting for the refound |
@mallenexpensify Refunded! 🙇 |
Thanks @jjcoffee and @hungvu193, confirming the refunds and that the total payment was $500 each. |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
Expected Result:
Both Patticpants names and avatars should be displayed at the top of the Money request message.
Actual Result:
Duplicate particpant name and avatar ( User A name and avatar ) at the top of the Money request message
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.3.57-0
Reproducible in staging?: Y
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screencast.From.14-08-23.19.38.44.mp4
Recording.1312.mp4
Expensify/Expensify Issue URL:
Issue reported by: @Ahmed-Abdella
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692033869443169
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: