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

[Awaiting Payment 29th May] [$500] Wrong message appears when the Expensify site is going down #37565

Closed
6 tasks done
m-natarajan opened this issue Feb 29, 2024 · 74 comments
Closed
6 tasks done
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Waiting for copy User facing verbiage needs polishing

Comments

@m-natarajan
Copy link

m-natarajan commented Feb 29, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number:
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: @flodnv
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1709063517335079

Action Performed:

  1. Open any chat or go to sign in page when the site is down

Expected Result:

When the site is down we should show: We might have a problem. Check out status.expensify.com.

Actual Result:

You appear to be offline appears confusing the user that they have a network problem when really it's the site.

Workaround:

unknown

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

image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ed912188740cf497
  • Upwork Job ID: 1764575263440932864
  • Last Price Increase: 2024-04-24
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 29, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

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

@flodnv flodnv added the Waiting for copy User facing verbiage needs polishing label Feb 29, 2024
Copy link

melvin-bot bot commented Feb 29, 2024

Triggered auto assignment to @NickTooker (Waiting for copy), see https://stackoverflow.com/c/expensify/questions/7025/ for more details.

@godofoutcasts94
Copy link
Contributor

godofoutcasts94 commented Feb 29, 2024

Proposal

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

Wrong message appears when the Expensify site is going down

What is the root cause of that problem?

New Feature but as discussed in the comment this is what we need to do.

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

Firstly, we need to fix the logical error of using the backend endpoint for determining isOffline :

  1. We will remove the backend ping logic from here :

