ground-android
ground-android copied to clipboard
[Code health] Fix duplicate / unclear EditObservation* flows
There are several problems with the lifecycle of EditObservationFragment and EditObservationViewModel while working on #1077:
initialize()is called when the ViewModel is being restored, even if it was already initialized. As a result, the Form is re-rendered multiple times: once when the ViewModel is observed, and once when the Observation reloads.- There are also several click event streams which use LiveData, which replays events which could lead to unexpected behaviors.
- New subscriptions are added each time the form is rebuilt, but are only disposed when the Fragment is destroyed. This causes a leak, and for event handlers to be called multiple times (once for each subscription created on each config change). One approach would be to add them to a
CompositeDisposablethat gets cleared each time the form is rebuilt. - How the
formLiveData is derived from the initialize stream is unintuitive; might be better to rely on side effects in this case (MutableLiveData.setValue()).
@shobhitagarwal1612 FYI.
Something we may also want to consider is moving the sync parts of the async flow into their own components. For example, if we could have view and view model contained in EditObservationFragment that is only created once the observation is loaded, we could simplify some of the async flows.
Also, ideally we would also persist the UI layout in the saved instance state state rather than reloading on each config change.
Confirmed that rebuildForm is called twice on changing screen orientation.
@JSunde @shobhitagarwal1612 Is this still relevant?
I think we can mark this as obsolete too. We haven't removed EditObservationFragment in a single shot but are constantly chipping away pieces from it when we implement new features to the redesigned data collection flow. Eventually, all of the bugs related to the previous edit/view observation flow should automatically get cleaned up.
I haven't necessarily been deleting things from the EditSubmissionFragment as I've been replicating the functionality in the new Data Collection flow. Would it be worth going in and deleting the functionality that we have working in the new flow, while keeping around anything remaining for context as we continue to build out the newer experience?
Sounds good to me. We can create a list of functionality pending in the current implementation and remove the old code.
@JSunde Can we close this now?