From 659413716c973596b364b0e080dbb91398bb7fd6 Mon Sep 17 00:00:00 2001
From: Radu Petrisel <radupetrisel@gmail.com>
Date: Tue, 25 Aug 2020 11:54:41 +0300
Subject: [PATCH] [Fix] SortedObservableCollectionAdaptor does Remove -> Add
 instead of Replace when index not changed (#392)

* Replace item if index not changed

Change-Id: Ib7564fb941849b4a12b30d3b6bc4dd6ab8dabfc8

* Add unit tests

Change-Id: Ib62b5916b63961e2b87b48f8d398ab37905d8497

Co-authored-by: Radu Petrisel <radu.petrisel@itiviti.com>
---
 ...ervableCollectionBindCacheSortedFixture.cs | 57 +++++++++++++++++++
 .../SortedObservableCollectionAdaptor.cs      | 12 +++-
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs b/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs
index 0fcb6ec7a..ec95c1287 100644
--- a/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs
+++ b/src/DynamicData.Tests/Binding/ObservableCollectionBindCacheSortedFixture.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
+using System.Reactive.Linq;
 using DynamicData.Binding;
 using DynamicData.Tests.Domain;
 using FluentAssertions;
@@ -174,5 +175,61 @@ public void TreatMovesAsRemoveAdd()
 			    latestSetWithMoves.Adds.Should().Be(0);
 		    }
 	    }
+
+        [Fact]
+        public void UpdateToSourceSendsReplaceIfSortingIsNotAffected()
+        {
+            var person1 = new Person("Adult1", 10);
+            var person2 = new Person("Adult2", 11);
+
+            NotifyCollectionChangedAction action = default;
+            _source.AddOrUpdate(person1);
+            _source.AddOrUpdate(person2);
+
+            var person2Updated = new Person("Adult2", 12);
+
+            using (_collection
+                .ObserveCollectionChanges()
+                .Select(change => change.EventArgs.Action)
+                .Subscribe(act => action = act))
+            {
+                _source.AddOrUpdate(person2Updated);
+            }
+
+            action.Should().Be(NotifyCollectionChangedAction.Replace, "The notification type should be Replace");
+            _collection.Should().Equal(person1, person2Updated);
+        }
+
+        [Fact]
+        public void UpdateToSourceSendsRemoveAndAddIfSortingIsAffected()
+        {
+            var person1 = new Person("Adult1", 10);
+            var person2 = new Person("Adult2", 11);
+            var person2Updated = new Person("Adult2", 1);
+
+            var actions = new List<NotifyCollectionChangedAction>();
+            var collection = new ObservableCollectionExtended<Person>();
+
+            using (var source = new SourceCache<Person, string>(person => person.Name))
+            using (source.Connect()
+                .Sort(SortExpressionComparer<Person>.Ascending(person => person.Age))
+                .Bind(collection)
+                .Subscribe())
+            {
+                source.AddOrUpdate(person1);
+                source.AddOrUpdate(person2);
+
+                using (collection
+                    .ObserveCollectionChanges()
+                    .Select(change => change.EventArgs.Action)
+                    .Subscribe(act => actions.Add(act)))
+                {
+                    source.AddOrUpdate(person2Updated);
+                }
+            }
+
+            actions.Should().Equal(NotifyCollectionChangedAction.Remove, NotifyCollectionChangedAction.Add);
+            collection.Should().Equal(person2Updated, person1);
+        }
     }
 }
diff --git a/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs b/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs
index 01c54c416..64fcf7ace 100644
--- a/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs
+++ b/src/DynamicData/Binding/SortedObservableCollectionAdaptor.cs
@@ -103,8 +103,16 @@ private void DoUpdate(ISortedChangeSet<TObject, TKey> updates, IObservableCollec
                         list.Move(update.PreviousIndex, update.CurrentIndex);
                         break;
                     case ChangeReason.Update:
-                        list.RemoveAt(update.PreviousIndex);
-                        list.Insert(update.CurrentIndex, update.Current);
+                        if (update.PreviousIndex != update.CurrentIndex)
+                        {
+                            list.RemoveAt(update.PreviousIndex);
+                            list.Insert(update.CurrentIndex, update.Current);
+                        }
+                        else
+                        {
+                            list.Replace(update.Previous.Value, update.Current);
+                        }
+
                         break;
                 }
             }