Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3480a090 authored by Ellen Poe's avatar Ellen Poe
Browse files

Merge branch 'ellenhp/no_saved_list_cycles' into 'main'

Prevent saved list cycles

See merge request e/os/cardinal!32
parents b64e4b4c f07da04a
Loading
Loading
Loading
Loading
Loading
+10 −1
Original line number Original line Diff line number Diff line
@@ -18,15 +18,24 @@


package earth.maps.cardinal.data
package earth.maps.cardinal.data


import earth.maps.cardinal.data.room.ItemType
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.MutableStateFlow
import javax.inject.Inject
import javax.inject.Inject
import javax.inject.Singleton
import javax.inject.Singleton


/**
 * Represents an item in the clipboard with its ID and type
 */
data class ClipboardItem(
    val itemId: String,
    val itemType: ItemType
)

@Singleton
@Singleton
class CutPasteRepository @Inject constructor() {
class CutPasteRepository @Inject constructor() {


    /**
    /**
     * The set of list item UUIDs and ItemTypes in the clipboard.
     * The set of list item UUIDs and ItemTypes in the clipboard.
     */
     */
    val clipboard: MutableStateFlow<Set<String>> = MutableStateFlow(emptySet())
    val clipboard: MutableStateFlow<Set<ClipboardItem>> = MutableStateFlow(emptySet())
}
}
 No newline at end of file
+68 −0
Original line number Original line Diff line number Diff line
@@ -288,6 +288,74 @@ class SavedListRepository @Inject constructor(
            return@mapLatest flows
            return@mapLatest flows
        }
        }


    /**
     * Checks if pasting the specified list items into the target list would create a cycle.
     * A cycle occurs when a list is being pasted into itself or any of its sublists.
     *
     * @param targetListId The ID of the list where items would be pasted
     * @param listIdsToPaste Set of list IDs that would be pasted
     * @return true if pasting would create a cycle, false otherwise
     */
    suspend fun wouldCreateCycle(
        targetListId: String,
        listIdsToPaste: Set<String>
    ): Boolean = withContext(Dispatchers.IO) {
        try {
            // If no lists are being pasted, no cycle can be created
            if (listIdsToPaste.isEmpty()) {
                return@withContext false
            }

            // Check if any of the lists to paste is the target list itself
            if (listIdsToPaste.contains(targetListId)) {
                return@withContext true
            }

            // For each list being pasted, check if the target list is in its hierarchy
            for (listId in listIdsToPaste) {
                if (isListInHierarchy(targetListId, listId)) {
                    return@withContext true
                }
            }

            false
        } catch (e: Exception) {
            Log.e(TAG, "Error checking for cycle", e)
            false // Default to allowing paste if there's an error
        }
    }

    /**
     * Recursively checks if targetListId is in the hierarchy of parentListId.
     * This means targetListId is either parentListId itself or one of its descendants.
     *
     * @param targetListId The list ID we're looking for
     * @param parentListId The list ID to start searching from
     * @return true if targetListId is in the hierarchy of parentListId
     */
    private suspend fun isListInHierarchy(
        targetListId: String,
        parentListId: String
    ): Boolean {
        // Base case: if we found the target list
        if (targetListId == parentListId) {
            return true
        }

        // Get all child lists of the parent list
        val childLists = listItemDao.getItemsInList(parentListId)
            .filter { it.itemType == ItemType.LIST }

        // Recursively check each child list
        for (childListItem in childLists) {
            if (isListInHierarchy(targetListId, childListItem.itemId)) {
                return true
            }
        }

        return false
    }

    companion object {
    companion object {
        private const val TAG = "SavedListRepository"
        private const val TAG = "SavedListRepository"
    }
    }
