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 on #24041] [$1000] User B's LHN is not updated with money request message sent by User A to User B #23694

Closed
1 of 6 tasks
kavimuru opened this issue Jul 26, 2023 · 37 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented Jul 26, 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!


Action Performed:

  1. Login as User A on one device.
  2. Login as User B on another device.
  3. With User A, request money from User B (click on FAB > Request money).

Expected Result:

User B's LHN should be updated with the message 'You owe $xxx'.

Actual Result:

Often, the LHN does not update and continues to display the previous message.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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:
Reproducible in staging?: needs reproduction
Reproducible in production?: needsreproduction
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

new.issue.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ygshbht
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690024821911749

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e5606632d74ace94
  • Upwork Job ID: 1684937748192702464
  • Last Price Increase: 2023-07-28
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 26, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 26, 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

@kavimuru kavimuru added the Needs Reproduction Reproducible steps needed label Jul 26, 2023
@kavimuru
Copy link
Author

Proposal from contributor

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

There is an inconsistency with the preview message update in the Left Hand Navigation (LHN) when money is requested between users.

What is the root cause of that problem?

The LHN preview message is determined based on reportActions. However, the display message only recalculates when the report or some other data changes, not the reportActions. Thus, if the server sends a report update first and reportActions update later, the display message does not update.

const optionItemRef = useRef();
const optionItem = useMemo(() => {
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData(fullReport, personalDetails, preferredLocale, policy);
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
}
optionItemRef.current = item;
return item;
}, [fullReport, personalDetails, preferredLocale, policy]);

*/
function getLastMessageTextForReport(report) {
const lastReportAction = lastReportActions[report.reportID];
let lastMessageTextFromReport = '';

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

To resolve this, we need to recalculate the display message in LHN when reportActions changes as well. This can be accomplished by including reportActions as a dependency in the useMemo hook, like so:

const optionItem = useMemo(() => {
    // Note: ideally we'd have this as a dependent selector in Onyx!
    const item = SidebarUtils.getOptionData(fullReport, personalDetails, preferredLocale, policy);
    if (deepEqual(item, optionItemRef.current)) {
        console.log('Deep equal');
        return optionItemRef.current;
    }
    optionItemRef.current = item;
    return item;
}, [fullReport, personalDetails, preferredLocale, policy, reportActions]);

This adjustment ensures that whenever a change in reportActions occurs, the display message will be recalculated, resulting in an accurate preview message in the LHN.

What alternative solutions did you explore? (Optional)

An alternate solution could be to pass reportActions as argument to the getOptionData function and any nested functions that require the lastReportAction. This way, these functions can directly access the latest reportActions, ensuring that they always operate on the most recent data. This change could help maintain accurate and updated preview messages in the Left Hand Navigation (LHN) section.

@ygshbht
Copy link
Contributor

ygshbht commented Jul 27, 2023

Thanks kavimuru.

Here's another video demonstrating the issue. If doesn't happen always but roughly a third of the times

copy_of_2023-07-25.18-45-30.mp4

@ygshbht
Copy link
Contributor

ygshbht commented Jul 27, 2023

Here's the solution video.

2023-07-22.17-00-27.mp4

@twisterdotcom
Copy link
Contributor

We have some internal API updates going on right now. I'm asking if they might solve this: https://expensify.slack.com/archives/C03SDMF9YJ2/p1690475932707729

@ygshbht
Copy link
Contributor

ygshbht commented Jul 27, 2023

Do contributors have access to the link https://expensify.slack.com/archives/C03SDMF9YJ2/p1690475932707729?
If you ask me, I don't think it's the backend issue here. Backend sends all the messages - if reportActions reaches client first, there is no issue, however if it reaches post the report message, you don't see the message in LHN as it depends on the reportActions and calculation to show message preview on LHN only happens when report changes. That being said, i'm really curious to see if backend tweaking is really the solution here

@twisterdotcom
Copy link
Contributor

They do not. Okay, we should open this up!

@twisterdotcom twisterdotcom added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Jul 28, 2023
@melvin-bot melvin-bot bot changed the title User B's LHN is not updated with money request message sent by User A to User B [$1000] User B's LHN is not updated with money request message sent by User A to User B Jul 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

Job added to Upwork: https://www.upwork.com/jobs/~01e5606632d74ace94

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

melvin-bot bot commented Jul 28, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Jul 28, 2023

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

@twisterdotcom
Copy link
Contributor

@parasharrajat could we just assign @ygshbht straight away on this one?

@s-alves10
Copy link
Contributor

#23189 (comment)

@s-alves10
Copy link
Contributor

@hannojg

Please share your idea here

@parasharrajat
Copy link
Member

Reviewing...

@melvin-bot melvin-bot bot added the Overdue label Jul 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

@twisterdotcom, @parasharrajat Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@parasharrajat
Copy link
Member

Aha! There is a proposal here. Didn't notice.

@melvin-bot melvin-bot bot removed the Overdue label Jul 31, 2023
@parasharrajat
Copy link
Member

@ygshbht Is this only happening when the last message is IOU action before the request message is received in the DM?

@ygshbht
Copy link
Contributor

ygshbht commented Jul 31, 2023

From what i see currently, yes, issue is only for IOU. This is probably related to translation of messages. Normal messages don't need translation whereas IOU type message with predefined text, do translate. So in these other messages, reportAction is not needed, they get text directly from report

@twisterdotcom twisterdotcom added Weekly KSv2 and removed Daily KSv2 labels Aug 4, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 14, 2023
@twisterdotcom
Copy link
Contributor

Still HELD

@melvin-bot melvin-bot bot removed the Overdue label Aug 14, 2023
@twisterdotcom
Copy link
Contributor

Apologies but I am finally going on Parental Leave. I am reassigning to another member of the team.

@twisterdotcom twisterdotcom added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Aug 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 18, 2023

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 18, 2023
@Expensify Expensify deleted a comment from melvin-bot bot Aug 18, 2023
@twisterdotcom twisterdotcom added Weekly KSv2 and removed Daily KSv2 labels Aug 18, 2023
@hannojg
Copy link
Contributor

hannojg commented Aug 21, 2023

The PR got merged so we are good for testing this again! 😊

@melvin-bot melvin-bot bot added the Overdue label Aug 28, 2023
@flaviadefaria
Copy link
Contributor

@parasharrajat are you able to test this and confirm next steps? Thanks!

@melvin-bot melvin-bot bot removed the Overdue label Aug 30, 2023
@flaviadefaria flaviadefaria added Daily KSv2 and removed Weekly KSv2 labels Aug 30, 2023
@melvin-bot melvin-bot bot added the Overdue label Sep 1, 2023
@flaviadefaria
Copy link
Contributor

I'll try and test this next week.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 1, 2023
@flaviadefaria
Copy link
Contributor

I'm struggling to test this on two separate devices. @kavimuru are you able to please re-test this? Or is anyone else able to help re-test this?

@melvin-bot melvin-bot bot removed the Overdue label Sep 5, 2023
@ygshbht
Copy link
Contributor

ygshbht commented Sep 5, 2023

@flaviadefaria I don't think this is reproducible now as the proposed solution has already been implemented by some other unknown PR :(

@flaviadefaria
Copy link
Contributor

Thanks for confirming @ygshbht so I'll close this out now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

7 participants