Skip to content
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 payment 2023-08-28] [$1000] IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view #24325

Closed
4 of 6 tasks
lanitochka17 opened this issue Aug 9, 2023 · 35 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 9, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Issue found when executing PR #23043

Action Performed:

  1. Go to staging.new.expensify.com
  2. Request money in a workspace chat
  3. Click on the IOU preview
  4. Click on the IOU again

Expected Result:

The workspace avatar shows the correct avatar

Actual Result:

The workspace avatar shows 'U' avatar before reverting to the actual avatar.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.52.1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6159056_20230809_224632.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01698449a42b0e216c
  • Upwork Job ID: 1689577432456220672
  • Last Price Increase: 2023-08-10
  • Automatic offers:
    • situchan | Reviewer | 26110136
    • StevenKKC | Contributor | 26110138
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

Triggered auto assignment to @flaviadefaria (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@StevenKKC
Copy link
Contributor

StevenKKC commented Aug 9, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view.

What is the root cause of that problem?

If we click IOU preview in the money request page, onIOUPreviewPressed is executed.
Since there is no child thread data in Onyx, app generates optimistic child thread data, and set the child thread's policyID to CONST.POLICY.OWNER_EMAIL_FAKE.
And then send OpenReport API to BE, and navigate to the child thread.

const thread = ReportUtils.buildOptimisticChatReport(
participantAccountIDs,
props.translate(ReportActionsUtils.isSentMoneyReportAction(props.action) ? 'iou.threadSentMoneyReportName' : 'iou.threadRequestReportName', {
formattedAmount: ReportActionsUtils.getFormattedAmount(props.action),
comment: props.action.originalMessage.comment,
}),
'',
CONST.POLICY.OWNER_EMAIL_FAKE,
CONST.POLICY.OWNER_ACCOUNT_ID_FAKE,
false,
'',
undefined,
undefined,
CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,
props.action.reportActionID,
props.requestReportID,
);
const userLogins = PersonalDetailsUtils.getLoginsByAccountIDs(thread.participantAccountIDs);
Report.openReport(thread.reportID, userLogins, thread, props.action.reportActionID);
Navigation.navigate(ROUTES.getReportRoute(thread.reportID));

Because the report's policyID is CONST.POLICY.OWNER_EMAIL_FAKE, ReportUtils.getPolicyName returns Unavailable Workspace, and then ReportUtils.getWorkspaceAvatar returns WorkspaceUavatar.
So 'U' is shown in money request header.

App/src/libs/ReportUtils.js

Lines 568 to 583 in 202f2eb

function getPolicyName(report, returnEmptyIfNotFound = false, policy = undefined) {
const noPolicyFound = returnEmptyIfNotFound ? '' : Localize.translateLocal('workspace.common.unavailable');
if (_.isEmpty(report)) {
return noPolicyFound;
}
if ((!allPolicies || _.size(allPolicies) === 0) && !report.policyName) {
return Localize.translateLocal('workspace.common.unavailable');
}
const finalPolicy = policy || _.get(allPolicies, `${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`);
// Public rooms send back the policy name with the reportSummary,
// since they can also be accessed by people who aren't in the workspace
return lodashGet(finalPolicy, 'name') || report.policyName || report.oldPolicyName || noPolicyFound;
}

App/src/libs/ReportUtils.js

Lines 859 to 862 in 202f2eb

function getWorkspaceAvatar(report) {
const workspaceName = getPolicyName(report, allPolicies);
return lodashGet(allPolicies, [`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`, 'avatar']) || getDefaultWorkspaceAvatar(workspaceName);
}

App/src/libs/ReportUtils.js

Lines 845 to 857 in 202f2eb

function getDefaultWorkspaceAvatar(workspaceName) {
if (!workspaceName) {
return defaultWorkspaceAvatars.WorkspaceBuilding;
}
// Remove all chars not A-Z or 0-9 including underscore
const alphaNumeric = workspaceName
.normalize('NFD')
.replace(/[^0-9a-z]/gi, '')
.toUpperCase();
return !alphaNumeric ? defaultWorkspaceAvatars.WorkspaceBuilding : defaultWorkspaceAvatars[`Workspace${alphaNumeric[0]}`];
}

After OpenReport API returns the thread report data, app showes the correct workspace avatar, since the thread has a corrrect policyID.

Therefore, workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view.

What changes do you think we should make in order to solve the problem?

We can set the optimistic child thread's policyID to correct value.
Because the props.iouReport has a correct policyID, we can set the optimistic thread's policyID to props.iouReport policyID instead of CONST.POLICY.OWNER_EMAIL_FAKE.

    lodashGet(props.iouReport, 'policyID', CONST.POLICY.OWNER_EMAIL_FAKE),

What alternative solutions did you explore? (Optional)

None.

@flaviadefaria flaviadefaria added the External Added to denote the issue can be worked on by a contributor label Aug 10, 2023
@melvin-bot melvin-bot bot changed the title IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view [$1000] IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view Aug 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 10, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01698449a42b0e216c

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 10, 2023

Current assignee @flaviadefaria is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Aug 10, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

@kaomoua
Copy link

kaomoua commented Aug 10, 2023

Hi. I can not see the proposal of @situchan. But how was he assigned to this issue?
I am new here and I really don't know how to contribute to Expensify.

1 similar comment
@kaomoua
Copy link

kaomoua commented Aug 10, 2023

Hi. I can not see the proposal of @situchan. But how was he assigned to this issue?
I am new here and I really don't know how to contribute to Expensify.

@pradeepmdk
Copy link
Contributor

@kaomoua he is a C+ member so he only going to review our proposal. you need to post the root cause and
solution here

@kaomoua
Copy link

kaomoua commented Aug 10, 2023

@pradeepmdk You mean this issue is still available?

@situchan
Copy link
Contributor

@StevenKKC thanks for your proposal. Please avoid code diff but explain in plain English

@situchan
Copy link
Contributor

@kaomoua as a new contributor, please review our contributing guidelines. As this GH has Help Wanted label, still open for proposals.

@kaomoua
Copy link

kaomoua commented Aug 10, 2023

#16108
Could you please check this issue? I posted my proposal and can not see other ones.
But this issues was assigned to other developer.
What happened?

@StevenKKC
Copy link
Contributor

@situchan Thanks for your review. I have updated my proposal.

@pradeepmdk
Copy link
Contributor

@kaomoua only the Help Wanted label we need to look that and check the guidelines contributing guidelines. like this
Screenshot 2023-08-10 at 6 26 06 PM

@situchan
Copy link
Contributor

@StevenKKC your root cause is correct. Does your solution work on paid IOU report as well?

@meedwire
Copy link

Contributor details
Your Expensify account email: l.jeronimo12345@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/leonardos24

The bug seems to be a relatively simple issue. It appears to be a remnant of a previous state and is being caused by a delay in state updating. The state should be cleared during the transition, or the state update should be optimized so that the transition isn't noticeable. There are several ways to accomplish this, such as handling it within the click action itself, forcing the dismounting and remounting of the screen, or creating a new instance of the screen. With my in-depth understanding of these processes and strong technical abilities, I am well-equipped to tackle this issue, ensuring a seamless user experience.

@melvin-bot
Copy link

melvin-bot bot commented Aug 10, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@StevenKKC
Copy link
Contributor

@situchan I have tested on paid IOU report and works as well.

@situchan
Copy link
Contributor

@StevenKKC's proposal looks good to me. Fixing optimistic data is the only solution.
🎀 👀 🎀 C+ reviewed

@melvin-bot
Copy link

melvin-bot bot commented Aug 14, 2023

Triggered auto assignment to @cristipaval, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 14, 2023
@StevenKKC
Copy link
Contributor

@situchan PR #24551 is ready for review.

@melvin-bot
Copy link

melvin-bot bot commented Aug 16, 2023

🎯 ⚡️ Woah @situchan / @StevenKKC, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

  • when @StevenKKC got assigned: 2023-08-14 11:00:21 Z
  • when the PR got merged: 2023-08-16 18:36:35 UTC

On to the next one 🚀

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 21, 2023
@melvin-bot melvin-bot bot changed the title [$1000] IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view [HOLD for payment 2023-08-28] [$1000] IOU - Workspace avatar shows 'U' avatar before reverting to actual avatar in IOU view Aug 21, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Aug 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 21, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Aug 21, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.55-8 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-08-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

For reference, here are some details about the assignees on this issue:

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 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:

  • [@situchan] The PR that introduced the bug has been identified. Link to the PR:
  • [@situchan] 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:
  • [@situchan] 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:
  • [@situchan] Determine if we should create a regression test for this bug.
  • [@situchan] 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.
  • [@flaviadefaria] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Aug 28, 2023
@StevenKKC
Copy link
Contributor

@flaviadefaria Could you please process payment?

@situchan
Copy link
Contributor

situchan commented Aug 30, 2023

This is minor UI bug and offline test is already in PR checklist. So no regression test is needed

@flaviadefaria
Copy link
Contributor

Thanks @situchan
@StevenKKC I was OoO, will process payment now.

@flaviadefaria
Copy link
Contributor

Payment is as follow:

  • No reporting bonus (bug reported by Applause)
  • @StevenKKC = $1000 + $500 bonus = $1500 (UW)
  • @situchan = $1000 + $500 bonus = $1500 (should request in ND)

@flaviadefaria
Copy link
Contributor

@StevenKKC has been paid. I'll switch this to weekly and close when @situchan has been paid.

@flaviadefaria flaviadefaria added Weekly KSv2 and removed Daily KSv2 labels Aug 30, 2023
@situchan
Copy link
Contributor

@flaviadefaria I am still using upwork. Already accepted offer made by Melvin - #24325 (comment)

@flaviadefaria flaviadefaria added Daily KSv2 and removed Weekly KSv2 labels Aug 30, 2023
@flaviadefaria
Copy link
Contributor

@situchan I just paid you in UW. Closing this as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

8 participants