[WIP] Select a start screen based upon datastore state. #169

jhugman · 2018-10-25T13:22:18Z

Fixes #144
Fixed #239
Depends on #22
Depends on #68

This PR continues to split up the graph into subgraphs, but starts sending UI actions through the DataStore.

devinreams · 2018-10-25T15:25:46Z

Holding on #22 and #68

RouteActions will be generated by the route presenter, in response to DataStore observables.

This won't be useful/testable until we start with the FxA login #22 and locking #68.

sashei · 2018-11-10T02:11:23Z

Ok!!!

I've taken a look at the integration tests & identified a few things that are going on here. The fixes / changes I've implemented are kind of hacky, so I'm going to push them up in a separate branch and you can take a look at them before you decide what you'd like to incorporate @jhugman .

I think my lack of expertise with the nav graph stuff is hindering getting this 💯 over the finish line, but I have at least introduced a new set of failures!! Maybe this will give you some ideas :)

Solutions:

I disabled animations in the tests by setting animationsDisabled = true in the build.gradle file, which I feel is a good idea but had no actual bearing on getting any of the integration tests to pass consistently.

Re-populating the item list when using in-memory Logins storage

All of the tests related to the ItemDetail screen were failing because after we reset the DataStore once, we never re-populated it, so it was impossible to tap on anything in the item list. The item detail and filter tests failed consistently, so I made sure to call updateList in the reset block of the DataStore for in-memory logins only.

Removing `graph_welcome` as the `startDestination`

The app was routing to the Welcome screen twice in many cases, first because it is the startDestination screen in the nav graph, and second because that's where it goes when a DataStoreAction.Reset comes down the queue. Many of the tests failed when looking for something related to the FxA login screen, because they had found the Get Started button & tapped on it but were promptly scooted back to the Welcome screen when the datastore finished resetting.

To fix that issue, I created an fragment_app_startup_placeholder that is displayed, as the name might indicate, as the startDestination in the graph_main. The RoutePresenter then displays the Welcome screen as dictated by data store state, not in relation to the nav graph at all.

I'll note that this solution is the one needing the most attention (see lack of expertise disclaimer above 😅), because the tests that are still failing do so when they try to tap on buttonGetStarted before anything is displayed.

devinreams · 2018-11-28T16:35:37Z

Will rebase against latest version, test, and then get reviewed...

ioana-farcas

Quick review: Code-wise looks great :) I was not able to see it working, maybe because of the sync issue. The list was not displayed and Lock Now was not displaying Lock screen.

ioana-farcas · 2018-11-29T10:23:14Z

app/src/main/java/mozilla/lockbox/presenter/LockedPresenter.kt

-                    dispatcher.dispatch(RouteAction.FingerprintDialog)
-                } else {
-                    unlockFallback()
+                when {


ioana-farcas · 2018-11-29T10:23:31Z

app/src/main/java/mozilla/lockbox/presenter/LockedPresenter.kt

                }
            }
            .addTo(compositeDisposable)

        lockedStore.onAuthentication
+            .filterByType(FingerprintAuthAction.OnAuthentication::class.java)


ioana-farcas · 2018-11-29T10:26:11Z

app/src/main/java/mozilla/lockbox/presenter/RoutePresenter.kt

