Fix test pref disappear on android if open app in offline mode by jatinsonijs · Pull Request #14897 · Expensify/App

jatinsonijs · 2023-02-07T06:25:38Z

Details

We are checking android app is in beta or not by comparing version from GitHub url. When we are offline we cannot fetch this information.
To solve this problem save last checked isBeta value in local storage and use them in fetch failed case like when offline.

Fixed Issues

$ #14047
PROPOSAL: #14047 (comment)

Tests

NOTE: This issue is only related to android native. There is not any impact on another platforms.

Comment out below code to always check beta version on native
App/src/libs/Environment/getEnvironment/index.native.js

 if (lodashGet(Config, 'ENVIRONMENT', CONST.ENVIRONMENT.DEV) === CONST.ENVIRONMENT.DEV) { 
     environment = CONST.ENVIRONMENT.DEV; 
     return resolve(environment); 
 }

Make sure STG badge is showing on header after the Chats label.
Open the Preferences page by navigate to Settings -> Preferences.
Check Test Preferences is showing.
Turn off network and kill / reload the app.
Open the app again and navigate to Settings -> Preferences.
Check Test Preferences is showing.

Verify that no errors appear in the JS console

Offline tests

Install / run app with staging env.
Make sure STG badge is showing on header after the Chats label.
Open the Preferences page by navigate to Settings -> Preferences.
Check Test Preferences is showing.
Turn off network and kill / reload the app.
Open the app again and navigate to Settings -> Preferences.
Check Test Preferences is showing.

QA Steps

NOTE: We cannot test it on web and mobile web as Its required to reload the app but in offline mode refresh page will not work.

Go to the home page.
Make sure STG badge is showing on header after the Chats label.
Open the Preferences page by navigate to Settings -> Preferences.
Check Test Preferences is showing.
Turn off network and kill / reload the app.
Open the app again and navigate to Settings -> Preferences.
Check Test Preferences is showing.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

web.mp4

Mobile Web - Chrome

mweb-chrome.mp4

Mobile Web - Safari

mweb-safari.mp4

Desktop

desktop.mp4

iOS

ios.mp4

Android

android.mp4

MelvinBot · 2023-02-07T06:25:44Z

@luacmartins @0xmiroslav One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

0xmiroslav · 2023-02-07T14:39:38Z

@jatinsonijs I think Tests step needs adjusted a bit.
Install / run app with staging env. - can you elaborate in detail? Default installation points to DEV

jatinsonijs · 2023-02-07T14:46:43Z

Hi @0xmiroslav we have to create .env file to use to use .env.staging configs

jatinsonijs · 2023-02-07T14:49:30Z

I have not found any direct way to test it, I think there is different process to generate staging apk, but we can test it by creating .env file and add a variable ENVIRONMENT=staging so that we can match the condition. we can only test it on android native. So I think there is not any correct steps to actually test it on all platform. Like iOS need TestFlight to test it. I have test iOS by change in code itself as we have to must add video we cannot skip it.

0xmiroslav · 2023-02-07T15:16:49Z

Tests step is for engineer review. So to be easy, we can just update step like this:

Comment these lines so that native environment depends on betaChecker

App/src/libs/Environment/getEnvironment/index.native.js

Lines 20 to 23 in 09f4dc3

if (lodashGet(Config, 'ENVIRONMENT', CONST.ENVIRONMENT.DEV) === CONST.ENVIRONMENT.DEV) {

environment = CONST.ENVIRONMENT.DEV;

return resolve(environment);

}

jatinsonijs · 2023-02-07T15:18:52Z

Yes it will work also, initially I have test by this.

jatinsonijs · 2023-02-07T15:19:30Z

can I include sometime like this in test steps ?

0xmiroslav · 2023-02-07T15:20:47Z

can I include sometime like this in test steps ?

I now have step. So up to @luacmartins

luacmartins · 2023-02-07T16:23:39Z

@jatinsonijs yes, you can include that in test steps only. QA steps shouldn't have any changes in code, since our QA team will be testing the staging APK.

