Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

Keys Backup can be setup twice #9510 #3127

Merged
merged 4 commits into from
May 22, 2019
Merged

Conversation

BillCarsonFr
Copy link
Member

Fixes #9510

Check for existing backup in the last step of the setup.
Check remote state when VectorHomeActivity is resumed to update banner.

image

processOnCreate()
} else {
//we should prompt
AlertDialog.Builder(context)
Copy link
Member

Choose a reason for hiding this comment

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

The dialog should not be in the model

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -1294,6 +1294,11 @@ Why choose Riot.im?
<string name="keys_backup_setup_step3_save_button_title">Save as File</string>
<string name="recovery_key_export_saved_as_warning">The recovery key has been saved to \'%s\'.\n\nWarning: this file may be deleted if the application is uninstalled.</string>

<string name="keys_backup_setup_override_backup_prompt_tile">A backup already exist in this location</string>
Copy link
Member

Choose a reason for hiding this comment

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

A backup already exists in this location on your homeserver

Copy link
Member Author

Choose a reason for hiding this comment

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

done


//Force remote backup state update to update the banner if needed
if (mSession.getCrypto() != null) {
mSession.getCrypto().getKeysBackup().checkAndStartKeysBackup();
Copy link
Member

Choose a reason for hiding this comment

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

Is it done elsewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

as discussed

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

Good work, some minor remarks

@@ -190,30 +203,22 @@ class KeysBackupSetupSharedViewModel : ViewModel() {
keysBackup.getCurrentVersion(object : SimpleApiCallback<KeysVersionResult?>(failureCallBack) {
override fun onSuccess(info: KeysVersionResult?) {
loadingStatus.value = null
if (info?.version.isNullOrBlank()) {
if (info?.version.isNullOrBlank() || forceOverride) {
//should not happen
Copy link
Member

Choose a reason for hiding this comment

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

Maybe remove this comment

processOnCreate()
} else {
//we should prompt
loadingStatus.value = null
Copy link
Member

Choose a reason for hiding this comment

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

This line is not necessary (it's done line 205)

}

fun processOnCreate() {
keysBackup.createKeysBackupVersion(megolmBackupCreationInfo!!, object : SimpleApiCallback<KeysVersion>(failureCallBack) {
Copy link
Member

Choose a reason for hiding this comment

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

The loading status is not set to true before this call?

@@ -1294,6 +1294,11 @@ Why choose Riot.im?
<string name="keys_backup_setup_step3_save_button_title">Save as File</string>
<string name="recovery_key_export_saved_as_warning">The recovery key has been saved to \'%s\'.\n\nWarning: this file may be deleted if the application is uninstalled.</string>

<string name="keys_backup_setup_override_backup_prompt_tile">A backup already exist on your HomeServer</string>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<string name="keys_backup_setup_override_backup_prompt_tile">A backup already exist on your HomeServer</string>
<string name="keys_backup_setup_override_backup_prompt_tile">A backup already exists on your homeserver</string>

<string name="keys_backup_setup_override_backup_prompt_tile">A backup already exist on your HomeServer</string>
<string name="keys_backup_setup_override_backup_prompt_description">It looks like you already have setup key backup from another device. Do you want to replace it with the one you’re creating?</string>
<string name="keys_backup_setup_override_replace">Replace</string>
<string name="keys_backup_setup_override_stop">Stop</string>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<string name="keys_backup_setup_override_stop">Stop</string>
<string name="keys_backup_setup_override_stop">Cancel</string>

@bmarty bmarty merged commit 733b344 into develop May 22, 2019
@bmarty bmarty deleted the feature/backup_setup_twice branch May 22, 2019 12:22
bmarty added a commit to element-hq/element-android that referenced this pull request Jun 17, 2019
bmarty added a commit to element-hq/element-android that referenced this pull request Jun 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants