Run migrations and refresh app state if relevant #7620
Conversation
|
Hmm it does seem to be causing an infinite loop, still need to fix that. |
TomatoToaster
changed the title
|
It looks like this is mostly working now! Noting here that I need to use |
TomatoToaster
changed the title
TomatoToaster
changed the title
|
The lint failure is related to this: #8882 (comment). This PR can be reviewed before the hold is lifted and despite that lint fail (which isn't relevant to the files changed here). |
src/libs/API.js
Outdated
| /** | ||
| * Runs command that will fix malformed data in a users account and also run migrations. | ||
| * It returns whether anything was changed. |
There was a problem hiding this comment.
NAB not sure if this comment is necessary. All methods defined in API.js do the same thing for different commands/params.
src/libs/actions/App.js
Outdated
| if (!response.changed) { | ||
| return; | ||
| } | ||
| Log.info('FixAccount found updates for this user, so data will be reinitalized', true, response); |
There was a problem hiding this comment.
| Log.info('FixAccount found updates for this user, so data will be reinitalized', true, response); | |
| Log.info('FixAccount found updates for this user, so data will be reinitialized', true, response); |
|
Tests steps are clear and it seemed to work! Note: I noticed that this call always returned a change in my dev account. However, I did verify that the migration ran and the NVP was set. |
|
HOLD will likely be released later today when the next deploy goes up. If there's no other comments, I'll merge it soon after that since we already have the approve and I don't think any other comments are likely to pop up. |
TomatoToaster
changed the title
|
Triggered auto assignment to @bondydaa ( |
|
@TomatoToaster looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
unassigning myself since amal said he will QA |
|
🚀 Deployed to staging by @TomatoToaster in version: 1.1.58-0 🚀
|
|
We've got a bad bug on production right now, and since we can't CP to production (yet), our only way to fix it is by doing another full production deploy. So I'm going to take a crack at the internalQA for this bad boi |
|
Nice, this was a pass. Going to check it off the checklist |
|
🚀 Deployed to production by @AndrewGable in version: 1.1.60-3 🚀
|
CC: @luacmartins (since you worked on this recently) @roryabraham @marcaaron
HOLD
Details
We will run FixAccount like we do in OldDot and reinitialize data if something changed for the user or we ran migrations. If FixAccount returns no changes (which it will 90% of the time), we will do nothing. This will work without refreshing the whole app state and users won't need to log out and log back in.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/195477
Tests
Same as QA but done locally (with the other Web-E PR linked above active)
QA Steps
You can reach out to
amal@expensify.com@TomatoToaster to test this on staging if needed.FixAccountIn another tab, log into the same account on OldDot, and then run this snippet in the console:
NVP.set('expensify_migration_2022_03_28_policyExpenseChatMigration')Then refresh the page on NewDot and verify that the three commands in the above screenshot are there.Refresh the page one more time and verify you only see the first 2 commands:
Tested On