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

[$500] mWeb/Chrome - Distance-In offline, distance request details page loading infinitely. #35087

Closed
1 of 6 tasks
lanitochka17 opened this issue Jan 24, 2024 · 39 comments
Closed
1 of 6 tasks
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

@lanitochka17
Copy link

lanitochka17 commented Jan 24, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 1.4.31-2
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #34604

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Go offline
  3. Tap on Workspace chat which has distance request created
  4. Tap on distance request to open distance request details page

Expected Result:

In offline, distance request details page must be displayed without loading

Actual Result:

In offline, distance request details page loading infinitely

Workaround:

Unknown

Workaround:

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6353506_1706118240428.az_recorder_20240124_214856.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ed39265b86a00161
  • Upwork Job ID: 1750223436772130816
  • Last Price Increase: 2024-02-07
@lanitochka17 lanitochka17 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 Jan 24, 2024
@melvin-bot melvin-bot bot changed the title mWeb/Chrome - Distance-In offline, distance request details page loading infinitely. [$500] mWeb/Chrome - Distance-In offline, distance request details page loading infinitely. Jan 24, 2024
Copy link

melvin-bot bot commented Jan 24, 2024

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

Copy link

melvin-bot bot commented Jan 24, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 24, 2024
Copy link

melvin-bot bot commented Jan 24, 2024

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

@rojiphil
Copy link
Contributor

rojiphil commented Jan 25, 2024

Bringing the proposal from here

Proposal

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

In offline, the distance request details page must be displayed without loading

What is the root cause of that problem?

As there is a purposeful reduction in data sent from BE in OpenApp, the report actions for a displayed report in LHN need not be present.
This would bring about interesting scenarios in offline cases.

  1. If the user is offline at this moment, the report screen will keep showing the loading status for the chat report. But, when online, the chat report actions would be fetched via openReport if the user clicks on the report in LHN.
  2. Now, if the user goes offline after fetching the chat report, a similar loading status will be shown for the IOU report when clicked.
  3. The same situation will repeat for the transaction report as well.

When offline, is showing a loading status in each of the above three cases intentional?

If not intentional, the proposal below can be considered.

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

To solve the problem, we can ensure that the displayed reports in LHN are fetched along with their linked reports up to a single level depth.

More specifically, we can leverage optionListItems here and ensure that all these reports are fetched once at app launch. Additionally, we can look for childReportID in Report Actions of these reports which is a good indicator of linked reports, and fetch them. This will solve points (2) and (3) mentioned in the root cause. For solving (1), we need to discuss how to arrive at this as BE support may be required. However, when minimalistic report action is sent on openApp, we can additionally look for linkedReportID.

We can implement a side effect here which looks something like this to solve the problem:

const fetchedReportIDsRef = useRef([]);
useEffect(() => {
    const arrayChildReportIDs = [];
    optionListItems.forEach((reportID) => {
        const reportActions = allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
        const childReportIDs = _.filter(_.pluck(reportActions, 'childReportID'),(childReportID) => childReportID);
        if(childReportIDs.length > 0) {
            arrayChildReportIDs.push(childReportIDs);
        } 
    });
    const allRecentReportIDs = _.union(optionListItems, ...arrayChildReportIDs);
    if(currentReportID) {
        allRecentReportIDs.push(currentReportID);
    }

    const unfetchedReportIDs = _.difference(allRecentReportIDs, fetchedReportIDsRef.current);
    unfetchedReportIDs.forEach((reportID) => {
        if(!reportID) {
            return;
        }
        Report.reconnect(reportID);
        fetchedReportIDsRef.current.push(reportID);
    });        
}, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode, allReportActions]);  

Additionally, we also need to include childReportID in reportActionsSelector here

What alternative solutions did you explore? (Optional)

@rojiphil
Copy link
Contributor

As there is a purposeful reduction in data sent from BE in OpenApp, the report actions for a displayed report in LHN need not be present. This would bring about interesting scenarios in offline cases.

  1. If the user is offline at this moment, the report screen will keep showing the loading status for the chat report. But, when online, the chat report actions would be fetched via openReport if the user clicks on the report in LHN.
  2. Now, if the user goes offline after fetching the chat report, a similar loading status will be shown for the IOU report when clicked.
  3. The same situation will repeat for the transaction report as well.