@@ -139,22 +157,22 @@ class RoutePresenter(
        // This maps two nodes in the graph_main.xml to the edge between them.
        // If a RouteAction is called from a place the graph doesn't know about then
        // the app will log.error.
-        when (Pair(from, to)) {
-            Pair(R.id.fragment_welcome, R.id.fragment_fxa_login) -> return R.id.action_welcome_to_fxaLogin
+        return when (Pair(from, to)) {


ioana-farcas · 2018-11-29T10:33:32Z

app/src/main/java/mozilla/lockbox/store/DataStore.kt

@@ -62,6 +63,7 @@ open class DataStore(
                        is DataStoreAction.Lock -> lock()
                        is DataStoreAction.Unlock -> unlock()
                        is DataStoreAction.Sync -> sync()
+                        is DataStoreAction.Reset -> reset()


Reset Action is only for test purpose? Did not see it dispatched from anywhere else beside test.

jhugman · 2018-11-30T16:24:12Z

Closing in favour of a new one.

jhugman requested a review from a team as a code owner October 25, 2018 13:22

ghost assigned jhugman Oct 25, 2018

ghost added the in progress label Oct 25, 2018

sashei mentioned this pull request Nov 2, 2018

FxA login #178

Merged

5 tasks

jhugman force-pushed the jhugman/144-start-screen-based-on-state branch from 53752c5 to 7837587 Compare November 6, 2018 15:20

jhugman added 12 commits November 7, 2018 16:38

Set the graph programmatically

0bf6d9d

Send lock and unlock actions instead of RouteActions.

c75ed80

RouteActions will be generated by the route presenter, in response to DataStore observables.

Enable routes to resond to locked, unlocked and welcome screens.

ee5d845

This won't be useful/testable until we start with the FxA login #22 and locking #68.

Move welcome flow into its own separate flow.

a8c485c

Send unlock actions via the datastore, rather than the router

6593316

Remove RouteAction.Back.

5cc5a40

Fixup tests following rebase

4deefc5

Hard code graph for the navhost.

0508f8b

Add DataStore reset

7ff53f9

Fun fact: espresso tests are not black box tests.

0f556cc

Add unlocking state.

19e6baf

Tidy error messages.

cf32395

jhugman force-pushed the jhugman/144-start-screen-based-on-state branch from 7837587 to cf32395 Compare November 8, 2018 20:06

sandysage mentioned this pull request Nov 9, 2018

Incorporate remaining Login Storage methods from android-components #40

Closed

4 tasks

sashei mentioned this pull request Nov 13, 2018

implement touch #206

Merged

6 tasks

devinreams requested a review from ioana-farcas November 28, 2018 16:35

ioana-farcas reviewed Nov 29, 2018

View reviewed changes

jhugman closed this Nov 30, 2018

ghost removed the in progress label Nov 30, 2018

jhugman mentioned this pull request Nov 30, 2018

[WIP] Select a start screen based upon datastore state. #249

Merged

6 tasks

devinreams deleted the jhugman/144-start-screen-based-on-state branch December 13, 2018 17:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Select a start screen based upon datastore state. #169

[WIP] Select a start screen based upon datastore state. #169

jhugman commented Oct 25, 2018 •

edited by devinreams

Loading

devinreams commented Oct 25, 2018

sashei commented Nov 10, 2018 •

edited

Loading

devinreams commented Nov 28, 2018

ioana-farcas left a comment

ioana-farcas Nov 29, 2018

ioana-farcas Nov 29, 2018

ioana-farcas Nov 29, 2018

ioana-farcas Nov 29, 2018

jhugman commented Nov 30, 2018

[WIP] Select a start screen based upon datastore state. #169

[WIP] Select a start screen based upon datastore state. #169

Conversation

jhugman commented Oct 25, 2018 • edited by devinreams Loading

devinreams commented Oct 25, 2018

sashei commented Nov 10, 2018 • edited Loading

Solutions:

Re-populating the item list when using in-memory Logins storage

Removing graph_welcome as the startDestination

devinreams commented Nov 28, 2018

ioana-farcas left a comment

Choose a reason for hiding this comment

ioana-farcas Nov 29, 2018

Choose a reason for hiding this comment

ioana-farcas Nov 29, 2018

Choose a reason for hiding this comment

ioana-farcas Nov 29, 2018

Choose a reason for hiding this comment

ioana-farcas Nov 29, 2018

Choose a reason for hiding this comment

jhugman commented Nov 30, 2018

jhugman commented Oct 25, 2018 •

edited by devinreams

Loading

sashei commented Nov 10, 2018 •

edited

Loading

Removing `graph_welcome` as the `startDestination`