hippware / rn-chat

MIT License
5 stars 0 forks source link

Migrate users from 3.9.3 to 4.x.x without forced relogin #3266

Closed southerneer closed 5 years ago

southerneer commented 5 years ago

~Now that we're a few months into the 2.x.x release we can safely remove tryMigrate in PersistableModel.ts~

Modify tryMigrate for v4

bengtan commented 5 years ago

Related, but maybe should be a separate topic:

We might want to also store the app version into the model.

Then, when we do upgrades, we can also apply logic to 'migrate' schema-from-an-old-version to schema-of-a-new-version. Sort-of like how database migrations work.

But this requires a chunk of infrastructure at the app data/model level, so ... maybe an idea for another time?

southerneer commented 5 years ago

We might want to also store the app version into the model.

We already do this. We check the new version against the stored version. If they don't match we load minimal (clear the cache).

aksonov commented 5 years ago

@southerneer I'm glad to remove it, but what about old users who uses old xmpp-based app? Why we should ask them to login again (I guess it will happen if we remove tryMigrate)? Can we try to get their username/password and login them automatically?

southerneer commented 5 years ago

Yep, based on the conversation this morning I will modify tryMigrate to work for migrating users to v4+ without the need for logging in again.

southerneer commented 5 years ago

Since we default to the firebase auth strategy in AuthStore.ts, we actually shouldn't need to do any special migration for production pre-v4 users. If they still have a valid firebase session then it should refresh their firebase token which will then be used in AuthStore (via Connectivity.tsx) to login.

@mstidham the test for this is to load 3.9.3 (I think that's the last 3.x.x version), login with phone number, then update to latest 4.x.x on TestFlight. If it reconnects successfully (without logging the user out) then we're good.

bengtan commented 5 years ago

the test for this is to load 3.9.3 (I think that's the last 3.x.x version), login with phone number, then update to latest 4.x.x on TestFlight. If it reconnects successfully (without logging the user out) then we're good.

Hmmm ... the complicating factor is that ... account-with-phone-number-XYZ on 3.9.3 and account-with-phone-number-XYZ on 4.x.x are two different accounts, with different IDs. One account lives in the Staging universe. The other account lives in the next universe.

So if QA tries this, and if the reconnection is successful, the app might just fall over anyway because the two account IDs are different. And then ... QA won't be able to tell whether the app failed because reconnection failed, or because the app couldn't handle the sudden change in account IDs.

I'd rather a dev test this, instead of QA.

southerneer commented 5 years ago

account-with-phone-number-XYZ on 3.9.3 and account-with-phone-number-XYZ on 4.x.x are two different accounts

That is an interesting wrinkle, but I don't think the different ids matter. As long as the firebase token is still valid and can be refreshed it should just work. I think QA should still give it a go.

mstidham commented 5 years ago

Test 1: I installed 3.9.3 and logged in with real number. Closed/killed the app and updated to 4.1.0 and received the Signup/Login screen. Test 2: I installed 3.9.3 and logged in with real number. Sent the app to the background and updated to 4.1.0 and also received the Signup/Login screen with app open.

I received a blank white screen and had to kill/reload when trying to Signup/Login after the update.

bengtan commented 5 years ago

@southerneer:

Bump.

Are you able to make sense of mstidham's test result?

Given that it's okay to have a one-off forced relogin when upgrading from 3.x to 4.x.x, I'm not sure how important this ticket is.

southerneer commented 5 years ago

This got a little more complicated after the login flow refactor. Now we would need to re-add tryMigrate to get it to "just work" moving from 3.x.x to 4.x.x

aksonov commented 5 years ago

@southerneer Do you know how to modify tryMigrate?

southerneer commented 5 years ago

I don't think we can do this with the new auth flow. 3.x.x auth tokens won't work with the new system so we have to log users out anyway.

southerneer commented 5 years ago

I can't remember why I reached that conclusion in the last comment. With the new auth flow we just need phone # + valid firebase token. I'm going to take another crack at tryMigrate.

bengtan commented 5 years ago

Okay, I'm going to hijack this ticket and adjust it so there's something to QA.

To QA this:

  1. Log in to the app (version 3.9.3) with a firebase user.
  2. Upgrade to 4.x.x and start it.

Observe whether the user needed to re-login.

If the user was automatically logged in (version 4.x.x) and could use the app, then this is a positive.

If the user needed to re-login, then this is a negative.

southerneer commented 5 years ago

Just make sure to do SMS login...bypass user migration won't work.

bengtan commented 5 years ago

Some positive test results reported at hippware/collaboration#7 (with the condition that there was a login subsequently after any codepushes).

mstidham commented 5 years ago

Moving this on to test on Prod.

bengtan commented 5 years ago

Yeah, let's test this on Prod.

bengtan commented 5 years ago

Overview

Plan for testing Production

Preparation (pre-Production server upgrade):

  1. If needed, install tinyrobot 3.9.3 (ie. the Production app) from the App Store.
  2. Login with firebase sms.
  3. Go to the 'edit profile' screen and look at the version. It should say '3.9.3.1 (3.9.3-v17)' (indicating that it has installed the latest codepush).
    • If it doesn't say this, wait a few minutes. The codepush should be automatically installed.
  4. After verifying that it is running '3.9.3.1 (3.9.3-v17)', logout and login (with firebase sms) again. (The login is needed so as to reset the expiry of the login token)
  5. Play around for a bit. Stay logged in.

Notes:

The important thing here is that ... after 3.9.3.1 has been installed, there is a subsequent (logout and) login with firebase sms.

If re-installing 3.9.3, the app might automagically restart itself after about 1 minute. This is the 3.9.3.1 codepush installing itself. This is okay.

This preparation needs to happen before the Production server upgrade. Since the login token lasts for 30 or 60 (?) days, QA can do the preparation a few days before the Production server upgrade is scheduled.

Then:

Test (after Productionn server upgrade):

  1. Upgrade to tinyrobot 4.14.x.
  2. Observe whether a re-login was needed or not (and write down the result)
  3. Continue using the app. Check old/existing bots, try to find bugs/issues/breakages etc.
bengtan commented 5 years ago

I tested this.

I had done the Preparation steps (0-4) a few days ago with 3.9.3.1.

Then, after the server upgrade, I installed 4.15.0 from TestFlight [1], opened it, and I was able to access my account without having to do a forced relogin.

Also, I was careful to skip the 'codepush brick' [2].

So I consider this a success.


[1] Interestingly, TestFlight showed an 'Install' button instead of an 'Update' button. Then, when I clicked on it, it said that I might lose all app data ... presumably since 3.9.3.1 was an App Store version. Dunno if this is relevant or not.

[2] Also, for those who received the 'codepush brick' screen on 3.9.3.1 before doing steps 5+, the 'codepush brick' codepush might mess up the Preparation state.


TL;DR: Verified on Prod

mstidham commented 5 years ago

I performed steps 0-4 and after the server deploy was unable to upgrade/install 4.15.0 . I received errors with every attempt.