From 1dc3190159846a5915c7c80ccc59c483723acf2f Mon Sep 17 00:00:00 2001 From: JRoberts <8711996+jr-simplex@users.noreply.github.com> Date: Mon, 9 May 2022 10:55:56 +0400 Subject: [PATCH] remove debug logs on deletion; android: context menu on contact link (#621) --- .../app/views/chat/item/ChatItemView.kt | 2 +- .../app/views/chatlist/ChatListNavLinkView.kt | 69 +++++++++++-------- src/Simplex/Chat.hs | 57 +++------------ src/Simplex/Chat/Store.hs | 8 +-- 4 files changed, 54 insertions(+), 82 deletions(-) diff --git a/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/ChatItemView.kt b/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/ChatItemView.kt index 8aed21baf..b14089783 100644 --- a/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/ChatItemView.kt +++ b/apps/android/app/src/main/java/chat/simplex/app/views/chat/item/ChatItemView.kt @@ -118,7 +118,7 @@ fun ChatItemView( } @Composable -private fun ItemAction(text: String, icon: ImageVector, onClick: () -> Unit, color: Color = MaterialTheme.colors.onBackground) { +fun ItemAction(text: String, icon: ImageVector, onClick: () -> Unit, color: Color = MaterialTheme.colors.onBackground) { DropdownMenuItem(onClick) { Row { Text( diff --git a/apps/android/app/src/main/java/chat/simplex/app/views/chatlist/ChatListNavLinkView.kt b/apps/android/app/src/main/java/chat/simplex/app/views/chatlist/ChatListNavLinkView.kt index 95bd343b1..888447965 100644 --- a/apps/android/app/src/main/java/chat/simplex/app/views/chatlist/ChatListNavLinkView.kt +++ b/apps/android/app/src/main/java/chat/simplex/app/views/chatlist/ChatListNavLinkView.kt @@ -1,20 +1,22 @@ package chat.simplex.app.views.chatlist import android.content.res.Configuration -import androidx.compose.foundation.clickable import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.* import androidx.compose.material.* -import androidx.compose.runtime.Composable -import androidx.compose.runtime.toMutableStateList +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.Delete +import androidx.compose.runtime.* import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import chat.simplex.app.R import chat.simplex.app.model.* import chat.simplex.app.ui.theme.SimpleXTheme +import chat.simplex.app.views.chat.item.ItemAction import chat.simplex.app.views.helpers.* import kotlinx.datetime.Clock @@ -34,27 +36,22 @@ fun ChatListNavLinkView(chat: Chat, chatModel: ChatModel) { } } }, - longClick = { - when (chat.chatInfo) { - is ChatInfo.Direct -> { - AlertManager.shared.showAlertMsg( - title = generalGetString(R.string.delete_contact__question), - text = generalGetString(R.string.delete_contact_all_messages_deleted_cannot_undo_warning), - confirmText = generalGetString(R.string.delete_verb), - onConfirm = { - val cInfo = chat.chatInfo - withApi { - val r = chatModel.controller.apiDeleteChat(cInfo.chatType, cInfo.apiId) - if (r) { - chatModel.removeChat(cInfo.id) - chatModel.chatId.value = null - } - } + deleteContact = { + AlertManager.shared.showAlertMsg( + title = generalGetString(R.string.delete_contact__question), + text = generalGetString(R.string.delete_contact_all_messages_deleted_cannot_undo_warning), + confirmText = generalGetString(R.string.delete_verb), + onConfirm = { + val cInfo = chat.chatInfo + withApi { + val r = chatModel.controller.apiDeleteChat(cInfo.chatType, cInfo.apiId) + if (r) { + chatModel.removeChat(cInfo.id) + chatModel.chatId.value = null } - ) + } } - else -> {} - } + ) }, ) } @@ -162,13 +159,14 @@ fun pendingContactAlertDialog(chatInfo: ChatInfo, chatModel: ChatModel) { } @Composable -fun ChatListNavLinkLayout(chat: Chat, click: () -> Unit, longClick: () -> Unit) { +fun ChatListNavLinkLayout(chat: Chat, click: () -> Unit, deleteContact: () -> Unit) { + val showMenu = remember { mutableStateOf(false) } Surface( modifier = Modifier .fillMaxWidth() .combinedClickable( onClick = click, - onLongClick = longClick + onLongClick = { if (chat.chatInfo is ChatInfo.Direct) showMenu.value = true } ) .height(88.dp) ) { @@ -187,6 +185,23 @@ fun ChatListNavLinkLayout(chat: Chat, click: () -> Unit, longClick: () -> Unit) is ChatInfo.ContactConnection -> ContactConnectionView(chat.chatInfo.contactConnection) } } + if (chat.chatInfo is ChatInfo.Direct) { + DropdownMenu( + expanded = showMenu.value, + onDismissRequest = { showMenu.value = false }, + Modifier.width(220.dp) + ) { + ItemAction( + stringResource(R.string.delete_verb), + Icons.Outlined.Delete, + onClick = { + deleteContact() + showMenu.value = false + }, + color = Color.Red + ) + } + } } Divider(Modifier.padding(horizontal = 8.dp)) } @@ -214,7 +229,7 @@ fun PreviewChatListNavLinkDirect() { chatStats = Chat.ChatStats() ), click = {}, - longClick = {} + deleteContact = {} ) } } @@ -242,7 +257,7 @@ fun PreviewChatListNavLinkGroup() { chatStats = Chat.ChatStats() ), click = {}, - longClick = {} + deleteContact = {} ) } } @@ -263,7 +278,7 @@ fun PreviewChatListNavLinkContactRequest() { chatStats = Chat.ChatStats() ), click = {}, - longClick = {} + deleteContact = {} ) } } diff --git a/src/Simplex/Chat.hs b/src/Simplex/Chat.hs index eb69eae86..234d17298 100644 --- a/src/Simplex/Chat.hs +++ b/src/Simplex/Chat.hs @@ -376,25 +376,15 @@ processChatCommand = \case ct@Contact {localDisplayName} <- withStore $ \st -> getContact st userId chatId withStore (\st -> getContactGroupNames st userId ct) >>= \case [] -> do - liftIO $ putStrLn "### files <- withStore $ \\st -> getContactFiles st userId ct" files <- withStore $ \st -> getContactFiles st userId ct - liftIO $ putStrLn "### conns <- withStore $ \\st -> getContactConnections st userId ct" conns <- withStore $ \st -> getContactConnections st userId ct - liftIO $ putStrLn "### withChatLock . procCmd $ do" withChatLock . procCmd $ do - liftIO $ putStrLn "### cancelFiles user (map (\\(fId, fStatus, _) -> (fId, fStatus)) files)" cancelFiles user (map (\(fId, fStatus, _) -> (fId, fStatus)) files) - liftIO $ putStrLn "### withFilesFolder $ \\filesFolder -> do" - withFilesFolder $ \filesFolder -> do - liftIO $ putStrLn "### deleteFiles filesFolder (map (\\(_, _, fPath) -> fPath) files)" + withFilesFolder $ \filesFolder -> deleteFiles filesFolder (map (\(_, _, fPath) -> fPath) files) - liftIO $ putStrLn "### withAgent $ \\a -> forM_ conns $ \\conn ->" - withAgent $ \a -> forM_ conns $ \conn -> do - liftIO $ putStrLn "### deleteConnection a (aConnId conn) `catchError` \\(_ :: AgentErrorType) -> pure ()" + withAgent $ \a -> forM_ conns $ \conn -> deleteConnection a (aConnId conn) `catchError` \(_ :: AgentErrorType) -> pure () - liftIO $ putStrLn "### withStore $ \\st -> deleteContact st userId ct" withStore $ \st -> deleteContact st userId ct - liftIO $ putStrLn "### unsetActive $ ActiveC localDisplayName" unsetActive $ ActiveC localDisplayName pure $ CRContactDeleted ct gs -> throwChatError $ CEContactGroups ct gs @@ -744,14 +734,11 @@ processChatCommand = \case withFilesFolder :: (FilePath -> m ()) -> m () withFilesFolder action = asks filesFolder >>= readTVarIO >>= mapM_ action deleteFiles :: FilePath -> [Maybe FilePath] -> m () - deleteFiles filesFolder filePaths = do + deleteFiles filesFolder filePaths = forM_ filePaths $ \filePath_ -> forM_ filePath_ $ \filePath -> do let fsFilePath = filesFolder <> "/" <> filePath - liftIO $ putStrLn ("in deleteFiles - fsFilePath " <> fsFilePath) - liftIO $ putStrLn "removeFile fsFilePath `E.catch` \\(_ :: E.SomeException) ->" - removeFile fsFilePath `E.catch` \(_ :: E.SomeException) -> do - liftIO $ putStrLn "removePathForcibly fsFilePath `E.catch` \\(_ :: E.SomeException) -> pure ()" + removeFile fsFilePath `E.catch` \(_ :: E.SomeException) -> removePathForcibly fsFilePath `E.catch` \(_ :: E.SomeException) -> pure () cancelFiles :: User -> [(Int64, ACIFileStatus)] -> m () cancelFiles user@User {userId} files = mapM_ maybeCancelFile files @@ -769,39 +756,26 @@ processChatCommand = \case AFS _ CIFSRcvComplete -> pure () cancelById :: Int64 -> m () cancelById fileId = do - liftIO $ putStrLn ("cancelById - fileId " <> show fileId) - liftIO $ putStrLn "ft <- withStore (\\st -> getFileTransfer st userId fileId)" ft <- withStore (\st -> getFileTransfer st userId fileId) - liftIO $ putStrLn "void $ cancelFile user fileId ft" void $ cancelFile user fileId ft cancelFile :: User -> Int64 -> FileTransfer -> m ChatResponse cancelFile user@User {userId} fileId ft = case ft of FTSnd ftm fts -> do - liftIO $ putStrLn "cancelFileTransfer CIFSSndCancelled" cancelFileTransfer CIFSSndCancelled - liftIO $ putStrLn "forM_ fts $ \\ft' -> cancelSndFileTransfer ft'" forM_ fts $ \ft' -> cancelSndFileTransfer ft' - liftIO $ putStrLn "ci <- withStore $ \\st -> getChatItemByFileId st user fileId" ci <- withStore $ \st -> getChatItemByFileId st user fileId - liftIO $ putStrLn "pure $ CRSndGroupFileCancelled ci ftm fts" pure $ CRSndGroupFileCancelled ci ftm fts FTRcv ftr -> do - liftIO $ putStrLn "cancelFileTransfer CIFSRcvCancelled" cancelFileTransfer CIFSRcvCancelled - liftIO $ putStrLn "cancelRcvFileTransfer ftr" cancelRcvFileTransfer ftr - liftIO $ putStrLn "pure $ CRRcvFileCancelled ftr" pure $ CRRcvFileCancelled ftr where cancelFileTransfer :: MsgDirectionI d => CIFileStatus d -> m () - cancelFileTransfer ciFileStatus = do - liftIO $ putStrLn "unless (fileTransferCancelled ft) $" + cancelFileTransfer ciFileStatus = unless (fileTransferCancelled ft) $ withStore $ \st -> do - liftIO $ putStrLn "updateFileCancelled st userId fileId" updateFileCancelled st userId fileId - liftIO $ putStrLn "updateCIFileStatus st userId fileId ciFileStatus" updateCIFileStatus st userId fileId ciFileStatus withCurrentCall :: ContactId -> (UserId -> Contact -> Call -> m (Maybe Call)) -> m ChatResponse withCurrentCall ctId action = withUser $ \User {userId} -> do @@ -1897,36 +1871,25 @@ isFileActive fileId files = do cancelRcvFileTransfer :: ChatMonad m => RcvFileTransfer -> m () cancelRcvFileTransfer ft@RcvFileTransfer {fileId, fileStatus} = do - liftIO $ putStrLn "closeFileHandle fileId rcvFiles" closeFileHandle fileId rcvFiles - liftIO $ putStrLn "withStore $ \\st -> do" withStore $ \st -> do - liftIO $ putStrLn "updateRcvFileStatus st ft FSCancelled" updateRcvFileStatus st ft FSCancelled - liftIO $ putStrLn "deleteRcvFileChunks st ft" deleteRcvFileChunks st ft case fileStatus of - RFSAccepted RcvFileInfo {agentConnId = AgentConnId acId} -> do - liftIO $ putStrLn "withAgent (`deleteConnection` acId)" + RFSAccepted RcvFileInfo {agentConnId = AgentConnId acId} -> withAgent (`deleteConnection` acId) - RFSConnected RcvFileInfo {agentConnId = AgentConnId acId} -> do - liftIO $ putStrLn "withAgent (`deleteConnection` acId)" + RFSConnected RcvFileInfo {agentConnId = AgentConnId acId} -> withAgent (`deleteConnection` acId) _ -> pure () cancelSndFileTransfer :: ChatMonad m => SndFileTransfer -> m () -cancelSndFileTransfer ft@SndFileTransfer {agentConnId = AgentConnId acId, fileStatus} = do - liftIO $ putStrLn "unless (fileStatus == FSCancelled || fileStatus == FSComplete) $ do" +cancelSndFileTransfer ft@SndFileTransfer {agentConnId = AgentConnId acId, fileStatus} = unless (fileStatus == FSCancelled || fileStatus == FSComplete) $ do withStore $ \st -> do - liftIO $ putStrLn "updateSndFileStatus st ft FSCancelled" updateSndFileStatus st ft FSCancelled - liftIO $ putStrLn "deleteSndFileChunks st ft" deleteSndFileChunks st ft withAgent $ \a -> do - liftIO $ putStrLn "void (sendMessage a acId $ smpEncode FileChunkCancel) `catchError` \\_ -> pure ()" void (sendMessage a acId $ smpEncode FileChunkCancel) `catchError` \_ -> pure () - liftIO $ putStrLn "deleteConnection a acId" deleteConnection a acId closeFileHandle :: ChatMonad m => Int64 -> (ChatController -> TVar (Map Int64 Handle)) -> m () @@ -2137,9 +2100,9 @@ withStore :: m a withStore action = asks chatStore - -- >>= runExceptT . action + >>= runExceptT . action -- use this line instead of above to log query errors - >>= (\st -> runExceptT $ action st `E.catch` \(e :: E.SomeException) -> liftIO (print e) >> E.throwIO e) + -- >>= (\st -> runExceptT $ action st `E.catch` \(e :: E.SomeException) -> liftIO (print e) >> E.throwIO e) >>= liftEither . first ChatErrorStore chatCommandP :: Parser ChatCommand diff --git a/src/Simplex/Chat/Store.hs b/src/Simplex/Chat/Store.hs index 7c9cc4b28..f9aafdbad 100644 --- a/src/Simplex/Chat/Store.hs +++ b/src/Simplex/Chat/Store.hs @@ -429,9 +429,7 @@ getContactGroupNames st userId Contact {contactId} = deleteContact :: MonadUnliftIO m => SQLiteStore -> UserId -> Contact -> m () deleteContact st userId Contact {contactId, localDisplayName} = do - liftIO $ putStrLn "in deleteContact, first liftIO . withTransaction st $ \\db -> do" liftIO . withTransaction st $ \db -> do - liftIO $ putStrLn "DB.execute DELETE FROM connections WHERE connection_id IN ..." DB.execute db [sql| @@ -443,15 +441,11 @@ deleteContact st userId Contact {contactId, localDisplayName} = do ) |] (userId, contactId) - liftIO $ putStrLn "DB.execute db \"DELETE FROM files WHERE user_id = ? AND contact_id = ?\" (userId, contactId)" DB.execute db "DELETE FROM files WHERE user_id = ? AND contact_id = ?" (userId, contactId) - liftIO $ putStrLn "in deleteContact, second liftIO . withTransaction st $ \\db -> do" + -- in separate transaction to prevent crashes on android (race condition on integrity check?) liftIO . withTransaction st $ \db -> do - liftIO $ putStrLn "DB.execute db \"DELETE FROM chat_items WHERE user_id = ? AND contact_id = ?\" (userId, contactId)" DB.execute db "DELETE FROM chat_items WHERE user_id = ? AND contact_id = ?" (userId, contactId) - liftIO $ putStrLn "DB.execute db \"DELETE FROM contacts WHERE user_id = ? AND contact_id = ?\" (userId, contactId)" DB.execute db "DELETE FROM contacts WHERE user_id = ? AND contact_id = ?" (userId, contactId) - liftIO $ putStrLn "DB.execute db \"DELETE FROM display_names WHERE user_id = ? AND local_display_name = ?\" (userId, localDisplayName)" DB.execute db "DELETE FROM display_names WHERE user_id = ? AND local_display_name = ?" (userId, localDisplayName) updateUserProfile :: StoreMonad m => SQLiteStore -> User -> Profile -> m ()