Support AddToFolder for Multiple Selected Threads #13973
Conversation
|
The issue was caused by allowing to add an archived chat to a folder. If an archived chat was added to any folder and then we tried to "select all" chats and then add them to any folder, the app crashed with |
Sagar0-0
force-pushed
the
support-addToFolder-for-multiple-threads
branch
from
f952222 to
d4980e0
Compare
| val threadIdsAndIsIncluded = threadIds.map { threadId -> | ||
| val isAlreadyIncluded = folders.find { it.chatFolder.id == folderId }?.chatFolder?.includedChats?.contains(threadId) ?: false | ||
| Pair(threadId,isAlreadyIncluded) | ||
| } | ||
| SignalDatabase.chatFolders.addToFolderIfNotIncluded(folderId, threadIdsAndIsIncluded) |
There was a problem hiding this comment.
Instead of mapping to pair, you can use threadIds.filter instead to filter out any included chats which will simplify the call and check made in addToFolderIfNotIncluded
| List<Conversation> conversations = new ArrayList<>(); | ||
| conversations.add(conversation); | ||
| items.add(new ActionItem(R.drawable.symbol_folder_add, getString(R.string.ConversationListFragment_add_to_folder), () -> | ||
| AddToFolderBottomSheet.showChatFolderSheet(folders, conversation.getThreadRecord().getThreadId(), conversation.getThreadRecord().getRecipient().isIndividual()).show(getParentFragmentManager(), BottomSheetUtil.STANDARD_BOTTOM_SHEET_FRAGMENT_TAG) | ||
| showAddToFolderBottomSheet(conversations) |
There was a problem hiding this comment.
Can use Collections.singletonList or Arrays.listOf() instead within showAddToFolderBottomSheet here. But also see my point below about just passing in conversation.
| @@ -1580,6 +1581,27 @@ public void onEvent(MessageSender.MessageSentEvent event) { | |||
| closeSearchIfOpen(); | |||
| } | |||
|
|
|||
| private void showAddToFolderBottomSheet(List<Conversation> conversations) { | |||
There was a problem hiding this comment.
Please correct me if I'm wrong - it looks like this only gets called once with the singular conversation. If so, you can drop the size check and stream map, but I'd actually consider changing the parameter for this to just Conversation and then wrapping it in a list right before the call to showAddToFolderBottomSheet(List<Long> threadIds, Boolean isIndividual) since there's no benefit in having it be a list at this point.
| val isIncludedViaChatType = (isIndividualChat && folder.showIndividualChats) || (!isIndividualChat && folder.showGroupChats) | ||
| val isIncludedExplicitly = folder.includedChats.contains(threadId) | ||
| val isExcludedExplicitly = folder.excludedChats.contains(threadId) | ||
| val isIncludedExplicitly = folder.includedChats.containsAll(threadIds) | ||
| val isExcludedExplicitly = folder.excludedChats.containsAll(threadIds) |
There was a problem hiding this comment.
The first check won't fully work because in ConversationListFragment Line 1653, you're always passing false in. This means if you have a folder that is only "All 1:1 chats", isAlreadyAdded will always be false if you try to add multiple 1:1 chats. To fix, you can do what you did for thread -> threads, but there might be a cleaner way too.
| Toast.makeText(context, requireContext().getString(R.string.AddToFolderBottomSheet_this_chat_is_already, folder.name), Toast.LENGTH_SHORT).show() | ||
| } else { |
There was a problem hiding this comment.
Small thing - this is a singular This chat is already in the folder x, but in cases where threadIds is plural, we'll want to change it to These chats are already in the folder x
First time contributor checklist
Contributor checklist
Fixes #1234syntaxDescription
Reason:
A user may want to add multiple chats in an existing or new folder, which was impossible YET.
I tested it thoroughly, previously I was having some FOREIGN KEY CONFLICT exception which crashed the app everytime when I try to "select all" chats and add it to an existing folder. This is not reproducing it for me. Let me know if anyone is able to reproduce it.
Next plans:
Remove from Folder for multi-select. Let me know if this sounds good.
I have also observed many initialization improvements in the code that we can improve to make the app opening time faster. Like Lazily adding the callbacks and more. Let me know if those PRs are welcomed.