[lib][web][native] Redux store to persist latest backup info
ClosedPublic
Actions

Authored by kamil on Tue, Nov 26, 7:17 AM.

Details

Reviewers

bartek
tomek

Commits

rCOMMa4bd9d515d2a: [lib][web][native] Redux store to persist latest backup info

Summary

ENG-9700.

Native-specific store where we can put backup-related data.

I am not sure if it is better to put this in lib, even if this won't be used on the web at all and has cleaner code (e.g., reducing in master reducer, not in native specific), or leave it as it is.

Test Plan

Check if migrations works

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Tue, Nov 26, 7:17 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptTue, Nov 26, 7:17 AM

kamil published this revision for review.Tue, Nov 26, 7:53 AM

kamil edited the summary of this revision. (Show Details)

bartek added inline comments.Tue, Nov 26, 7:53 AM

lib/types/backup-types.js
59 ↗	(On Diff #46083)

kamil added a child revision: D14050: [lib][native] create action to set latest `backupID` on User Keys upload.Tue, Nov 26, 8:07 AM

Harbormaster completed remote builds in B32868: Diff 46083.Tue, Nov 26, 9:06 AM

Do we really need the migration? Won't it get merged with defaultState anyways?

lib/types/backup-types.js
53 ↗	(On Diff #46083)	Typo

In D14049#391563, @ashoat wrote:

Do we really need the migration? Won't it get merged with defaultState anyways?

In the past, we had some bugs caused by relying on the default state instead of the migration. They can happen in the following scenario:

A new store is added without a migration
A new migration is added that uses this store
A user updates the app from before the 1st step

Then migration 2 fails.

tomek accepted this revision.Wed, Nov 27, 7:12 AM

This revision is now accepted and ready to land.Wed, Nov 27, 7:12 AM

In D14049#391602, @tomek wrote:

In D14049#391563, @ashoat wrote:

Do we really need the migration? Won't it get merged with defaultState anyways?

In the past, we had some bugs caused by relying on the default state instead of the migration. They can happen in the following scenario:

A new store is added without a migration

A new migration is added that uses this store

A user updates the app from before the 1st step

Then migration 2 fails.

In this case, the store can be null, so it's hard to imagine such a scenario occurring... the code in the later migration would have to crash specifically on undefined but not on null.