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

Fix App deprecation check mechanism #1358

Merged
merged 25 commits into from
Nov 12, 2019
Merged

Conversation

rohit-dua
Copy link
Contributor

@rohit-dua rohit-dua commented Oct 16, 2019

Description

The app deprecation check mechanism was not working as it required firebase login with web3 account. This requirement is not required for the version check.

Atm we need to register every single version and mark it deprecated in the firebase DB. Now a minimum version in firebase db should do the task

Testing

The deprecation system was tested on the device by:

  • Changing the deprecation flag of the testing version (1.5.0) in the firebase db. The format versions > 1 > 5 > 0 > deprecated > true was used.
  • Adding a minVersion flag to versions and providing it a higher value than the current version

In both the cases the deprecation screen was showed at app start.

Related issues

Backwards compatibility

It is backwards compatible.

@rohit-dua rohit-dua requested a review from jmrossy October 16, 2019 13:48
@rohit-dua rohit-dua requested a review from cmcewen as a code owner October 16, 2019 13:48
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include info about how you tested this

packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #1358 into master will increase coverage by 0.51%.
The diff coverage is 65.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1358      +/-   ##
==========================================
+ Coverage   73.78%   74.29%   +0.51%     
==========================================
  Files         279      278       -1     
  Lines        7606     7540      -66     
  Branches      657      660       +3     
==========================================
- Hits         5612     5602      -10     
+ Misses       1881     1825      -56     
  Partials      113      113
Flag Coverage Δ
#mobile 74.29% <65.38%> (+0.51%) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/app/saga.ts 59.03% <55.55%> (-2.41%) ⬇️
packages/mobile/src/firebase/firebase.ts 43.75% <70.58%> (+7.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 722945f...9c5bb54. Read the comment docs.

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I asked in my last review, please add info about how this was tested.

packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
packages/mobile/src/firebase/firebase.ts Outdated Show resolved Hide resolved
packages/mobile/src/firebase/firebase.ts Show resolved Hide resolved
@jmrossy jmrossy assigned rohit-dua and unassigned jmrossy Oct 20, 2019
@rohit-dua
Copy link
Contributor Author

As I asked in my last review, please add info about how this was tested.

Added the Testing information in the description:

Testing

The deprecation system was tested on the device by:

  • Changing the deprecation flag of the testing version (1.5.0) in the firebase db. The format versions > 1 > 5 > 0 > deprecated > true was used.
  • Adding a minVersion flag to versions and providing it a higher value than the current version

In both the cases the deprecation screen was showed at app start.

@rohit-dua rohit-dua assigned jmrossy and unassigned rohit-dua Oct 21, 2019
@jmrossy jmrossy assigned rohit-dua and unassigned jmrossy Oct 22, 2019
@rohit-dua rohit-dua assigned jmrossy and unassigned rohit-dua Oct 22, 2019
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with one small comment, please fix before merging.
Once you merge this, please copy over the updated db-rules.json to the firebase dbs and ping me when that's done :)

packages/mobile/test/values.ts Outdated Show resolved Hide resolved
@jmrossy jmrossy removed their assignment Oct 23, 2019
@jmrossy jmrossy removed their assignment Oct 23, 2019
@jmrossy jmrossy merged commit cc69980 into master Nov 12, 2019
@jmrossy jmrossy deleted the rohit-dua/app-version-check branch November 12, 2019 11:28
aaronmgdr added a commit that referenced this pull request Nov 14, 2019
* master: (56 commits)
  Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585)
  [Wallet] Add more local currencies (#1698)
  Switch to correct cluster when fauceting (#1687)
  [Wallet] Use the country of the phone number for determining the default local currency (#1684)
  [Wallet] Limit QR code scanner to 1 code per second (#1676)
  Update Dark backgrounds text color (#1677)
  Remove integration sync test
  Minor attestation service fixes (#1680)
  [wallet] Fixed Native phone picker Use native API instead (#1669)
  Fix token addresses for notification service (#1674)
  Add golang to setup docs
  [wallet] Hide invite education copy after invite was redeemed (#1670)
  [Wallet] Add spinner, timer, and tip text to Verification input screen (#1656)
  [Wallet] Fix app deprecation check mechanism (#1358)
  Point end-to-end governance test back to master (#1665)
  Add EpochRewards smart contract to calculate epoch rewards and payments (#1558)
  Optimized Attestation view calls and removal of the reveal TX (#1578)
  Support claim signatures and support Keybase claims (#1575)
  [Wallet] Add timestamp to top banner messages (#1657)
  Export geth metrics on VM testnet (#1351)
  ...

# Conflicts:
#	yarn.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App deprecation mechanism seems broken
3 participants