From 7c5dfd93cc73f145185d1d258736a398c88ef452 Mon Sep 17 00:00:00 2001 From: Sasikanth Miriyampalli Date: Wed, 22 Nov 2023 08:48:43 +0530 Subject: [PATCH 1/4] Remove `Continuator#ContinueToScreen_Old` We are no longer using this continuation --- .../facility/alertchange/AlertFacilityChangeSheet.kt | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/app/src/main/java/org/simple/clinic/facility/alertchange/AlertFacilityChangeSheet.kt b/app/src/main/java/org/simple/clinic/facility/alertchange/AlertFacilityChangeSheet.kt index da21d33053f..b2cda557a30 100644 --- a/app/src/main/java/org/simple/clinic/facility/alertchange/AlertFacilityChangeSheet.kt +++ b/app/src/main/java/org/simple/clinic/facility/alertchange/AlertFacilityChangeSheet.kt @@ -14,7 +14,6 @@ import org.simple.clinic.R import org.simple.clinic.databinding.SheetAlertFacilityChangeBinding import org.simple.clinic.di.injector import org.simple.clinic.facility.alertchange.Continuation.ContinueToActivity -import org.simple.clinic.facility.alertchange.Continuation.ContinueToScreen_Old import org.simple.clinic.facility.change.FacilityChangeScreen import org.simple.clinic.feature.Features import org.simple.clinic.mobius.ViewRenderer @@ -22,8 +21,6 @@ import org.simple.clinic.navigation.v2.Router import org.simple.clinic.navigation.v2.ScreenKey import org.simple.clinic.navigation.v2.ScreenResultBus import org.simple.clinic.navigation.v2.Succeeded -import org.simple.clinic.navigation.v2.compat.FullScreenKey -import org.simple.clinic.navigation.v2.compat.wrap import org.simple.clinic.navigation.v2.fragments.BaseBottomSheet import org.simple.clinic.util.resolveFloat import org.simple.clinic.util.setFragmentResultListener @@ -142,10 +139,6 @@ class AlertFacilityChangeSheet : private fun closeSheetWithContinuation() { when (continuation) { - is ContinueToScreen_Old -> { - val screenKey = (continuation as ContinueToScreen_Old).screenKey - router.replaceTop(screenKey.wrap()) - } is ContinueToActivity -> { val (intent, requestCode) = (continuation as ContinueToActivity).run { intent to requestCode @@ -191,9 +184,6 @@ class AlertFacilityChangeSheet : sealed class Continuation : Parcelable { - @Parcelize - data class ContinueToScreen_Old(val screenKey: FullScreenKey) : Continuation() - @Parcelize data class ContinueToScreen(val screenKey: ScreenKey) : Continuation() From f7687b3222923e139131fe2e38d491844f9124e0 Mon Sep 17 00:00:00 2001 From: Sasikanth Miriyampalli Date: Wed, 22 Nov 2023 09:12:22 +0530 Subject: [PATCH 2/4] Check if the new request matches the current top request when replacing top in `Router` We don't have any flows that replace the current screen with same screen, so added a check that prevents replacing current key with same key. This is to prevent removing the last key when there is only one key left. --- .../java/org/simple/clinic/navigation/v2/Router.kt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/simple/clinic/navigation/v2/Router.kt b/app/src/main/java/org/simple/clinic/navigation/v2/Router.kt index 3ea2d601a4b..d571bbfed41 100644 --- a/app/src/main/java/org/simple/clinic/navigation/v2/Router.kt +++ b/app/src/main/java/org/simple/clinic/navigation/v2/Router.kt @@ -90,9 +90,14 @@ class Router( } fun replaceTop(screenKey: ScreenKey, transactionOptions: TransactionOptions? = null) { + val navRequest = Normal(screenKey) + if (history.matchesTop(navRequest)) { + return + } + val newHistory = history .removeLast() - .add(Normal(screenKey)) + .add(navRequest) executeStateChange(newHistory, Direction.Replace, null, transactionOptions) } @@ -102,9 +107,14 @@ class Router( screenKey: ScreenKey, transactionOptions: TransactionOptions? = null ) { + val navRequest = ExpectingResult(requestType, screenKey) + if (history.matchesTop(navRequest)) { + return + } + val newHistory = history .removeLast() - .add(ExpectingResult(requestType, screenKey)) + .add(navRequest) executeStateChange(newHistory, Direction.Forward, null, transactionOptions) } From 83c98c3ffc9cacac37eab63ed54f0361e85fc3ff Mon Sep 17 00:00:00 2001 From: Sasikanth Miriyampalli Date: Wed, 22 Nov 2023 08:58:06 +0530 Subject: [PATCH 3/4] Use analytics name when throwing remove last error Since certain screen keys can contain potential PII, we are just including the analytics name in the error. --- app/src/main/java/org/simple/clinic/navigation/v2/History.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/simple/clinic/navigation/v2/History.kt b/app/src/main/java/org/simple/clinic/navigation/v2/History.kt index 8fa56aefcd0..762fb8f91d2 100644 --- a/app/src/main/java/org/simple/clinic/navigation/v2/History.kt +++ b/app/src/main/java/org/simple/clinic/navigation/v2/History.kt @@ -23,7 +23,7 @@ data class History(val requests: List) : Parcelable { fun removeLast(): History { require(requests.size > 1) { val lastKey = requests.lastOrNull()?.key - "Cannot remove last key ($lastKey) when there is only one key left" + "Cannot remove last key (${lastKey?.analyticsName}) when there is only one key left" } return copy(requests = requests.dropLast(1)) From 4ba7271df9742318c4cf890031733436ebdae8f9 Mon Sep 17 00:00:00 2001 From: Sasikanth Miriyampalli Date: Wed, 22 Nov 2023 09:08:59 +0530 Subject: [PATCH 4/4] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e66e15b85b..a13f476cd07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # CHANGELOG +## Next Release + +### Fixes + +- Fix app crashing when trying to replacing the last screen during navigation + ## 2023-11-20-8926 ### Internal