From 4611e2edb4ba2ee23dbf4a85a38a8879a4edff42 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Mon, 24 Feb 2025 12:50:11 +1100 Subject: [PATCH 1/8] prioritiseStops based on priority order --- .../ui/searchstop/SearchStopViewModel.kt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index f2193550..00b1ae82 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -22,6 +22,7 @@ import xyz.ksharma.krail.sandook.Sandook import xyz.ksharma.krail.sandook.SelectProductClassesForStop import xyz.ksharma.krail.trip.planner.network.api.service.TripPlanningService import xyz.ksharma.krail.trip.planner.ui.state.TransportMode +import xyz.ksharma.krail.trip.planner.ui.state.TransportModeSortOrder import xyz.ksharma.krail.trip.planner.ui.state.searchstop.SearchStopState import xyz.ksharma.krail.trip.planner.ui.state.searchstop.SearchStopUiEvent @@ -73,7 +74,7 @@ class SearchStopViewModel( } val stopResults = resultsDb.map { it.toStopResult() - } + }.let(::prioritiseStops) updateUiState { displayData(stopResults) } }.getOrElse { @@ -84,6 +85,21 @@ class SearchStopViewModel( } } + // TODO - move to another file and add UT for it. Inject and use. + private fun prioritiseStops(stopResults: List): List { + val sortedTransportModes = TransportMode.sortedValues(TransportModeSortOrder.PRIORITY) + val transportModePriorityMap = sortedTransportModes.mapIndexed { index, transportMode -> + transportMode.productClass to index + }.toMap() + + return stopResults.sortedWith(compareBy( + { stopResult -> + stopResult.transportModeType.minOfOrNull { transportModePriorityMap[it.productClass] ?: Int.MAX_VALUE } ?: Int.MAX_VALUE + }, + { it.stopName } + )) + } + private fun SearchStopState.displayData(stopsResult: List) = copy( stops = stopsResult.toImmutableList(), isLoading = false, @@ -104,6 +120,7 @@ class SearchStopViewModel( } } +/// TODO - move to mapper: private fun SelectProductClassesForStop.toStopResult() = SearchStopState.StopResult( stopId = stopId, stopName = stopName, From 84615fe328aae1afa606ddc4b352ce3ee8e110fe Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Mon, 24 Feb 2025 12:54:51 +1100 Subject: [PATCH 2/8] Take 50 at the end after sorting --- .../ui/searchstop/SearchStopViewModel.kt | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 00b1ae82..5fe337af 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -68,13 +68,16 @@ class SearchStopViewModel( sandook.selectStops( stopName = query, excludeProductClassList = emptyList(), - ).take(50) - resultsDb.forEach { - log("resultsDb [$query]: ${it.stopName}") - } - val stopResults = resultsDb.map { - it.toStopResult() - }.let(::prioritiseStops) + ) + /* + resultsDb.forEach { + log("resultsDb [$query]: ${it.stopName}") + } + */ + val stopResults = resultsDb + .map { it.toStopResult() } + .let(::prioritiseStops) + .take(50) updateUiState { displayData(stopResults) } }.getOrElse { @@ -94,7 +97,9 @@ class SearchStopViewModel( return stopResults.sortedWith(compareBy( { stopResult -> - stopResult.transportModeType.minOfOrNull { transportModePriorityMap[it.productClass] ?: Int.MAX_VALUE } ?: Int.MAX_VALUE + stopResult.transportModeType.minOfOrNull { + transportModePriorityMap[it.productClass] ?: Int.MAX_VALUE + } ?: Int.MAX_VALUE }, { it.stopName } )) From 20fd36d7ad92e67fd0a344dd3675bbcc7af02601 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Tue, 25 Feb 2025 20:18:22 +1100 Subject: [PATCH 3/8] Move filter logic to ViewModel rather than SQL --- .../ui/searchstop/SearchStopViewModel.kt | 19 ++++++++++++++----- .../xyz/ksharma/krail/sandook/RealSandook.kt | 5 ++--- .../xyz/ksharma/krail/sandook/Sandook.kt | 4 +--- .../xyz/ksharma/krail/sandook/NswStops.sq | 12 ++++-------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 5fe337af..97f87550 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -17,7 +17,6 @@ import xyz.ksharma.krail.core.analytics.Analytics import xyz.ksharma.krail.core.analytics.AnalyticsScreen import xyz.ksharma.krail.core.analytics.event.AnalyticsEvent import xyz.ksharma.krail.core.analytics.event.trackScreenViewEvent -import xyz.ksharma.krail.core.log.log import xyz.ksharma.krail.sandook.Sandook import xyz.ksharma.krail.sandook.SelectProductClassesForStop import xyz.ksharma.krail.trip.planner.network.api.service.TripPlanningService @@ -65,10 +64,7 @@ class SearchStopViewModel( log("results: $results")*/ val resultsDb: List = - sandook.selectStops( - stopName = query, - excludeProductClassList = emptyList(), - ) + sandook.selectStops(stopName = query) /* resultsDb.forEach { log("resultsDb [$query]: ${it.stopName}") @@ -76,6 +72,12 @@ class SearchStopViewModel( */ val stopResults = resultsDb .map { it.toStopResult() } + .let { + filterProductClasses( + stopResults = it, + excludedProductClasses = listOf(TransportMode.Ferry()) + ) + } .let(::prioritiseStops) .take(50) @@ -125,6 +127,13 @@ class SearchStopViewModel( } } +fun filterProductClasses( + stopResults: List, + excludedProductClasses: Any, +): List { + +} + /// TODO - move to mapper: private fun SelectProductClassesForStop.toStopResult() = SearchStopState.StopResult( stopId = stopId, diff --git a/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/RealSandook.kt b/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/RealSandook.kt index 23eaf464..c23d66a9 100644 --- a/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/RealSandook.kt +++ b/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/RealSandook.kt @@ -123,9 +123,8 @@ internal class RealSandook(factory: SandookDriverFactory) : Sandook { excludeProductClassList: List, ): List { return nswStopsQueries.selectProductClassesForStop( - stopName, - stopName, - productClass = excludeProductClassList.map { it.toLong() }, + stopId = stopName, + stopName = stopName, ).executeAsList() } diff --git a/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/Sandook.kt b/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/Sandook.kt index 7b377f00..6d474038 100644 --- a/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/Sandook.kt +++ b/sandook/src/commonMain/kotlin/xyz/ksharma/krail/sandook/Sandook.kt @@ -55,8 +55,6 @@ interface Sandook { */ fun selectStops( stopName: String, - excludeProductClassList: List = emptyList(), + excludeProductClassList: List, ): List - - // endregion } diff --git a/sandook/src/commonMain/sqldelight/xyz/ksharma/krail/sandook/NswStops.sq b/sandook/src/commonMain/sqldelight/xyz/ksharma/krail/sandook/NswStops.sq index b4e57585..55371a07 100644 --- a/sandook/src/commonMain/sqldelight/xyz/ksharma/krail/sandook/NswStops.sq +++ b/sandook/src/commonMain/sqldelight/xyz/ksharma/krail/sandook/NswStops.sq @@ -42,14 +42,10 @@ selectProductClassesForStop: SELECT s.*, COALESCE(GROUP_CONCAT(p.productClass), '') AS productClasses FROM NswStops AS s -LEFT JOIN NswStopProductClass AS p ON s.stopId = p.stopId +LEFT JOIN NswStopProductClass AS p + ON s.stopId = p.stopId WHERE ( - s.stopId = ? -- Exact match scenario - OR s.stopName LIKE '%' || ? || '%' -- Partial match scenario -) -AND s.stopId NOT IN ( - SELECT stopId - FROM NswStopProductClass - WHERE productClass IN ? + s.stopId = :stopId -- Use named parameter :stopId + OR s.stopName LIKE '%' || :stopName || '%' -- Use named parameter :stopName ) GROUP BY s.stopId; From 5ed1607c586f85f1f261a7cfde8063b9502d90f5 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Tue, 25 Feb 2025 20:34:16 +1100 Subject: [PATCH 4/8] Add tests --- .../ui/searchstop/SearchStopViewModel.kt | 9 +- .../StopFilterByProductClassTest.kt | 89 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 feature/trip-planner/ui/src/commonTest/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/StopFilterByProductClassTest.kt diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 97f87550..548b57dc 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -75,7 +75,7 @@ class SearchStopViewModel( .let { filterProductClasses( stopResults = it, - excludedProductClasses = listOf(TransportMode.Ferry()) + excludedProductClasses = listOf(TransportMode.Ferry().productClass).toImmutableList() ) } .let(::prioritiseStops) @@ -129,9 +129,12 @@ class SearchStopViewModel( fun filterProductClasses( stopResults: List, - excludedProductClasses: Any, + excludedProductClasses: List, ): List { - + return stopResults.filter { stopResult -> + val productClasses = stopResult.transportModeType.map { it.productClass } + productClasses.any { it !in excludedProductClasses } + } } /// TODO - move to mapper: diff --git a/feature/trip-planner/ui/src/commonTest/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/StopFilterByProductClassTest.kt b/feature/trip-planner/ui/src/commonTest/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/StopFilterByProductClassTest.kt new file mode 100644 index 00000000..703cef56 --- /dev/null +++ b/feature/trip-planner/ui/src/commonTest/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/StopFilterByProductClassTest.kt @@ -0,0 +1,89 @@ +package xyz.ksharma.krail.trip.planner.ui.searchstop + +import kotlinx.collections.immutable.toImmutableList +import kotlinx.coroutines.test.runTest +import xyz.ksharma.krail.trip.planner.ui.state.TransportMode +import xyz.ksharma.krail.trip.planner.ui.state.searchstop.SearchStopState +import kotlin.test.Test +import kotlin.test.assertEquals + +class StopFilterByProductClassTest { + + @Test + fun `should return stops excluding given product classes`() = runTest { + // Given + val testCases = listOf( + TestCase( + excludedClasses = listOf(1), + expectedStopIds = listOf("10101101", "10101105", "12349", "12356") + ), + TestCase( + excludedClasses = listOf(1, 2), + expectedStopIds = listOf("12349", "12356") + ), + TestCase( + excludedClasses = listOf(5), + expectedStopIds = listOf("10101101", "10101100", "10101105", "12349") + ), + // All product classes are excluded + TestCase( + excludedClasses = listOf(1, 2, 5), + expectedStopIds = listOf() + ) + ) + + testCases.forEach { testCase -> + // When + val actualResults = filterProductClasses( + stopResults = stopResults, + excludedProductClasses = testCase.excludedClasses + ) + + val actualStopIds = actualResults.map { it.stopId } + + // Then + assertEquals(testCase.expectedStopIds.sorted(), actualStopIds.sorted()) + } + } + + private data class TestCase( + val excludedClasses: List, + val expectedStopIds: List, + ) + + companion object { + private val stopResults = listOf( + SearchStopState.StopResult( + stopName = "Town Hall Station", + stopId = "10101101", + transportModeType = listOf( + TransportMode.Train(), + TransportMode.Metro() + ).toImmutableList(), + ), + SearchStopState.StopResult( + stopName = "Wynyard Station", + stopId = "10101100", + transportModeType = listOf(TransportMode.Train()).toImmutableList(), + ), + SearchStopState.StopResult( + stopName = "Metro Only Station", + stopId = "10101105", + transportModeType = listOf(TransportMode.Metro()).toImmutableList(), + ), + SearchStopState.StopResult( + stopName = "Schofields Station", + stopId = "12349", + transportModeType = listOf( + TransportMode.Bus(), + TransportMode.Train() + ).toImmutableList(), + ), + SearchStopState.StopResult( + stopName = "103 ABC Rd, Hallway", + stopId = "12356", + transportModeType = listOf(TransportMode.Bus()).toImmutableList(), + ), + ) + } +} From ab7559118d6bf8ed604ddb393473d3ef3b1dd7e5 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Tue, 25 Feb 2025 20:45:32 +1100 Subject: [PATCH 5/8] High priority stopIDs are displayed on Top --- .../trip/planner/ui/searchstop/SearchStopViewModel.kt | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 548b57dc..f08bc02c 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -65,11 +65,7 @@ class SearchStopViewModel( val resultsDb: List = sandook.selectStops(stopName = query) - /* - resultsDb.forEach { - log("resultsDb [$query]: ${it.stopName}") - } - */ + val stopResults = resultsDb .map { it.toStopResult() } .let { @@ -97,7 +93,12 @@ class SearchStopViewModel( transportMode.productClass to index }.toMap() + val highPriorityStopIds = listOf("10101234", "10101235", "10101236", "10101237", "10101238") + return stopResults.sortedWith(compareBy( + { stopResult -> + if (stopResult.stopId in highPriorityStopIds) 0 else 1 + }, { stopResult -> stopResult.transportModeType.minOfOrNull { transportModePriorityMap[it.productClass] ?: Int.MAX_VALUE From 3f8a1c1e47cdf84303026682f287508995d949d1 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Tue, 25 Feb 2025 20:53:28 +1100 Subject: [PATCH 6/8] Add list of stops that are high priority --- .../planner/ui/searchstop/SearchStopViewModel.kt | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index f08bc02c..0157f96a 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -93,7 +93,19 @@ class SearchStopViewModel( transportMode.productClass to index }.toMap() - val highPriorityStopIds = listOf("10101234", "10101235", "10101236", "10101237", "10101238") + // TODO - these should come from Firebase config and have only these hardcoded as fallback. + val highPriorityStopIds = listOf( + "200060", + "200070", + "200080", + "206010", + "2150106", + "200017", + "200039", + "201016", + "201039", + "201080" + ) return stopResults.sortedWith(compareBy( { stopResult -> From 44a0b94341e244e9df123314f9344c85366d147e Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Wed, 26 Feb 2025 22:15:58 +1100 Subject: [PATCH 7/8] Add couple more high priority stops --- .../trip/planner/ui/searchstop/SearchStopViewModel.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 0157f96a..22d21299 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -104,7 +104,12 @@ class SearchStopViewModel( "200039", "201016", "201039", - "201080" + "201080", + "200066", + "200030", + "200046", + "200050", + ) return stopResults.sortedWith(compareBy( From 81f22de3e518d2141f8ed3df1c81fd09cb9dede7 Mon Sep 17 00:00:00 2001 From: Karan Sharma <55722391+ksharma-xyz@users.noreply.github.com> Date: Fri, 28 Feb 2025 00:00:15 +1100 Subject: [PATCH 8/8] Update tests --- .../xyz/ksharma/core/test/viewmodels/SearchStopViewModelTest.kt | 2 +- .../krail/trip/planner/ui/searchstop/SearchStopViewModel.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/test/src/commonTest/kotlin/xyz/ksharma/core/test/viewmodels/SearchStopViewModelTest.kt b/core/test/src/commonTest/kotlin/xyz/ksharma/core/test/viewmodels/SearchStopViewModelTest.kt index 87a73061..1fce79f9 100644 --- a/core/test/src/commonTest/kotlin/xyz/ksharma/core/test/viewmodels/SearchStopViewModelTest.kt +++ b/core/test/src/commonTest/kotlin/xyz/ksharma/core/test/viewmodels/SearchStopViewModelTest.kt @@ -72,7 +72,7 @@ class SearchStopViewModelTest { } } -/* +/* This test is not valid anymore, as we're not calling API. @Test fun `GIVEN search query WHEN SearchTextChanged is triggered and api is success THEN uiState is updated with results`() = runTest { diff --git a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt index 22d21299..8db2c2a3 100644 --- a/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt +++ b/feature/trip-planner/ui/src/commonMain/kotlin/xyz/ksharma/krail/trip/planner/ui/searchstop/SearchStopViewModel.kt @@ -64,7 +64,7 @@ class SearchStopViewModel( log("results: $results")*/ val resultsDb: List = - sandook.selectStops(stopName = query) + sandook.selectStops(stopName = query, excludeProductClassList = listOf()) val stopResults = resultsDb .map { it.toStopResult() }