Skip to content

Fix IllegalStateException after deleting LOI #3618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andreia-ferreira merged 3 commits into from
Mar 20, 2026
Merged

Fix IllegalStateException after deleting LOI #3618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e8d79d
fix delete loi called on every started lifecycle event
andreia-ferreira Mar 18, 2026
603a15f
Merge branch 'master' into andreia/3468/fix-illegal-state-exception
andreia-ferreira Mar 20, 2026
ab9aacd
Merge branch 'master' into andreia/3468/fix-illegal-state-exception
andreia-ferreira Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...in/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class HomeScreenMapContainerFragment : AbstractMapContainerFragment() {

/** Invoked when user clicks delete on a site. */
private fun onDeleteSite(loiData: SelectedLoiSheetData) {
launchWhenStarted { mapContainerViewModel.deleteLoi(loiData.loi) }
mapContainerViewModel.deleteLoi(loiData.loi)
}

override fun onCreateView(
Expand Down
9 changes: 6 additions & 3 deletions ...n/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import org.groundplatform.android.common.Constants.CLUSTERING_ZOOM_THRESHOLD
import org.groundplatform.android.data.local.LocalValueStore
import org.groundplatform.android.model.Survey
Expand Down Expand Up @@ -256,9 +257,11 @@ internal constructor(
/**
* Deletes the given LOI and all associated data. This should only be called for free-form jobs.
*/
suspend fun deleteLoi(loi: LocationOfInterest) {
loiRepository.deleteLoi(loi)
selectLocationOfInterest(null)
fun deleteLoi(loi: LocationOfInterest) {
viewModelScope.launch {
loiRepository.deleteLoi(loi)
selectLocationOfInterest(null)
Comment on lines +262 to +263
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The loiRepository.deleteLoi(loi) call can throw an exception (e.g., if the user doesn't have permission to delete the LOI). An unhandled exception here would cause the coroutine to fail and could crash the application. To make this operation more robust, it's recommended to wrap these calls in a try-catch block to gracefully handle any potential errors and log them.

      try {
        loiRepository.deleteLoi(loi)
        selectLocationOfInterest(null)
      } catch (e: Exception) {
        Timber.e(e, "Error deleting LOI %s", loi.id)
      }

}
}

private fun getLocationOfInterestFeatures(survey: Survey): Flow<Set<Feature>> =
Expand Down
15 changes: 14 additions & 1 deletion ...va/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.robolectric.RobolectricTestRunner

@OptIn(ExperimentalCoroutinesApi::class)
@HiltAndroidTest
@RunWith(RobolectricTestRunner::class)
class HomeScreenMapContainerViewModelTest : BaseHiltTest() {
Expand All @@ -59,7 +61,6 @@ class HomeScreenMapContainerViewModelTest : BaseHiltTest() {
@Inject lateinit var activateSurvey: ActivateSurveyUseCase
@BindValue @Mock lateinit var loiRepository: LocationOfInterestRepository

@OptIn(ExperimentalCoroutinesApi::class)
@Before
override fun setUp() {
super.setUp()
Expand Down Expand Up @@ -90,6 +91,18 @@ class HomeScreenMapContainerViewModelTest : BaseHiltTest() {
.isEqualTo(listOf(AdHocDataCollectionButtonData(canCollectData = true, ADHOC_JOB)))
}

@Test
fun `deleteLoi deletes the loi and deselects it`() = runWithTestDispatcher {
viewModel.onFeatureClicked(setOf(LOCATION_OF_INTEREST_FEATURE))
assertThat(viewModel.featureClicked.value).isNotNull()

viewModel.deleteLoi(LOCATION_OF_INTEREST)
advanceUntilIdle()

verify(loiRepository).deleteLoi(LOCATION_OF_INTEREST)
assertThat(viewModel.featureClicked.value).isNull()
}

companion object {
private val BOUNDS = Bounds(Coordinates(-20.0, -20.0), Coordinates(-10.0, -10.0))
val CAMERA_POSITION =
Expand Down
Loading