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

[$1000] Web - Sending message in two open chat page in different tabs is not reflected on the other tabs. #27034

Closed
6 tasks
kbecciv opened this issue Sep 8, 2023 · 40 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Sep 8, 2023

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:

  1. Open the website
  2. Open any chat from LHN
  3. Hover the message and click the copy link
  4. Paste the link in the new tab
  5. Send a message
  6. Observe the result from the other tab

Expected Result:

The message should be reflected in the tabs

Actual Result:

The message is not reflected in the tabs

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.66.3
Reproducible in staging?: y
Reproducible in production?: y
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

Screenshare.-.2023-09-04.3_46_39.PM.mp4
Recording.4337.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c1fd00c75672684e
  • Upwork Job ID: 1700171246406656000
  • Last Price Increase: 2023-10-04
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 8, 2023
@melvin-bot melvin-bot bot changed the title Web - Sending message in two open chat page in different tabs is not reflected on the other tabs. [$500] Web - Sending message in two open chat page in different tabs is not reflected on the other tabs. Sep 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

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

melvin-bot bot commented Sep 8, 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

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

Triggered auto assignment to @zanyrenney (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@khanumar03
Copy link

/assign

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

📣 @khanumar03! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@garrettmknight
Copy link
Contributor

Wow, Melv. Really going nuts on this one. Bug is open and we're waiting on proposals.

@rakshitjain13
Copy link
Contributor

rakshitjain13 commented Sep 12, 2023

Proposal

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

The message is not reflected in the tabs

What is the root cause of that problem?

In SequentialQueue.js in this file, the flush method checks that only the leader in tabs of a single browser can make requests.

// ONYXKEYS.PERSISTED_REQUESTS is shared across clients, thus every client/tab will have a copy
// It is very important to only process the queue from leader client otherwise requests will be duplicated.
if (!ActiveClientManager.isClientTheLeader()) {
return;
}

It is clearly return in src/libs/ActiveClientManager/index.js that

/**
* When you have many tabs in one browser, the data of Onyx is shared between all of them. Since we persist write requests in Onyx, we need to ensure that
* only one tab is processing those saved requests or we would be duplicating data (or creating errors).
* This file ensures exactly that by tracking all the clientIDs connected, storing the most recent one last and it considers that last clientID the "leader".
*/

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

So now if a tab which is not a leader makes a request we will do the same thing. i.e. Ensure persistedRequests are read from storage which is shown below.

process().finally(() => {
isSequentialQueueRunning = false;
resolveIsReadyPromise();
currentRequest = null;
flushOnyxUpdatesQueue();
});

Something like this :

 if (!ActiveClientManager.isClientTheLeader()) { 
      process().finally(() => { 
         isSequentialQueueRunning = false; 
         resolveIsReadyPromise(); 
         currentRequest = null; 
         flushOnyxUpdatesQueue(); 
     }); 
     return; 
 } 

What alternative solutions did you explore? (Optional)

NA

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2023
@garrettmknight
Copy link
Contributor

@Ollyws when you get a chance, what do you think of that proposal?

@melvin-bot melvin-bot bot removed the Overdue label Sep 13, 2023
@rakshitjain13
Copy link
Contributor

@Ollyws @garrettmknight Any thoughts about the above proposal

@melvin-bot
Copy link

melvin-bot bot commented Sep 15, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Sep 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 18, 2023

@garrettmknight, @Ollyws Huh... This is 4 days overdue. Who can take care of this?

@melvin-bot melvin-bot bot removed the Overdue label Sep 19, 2023
@Ollyws
Copy link
Contributor

Ollyws commented Sep 19, 2023

@rakshitjain13 Is this still reproducible for you? I'm struggling to reproduce it on the latest main.

@misgana96
Copy link

@Ollyws I can reproduce it in the latest main
Screencast from 19-09-2023 7:20:04 ከሰዓት.webm

@Ollyws
Copy link
Contributor

Ollyws commented Sep 26, 2023

@rakshitjain13 If you go offline with multiple tabs open, send a message and then go back online you should see the request duplicated on each one.
Either way I don't think it's a good idea to be removing it or allowing the queue to be flushed from tabs that aren't the leader.

@Ollyws
Copy link
Contributor

Ollyws commented Sep 26, 2023

Awaiting proposals....

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

@garrettmknight @Ollyws this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

@melvin-bot
Copy link

melvin-bot bot commented Sep 29, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot
Copy link

melvin-bot bot commented Oct 2, 2023

@garrettmknight, @Ollyws Huh... This is 4 days overdue. Who can take care of this?

@Ollyws
Copy link
Contributor

Ollyws commented Oct 2, 2023

Still awaiting proposals....

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 2, 2023
@garrettmknight garrettmknight changed the title [$500] Web - Sending message in two open chat page in different tabs is not reflected on the other tabs. [$1000] Web - Sending message in two open chat page in different tabs is not reflected on the other tabs. Oct 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 4, 2023

Upwork job price has been updated to $1000

@garrettmknight
Copy link
Contributor

Upping the price to see if we can give any other proposals.

@melvin-bot melvin-bot bot removed the Overdue label Oct 4, 2023
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

@garrettmknight @Ollyws this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

  • Decide whether any proposals currently meet our guidelines and can be approved as-is today
  • If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@melvin-bot melvin-bot bot added Internal Requires API changes or must be handled by Expensify staff and removed 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 labels Oct 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

Current assignee @Ollyws is eligible for the Internal assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added the Overdue label Oct 9, 2023
@garrettmknight
Copy link
Contributor

@Ollyws can you double check this is still happening in dev before we find an internal assignee?

@melvin-bot melvin-bot bot removed the Overdue label Oct 9, 2023
@Ollyws
Copy link
Contributor

Ollyws commented Oct 9, 2023

@garrettmknight I can't reproduce it, and seems like neither can the QA team.

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Third week) Lets close it?

@melvin-bot melvin-bot bot added the Overdue label Oct 12, 2023
@garrettmknight
Copy link
Contributor

Awesome, closing!

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 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

8 participants