From 7de89ffe5786c201df61a8b3c7302015bda29bbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Kaan=20K=C3=B6se?= Date: Mon, 15 Jul 2024 00:00:38 +0200 Subject: [PATCH] Fixing an issue with thread creation and selected items notifications. --- Wino.Core.Domain/Entities/MailCopy.cs | 3 +- .../Models/MailItem/IMailHashContainer.cs | 15 ++ Wino.Core.Domain/Models/MailItem/IMailItem.cs | 2 +- .../Models/MailItem/ThreadMailItem.cs | 3 + .../Collections/WinoMailCollection.cs | 174 +++++++++++------- .../Data/MailItemViewModel.cs | 3 + .../Data/ThreadMailItemViewModel.cs | 12 +- Wino.Mail.ViewModels/MailListPageViewModel.cs | 53 +++--- 8 files changed, 160 insertions(+), 105 deletions(-) create mode 100644 Wino.Core.Domain/Models/MailItem/IMailHashContainer.cs diff --git a/Wino.Core.Domain/Entities/MailCopy.cs b/Wino.Core.Domain/Entities/MailCopy.cs index 5075227f..489bc916 100644 --- a/Wino.Core.Domain/Entities/MailCopy.cs +++ b/Wino.Core.Domain/Entities/MailCopy.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using SQLite; using Wino.Core.Domain.Enums; using Wino.Core.Domain.Models.MailItem; @@ -140,7 +141,7 @@ namespace Wino.Core.Domain.Entities /// [Ignore] public MailAccount AssignedAccount { get; set; } - + public IEnumerable GetContainingIds() => new[] { UniqueId }; public override string ToString() => $"{Subject} <-> {Id}"; } } diff --git a/Wino.Core.Domain/Models/MailItem/IMailHashContainer.cs b/Wino.Core.Domain/Models/MailItem/IMailHashContainer.cs new file mode 100644 index 00000000..1426bc04 --- /dev/null +++ b/Wino.Core.Domain/Models/MailItem/IMailHashContainer.cs @@ -0,0 +1,15 @@ +using System; +using System.Collections.Generic; + +namespace Wino.Core.Domain.Models.MailItem +{ + /// + /// An interface that returns the UniqueId store for IMailItem. + /// For threads, it may be multiple items. + /// For single mails, it'll always be one item. + /// + public interface IMailHashContainer + { + IEnumerable GetContainingIds(); + } +} diff --git a/Wino.Core.Domain/Models/MailItem/IMailItem.cs b/Wino.Core.Domain/Models/MailItem/IMailItem.cs index b75c24af..73f01e02 100644 --- a/Wino.Core.Domain/Models/MailItem/IMailItem.cs +++ b/Wino.Core.Domain/Models/MailItem/IMailItem.cs @@ -6,7 +6,7 @@ namespace Wino.Core.Domain.Models.MailItem /// /// Interface of simplest representation of a MailCopy. /// - public interface IMailItem + public interface IMailItem : IMailHashContainer { Guid UniqueId { get; } string Id { get; } diff --git a/Wino.Core.Domain/Models/MailItem/ThreadMailItem.cs b/Wino.Core.Domain/Models/MailItem/ThreadMailItem.cs index 8eb8085e..7a0d0656 100644 --- a/Wino.Core.Domain/Models/MailItem/ThreadMailItem.cs +++ b/Wino.Core.Domain/Models/MailItem/ThreadMailItem.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; using Wino.Core.Domain.Entities; @@ -40,6 +41,8 @@ namespace Wino.Core.Domain.Models.MailItem } } + public IEnumerable GetContainingIds() => ThreadItems?.Select(a => a.UniqueId) ?? default; + #region IMailItem public Guid UniqueId => LatestMailItem?.UniqueId ?? Guid.Empty; diff --git a/Wino.Mail.ViewModels/Collections/WinoMailCollection.cs b/Wino.Mail.ViewModels/Collections/WinoMailCollection.cs index bd3b1a23..3be2e0d5 100644 --- a/Wino.Mail.ViewModels/Collections/WinoMailCollection.cs +++ b/Wino.Mail.ViewModels/Collections/WinoMailCollection.cs @@ -18,7 +18,9 @@ namespace Wino.Mail.ViewModels.Collections // If the item provider here for update or removal doesn't exist here // we can ignore the operation. - public HashSet MailCopyIdHashSet = new HashSet(); + public HashSet MailCopyIdHashSet = []; + + public event EventHandler MailItemRemoved; private ListItemComparer listComparer = new ListItemComparer(); @@ -61,45 +63,51 @@ namespace Wino.Mail.ViewModels.Collections return mailItem.FromName; } - private async Task InsertItemInternalAsync(object groupKey, IMailItem mailItem) - => await ExecuteUIThread(() => - { - if (mailItem is MailCopy mailCopy) - { - MailCopyIdHashSet.Add(mailCopy.UniqueId); - - _mailItemSource.InsertItem(groupKey, listComparer, new MailItemViewModel(mailCopy), listComparer.GetItemComparer()); - } - else if (mailItem is ThreadMailItem threadMailItem) - { - foreach (var item in threadMailItem.ThreadItems) - { - MailCopyIdHashSet.Add(item.UniqueId); - } - - _mailItemSource.InsertItem(groupKey, listComparer, new ThreadMailItemViewModel(threadMailItem), listComparer.GetItemComparer()); - } - else if (mailItem is MailItemViewModel) - { - MailCopyIdHashSet.Add(mailItem.UniqueId); - - _mailItemSource.InsertItem(groupKey, listComparer, mailItem, listComparer.GetItemComparer()); - } - }); - - private async Task RemoveItemInternalAsync(ObservableGroup group, IMailItem mailItem) + private void UpdateUniqueIdHashes(IMailHashContainer itemContainer, bool isAdd) { - MailCopyIdHashSet.Remove(mailItem.UniqueId); - - await ExecuteUIThread(() => + foreach (var item in itemContainer.GetContainingIds()) { - group.Remove(mailItem); - - if (group.Count == 0) + if (isAdd) { - _mailItemSource.RemoveGroup(group.Key); + MailCopyIdHashSet.Add(item); } - }); + else + { + MailCopyIdHashSet.Remove(item); + } + } + } + + private void InsertItemInternal(object groupKey, IMailItem mailItem) + { + UpdateUniqueIdHashes(mailItem, true); + + if (mailItem is MailCopy mailCopy) + { + _mailItemSource.InsertItem(groupKey, listComparer, new MailItemViewModel(mailCopy), listComparer.GetItemComparer()); + } + else if (mailItem is ThreadMailItem threadMailItem) + { + _mailItemSource.InsertItem(groupKey, listComparer, new ThreadMailItemViewModel(threadMailItem), listComparer.GetItemComparer()); + } + else + { + _mailItemSource.InsertItem(groupKey, listComparer, mailItem, listComparer.GetItemComparer()); + } + } + + private void RemoveItemInternal(ObservableGroup group, IMailItem mailItem) + { + UpdateUniqueIdHashes(mailItem, false); + + MailItemRemoved?.Invoke(this, mailItem); + + group.Remove(mailItem); + + if (group.Count == 0) + { + _mailItemSource.RemoveGroup(group.Key); + } } public async Task AddAsync(MailCopy addedItem) @@ -109,6 +117,9 @@ namespace Wino.Mail.ViewModels.Collections var groupCount = _mailItemSource.Count; + var addedAccountProviderType = addedItem.AssignedAccount.ProviderType; + var threadingStrategy = ThreadingStrategyProvider?.GetStrategy(addedAccountProviderType); + for (int i = 0; i < groupCount; i++) { if (shouldExit) break; @@ -119,10 +130,6 @@ namespace Wino.Mail.ViewModels.Collections { var item = group[k]; - var addedAccountProviderType = addedItem.AssignedAccount.ProviderType; - - var threadingStrategy = ThreadingStrategyProvider?.GetStrategy(addedAccountProviderType); - if (threadingStrategy?.ShouldThreadWithItem(addedItem, item) ?? false) { shouldExit = true; @@ -140,23 +147,32 @@ namespace Wino.Mail.ViewModels.Collections var existingGroupKey = GetGroupingKey(threadMailItemViewModel); - threadMailItemViewModel.AddMailItemViewModel(addedItem); + await ExecuteUIThread(() => { threadMailItemViewModel.AddMailItemViewModel(addedItem); }); var newGroupKey = GetGroupingKey(threadMailItemViewModel); if (!existingGroupKey.Equals(newGroupKey)) { - await RemoveItemInternalAsync(group, threadMailItemViewModel); - await InsertItemInternalAsync(newGroupKey, threadMailItemViewModel); + var mailThreadItems = threadMailItemViewModel.GetThreadMailItem(); + + await ExecuteUIThread(() => + { + // Group must be changed for this thread. + // Remove the thread first. + + RemoveItemInternal(group, threadMailItemViewModel); + + // Insert new view model because the previous one might've been deleted with the group. + InsertItemInternal(newGroupKey, new ThreadMailItemViewModel(mailThreadItems)); + }); } - - await ExecuteUIThread(() => { threadMailItemViewModel.NotifyPropertyChanges(); }); - - if (!MailCopyIdHashSet.Contains(addedItem.UniqueId)) + else { - MailCopyIdHashSet.Add(addedItem.UniqueId); + await ExecuteUIThread(() => { threadMailItemViewModel.NotifyPropertyChanges(); }); } + UpdateUniqueIdHashes(addedItem, true); + break; } else @@ -177,10 +193,10 @@ namespace Wino.Mail.ViewModels.Collections if (item is MailItemViewModel itemViewModel) { - itemViewModel.Update(addedItem); + await ExecuteUIThread(() => { itemViewModel.Update(addedItem); }); - MailCopyIdHashSet.Remove(itemViewModel.UniqueId); - MailCopyIdHashSet.Add(addedItem.UniqueId); + UpdateUniqueIdHashes(itemViewModel, false); + UpdateUniqueIdHashes(addedItem, true); } } else @@ -189,15 +205,18 @@ namespace Wino.Mail.ViewModels.Collections var threadMailItem = new ThreadMailItem(); - threadMailItem.AddThreadItem(item); - threadMailItem.AddThreadItem(addedItem); + await ExecuteUIThread(() => + { + threadMailItem.AddThreadItem(item); + threadMailItem.AddThreadItem(addedItem); - if (threadMailItem.ThreadItems.Count == 1) return; + if (threadMailItem.ThreadItems.Count == 1) return; - var newGroupKey = GetGroupingKey(threadMailItem); + var newGroupKey = GetGroupingKey(threadMailItem); - await RemoveItemInternalAsync(group, item); - await InsertItemInternalAsync(newGroupKey, threadMailItem); + RemoveItemInternal(group, item); + InsertItemInternal(newGroupKey, threadMailItem); + }); } break; @@ -208,6 +227,9 @@ namespace Wino.Mail.ViewModels.Collections // Update properties. if (item.Id == addedItem.Id && item is MailItemViewModel itemViewModel) { + UpdateUniqueIdHashes(itemViewModel, false); + UpdateUniqueIdHashes(addedItem, true); + await ExecuteUIThread(() => { itemViewModel.Update(addedItem); }); shouldExit = true; @@ -224,7 +246,7 @@ namespace Wino.Mail.ViewModels.Collections var groupKey = GetGroupingKey(addedItem); - await InsertItemInternalAsync(groupKey, addedItem); + await ExecuteUIThread(() => { InsertItemInternal(groupKey, addedItem); }); } } @@ -268,12 +290,9 @@ namespace Wino.Mail.ViewModels.Collections } else { - foreach (var item in group) { existingGroup.Add(item); - - // _mailItemSource.InsertItem(existingGroup, item); } } } @@ -325,11 +344,14 @@ namespace Wino.Mail.ViewModels.Collections if (itemContainer == null) return; - // mailCopyIdHashSet.Remove(itemContainer.ItemViewModel.UniqueId); + if (itemContainer.ItemViewModel != null) + { + UpdateUniqueIdHashes(itemContainer.ItemViewModel, false); + } itemContainer.ItemViewModel?.Update(updatedMailCopy); - // mailCopyIdHashSet.Add(updatedMailCopy.UniqueId); + UpdateUniqueIdHashes(updatedMailCopy, true); // Call thread notifications if possible. itemContainer.ThreadViewModel?.NotifyPropertyChanges(); @@ -426,6 +448,8 @@ namespace Wino.Mail.ViewModels.Collections * -> Remove the thread. */ + var oldGroupKey = GetGroupingKey(threadMailItemViewModel); + await ExecuteUIThread(() => { threadMailItemViewModel.RemoveCopyItem(removalItem); }); if (threadMailItemViewModel.ThreadItems.Count == 1) @@ -435,25 +459,38 @@ namespace Wino.Mail.ViewModels.Collections var singleViewModel = threadMailItemViewModel.GetSingleItemViewModel(); var groupKey = GetGroupingKey(singleViewModel); - await RemoveItemInternalAsync(group, threadMailItemViewModel); + await ExecuteUIThread(() => { RemoveItemInternal(group, threadMailItemViewModel); }); // If thread->single conversion is being done, we should ignore it for non-draft items. // eg. Deleting a reply message from draft folder. Single non-draft item should not be re-added. if (!PruneSingleNonDraftItems || singleViewModel.IsDraft) { - await InsertItemInternalAsync(groupKey, singleViewModel); + await ExecuteUIThread(() => { InsertItemInternal(groupKey, singleViewModel); }); } } else if (threadMailItemViewModel.ThreadItems.Count == 0) { - await RemoveItemInternalAsync(group, threadMailItemViewModel); + await ExecuteUIThread(() => { RemoveItemInternal(group, threadMailItemViewModel); }); } else { // Item inside the thread is removed. + await ExecuteUIThread(() => { threadMailItemViewModel.ThreadItems.Remove(removalItem); }); - threadMailItemViewModel.ThreadItems.Remove(removalItem); + UpdateUniqueIdHashes(removalItem, false); + } + + var newGroupKey = GetGroupingKey(threadMailItemViewModel); + + // Make sure to update group key after the thread is updated. + if (!oldGroupKey.Equals(newGroupKey)) + { + await ExecuteUIThread(() => + { + RemoveItemInternal(group, threadMailItemViewModel); + InsertItemInternal(newGroupKey, threadMailItemViewModel); + }); } shouldExit = true; @@ -461,7 +498,8 @@ namespace Wino.Mail.ViewModels.Collections } else if (item.UniqueId == removeItem.UniqueId) { - await RemoveItemInternalAsync(group, item); + await ExecuteUIThread(() => { RemoveItemInternal(group, item); }); + shouldExit = true; break; diff --git a/Wino.Mail.ViewModels/Data/MailItemViewModel.cs b/Wino.Mail.ViewModels/Data/MailItemViewModel.cs index e7104246..aa77ead0 100644 --- a/Wino.Mail.ViewModels/Data/MailItemViewModel.cs +++ b/Wino.Mail.ViewModels/Data/MailItemViewModel.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using CommunityToolkit.Mvvm.ComponentModel; using Wino.Core.Domain.Entities; using Wino.Core.Domain.Models.MailItem; @@ -94,5 +95,7 @@ namespace Wino.Mail.ViewModels.Data OnPropertyChanged(nameof(Subject)); OnPropertyChanged(nameof(PreviewText)); } + + public IEnumerable GetContainingIds() => new[] { UniqueId }; } } diff --git a/Wino.Mail.ViewModels/Data/ThreadMailItemViewModel.cs b/Wino.Mail.ViewModels/Data/ThreadMailItemViewModel.cs index 74360ff5..3594b40d 100644 --- a/Wino.Mail.ViewModels/Data/ThreadMailItemViewModel.cs +++ b/Wino.Mail.ViewModels/Data/ThreadMailItemViewModel.cs @@ -12,18 +12,14 @@ namespace Wino.Mail.ViewModels.Data /// /// Thread mail item (multiple IMailItem) view model representation. /// - public class ThreadMailItemViewModel : ObservableObject, IMailItemThread, IComparable, IComparable + public partial class ThreadMailItemViewModel : ObservableObject, IMailItemThread, IComparable, IComparable { public ObservableCollection ThreadItems => ((IMailItemThread)_threadMailItem).ThreadItems; private readonly ThreadMailItem _threadMailItem; + [ObservableProperty] private bool isThreadExpanded; - public bool IsThreadExpanded - { - get => isThreadExpanded; - set => SetProperty(ref isThreadExpanded, value); - } public ThreadMailItemViewModel(ThreadMailItem threadMailItem) { @@ -36,6 +32,8 @@ namespace Wino.Mail.ViewModels.Data } } + public ThreadMailItem GetThreadMailItem() => _threadMailItem; + public IEnumerable GetMailCopies() => ThreadItems.OfType().Select(a => a.MailCopy); @@ -123,5 +121,7 @@ namespace Wino.Mail.ViewModels.Data // Get single mail item view model out of the only item in thread items. public MailItemViewModel GetSingleItemViewModel() => ThreadItems.First() as MailItemViewModel; + + public IEnumerable GetContainingIds() => ((IMailItemThread)_threadMailItem).GetContainingIds(); } } diff --git a/Wino.Mail.ViewModels/MailListPageViewModel.cs b/Wino.Mail.ViewModels/MailListPageViewModel.cs index 06d3fa87..423bdd76 100644 --- a/Wino.Mail.ViewModels/MailListPageViewModel.cs +++ b/Wino.Mail.ViewModels/MailListPageViewModel.cs @@ -172,6 +172,24 @@ namespace Wino.Mail.ViewModels { await ExecuteUIThread(() => { SelectedItemCollectionUpdated(a.EventArgs); }); }); + + MailCollection.MailItemRemoved += (c, removedItem) => + { + if (removedItem is ThreadMailItemViewModel removedThreadViewModelItem) + { + foreach (var viewModel in removedThreadViewModelItem.ThreadItems.Cast()) + { + if (SelectedItems.Contains(viewModel)) + { + SelectedItems.Remove(viewModel); + } + } + } + else if (removedItem is MailItemViewModel removedMailItemViewModel && SelectedItems.Contains(removedMailItemViewModel)) + { + SelectedItems.Remove(removedMailItemViewModel); + } + }; } #region Properties @@ -310,25 +328,6 @@ namespace Wino.Mail.ViewModels MailCollection.CoreDispatcher = Dispatcher; } - //protected override async void OnFolderUpdated(MailItemFolder updatedFolder, MailAccount account) - //{ - // base.OnFolderUpdated(updatedFolder, account); - - // // Don't need to update if the folder update does not belong to the current folder menu item. - // if (ActiveFolder == null || updatedFolder == null || !ActiveFolder.HandlingFolders.Any(a => a.Id == updatedFolder.Id)) return; - - // await ExecuteUIThread(() => - // { - // ActiveFolder.UpdateFolder(updatedFolder); - - // OnPropertyChanged(nameof(CanSynchronize)); - // OnPropertyChanged(nameof(IsFolderSynchronizationEnabled)); - // }); - - // // Force synchronization after enabling the folder. - // SyncFolder(); - //} - private async void UpdateBarMessage(InfoBarMessageType severity, string title, string message) { await ExecuteUIThread(() => @@ -618,12 +617,9 @@ namespace Wino.Mail.ViewModels if (!shouldPreventIgnoringFilter && ShouldPreventItemAdd(addedMail)) return; - await ExecuteUIThread(async () => - { - await MailCollection.AddAsync(addedMail); + await MailCollection.AddAsync(addedMail); - NotifyItemFoundState(); - }); + await ExecuteUIThread(() => { NotifyItemFoundState(); }); } catch { } finally @@ -693,6 +689,7 @@ namespace Wino.Mail.ViewModels gmailUnreadFolderMarkedAsReadUniqueIds.Remove(removedMail.UniqueId); } } + protected override async void OnDraftCreated(MailCopy draftMail, MailAccount account) { base.OnDraftCreated(draftMail, account); @@ -704,10 +701,10 @@ namespace Wino.Mail.ViewModels await listManipulationSemepahore.WaitAsync(); // Create the item. Draft folder navigation is already done at this point. - await ExecuteUIThread(async () => - { - await MailCollection.AddAsync(draftMail); + await MailCollection.AddAsync(draftMail); + await ExecuteUIThread(() => + { // New draft is created by user. Select the item. Messenger.Send(new MailItemNavigationRequested(draftMail.UniqueId, ScrollToItem: true)); @@ -940,8 +937,6 @@ namespace Wino.Mail.ViewModels if (navigatingMailItem != null) WeakReferenceMessenger.Default.Send(new SelectMailItemContainerEvent(navigatingMailItem, message.ScrollToItem)); - else - Debugger.Break(); } #endregion