Fix: Renaming file the same as an older deleted one causes new file to be grouped incorrectly

src/Files.App/Utils/Storage/Collection/BulkConcurrentObservableCollection.cs

@@ -2,12 +2,14 @@
 // Licensed under the MIT License.
 
 using System.Collections.Specialized;
+using System.Runtime.CompilerServices;
 
 namespace Files.App.Utils.Storage
 {
 	[DebuggerTypeProxy(typeof(CollectionDebugView<>))]
 	[DebuggerDisplay("Count = {Count}")]
 	public class BulkConcurrentObservableCollection<T> : INotifyCollectionChanged, INotifyPropertyChanged, ICollection<T>, IList<T>, ICollection, IList
+		where T : class
 	{
 		protected bool isBulkOperationStarted;
 		private readonly object syncRoot = new object();
@@ -200,7 +202,10 @@ private void AddItemsToGroup(IEnumerable<T> items, CancellationToken token = def
 					GroupedCollection?.Add(group);
 					GroupedCollection!.IsSorted = false;
 				}
+				// Register property changed handler to react to date changes so the item can be moved
+				RegisterPropertyChanged(item);
 			}
+
 		}
 
 		private void RemoveItemsFromGroup(IEnumerable<T> items)
@@ -216,6 +221,100 @@ private void RemoveItemsFromGroup(IEnumerable<T> items)
 					if (group.Count == 0)
 						GroupedCollection?.Remove(group);
 				}
+
+				// Unregister change handler when item is removed from groups/collection
+				UnregisterPropertyChanged(item);
+			}
+		}
+
+		private readonly ConditionalWeakTable<T, PropertyChangedEventHandler> propertyChangedHandlers = new();
+
+		private void RegisterPropertyChanged(T item)
+		{
+			if (item is INotifyPropertyChanged notifier)
+			{
+				// avoid duplicate handler
+				if (propertyChangedHandlers.TryGetValue(item, out _))
+					return;
+
+				PropertyChangedEventHandler handler = (s, e) =>
+				{
+					// React to date fields changing — move item between groups if needed
+					if (e.PropertyName is "ItemDateModifiedReal" or "ItemDateCreatedReal" or "ItemDateAccessedReal" or "ItemDateDeletedReal")
+						OnItemDatePropertyChanged((T)s);
+				};
+
+				propertyChangedHandlers.Add(item, handler);
+				notifier.PropertyChanged += handler;
+			}
+		}
+
+		private void UnregisterPropertyChanged(T item)
+		{
+			if (item is INotifyPropertyChanged notifier)
+			{
+				if (propertyChangedHandlers.TryGetValue(item, out var handler))
+				{
+					notifier.PropertyChanged -= handler;
+					propertyChangedHandlers.Remove(item);
+				}
+			}
+		}
+
+		private void OnItemDatePropertyChanged(T item)
+		{
+			if (!IsGrouped || ItemGroupKeySelector is null)
+				return;
+
+			var newKey = GetGroupKeyForItem(item);
+			if (newKey is null)
+				return;
+
+			var oldKey = (item is IGroupableItem groupable) ? groupable.Key : null;
+			if (oldKey == newKey)
+				return;
+
+			// Move item between groups under a lock to keep collection consistent
+			lock (syncRoot)
+			{
+				// remove from old group
+				if (!string.IsNullOrEmpty(oldKey))
+				{
+					var oldGroup = GroupedCollection?.Where(x => x.Model.Key == oldKey).FirstOrDefault();
+					if (oldGroup is not null && oldGroup.Contains(item))
+					{
+						oldGroup.Remove(item);
+						if (oldGroup.Count == 0)
+							GroupedCollection?.Remove(oldGroup);
+					}
+				}
+
+				// add to new group
+				var groups = GroupedCollection?.Where(x => x.Model.Key == newKey);
+				if (item is IGroupableItem gp)
+					gp.Key = newKey;
+
+				if (groups is not null && groups.Any())
+				{
+					var gp = groups.First();
+					if (!gp.Contains(item))
+						gp.Add(item);
+					gp.IsSorted = false;
+				}
+				else
+				{
+					var group = new GroupedCollection<T>(newKey)
+					{
+						item
+					};
+
+					group.GetExtendedGroupHeaderInfo = GetExtendedGroupHeaderInfo;
+					if (GetGroupHeaderInfo is not null)
+						GetGroupHeaderInfo.Invoke(group);
+
+					GroupedCollection?.Add(group);
+					GroupedCollection!.IsSorted = false;
+				}
 			}
 		}
 
