Release v. 1.1.3 (#845)

* Add release notes for 1.1.2.4405 * Bump version to 1.2.0 * Add ignore to the flaky test. This needs to be fixed later. (#793) * 626, 627: Add kebab menu for delete (#764) * Add kebab menu icon and strings. Set up fragment for addition to toolbar. * Create fragment and dialog. * Add confirmation dialog, actions, and presenters * Routing * Ensure item is actually deleted and item list is refreshed before navigating back. * Refactor and remove edit for now * Keeping edit button, just not doing any functionality with it * Test for item detail presenter * Ellipsize long item titles in detail view * Delete confirmation toast * Delete toast and tests * Consumable toast notification for delete * Specify return type on Optional extension * ItemListPresenterTest needed a consumable for the delete toast test * Cleanup * Fix toast notifications * Import strings from android-l10n (#809) State: mozilla-l10n/android-l10n@63a5240 * 806-uitest-add-delete (#808) * Adjust spinner dropdown size to fit longer words (l10n) (#812) * Ellipsize text in item list (#811) * 810: Reset support when syncing (#826) * Reset support when info is null * Reset support every time we sync. * Removing timeout to be fixed in #791 (#833) * Removing timeout to be fixed in #791 * Remove strings * 828-l10nScreenshotsTest-delete (#829) * Add dot to sentence (#837) * 814: Telemetry for sync (#835) * Add telemetry actions for sync, list update, and error states. * Sync timeout instead of sync end * Add sync complete event * Add errors and ids to telemetry calls * Update metrics docs to reflect telemetry sync changes (#841) * Revert delete PRs (#836) * Revert "828-l10nScreenshotsTest-delete (#829)" This reverts commit bab9fe8. * Revert "806-uitest-add-delete (#808)" This reverts commit ec56009. * Revert "626, 627: Add kebab menu for delete (#764)" This reverts commit 17605c3. * Remove unused imports and fix lint error. * Rebase and lint * Update to version 1.1.3 (#844) * Update version to 1.1.3 * Add release notes for 1.1.3
mozilla-lockwise · Jul 25, 2019 · f2bf091 · f2bf091
1 parent 2d46cc6
commit f2bf091
Show file tree

Hide file tree

Showing 24 changed files with 295 additions and 112 deletions.
diff --git a/README.md b/README.md
@@ -29,7 +29,7 @@ version 2.0][license-link].
 
 Note that some test fixtures and all code in the `thirdparty` directory use source code from third-party services, and are not subject to this license.
 
-All alternative licenses and the origins of third party code is cited in the [third party readme][third-party-link]
+All alternative licenses and the origins of third party code is cited in the [third party readme][third-party-link].
 
 [bitrise-image]: https://app.bitrise.io/app/20089a88380dd14d/status.svg?token=41PRDjKSm0fQCUiS2EmCkQ&branch=master
 [bitrise-link]: https://app.bitrise.io/app/20089a88380dd14d

diff --git a/app/build.gradle b/app/build.gradle
@@ -21,8 +21,8 @@ android {
         minSdkVersion 24
         //noinspection OldTargetApi
         targetSdkVersion 26
-        versionCode 4103
-        versionName "1.1.2"
+        versionCode 4584
+        versionName "1.1.3"
         testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
         // The following argument makes the Android Test Orchestrator run its
         // "pm clear" command after each test invocation. This command ensures

diff --git a/app/src/main/java/mozilla/lockbox/action/DataStoreAction.kt b/app/src/main/java/mozilla/lockbox/action/DataStoreAction.kt
@@ -6,16 +6,92 @@
 
 package mozilla.lockbox.action
 
+import mozilla.components.service.sync.logins.ServerPassword
 import mozilla.lockbox.model.SyncCredentials
 
 sealed class DataStoreAction(
     override val eventMethod: TelemetryEventMethod,
-    override val eventObject: TelemetryEventObject
+    override val eventObject: TelemetryEventObject,
+    override val value: String? = null,
+    override val extras: Map<String, Any>? = null
 ) : TelemetryAction {
     object Lock : DataStoreAction(TelemetryEventMethod.lock, TelemetryEventObject.datastore)
     object Unlock : DataStoreAction(TelemetryEventMethod.unlock, TelemetryEventObject.datastore)
     object Reset : DataStoreAction(TelemetryEventMethod.reset, TelemetryEventObject.datastore)
-    object Sync : DataStoreAction(TelemetryEventMethod.sync, TelemetryEventObject.datastore)
-    data class Touch(val id: String) : DataStoreAction(TelemetryEventMethod.touch, TelemetryEventObject.datastore)
-    data class UpdateCredentials(val syncCredentials: SyncCredentials) : DataStoreAction(TelemetryEventMethod.update_credentials, TelemetryEventObject.datastore)
+
+    /**
+     * Emitted when the app requests a sync.
+     */
+    object Sync : DataStoreAction(TelemetryEventMethod.sync_start, TelemetryEventObject.datastore)
+
+    /**
+     * Emitted when a sync request completes.
+     */
+    object SyncEnd : DataStoreAction(TelemetryEventMethod.sync_end, TelemetryEventObject.datastore)
+
+    /**
+     * Emitted when the app times out when listening for a response from sync.
+     */
+    data class SyncTimeout(val error: String)
+        : DataStoreAction(
+            TelemetryEventMethod.sync_timeout,
+            TelemetryEventObject.datastore,
+            error
+        )
+
+    /**
+     * Emitted when the app receives an error response from sync.
+     */
+    data class SyncError(val error: String) : DataStoreAction(
+        TelemetryEventMethod.sync_error,
+        TelemetryEventObject.datastore,
+        error
+    )
+
+    /**
+     * Emitted when the item list has been updated.
+     */
+    object ListUpdate : DataStoreAction(TelemetryEventMethod.list_update, TelemetryEventObject.datastore)
+
+    /**
+     * Emitted when an update to the item list has failed with an error.
+     */
+    data class ListUpdateError(val error: String)
+        : DataStoreAction(
+            TelemetryEventMethod.list_update_error,
+            TelemetryEventObject.datastore,
+            error
+        )
+
+    /**
+     * Emitted when a DataStore action results in an error.
+     */
+    data class Errors(val error: String)
+        : DataStoreAction(
+            TelemetryEventMethod.list_update_error,
+            TelemetryEventObject.datastore,
+            error
+        )
+
+    data class Touch(val id: String)
+        : DataStoreAction(
+            TelemetryEventMethod.touch,
+            TelemetryEventObject.datastore,
+            "ID: $id"
+        )
+
+    data class UpdateCredentials(val syncCredentials: SyncCredentials)
+        : DataStoreAction(
+            TelemetryEventMethod.update_credentials,
+            TelemetryEventObject.datastore
+        )
+
+    data class Delete(val item: ServerPassword?)
+        : DataStoreAction(
+            TelemetryEventMethod.delete,
+            TelemetryEventObject.delete_credential,
+            item?.id
+        )
+
+    data class Edit(val itemId: Int) : DataStoreAction(TelemetryEventMethod.edit, TelemetryEventObject.edit_credential)
 }
diff --git a/app/src/main/java/mozilla/lockbox/action/TelemetryAction.kt b/app/src/main/java/mozilla/lockbox/action/TelemetryAction.kt
@@ -37,6 +37,8 @@ enum class TelemetryEventMethod {
     background,
     setting_changed,
     show,
+    delete,
+    edit,
     canceled,
     login_selected,
     autofill_locked,
@@ -46,7 +48,12 @@ enum class TelemetryEventMethod {
     lock,
     unlock,
     reset,
-    sync,
+    sync_start,
+    sync_end,
+    sync_timeout,
+    sync_error,
+    list_update,
+    list_update_error,
     touch,
     update_credentials,
     autofill_single,
@@ -93,5 +100,7 @@ enum class TelemetryEventObject {
     filter,
     back,
     dialog,
-    datastore
+    datastore,
+    delete_credential,
+    edit_credential
 }
diff --git a/app/src/main/java/mozilla/lockbox/presenter/ItemListPresenter.kt b/app/src/main/java/mozilla/lockbox/presenter/ItemListPresenter.kt
@@ -70,7 +70,7 @@ class ItemListPresenter(
             .map {
                 when (it) {
                     DataStore.SyncState.Syncing -> true
-                    DataStore.SyncState.NotSyncing, DataStore.SyncState.TimedOut -> false
+                    DataStore.SyncState.NotSyncing -> false
                 }
             }
             .subscribe { syncing ->
@@ -83,13 +83,14 @@ class ItemListPresenter(
             }
             .addTo(compositeDisposable)
 
-        dataStore.syncState
-            .filter { it == DataStore.SyncState.TimedOut }
-            .map { R.string.sync_timed_out }
-            .observeOn(AndroidSchedulers.mainThread())
-            .subscribe(view::showToastNotification)
-            .addTo(compositeDisposable)
-
+        /* timeout to be fixed in https://github.com/mozilla-lockwise/lockwise-android/issues/791
+              dataStore.syncState
+                  .filter { it == DataStore.SyncState.TimedOut }
+                  .map { R.string.sync_timed_out }
+                  .observeOn(AndroidSchedulers.mainThread())
+                  .subscribe(view::showToastNotification)
+                  .addTo(compositeDisposable)
+        */
         Observables.combineLatest(dataStore.list, settingStore.itemListSortOrder)
             .distinctUntilChanged()
             .map { pair ->
@@ -145,8 +146,12 @@ class ItemListPresenter(
             .addTo(compositeDisposable)
 
         view.refreshItemList
-            .doOnDispose { view.stopRefreshing() }
-            .subscribe { dispatcher.dispatch(DataStoreAction.Sync) }
+            .doOnDispose {
+                view.stopRefreshing()
+            }
+            .subscribe {
+                dispatcher.dispatch(DataStoreAction.Sync)
+            }
             .addTo(compositeDisposable)
 
         accountStore.profile

diff --git a/app/src/main/java/mozilla/lockbox/store/AccountStore.kt b/app/src/main/java/mozilla/lockbox/store/AccountStore.kt
@@ -22,11 +22,11 @@ import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.rx2.asMaybe
 import kotlinx.coroutines.rx2.asSingle
+import mozilla.appservices.fxaclient.Config
 import mozilla.appservices.fxaclient.FxaException
 import mozilla.components.concept.sync.AccessTokenInfo
 import mozilla.components.concept.sync.Avatar
 import mozilla.components.concept.sync.Profile
-import mozilla.components.service.fxa.Config
 import mozilla.components.service.fxa.FirefoxAccount
 import mozilla.lockbox.action.AccountAction
 import mozilla.lockbox.action.DataStoreAction

diff --git a/app/src/main/java/mozilla/lockbox/store/DataStore.kt b/app/src/main/java/mozilla/lockbox/store/DataStore.kt
@@ -31,7 +31,6 @@ import mozilla.lockbox.support.DataStoreSupport
 import mozilla.lockbox.support.FxASyncDataStoreSupport
 import mozilla.lockbox.support.Optional
 import mozilla.lockbox.support.TimingSupport
-import mozilla.lockbox.support.asOptional
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.TimeoutException
 import kotlin.coroutines.CoroutineContext
@@ -55,7 +54,7 @@ open class DataStore(
     }
 
     enum class SyncState {
-        Syncing, NotSyncing, TimedOut
+        Syncing, NotSyncing
     }
 
     internal val compositeDisposable = CompositeDisposable()
@@ -144,7 +143,9 @@ open class DataStore(
 
     open fun get(id: String): Observable<Optional<ServerPassword>> {
         return list.map { items ->
-            items.findLast { item -> item.id == id }.asOptional()
+            Optional(
+                items.findLast { item -> item.id == id }
+            )
         }
     }
 
@@ -211,25 +212,29 @@ open class DataStore(
     }
 
     private fun sync() {
-        val syncConfig = support.syncConfig ?: return {
-            val throwable = IllegalStateException("syncConfig should already be defined")
-            pushError(throwable)
-        }()
+        resetSupport(support)
 
         // ideally, we don't sync unless we are connected to the network
         syncStateSubject.accept(SyncState.Syncing)
 
-        backend.sync(syncConfig)
+        backend.sync(support.syncConfig!!)
             .asSingle(coroutineContext)
             .timeout(Constant.App.syncTimeout, TimeUnit.SECONDS)
             .doOnEvent { _, err ->
                 (err as? TimeoutException).let {
-                    syncStateSubject.accept(SyncState.TimedOut)
+                    // syncStateSubject.accept(SyncState.TimedOut)
+                    dispatcher.dispatch(DataStoreAction.SyncTimeout(it?.message ?: ""))
                 }.run {
                     syncStateSubject.accept(SyncState.NotSyncing)
                 }
             }
-            .subscribe(this::updateList, this::pushError)
+            .subscribe({
+                    this.updateList(it)
+                    dispatcher.dispatch(DataStoreAction.SyncEnd)
+            }, {
+                    this.pushError(it)
+                    dispatcher.dispatch(DataStoreAction.SyncError(it.message ?: ""))
+            })
             .addTo(compositeDisposable)
     }
 
@@ -245,7 +250,13 @@ open class DataStore(
         if (!backend.isLocked()) {
             backend.list()
                 .asSingle(coroutineContext)
-                .subscribe(listSubject::accept, this::pushError)
+                .subscribe({
+                    listSubject.accept(it)
+                    dispatcher.dispatch(DataStoreAction.ListUpdate)
+                }, {
+                    this.pushError(it)
+                    dispatcher.dispatch(DataStoreAction.ListUpdateError(it.message ?: ""))
+                })
                 .addTo(compositeDisposable)
         }
     }
@@ -297,11 +308,17 @@ open class DataStore(
         val loginsException = e as? LoginsStorageException
 
         loginsException?.let {
-            this.stateSubject.accept(DataStore.State.Errored(it))
+            this.stateSubject.accept(State.Errored(it))
         }
 
         when (loginsException) {
-            is SyncAuthInvalidException, is InvalidKeyException -> dispatcher.dispatch(LifecycleAction.UserReset)
+            is SyncAuthInvalidException,
+            is InvalidKeyException,
+            is LoginsStorageException -> {
+                dispatcher.dispatch(DataStoreAction.Errors(e.message ?: ""))
+                resetSupport(support)
+                dispatcher.dispatch(LifecycleAction.UserReset)
+            }
         }
     }
 

diff --git a/app/src/main/java/mozilla/lockbox/support/Constant.kt b/app/src/main/java/mozilla/lockbox/support/Constant.kt
@@ -36,6 +36,12 @@ object Constant {
         val scopes = setOf(profileScope, lockboxScope, oldSyncScope)
     }
 
+    object FxAErrors {
+        const val SyncAuthInvalid = "SyncAuthInvalidException"
+        const val InvalidKey = "InvalidKeyException"
+        const val LoginsStorage = "LoginsStorageException"
+    }
+
     object Faq {
         const val uri = "https://lockbox.firefox.com/faq.html"
         const val topUri = uri + "#top"

diff --git a/app/src/main/java/mozilla/lockbox/support/Optional.kt b/app/src/main/java/mozilla/lockbox/support/Optional.kt
@@ -7,4 +7,4 @@
 package mozilla.lockbox.support
 
 data class Optional<T>(val value: T?)
-fun <T> T?.asOptional() = Optional(this)
+fun <T> T?.asOptional(): Optional<T> = Optional(this)