Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) #13

ssawchenko · 2021-09-28T02:04:45Z

Summary of Problem:

Android project had some initial work done on it, but didn't end up getting fully flushed out.

Sorry, there's a few issues being addressed here all at the same time, I should have broken them up but just kept going down rabbit holes and they were somewhat intertwined.

Proposed Solution:

This PR addresses the following tickets:

This PR adds:

PlatformVersionData, VersionData, Status and DisplayState data models; named to match iOS as close as possible
URLFetcher interface and a default implementation that pulls down a string from a given URL
VersionCheck contains flows for exposing both the display and status states; it can also be setup as a lifecycle listener to automatically run checks "on start"
DefaultUpgradeDialog collects display flow from VersionCheck and handles creation of a default alert dialog; it should be setup as an activity lifecycle listener to automatically handle the show/dismiss of the dialog.
VersionDataConverter interface and a default implementation to parse JSON into a VersionData object; to avoid having to import specific parsing libraries, this is done with basic JSONObject and JSONArray manual parsing.
Unit tests

Setup and Usage

Currently, to add version checking at the application level, setup is as follows (the MainActivity is still playing around with using the ViewModel as a proof of concept, but in practice I am guessing we'd be applying this at the application level)

class App: Application(), LifecycleObserver {

    override fun onCreate() {
        super.onCreate()
        setupVersionCheck()
    }

    private fun setupVersionCheck() {
        val versionChecker = VersionCheck(
            VersionCheckConfig(
              appVersionName = BuildConfig.VERSION_NAME,
              appVersionCode = BuildConfig.VERSION_CODE,
              url = "https://doesn't_matter",
              urlFetcher = MockURLFetcher
          )
        )
        val upgradeDialog = DefaultUpgradeDialog(versionChecker.displayStateFlow)

        ProcessLifecycleOwner.get().lifecycle.addObserver(versionChecker)
        registerActivityLifecycleCallbacks(upgradeDialog)
    }
}

Testing Steps:

Run sample app
Default dialog should automatically pop up indicating that an upgrade is required

Still todo:

Allow "Upgrade" button to launch Play Store #14
Add support for latestTestVersion #10
Test URLFetcher with actual live URL; right now all tests have been done by using a mock URLFetcher that is loading in test data.

* Moved Version into models package

…t; added more unit tests

jacobminer

My biggest concern with using a viewModel is that I think it means we lose the ability to validate the version from the App class, and to replicate it we'd need to make sure that every activity in the app is calling runVersionCheck in the onResume via the ViewModel. I think we could get around that and keep most of this functionality if we refactored out the runVersionCheck into a separate Repository which could be used at the app level directly, but keep the VersionCheckViewModel, and have it monitor the repository instead.

So it might looks something like:
VersionCheckRepository

Contains a shared flow of Status
has runVersionCheck

VersionCheckViewModel

monitors the VersionCheckRepository's flow, converts it to LiveData for observers

Activity/Fragment can monitor VersionCheckViewModel (essentially the same)

App can call runVersionCheck via the VersionCheckRepository if necessary

jacobminer · 2021-09-28T21:29:16Z

versioncheckkotlin/src/main/java/com/steamclock/versioncheckkotlin/interfaces/UpgradeDialog.kt

+}
+
+class DefaultUpgradeDialog(
+    private val activity: Activity): UpgradeDialog, LifecycleObserver {


Will this leak activity?

Hrm, good question. I didn't run leak canary, but this felt like the only way to get access to the activity to get context to show the dialog. Maybe I could extract this out by having a delegate/interface that the Activity or App implements to give context or create the dialog. I'll ponder that a little more.

This should no longer be an issue.

jacobminer · 2021-09-28T21:29:30Z

versioncheckkotlin/src/main/java/com/steamclock/versioncheckkotlin/interfaces/UpgradeDialog.kt

+                                  requiresUpdate: Boolean,
+                                  canDismiss: Boolean) {
+
+        val builder = AlertDialog.Builder(activity).apply {


Should we check if activity is still active here?

This should no longer be an issue

...eckkotlin/src/main/java/com/steamclock/versioncheckkotlin/interfaces/VersionDataConverter.kt

ssawchenko · 2021-09-28T23:18:48Z

My biggest concern with using a viewModel is that I think it means we lose the ability to validate the version from the App class, and to replicate it we'd need to make sure that every activity in the app is calling runVersionCheck in the onResume via the ViewModel. I think we could get around that and keep most of this functionality if we refactored out the runVersionCheck into a separate Repository which could be used at the app level directly, but keep the VersionCheckViewModel, and have it monitor the repository instead.

So it might looks something like:
VersionCheckRepository

Contains a shared flow of Status
has runVersionCheck
VersionCheckViewModel

monitors the VersionCheckRepository's flow, converts it to LiveData for observers
Activity/Fragment can monitor VersionCheckViewModel (essentially the same)

App can call runVersionCheck via the VersionCheckRepository if necessary

Yeah, I agree with you on these points for sure. I went this route for a few reasons:

I figured we could add this to a BaseActivity class to easily get the version checking across all pages
I wanted to make the required setup code as small as possible (ie. avoiding having to listen for the activity lifecycles in the Application object, but maybe that's just the way it has to be. I'll play around with that option shortly.

ssawchenko · 2021-09-29T19:54:01Z

@jacobminer Ok ready for another round of review! Big refactor here - I have moved the bulk of the logic into the VersionRepository and exposed those updates via state flows. This allows us to setup the version checks easily on the application object. Please see my updated PR description for comments on what the new classes are. I'm still super new to flows, so it is possible there are nuances I have not got correct.

Let me know if this is closer to what you were thinking!

Edit: This broke some of my unit tests, but once we have accepted a final implementation I will update them to use the repositories instead.

ssawchenko · 2021-09-29T19:50:21Z

versioncheckkotlin/src/main/java/com/steamclock/versioncheckkotlin/VersionChecker.kt

+        // Looking at documentation we may be able to trigger the update with each collection so
+        // we do not actually have to call this manually? <-- todo
+        applicationScope.launch(Dispatchers.IO) {
+            // Delay to show that we do get the initial state value before we call the update


Need to remove this before committing this is mostly for testing right now

No longer an issue

ssawchenko · 2021-09-29T19:57:03Z

versioncheckkotlin/src/main/java/com/steamclock/versioncheckkotlin/VersionChecker.kt

+            // repeatOnLifecycle(Lifecycle.State.STARTED) as our collection scope.
+            versionRepository.displayStateFlow.collect {
+                upgradeDialog.show(activity, it)
+                Toast.makeText(activity, it.toString(), Toast.LENGTH_LONG).show()


todo: Remove toast before releasing

No longer an issue

…ects flow and reacts to activity lifecycle changes

ssawchenko · 2021-09-29T23:29:04Z

@jacobminer Ok, 3rd time's the charm? Hopefully my changes reflect what you were suggesting in our talk. I have

Moved flow logic into VersionCheck (repository is gone); this class can be setup as a process lifecycle observer if desired to automatically trigger the version check on app "start".
Changed DefaultUpgradeDialog to take the displayState flow; this class now handles activity lifecycle changes regarding the dialog.
Nuked the ViewModel; as it is no longer really useful.
Added a config object since I think we may be able to create/inject it nicer using koin.

ssawchenko · 2021-09-29T23:40:35Z

versioncheckkotlin/src/test/java/com/steamclock/versioncheckkotlin/utils/TestUtils.kt

@@ -0,0 +1,55 @@
+package com.steamclock.versioncheckkotlin.utils
+


I can probably remove this file now that we are no longer using LiveData; when I update the test scripts I will look into this

ssawchenko · 2021-09-29T23:41:02Z

versioncheckkotlin/src/test/java/com/steamclock/versioncheckkotlin/VersionCheckTest.kt

@@ -0,0 +1,96 @@
+//import androidx.arch.core.executor.testing.InstantTaskExecutorRule
+//import com.steamclock.versioncheckkotlin.utils.TestConstants
+//import com.steamclock.versioncheckkotlin.VersionCheckViewModel


Will update this to test VersionCheck once we have finalized implementation

Updated with new unit test

ssawchenko · 2021-09-29T23:47:29Z

app/src/main/java/com/steamclock/versioncheckkotlinsample/App.kt

+        )
+        val upgradeDialog = DefaultUpgradeDialog(versionChecker.displayStateFlow)
+
+        ProcessLifecycleOwner.get().lifecycle.addObserver(versionChecker)


I went back and looked at the Swift library and I now see what you mean about how they are listening for the application events.

I'm totally fine with pulling out the lifecycle listener code out of VersionCheck and simply making the app itself handle the lifecycle observation manually. Thoughts?

Either way works for me! From an app standpoint, it's pretty convenient to use it this way, but the runVersionCheck function is also exposed, so it could be used either way. Maybe leave it as is, then note in the docs that you can choose to observe the lifecycle in the app and call runVersionCheck or call ProcessLifecycleOwner.get().lifecycle.addObserver(versionChecker) depending on what behaviour best fits the project?

jacobminer

Looks great! 🚀

versioncheckkotlin/src/main/java/com/steamclock/versioncheckkotlin/VersionCheckViewModel.kt

app/src/main/java/com/steamclock/versioncheckkotlinsample/MainActivity.kt

jacobminer · 2021-09-30T00:15:41Z

app/src/main/java/com/steamclock/versioncheckkotlinsample/App.kt

+        )
+        val upgradeDialog = DefaultUpgradeDialog(versionChecker.displayStateFlow)
+
+        ProcessLifecycleOwner.get().lifecycle.addObserver(versionChecker)


Either way works for me! From an app standpoint, it's pretty convenient to use it this way, but the runVersionCheck function is also exposed, so it could be used either way. Maybe leave it as is, then note in the docs that you can choose to observe the lifecycle in the app and call runVersionCheck or call ProcessLifecycleOwner.get().lifecycle.addObserver(versionChecker) depending on what behaviour best fits the project?

jacobminer · 2021-10-01T16:41:56Z

Sorry, it looks like my comments from the previous version got included in this PR review. Disregard those!

ssawchenko added 11 commits September 27, 2021 14:19

Added VersionData and VersionDataConveter (and test)

3fcbc8c

Added DisplayState and Status data classes

62d1847

Added URLFetcher interface and NetworkURLFetcher implementation

0cb1f9e

Added VersionCheckViewModel and basic tests

b1c2afc

* Moved Version into models package

Added UpgradeDialog and default implementation

6cdb064

Updated dialog text; suppress ability for user to close dialog

8d5c27f

Moved library versions to project gradle

2c7b8c5

Removed unused libraries

e34acaa

Fixed incorrect logic when determining if build is in the blocked lis…

5ca9af0

…t; added more unit tests

Fixed containsBlockedVersion logic and added test case

56adc43

Removed comments

bd8dbc6

ssawchenko requested a review from jacobminer September 28, 2021 19:59

ssawchenko assigned jacobminer Sep 28, 2021

ssawchenko changed the title ~~[WIP] Ss/adding main functionality~~ Added VersionCheckViewModel and support for fetching, parsing, verifying data and showing a default dialog (#5, #6, #7) Sep 28, 2021

ssawchenko changed the title ~~Added VersionCheckViewModel and support for fetching, parsing, verifying data and showing a default dialog (#5, #6, #7)~~ Added VersionCheckViewModel and support for verifying data and showing a default dialog (#5, #6, #7) Sep 28, 2021

jacobminer reviewed Sep 28, 2021

View reviewed changes

jacobminer assigned ssawchenko and unassigned jacobminer Sep 28, 2021

ssawchenko added 3 commits September 29, 2021 12:30

Added flows via repository; viewmodel now broken

f098f23

Extracted activity lifecycle handling to VersionCheck class

9758bd7

Updated viewmodel; commented out now broken test case

1b8ea43

ssawchenko assigned jacobminer and unassigned ssawchenko Sep 29, 2021

ssawchenko commented Sep 29, 2021

View reviewed changes

ssawchenko added 2 commits September 29, 2021 16:20

Removed repository, VersionCheck now exposes flows; UpradeDialog coll…

4007435

…ects flow and reacts to activity lifecycle changes

Added comment

6ef822f

ssawchenko requested a review from jacobminer September 29, 2021 23:29

ssawchenko added 3 commits September 29, 2021 16:35

Reset needToShowDialog

206b521

Fixed double dialog show

87a03a6

Removed old comment

c345390

ssawchenko commented Sep 29, 2021

View reviewed changes

jacobminer approved these changes Sep 30, 2021

View reviewed changes

jacobminer assigned ssawchenko and unassigned jacobminer Oct 1, 2021

ssawchenko added 2 commits October 1, 2021 13:10

Updating unit tests

5f88505

Removed old functions

e383359

ssawchenko changed the title ~~Added VersionCheckViewModel and support for verifying data and showing a default dialog (#5, #6, #7)~~ Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) Oct 1, 2021

ssawchenko merged commit 1c51994 into main Oct 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) #13

Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) #13

ssawchenko commented Sep 28, 2021 •

edited

Loading

jacobminer left a comment

jacobminer Sep 28, 2021

ssawchenko Sep 28, 2021

ssawchenko Sep 29, 2021

jacobminer Sep 28, 2021

ssawchenko Sep 29, 2021

ssawchenko commented Sep 28, 2021 •

edited

Loading

ssawchenko commented Sep 29, 2021 •

edited

Loading

ssawchenko Sep 29, 2021

ssawchenko Oct 1, 2021

ssawchenko Sep 29, 2021

ssawchenko Oct 1, 2021

ssawchenko commented Sep 29, 2021 •

edited

Loading

ssawchenko Sep 29, 2021

ssawchenko Oct 1, 2021

ssawchenko Sep 29, 2021

ssawchenko Oct 1, 2021

ssawchenko Sep 29, 2021 •

edited

Loading

jacobminer Sep 30, 2021

jacobminer left a comment

jacobminer Sep 30, 2021

jacobminer commented Oct 1, 2021

		@@ -0,0 +1,55 @@
		package com.steamclock.versioncheckkotlin.utils

Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) #13

Added VersionCheck class and support for verifying data and showing a default dialog (#5, #6, #7) #13

Conversation

ssawchenko commented Sep 28, 2021 • edited Loading

Summary of Problem:

Proposed Solution:

Setup and Usage

Testing Steps:

Still todo:

jacobminer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssawchenko commented Sep 28, 2021 • edited Loading

ssawchenko commented Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssawchenko commented Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssawchenko Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobminer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobminer commented Oct 1, 2021

ssawchenko commented Sep 28, 2021 •

edited

Loading

ssawchenko commented Sep 28, 2021 •

edited

Loading

ssawchenko commented Sep 29, 2021 •

edited

Loading

ssawchenko commented Sep 29, 2021 •

edited

Loading

ssawchenko Sep 29, 2021 •

edited

Loading