luacmartins · 2023-02-07T16:25:53Z

            })
            .catch(() => {
-                resolve(false);
+                // In case version fetch failed like in offline mode use last saved value


Suggested change

// In case version fetch failed like in offline mode use last saved value

// Use lastBetaValue in case we fail to fetch the new one, e.g. when we are offline

luacmartins · 2023-02-07T16:29:55Z

+import ONYXKEYS from '../../../ONYXKEYS';
+import * as AppUpdate from '../../actions/AppUpdate';
+
+let lastBetaValue = false;


This name suggests that we are storing a version string. I think we should change it to better describe that it's storing a bool:

Suggested change

let lastBetaValue = false;

let isBeta = false;

Good suggestion! Since this is boolean flag, it's standard to add is prefix

@luacmartins actually its conflict with inner const isBeta = semver.gt(pkg.version, productionVersion); isBeta variable name that's why I have used this name.

Something like isLastSavedBeta?

Yes @0xmiroslav let me change by isLastSavedBeta

Changes done

0xmiroslav · 2023-02-07T21:11:10Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web.mov

Mobile Web - Chrome

mchrome.mov

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

Android

android.mp4

OSBotify · 2023-02-08T13:26:02Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2023-02-08T13:55:24Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	699.810 ms → 723.490 ms (+23.680 ms, +3.4%)
App start runJsBundle	195.484 ms → 204.063 ms (+8.579 ms, +4.4%)
App start regularAppStart	0.014 ms → 0.016 ms (+0.001 ms, +8.4%)
App start nativeLaunch	20.188 ms → 19.871 ms (-0.317 ms, -1.6%)
Open Search Page TTI	611.099 ms → 604.355 ms (-6.744 ms, -1.1%)

Show details

Name	Duration
App start TTI	Baseline Mean: 699.810 ms Stdev: 26.332 ms (3.8%) Runs: 648.7622720003128 653.8662959998474 658.5297229997814 664.1737909996882 667.9541469998658 674.4979480002075 677.5543099995703 679.728850999847 683.0751670002937 686.5572279999033 689.6050129998475 695.6500359997153 697.3064909996465 697.7819400001317 699.2447380004451 701.952685999684 702.0722300000489 706.6844619996846 708.5359359998256 709.8154450003058 715.9815840004012 717.0406149998307 717.9122980004176 721.787605999969 727.3907819995657 731.0822970001027 732.16772700008 737.8459480004385 738.8035030001774 750.9263070002198 Current Mean: 723.490 ms Stdev: 30.608 ms (4.2%) Runs: 679.9186100000516 680.1864710003138 683.8597870003432 684.8366860002279 693.0369279999286 695.3503379998729 698.6124459998682 701.5434400001541 702.1932789999992 702.6664739996195 703.6830900004134 711.8097080001608 713.2423499999568 714.2124340003356 716.3276030002162 718.7961100004613 722.4217220004648 723.7279930002987 729.4713239995763 729.6126469997689 730.7118359999731 734.572995999828 735.6184350000694 738.6239919997752 739.3543090000749 744.8087510000914 761.0044470001012 778.026786999777 784.7537620002404 784.826795999892 790.3805679995567
App start runJsBundle	Baseline Mean: 195.484 ms Stdev: 20.019 ms (10.2%) Runs: 168 169 170 171 171 174 176 180 180 182 183 189 191 192 192 193 193 196 196 197 200 203 204 207 217 219 219 225 226 237 240 Current Mean: 204.063 ms Stdev: 22.847 ms (11.2%) Runs: 177 178 179 180 181 184 184 185 185 185 190 192 195 195 196 197 198 200 202 202 210 211 212 214 217 220 221 235 247 247 249 262
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (6.1%) Runs: 0.01285799965262413 0.013224000111222267 0.013468999415636063 0.013509000651538372 0.013631000183522701 0.013670999556779861 0.01375299971550703 0.013833999633789062 0.013997000642120838 0.0139979999512434 0.014161000028252602 0.014241999946534634 0.014241999946534634 0.014322999864816666 0.014403999783098698 0.01444500032812357 0.014525999315083027 0.014689000323414803 0.014729000627994537 0.014811000786721706 0.014852000400424004 0.0148930000141263 0.014973999932408333 0.015055999159812927 0.015258999541401863 0.015584000386297703 0.016153999604284763 0.0163569999858737 0.016439000144600868 Current Mean: 0.016 ms Stdev: 0.001 ms (7.1%) Runs: 0.013427999801933765 0.013957000337541103 0.014241000637412071 0.014527000486850739 0.01464799977838993 0.014648000709712505 0.014649000018835068 0.014932999387383461 0.014932999387383461 0.014973999932408333 0.01501499954611063 0.015054999850690365 0.015095999464392662 0.015259000472724438 0.015583999454975128 0.015584999695420265 0.01570700015872717 0.015787999145686626 0.015868999995291233 0.016032000072300434 0.01607299968600273 0.016112999990582466 0.016316999681293964 0.016439000144600868 0.016600999981164932 0.016885999590158463 0.017009000293910503 0.017048999667167664 0.017211999744176865 0.017253000289201736 0.01733400020748377 0.0183100001886487
App start nativeLaunch	Baseline Mean: 20.188 ms Stdev: 2.311 ms (11.4%) Runs: 17 18 18 18 18 18 18 18 18 18 18 18 19 19 19 19 20 20 20 21 21 22 22 22 22 22 23 23 23 24 25 25 Current Mean: 19.871 ms Stdev: 1.581 ms (8.0%) Runs: 17 18 18 18 18 18 18 18 19 19 19 19 19 20 20 20 20 20 20 20 20 21 21 21 22 22 22 22 22 22 23
Open Search Page TTI	Baseline Mean: 611.099 ms Stdev: 15.626 ms (2.6%) Runs: 577.765746999532 582.6092940000817 584.0536700002849 592.1975100003183 593.4116210006177 596.7820640001446 598.138184000738 598.1999110002071 602.9900719998404 605.7882900005206 607.653157999739 609.628866000101 610.1620289999992 610.3900150004774 611.7221679994836 612.9793290002272 613.1670730002224 613.2777909999713 613.7885339995846 614.3038739999756 615.5137939993292 616.5901290001348 619.8176679993048 621.3995360005647 622.8422449994832 623.2991129998118 625.2409260002896 633.9328209999949 635.9972740001976 637.8345550000668 642.5858160005882 Current Mean: 604.355 ms Stdev: 9.088 ms (1.5%) Runs: 586.6307789999992 589.3795170001686 593.600056999363 594.322103000246 594.4026690004393 594.5511069996282 597.5484210001305 598.2108970005065 598.2114260001108 598.9246020000428 599.7276609996334 599.87084900029 599.9943850003183 600.4674080004916 602.4549569999799 603.2040210003033 603.7809649994597 604.5053709996864 606.3579509994015 606.7215989995748 607.7895510001108 607.8144129998982 608.1206459999084 610.5884610004723 610.9802249995992 611.2277839994058 612.469197999686 613.5448000002652 617.2084149997681 620.0874840002507 623.0558679997921 623.6097009992227

OSBotify · 2023-02-09T00:30:52Z

🚀 Deployed to staging by https://github.com/luacmartins in version: 1.2.68-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-02-10T00:31:41Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.2.68-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Fix test pref disappear on android in offline mode

696b6f1

jatinsonijs requested a review from a team as a code owner February 7, 2023 06:25

melvin-bot Bot requested review from 0xmiroslav and luacmartins and removed request for a team February 7, 2023 06:25

jatinsonijs mentioned this pull request Feb 7, 2023

[HOLD for payment 2023-06-30] [HOLD for payment 2023-02-17] Test Preferences section disappear when the android app is reloaded in offline mode #14047

Closed

1 task

luacmartins requested changes Feb 7, 2023

View reviewed changes

Change comment and variable name

c8925c5

0xmiroslav approved these changes Feb 7, 2023

View reviewed changes

luacmartins approved these changes Feb 8, 2023

View reviewed changes

luacmartins merged commit 8b01a44 into Expensify:main Feb 8, 2023

OSBotify mentioned this pull request Feb 8, 2023

Deploy Checklist: New Expensify 2023-02-08 #14977

Closed

6 tasks

	// In case version fetch failed like in offline mode use last saved value
	// Use lastBetaValue in case we fail to fetch the new one, e.g. when we are offline

Uh oh!

Conversation

jatinsonijs commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

MelvinBot commented Feb 7, 2023

Uh oh!

0xmiroslav commented Feb 7, 2023

Uh oh!

jatinsonijs commented Feb 7, 2023

Uh oh!

jatinsonijs commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xmiroslav commented Feb 7, 2023

Uh oh!

jatinsonijs commented Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jatinsonijs commented Feb 7, 2023

Uh oh!

0xmiroslav commented Feb 7, 2023

Uh oh!

luacmartins commented Feb 7, 2023

Uh oh!

luacmartins Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

luacmartins Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

0xmiroslav Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

jatinsonijs Feb 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmiroslav Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

jatinsonijs Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

jatinsonijs Feb 7, 2023

Choose a reason for hiding this comment

Uh oh!

0xmiroslav commented Feb 7, 2023

Reviewer Checklist

Screenshots/Videos

Uh oh!

OSBotify commented Feb 8, 2023

Uh oh!

github-actions Bot commented Feb 8, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

Uh oh!

OSBotify commented Feb 9, 2023

Uh oh!

OSBotify commented Feb 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jatinsonijs commented Feb 7, 2023 •

edited

Loading

jatinsonijs commented Feb 7, 2023 •

edited

Loading

jatinsonijs commented Feb 7, 2023 •

edited

Loading

jatinsonijs Feb 7, 2023 •

edited

Loading