When offline, is showing a loading status in each of the above three cases intentional?

@sophiepintoraetz @parasharrajat
Can you please help clarify the above query as mentioned in my proposal?
Also, what would be the expected behaviour here?

@parasharrajat
Copy link
Member

I will take a look. Currently away from laptop.

@melvin-bot melvin-bot bot added the Overdue label Jan 29, 2024
@sophiepintoraetz
Copy link
Contributor

Still waiting on @parasharrajat

@melvin-bot melvin-bot bot removed the Overdue label Jan 30, 2024
Copy link

melvin-bot bot commented Jan 31, 2024

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

@parasharrajat
Copy link
Member

I will checking this today,

@strepanier03 strepanier03 added Bug Something is broken. Auto assigns a BugZero manager. and removed Bug Something is broken. Auto assigns a BugZero manager. labels Feb 1, 2024
Copy link

melvin-bot bot commented Feb 1, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
Copy link

melvin-bot bot commented Feb 5, 2024

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

@johncschuster
Copy link
Contributor

@parasharrajat, did you get a chance to look at the above proposal?

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
Copy link

melvin-bot bot commented Feb 7, 2024

@johncschuster @parasharrajat this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Feb 7, 2024

📣 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 Feb 8, 2024
@johncschuster
Copy link
Contributor

This issue has the retest label applied. Coming from this process, it looks like Applause will re-test by Friday and comment with their results on Monday.

@melvin-bot melvin-bot bot removed the Overdue label Feb 26, 2024
@johncschuster
Copy link
Contributor

That said, this should have been tested this previous Friday and the results should be posted today-ish. @Expensify/applause, did this issue get retested in the last batch? If not, can you please add it? Thank you!

@mvtglobally
Copy link

@johncschuster Updating results right now. Give me a couple minutes. Working on it

@mvtglobally
Copy link

Issue is reproducible during KI retests.

@mvtglobally mvtglobally removed the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Feb 27, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 29, 2024
@neil-marcellini
Copy link
Contributor

I took a look and I was able to reproduce the bug as described by the test steps. Are we sure that it's specific to distance requests? Does it happen for manual or scan requests with the same steps?

Please assign me once it's time to review a proposal.

Copy link

melvin-bot bot commented Mar 1, 2024

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

@johncschuster
Copy link
Contributor

@Expensify/applause

Are we sure that it's specific to distance requests? Does it happen for manual or scan requests with the same steps?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Mar 1, 2024
Copy link

melvin-bot bot commented Mar 5, 2024

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

@johncschuster
Copy link
Contributor

@lanitochka17 / @Expensify/applause

Are we sure that it's specific to distance requests? Does it happen for manual or scan requests with the same steps?

Can you please confirm this?

Copy link

melvin-bot bot commented Mar 11, 2024

@mananjadhav, @johncschuster, @lanitochka17 Huh... This is 4 days overdue. Who can take care of this?

@johncschuster
Copy link
Contributor

@lanitochka17 / @applausebot bump on this!

Are we sure that it's specific to distance requests? Does it happen for manual or scan requests with the same steps?

Can you please confirm?

@melvin-bot melvin-bot bot removed the Overdue label Mar 11, 2024
@johncschuster johncschuster reopened this Mar 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 14, 2024
@Expensify Expensify deleted a comment from melvin-bot bot Mar 14, 2024
@johncschuster
Copy link
Contributor

Bumped in the QA channel

@melvin-bot melvin-bot bot removed the Overdue label Mar 14, 2024
@kavimuru
Copy link

@johncschuster not able to reproduce this bug now.

six.mp4
dt.mp4

@johncschuster
Copy link
Contributor

Great! In that case, I'm going to go ahead and close it. Thanks, @kavimuru!

Copy link

melvin-bot bot commented Mar 15, 2024

@johncschuster Be sure to fill out the Contact List!

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

10 participants