Details
Checked it on all platforms in dev mode
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
I would split this diff into two. One with changing fullBackupSupport -> hook, and the second with only flipping the switch to make it easier to revert just in case, but it is probably fine as it is
| lib/components/secondary-device-qr-auth-context-provider.react.js | ||
|---|---|---|
| 244 | Shouldn't we pass identityAuthResult.userID here to avoid any possible race conditions? And use fullBackupSupportEnabled instead of hook | |
| native/account/restore.js | ||
| 215 | In theory, passing identityAuthResult.userID here to avoid a race condition might be reasonable too, but since it is native specific, we'll base this on whether this is staff release | |
| native/cpp/CommonCpp/Tools/ServicesUtils.cpp | ||
| 5–10 | Unfortunately, that's not great and different from JS; we don't check the userID here, which means if a staff user tries to restore on a non-staff release app, it will fail. We should either:
This is all caused by the fact that we've never prioritized ENG-10124 | |
| native/cpp/CommonCpp/Tools/ServicesUtils.cpp | ||
|---|---|---|
| 5–10 | For option 1, we should also inform staff users that it won't work on non-staff release's device but shouldn't be a problem, because in this case app should work (without backup but not crash) | |
This is great idea, I'll do that.
| lib/components/secondary-device-qr-auth-context-provider.react.js | ||
|---|---|---|
| 244 | Good idea | |
| native/account/restore.js | ||
| 215 | Actually, using identityAuthResult is better; the less hooks, the better. Thanks! | |
| native/cpp/CommonCpp/Tools/ServicesUtils.cpp | ||
| 5–10 | As we discussed offline, this isn't that simple, and basing this on userID everywhere requires significant changes. I think it's not worth doing anything beyond informing that on non-staff release, logs/compactions won't be generated/uploaded. The problem disappears when we launch backup to public. | |