+25 −6
Original line number Original line Diff line number Diff line
@@ -50,6 +50,8 @@ import androidx.compose.material3.IconButton
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.OutlinedTextField
import androidx.compose.material3.OutlinedTextField
import androidx.compose.material3.Scaffold
import androidx.compose.material3.Scaffold
import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState
import androidx.compose.material3.Text
import androidx.compose.material3.Text
import androidx.compose.material3.ToggleFloatingActionButton
import androidx.compose.material3.ToggleFloatingActionButton
import androidx.compose.material3.ToggleFloatingActionButtonDefaults.animateIcon
import androidx.compose.material3.ToggleFloatingActionButtonDefaults.animateIcon
@@ -77,6 +79,7 @@ import androidx.navigation.NavController
import earth.maps.cardinal.R.dimen
import earth.maps.cardinal.R.dimen
import earth.maps.cardinal.R.drawable
import earth.maps.cardinal.R.drawable
import earth.maps.cardinal.R.string
import earth.maps.cardinal.R.string
import earth.maps.cardinal.data.ClipboardItem
import earth.maps.cardinal.data.room.ListContent
import earth.maps.cardinal.data.room.ListContent
import earth.maps.cardinal.data.room.ListContentItem
import earth.maps.cardinal.data.room.ListContentItem
import earth.maps.cardinal.data.room.PlaceContent
import earth.maps.cardinal.data.room.PlaceContent
@@ -102,11 +105,13 @@ fun ManagePlacesScreen(
    val clipboard by viewModel.clipboard.collectAsState(emptySet())
    val clipboard by viewModel.clipboard.collectAsState(emptySet())
    val selectedItems by viewModel.selectedItems.collectAsState()
    val selectedItems by viewModel.selectedItems.collectAsState()
    val isAllSelected by viewModel.isAllSelected.collectAsState(initial = false)
    val isAllSelected by viewModel.isAllSelected.collectAsState(initial = false)
    val errorMessage by viewModel.errorMessage.collectAsState()
    var showDeleteConfirmation by remember { mutableStateOf(false) }
    var showDeleteConfirmation by remember { mutableStateOf(false) }
    var showCreateListDialog by remember { mutableStateOf(false) }
    var showCreateListDialog by remember { mutableStateOf(false) }
    var showEditDialog by remember { mutableStateOf(false) }
    var showEditDialog by remember { mutableStateOf(false) }
    var editingItem by remember { mutableStateOf<ListContent?>(null) }
    var editingItem by remember { mutableStateOf<ListContent?>(null) }
    var fabMenuExpanded by remember { mutableStateOf(false) }
    var fabMenuExpanded by remember { mutableStateOf(false) }
    val snackbarHostState = remember { SnackbarHostState() }


    // Initialize the view model with the listId if provided
    // Initialize the view model with the listId if provided
    LaunchedEffect(listId) {
    LaunchedEffect(listId) {
@@ -114,13 +119,28 @@ fun ManagePlacesScreen(
    }
    }


    Scaffold(
    Scaffold(
        contentWindowInsets = WindowInsets.safeDrawing, topBar = {
        contentWindowInsets = WindowInsets.safeDrawing,
        snackbarHost = { SnackbarHost(snackbarHostState, modifier = Modifier.padding(bottom = TOOLBAR_HEIGHT_DP)) },
        topBar = {
            ManagePlacesTopBar(
            ManagePlacesTopBar(
                navController = navController,
                navController = navController,
                title = currentListName ?: stringResource(string.saved_places_title_case),
                title = currentListName ?: stringResource(string.saved_places_title_case),
                breadcrumbNames = parents.plus(currentListName ?: ""),
                breadcrumbNames = parents.plus(currentListName ?: ""),
            )
            )
        }) { paddingValues ->
        }) { paddingValues ->

        val coroutineScope = rememberCoroutineScope()

        // Show error message in snackbar when it changes
        LaunchedEffect(errorMessage) {
            errorMessage?.let { message ->
                coroutineScope.launch {
                    snackbarHostState.showSnackbar(message)
                }
                viewModel.clearErrorMessage()
            }
        }

        if (currentListContent?.isEmpty() == true) {
        if (currentListContent?.isEmpty() == true) {
            Column(
            Column(
                modifier = Modifier.padding(paddingValues)
                modifier = Modifier.padding(paddingValues)
@@ -129,7 +149,6 @@ fun ManagePlacesScreen(
            }
            }
        }
        }


        val coroutineScope = rememberCoroutineScope()
        val currentListContent = currentListContent
        val currentListContent = currentListContent
        Box(modifier = Modifier.padding(paddingValues)) {
        Box(modifier = Modifier.padding(paddingValues)) {
            AnimatedVisibility(
            AnimatedVisibility(
@@ -453,7 +472,7 @@ private fun EmptyListContent() {
private fun ListContentGrid(
private fun ListContentGrid(
    viewModel: ManagePlacesViewModel,
    viewModel: ManagePlacesViewModel,
    content: List<Flow<ListContent?>>,
    content: List<Flow<ListContent?>>,
    clipboard: Set<String>,
    clipboard: Set<ClipboardItem>,
    selectedItems: Set<String>,
    selectedItems: Set<String>,
    onItemClick: (ListContent) -> Unit,
    onItemClick: (ListContent) -> Unit,
    onEditClick: (ListContent) -> Unit,
    onEditClick: (ListContent) -> Unit,
@@ -483,7 +502,7 @@ private fun ListContentGrid(
        items(lists) { item ->
        items(lists) { item ->
            ListItem(
            ListItem(
                item = item,
                item = item,
                isInClipboard = clipboard.contains(item.id),
                isInClipboard = clipboard.any { it.itemId == item.id },
                isSelected = selectedItems.contains(item.id),
                isSelected = selectedItems.contains(item.id),
                onSelectionChange = { viewModel.toggleSelection(item.id) },
                onSelectionChange = { viewModel.toggleSelection(item.id) },
                onClick = { onItemClick(item) },
                onClick = { onItemClick(item) },
@@ -505,7 +524,7 @@ private fun ListContentGrid(
            items(pinnedPlaces) { item ->
            items(pinnedPlaces) { item ->
                PlaceItem(
                PlaceItem(
                    item = item,
                    item = item,
                    isInClipboard = clipboard.contains(item.id),
                    isInClipboard = clipboard.any { it.itemId == item.id },
                    isSelected = selectedItems.contains(item.id),
                    isSelected = selectedItems.contains(item.id),
                    onSelectionChange = { viewModel.toggleSelection(item.id) },
                    onSelectionChange = { viewModel.toggleSelection(item.id) },
                    onClick = { onItemClick(item) },
                    onClick = { onItemClick(item) },
@@ -530,7 +549,7 @@ private fun ListContentGrid(
        items(unpinnedPlaces) { item ->
        items(unpinnedPlaces) { item ->
            PlaceItem(
            PlaceItem(
                item = item,
                item = item,
                isInClipboard = clipboard.contains(item.id),
                isInClipboard = clipboard.any { it.itemId == item.id },
                isSelected = selectedItems.contains(item.id),
                isSelected = selectedItems.contains(item.id),
                onSelectionChange = { viewModel.toggleSelection(item.id) },
                onSelectionChange = { viewModel.toggleSelection(item.id) },
                onClick = { onItemClick(item) },
                onClick = { onItemClick(item) },
+47 −6
Original line number Original line Diff line number Diff line
@@ -18,9 +18,13 @@


package earth.maps.cardinal.ui.saved
package earth.maps.cardinal.ui.saved


import android.content.Context
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import androidx.lifecycle.viewModelScope
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.lifecycle.HiltViewModel
import dagger.hilt.android.qualifiers.ApplicationContext
import earth.maps.cardinal.R
import earth.maps.cardinal.data.ClipboardItem
import earth.maps.cardinal.data.CutPasteRepository
import earth.maps.cardinal.data.CutPasteRepository
import earth.maps.cardinal.data.Place
import earth.maps.cardinal.data.Place
import earth.maps.cardinal.data.room.ItemType
import earth.maps.cardinal.data.room.ItemType
@@ -42,6 +46,7 @@ import javax.inject.Inject


@HiltViewModel
@HiltViewModel
class ManagePlacesViewModel @Inject constructor(
class ManagePlacesViewModel @Inject constructor(
    @param:ApplicationContext private val context: Context,
    private val savedPlaceRepository: SavedPlaceRepository,
    private val savedPlaceRepository: SavedPlaceRepository,
    private val savedListRepository: SavedListRepository,
    private val savedListRepository: SavedListRepository,
    private val listItemDao: ListItemDao,
    private val listItemDao: ListItemDao,
@@ -92,7 +97,11 @@ class ManagePlacesViewModel @Inject constructor(
        list?.let { savedListRepository.getListContent(it.id) } ?: flowOf(null)
        list?.let { savedListRepository.getListContent(it.id) } ?: flowOf(null)
    }
    }


    val clipboard: Flow<Set<String>> = cutPasteRepository.clipboard
    val clipboard: Flow<Set<ClipboardItem>> = cutPasteRepository.clipboard

    // Error messages for UI display
    private val _errorMessage = MutableStateFlow<String?>(null)
    val errorMessage: StateFlow<String?> = _errorMessage


    suspend fun setInitialList(listId: String?) {
    suspend fun setInitialList(listId: String?) {
        if (listId != null) {
        if (listId != null) {
@@ -174,23 +183,55 @@ class ManagePlacesViewModel @Inject constructor(
    }
    }


    fun cutSelected() {
    fun cutSelected() {
        val newClipboard = _selectedItems.value.toSet()
        viewModelScope.launch {
        cutPasteRepository.clipboard.value = newClipboard
            val currentListId = _currentListId.value ?: return@launch
            val itemsResult = savedListRepository.getItemsInList(currentListId)
            if (itemsResult.isFailure) return@launch
            val items = itemsResult.getOrNull() ?: return@launch

            val clipboardItems = _selectedItems.value.mapNotNull { itemId ->
                val item = items.find { it.itemId == itemId } ?: return@mapNotNull null
                ClipboardItem(itemId, item.itemType)
            }.toSet()

            cutPasteRepository.clipboard.value = clipboardItems
            clearSelection()
            clearSelection()
        }
        }
    }


    fun pasteSelected() {
    fun pasteSelected() {
        val currentListId = currentListId.value ?: return
        val currentListId = currentListId.value ?: return
        viewModelScope.launch {
        viewModelScope.launch {
            val clipboardItems = cutPasteRepository.clipboard.value
            
            // Check if any lists in the clipboard would create a cycle
            val listIdsToCheck = clipboardItems
                .filter { it.itemType == ItemType.LIST }
                .map { it.itemId }
                .toSet()
            
            val wouldCreateCycle = savedListRepository.wouldCreateCycle(currentListId, listIdsToCheck)
            
            if (wouldCreateCycle) {
                // Don't paste if it would create a cycle
                _errorMessage.value =
                    context.getString(R.string.cannot_paste_a_list_into_itself_or_one_of_its_sublists)
                return@launch
            }
            
            val itemsInListCount = listItemDao.getItemsInList(currentListId).size
            val itemsInListCount = listItemDao.getItemsInList(currentListId).size


            cutPasteRepository.clipboard.value.forEachIndexed { index, id ->
            clipboardItems.forEachIndexed { index, clipboardItem ->
                listItemDao.moveItem(id, currentListId, itemsInListCount + index)
                listItemDao.moveItem(clipboardItem.itemId, currentListId, itemsInListCount + index)
            }
            }
            cutPasteRepository.clipboard.value = emptySet()
            cutPasteRepository.clipboard.value = emptySet()
        }
        }
    }
    }


    fun clearErrorMessage() {
        _errorMessage.value = null
    }

    fun updatePlace(
    fun updatePlace(
        id: String,
        id: String,
        customName: String?,
        customName: String?,
+1 −0
Original line number Original line Diff line number Diff line
@@ -264,4 +264,5 @@
    <string name="category_transportation">Transportation</string>
    <string name="category_transportation">Transportation</string>
    <string name="category_entertainment">Entertainment</string>
    <string name="category_entertainment">Entertainment</string>
    <string name="category_nightlife">Nightlife</string>
    <string name="category_nightlife">Nightlife</string>
    <string name="cannot_paste_a_list_into_itself_or_one_of_its_sublists">Cannot paste a list into itself or one of its sublists</string>
</resources>
</resources>
Loading