-
Notifications
You must be signed in to change notification settings - Fork 3k
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 2024-05-30] HIGH: Fix extraneous ReconnectApp
on refresh
#40995
Comments
ReconnectApp
and OpenReport
on refresh
Hey Edu here from Callstack |
Updates on the ProblemsThere are 2 cases that I’m trying to prevent:
Solution
At least for now seems to be working, I'm testing and soon I can make a formal proposal or a PR if you want |
I don't think that's the right solution -- we don't want to suppress multiple calls, we want to figure out why the code is even attempting to make multiple calls, and stop it. |
ReconnectApp
and OpenReport
on refreshReconnectApp
and OpenReport
on refresh
actually, is this a dupe of: #39673? Can we close this? |
Actually, the |
ReconnectApp
and OpenReport
on refreshReconnectApp
on refresh
@quinthar ok, The "suppress" is mainly because we create multiple ReportScreen, and even when you switch a report I saw |
@quinthar ProblemOn the other hand, the app register to the Network status, when it is registered it receives updates
Given that SolutionGiven that the Lib for web is still experimental and in the description for isInternetReachable if it is unknown is null. Also checking the code for web they send basic information until they collect the real
I suggest to remove the |
I'm temporarily out of the C+ team as I'm about to go OOO, so the auto-assigning isn't working here. Tagging @tgolen on the PR to give it a quick review, otherwise once a BZ is assigned maybe they can find someone? |
Job added to Upwork: https://www.upwork.com/jobs/~01008cff119674ebe1 |
Triggered auto assignment to Contributor Plus for review of internal employee PR - @s77rt ( |
Triggered auto assignment to @dylanexpensify ( |
ReconnectApp
on refreshReconnectApp
on refresh
Hiya! @mallenexpensify mind confirming what's needed here? |
@dylanexpensify The PR hasn't hit staging yet, but eventually payment will be due for C+ review. |
What's the next step here? Who's doing it? What is the ETA? |
So we just need to pay this out? |
Yep this was only deployed to staging so far though |
Still waiting then! |
Hit prod on 23rd, tomorrow will pay out! |
ReconnectApp
on refreshReconnectApp
on refresh
Payment Summary
BugZero Checklist (@dylanexpensify)
|
@dylanexpensify Friendly bump for payment 🙇 |
Sorry, on it now! Thanks @jjcoffee! |
Invited to apply! |
@dylanexpensify Thank you, I've applied! 🙏 |
@dylanexpensify Friendly bump 🙇 |
@jjcoffee sent offer, sorry! |
@dylanexpensify No worries! I've accepted the offer now. |
done! |
Being discussed here: https://expensify.slack.com/archives/C049HHMV9SM/p1714059210537139
Often occurs in conjunction with HIGH: [Reliability] [$500] Multiple calls to GetNewerActions on report open and HIGH: [API Reliability] [$250] Multiple calls to OpenReport on signin, but seems to be a different root cause.
Actions Performed:
OpenApp
ReconnectApp
andOpenReport
Expected Result:
There should be only one
ReconnectApp
OpenReport
OpenApp
callsActual Result:
Multiple calls for
ReconnectApp
OpenReport
Platforms:
Mac OS: Chrome/Safari
Screenshots/Videos:
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @dylanexpensifyThe text was updated successfully, but these errors were encountered: