Automatically sync with server when Match Exactly enabled #3960
Conversation
seadowg
force-pushed
the
auto-match-exactly
branch
4 times, most recently
from
5fde62f to
14ec8de
Compare
|
|
||
| class AudioPlayerViewModel extends ViewModel implements MediaPlayer.OnCompletionListener { | ||
|
|
||
| private final MediaPlayerFactory mediaPlayerFactory; | ||
| private final Supplier<MediaPlayer> mediaPlayerFactory; |
There was a problem hiding this comment.
I ran into a problem with the audio stuff due to it instantiating a Scheduler so had to do a slight rework. I like the idea of using Supplier instead of an explicit ...Factory or ...Provider type now the new desugaring gives us access to it. We might still need a concrete class for Dagger but everything else should be able to use Supplier.
There was a problem hiding this comment.
Sounds good. Add a note in the "state of the union"?
| @@ -13,6 +14,10 @@ private MultiClickGuard() { | |||
|
|
|||
| // Debounce multiple clicks within the same screen | |||
| public static boolean allowClick(String className) { | |||
| if (test) { | |||
There was a problem hiding this comment.
I ran into problems with this blocking clicks in tests locally
And make MatchExactly test work with spike WorkManager implementation
This was being used to check media files when a form has hadn't changed. As the form download will always go ahead and download the manifest and any new/changed files there was no reason to do this.
seadowg
force-pushed
the
auto-match-exactly
branch
from
14ec8de to
20b3a11
Compare
async/src/main/java/org/odk/collect/async/CoroutineAndWorkManagerScheduler.kt
Show resolved
Hide resolved
collect_app/src/main/java/org/odk/collect/android/formmanagement/ServerFormDetails.java
Outdated
Show resolved
Hide resolved
|
|
||
| class AudioPlayerViewModel extends ViewModel implements MediaPlayer.OnCompletionListener { | ||
|
|
||
| private final MediaPlayerFactory mediaPlayerFactory; | ||
| private final Supplier<MediaPlayer> mediaPlayerFactory; |
There was a problem hiding this comment.
Sounds good. Add a note in the "state of the union"?
...ct_app/src/test/java/org/odk/collect/android/formmanagement/BlankFormsListViewModelTest.java
Outdated
Show resolved
Hide resolved
collect_app/src/main/java/org/odk/collect/android/backgroundwork/BackgroundWorkManager.java
Show resolved
Hide resolved
...ct_app/src/test/java/org/odk/collect/android/formmanagement/ServerFormsSynchronizerTest.java
Show resolved
Hide resolved
|
@seadowg can you please keep a running list of sources of risk so that QA can do a regression check once the feature is complete? Here I see:
|
|
@lognaturel that's a good shout. I'll keep a list in my local notes and then post when we get to doing QA on |
Closes #3939
What has been done to verify that this works as intended?
Lots of new tests and played with automatic sync manually on the emulator.
Why is this the best possible solution? Were any other approaches considered?
The biggest decision here was whether to just use
WorkManageras is or include it in ourasync.Schedulerabstraction. I ended up going with the latter as it means we can write tests for the automatic sync we wouldn't be able to write otherwise (like those found inMatchExactlyTest). This does mean we end up with a weighty abstraction inTaskSpec. I'm planning the other background jobs to using this as part of #3940.How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
As with previous Match Exactly PRs we're going to skip QA on this and wait until the feature is complete.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.