reachabilityUrl: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api?command=Ping`,
reachabilityMethod: 'GET',
reachabilityTest: (response) => {
if (!response.ok) {
return Promise.resolve(false);
}
return response
.json()
.then((json) => Promise.resolve(json.jsonCode === 200))
.catch(() => Promise.resolve(false));
},
// If a check is taking longer than this time we're considered offline
reachabilityRequestTimeout: CONST.NETWORK.MAX_PENDING_TIME_MS,

and after removing the backend ping logic.
2. We need to add a new method that seperates the backend ping logic seperately like this :

async function testBackendReachability() {
  try {
    const response = await fetch(`${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api?command=Ping`);
    return response.ok; 
  } catch {
    return false;
  }
}

So, that we have a seperate logic to ping backend seperately now.

  1. Now, since we have seperated backend pinging logic seperately. Here :
    let isOffline = false;
    let hasPendingNetworkCheck = false;

    we can add a new boolean Onyx property called isBackendUnavailable and initially its value will be false. So that we don't directly say that backend is unavailable.
let isBackendUnavailable = false;
  1. Then, we need to update the setOfflineStatus function here :
    function setOfflineStatus(isCurrentlyOffline: boolean): void {
    NetworkActions.setIsOffline(isCurrentlyOffline);
    // When reconnecting, ie, going from offline to online, all the reconnection callbacks
    // are triggered (this is usually Actions that need to re-download data from the server)
    if (isOffline && !isCurrentlyOffline) {
    triggerReconnectionCallbacks('offline status changed');
    }
    isOffline = isCurrentlyOffline;
    }

to also set isBackendUnavailable in Onyx like this :

function setOfflineStatus(isCurrentlyOffline: boolean) {

  // Existing logic to set isOffline
  
  const isBackendUp = await testBackendReachability();
  isBackendUnavailable = !isBackendUp;

  // Dispatch new action to set isBackendUnavailable in Onyx
}
  1. In NetInfo.addEventListener i.e. here :
    NetInfo.addEventListener((state) => {
    Log.info('[NetworkConnection] NetInfo state change', false, {...state});
    if (shouldForceOffline) {
    Log.info('[NetworkConnection] Not setting offline status because shouldForceOffline = true');
    return;
    }
    setOfflineStatus(state.isInternetReachable === false);
    });
    }

we need to call testBackendReachability() on network changes like this :

NetInfo.addEventListener(state => {

  await setOfflineStatus(state.isInternetReachable === false);

})

This will help us to separates the offline and backend unavailable logic, removing the dependency on the backend endpoint for determining offline status.

Also, at last in en.ts file we can add a new key and value for saying this We might have a problem. Check out status.expensify.com when the Expensify App is literally down.

What alternative solutions did you explore? (Optional)

N/A

@NickTooker
Copy link

How about:

We might have a problem. Check out status.expensify.com.

@neonbhai
Copy link
Contributor

neonbhai commented Mar 1, 2024

Proposal

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

Wrong message appears when the Expensify site is going down

What is the root cause of that problem?

We need to update the Copy in Offline Indicator here:

<Text style={[styles.ml3, styles.chatItemComposeSecondaryRowSubText]}>{translate('common.youAppearToBeOffline')}</Text>

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

We will be creating a new key in src/languages according to the approved copy:

offlineMessage: "We might have a problem. Check out "

Then use it here:

<Text style={[styles.ml3, styles.chatItemComposeSecondaryRowSubText]}>{translate('common.youAppearToBeOffline')}</Text>

We will be rendering the link in the approved copy with the TextLink component:

<TextLink onPress={() => Link.openExternalLink('https://status.expensify.com/')}>
  status.expensify.com
</TextLink>

@NickTooker
Copy link

This is the approved copy @neonbhai.

We might have a problem. Check out status.expensify.com.

@godofoutcasts94

This comment was marked as off-topic.

@melvin-bot melvin-bot bot added the Overdue label Mar 4, 2024
@trjExpensify
Copy link
Contributor

Sounds like we have the copy, adding External to get a C+ assigned.

@melvin-bot melvin-bot bot removed the Overdue label Mar 4, 2024
@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Mar 4, 2024
@melvin-bot melvin-bot bot changed the title Wrong message appears when the Expensify site is going down [$500] Wrong message appears when the Expensify site is going down Mar 4, 2024
Copy link

melvin-bot bot commented Mar 4, 2024

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

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

melvin-bot bot commented Mar 4, 2024

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

@trjExpensify trjExpensify added NewFeature Something to build that is a new item. and removed Help Wanted Apply this label when an issue is open to proposals by contributors Bug Something is broken. Auto assigns a BugZero manager. labels Mar 4, 2024
Copy link

melvin-bot bot commented Mar 4, 2024

Current assignee @trjExpensify is eligible for the NewFeature assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Mar 4, 2024
@trjExpensify trjExpensify added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 4, 2024
@trjExpensify
Copy link
Contributor

We haven't implemented this feature previously, so adding New feature label. I've also updated the OP with the expected results copy.

@godofoutcasts94
Copy link
Contributor

Updated Proposal @trjExpensify

Copy link

melvin-bot bot commented May 1, 2024

@trjExpensify, @aldo-expensify, @tienifr, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick!

@trjExpensify
Copy link
Contributor

I've assigned you to the PR @DylanDylann. @tienifr there are conflicts here.

Copy link

melvin-bot bot commented May 9, 2024

@trjExpensify, @aldo-expensify, @tienifr, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented May 13, 2024

@trjExpensify, @aldo-expensify, @tienifr, @DylanDylann 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Copy link

melvin-bot bot commented May 15, 2024

@trjExpensify, @aldo-expensify, @tienifr, @DylanDylann Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@trjExpensify
Copy link
Contributor

Hit staging yesterday melv, chill.

@flodnv
Copy link
Contributor

flodnv commented May 21, 2024

I did notice the change working last week 😅 👍 🙇

@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

Copy link

melvin-bot bot commented May 24, 2024

@trjExpensify, @aldo-expensify, @tienifr, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick!

@trjExpensify trjExpensify changed the title [$500] Wrong message appears when the Expensify site is going down [Awaiting Payment 29th May] [$500] Wrong message appears when the Expensify site is going down May 24, 2024
@trjExpensify trjExpensify added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review Help Wanted Apply this label when an issue is open to proposals by contributors labels May 24, 2024
@trjExpensify
Copy link
Contributor

Not overdue, Melv. Awaiting payment on the 29th.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels May 24, 2024
@trjExpensify
Copy link
Contributor

Payment summary for tomorrow as follows:

@melvin-bot melvin-bot bot removed the Overdue label May 28, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@trjExpensify
Copy link
Contributor

Offers sent!

@tienifr
Copy link
Contributor

tienifr commented May 30, 2024

Thanks @trjExpensify I accepted the offer!

@trjExpensify
Copy link
Contributor

Paid, @tienifr! @DylanDylann, I think you need to accept? 🤔

@DylanDylann
Copy link
Contributor

@trjExpensify Accepted! Thanks

@melvin-bot melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels May 31, 2024
@trjExpensify
Copy link
Contributor

Paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item. Waiting for copy User facing verbiage needs polishing
Projects
None yet
Development

No branches or pull requests