@@ -265,6 +364,10 @@ public void Clear()
 		{
 			lock (syncRoot)
 			{
+				// Unregister handlers for all items before clearing
+				foreach (var it in collection.ToList())
+					UnregisterPropertyChanged(it);
+
 				collection.Clear();
 				GroupedCollection?.Clear();
 

tests/Files.App.UnitTests/BulkConcurrentObservableCollectionTests.cs

@@ -0,0 +1,63 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.Linq;
+
+namespace Files.App.UnitTests
+{
+    [TestClass]
+    public class BulkConcurrentObservableCollectionTests
+    {
+        private class TestItem : INotifyPropertyChanged, Utils.Storage.IGroupableItem
+        {
+            public string Key { get; set; }
+
+            private DateTimeOffset _date;
+            public DateTimeOffset ItemDateModifiedReal
+            {
+                get => _date;
+                set
+                {
+                    if (_date != value)
+                    {
+                        _date = value;
+                        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(ItemDateModifiedReal)));
+                    }
+                }
+            }
+
+            public event PropertyChangedEventHandler? PropertyChanged;
+
+            public override string ToString() => ItemDateModifiedReal.ToString();
+        }
+
+        [TestMethod]
+        public void When_ItemDateChanges_ItemMovesBetweenGroups()
+        {
+            // Group by logic: within 7 days = "Recent", else "Old"
+            var col = new Utils.Storage.BulkConcurrentObservableCollection<TestItem>();
+            col.ItemGroupKeySelector = item => (DateTimeOffset.Now - item.ItemDateModifiedReal).TotalDays <= 7 ? "Recent" : "Old";
+
+            var recentItem = new TestItem { ItemDateModifiedReal = DateTimeOffset.Now.AddDays(-3) };
+            var oldItem = new TestItem { ItemDateModifiedReal = DateTimeOffset.Now.AddDays(-400) };
+
+            col.Add(recentItem);
+            col.Add(oldItem);
+
+            Assert.IsNotNull(col.GroupedCollection);
+            Assert.AreEqual(2, col.GroupedCollection.Count);
+
+            // Now change recentItem date so it becomes old
+            recentItem.ItemDateModifiedReal = DateTimeOffset.Now.AddDays(-400);
+
+            // It should have been moved to the old group
+            var recentGroup = col.GroupedCollection.FirstOrDefault(g => g.Model.Key == "Recent");
+            var oldGroup = col.GroupedCollection.FirstOrDefault(g => g.Model.Key == "Old");
+
+            Assert.IsTrue(recentGroup == null || !recentGroup.Contains(recentItem), "recentItem should not be in recent group anymore");
+            Assert.IsNotNull(oldGroup, "old group should exist");
+            Assert.IsTrue(oldGroup.Contains(recentItem), "recentItem should be in old group now");
+        }
+    }
+}

tests/Files.App.UnitTests/Files.App.UnitTests.csproj

@@ -0,0 +1,18 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net9.0</TargetFramework>
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.10.0" />
+    <PackageReference Include="MSTest.TestAdapter" Version="3.1.3" />
+    <PackageReference Include="MSTest.TestFramework" Version="3.1.3" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="../../src/Files.App/Files.App.csproj" />
+  </ItemGroup>
+
+</Project>