App
App copied to clipboard
[$500] mWeb/Chrome - Distance-In offline, distance request details page loading infinitely.
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 https://github.com/Expensify/App/pull/34604
Action Performed:
- Go to https://staging.new.expensify.com/
- Go offline
- Tap on Workspace chat which has distance request created
- 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
- [x] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/Expensify/App/assets/78819774/878bd4aa-e457-4bee-a6f2-43e1b429fe79
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01ed39265b86a00161
- Upwork Job ID: 1750223436772130816
- Last Price Increase: 2024-01-31
Triggered auto assignment to @sophiepintoraetz (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Job added to Upwork: https://www.upwork.com/jobs/~01ed39265b86a00161
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External
)
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.
- 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 viaopenReport
if the user clicks on the report in LHN. - Now, if the user goes
offline
after fetching the chat report, a similar loading status will be shown for the IOU report when clicked. - 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)
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.
- 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 viaopenReport
if the user clicks on the report in LHN.- Now, if the user goes
offline
after fetching the chat report, a similar loading status will be shown for the IOU report when clicked.- 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?
I will take a look. Currently away from laptop.
Still waiting on @parasharrajat
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
I will checking this today,
Triggered auto assignment to @johncschuster (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
@johncschuster, @parasharrajat Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@parasharrajat, did you get a chance to look at the above proposal?
@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!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@mananjadhav is taking over C+ review while Rajat is away
Just took over going through the issue history, and the linked issue discussion where the proposal was originally posted.
@mananjadhav, @johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
@lanitochka17 @rojiphil Can you confirm if this is reproducible in all platforms? I can see only Android mWeb marked? If it's specific platform, then I am wondering how can backend have impact on one platform only?
@mananjadhav @johncschuster 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!
Issue is not reproducible on the latest build 1.4.41-4
https://github.com/Expensify/App/assets/78819774/aacb03b8-1913-457d-b4df-f6f417e2e7c7
I've applied the retest-weekly label. If we find this behavior is no longer present on the next retest, I think we can close it.
@mananjadhav @johncschuster 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!
Current assignee @mananjadhav is eligible for the Internal assigner, not assigning anyone new.
@mananjadhav, @johncschuster Eep! 4 days overdue now. Issues have feelings too...
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.
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!
@johncschuster Updating results right now. Give me a couple minutes. Working on it
Issue is reproducible during KI retests.
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.
@mananjadhav, @johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!