Skip to content

Commit eb0615f

Browse files
committed
Merge branch 'bugfix/TopicRepository.Move-first-child' into develop
Fixed a bug where moving the first (or only) child of a topic to after a sibling in another topic would be inadvertently trip an escape condition meant to bypass cases where the topic was being moved to the same location that it was already located. As part of this, introduced two unit tests—one to validate that the bug no longer appears, and another to ensure that the escape condition still works. This resolves Issue #76.
2 parents 8d460ae + 858774f commit eb0615f

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

OnTopic.Tests/TopicRepositoryBaseTest.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,30 @@ public void Delete_ContentTypeDescriptor_UpdatesContentTypeCache() {
849849

850850
}
851851

852+
/*==========================================================================================================================
853+
| TEST: MOVE: AFTER SIBLING: SET CORRECTLY
854+
\-------------------------------------------------------------------------------------------------------------------------*/
855+
/// <summary>
856+
/// Moves a <see cref="Topic"/> after a sibling in another parent, and ensures it is set correctly.
857+
/// </summary>
858+
[Fact]
859+
public void Move_AfterSibling_SetCorrectly() {
860+
861+
var source = new Topic("Source", "Page");
862+
var topic = new Topic("Test", "Page", source);
863+
var target = new Topic("Target", "Page");
864+
var sibling = new Topic("Sibling", "Page", target);
865+
var olderSibling = new Topic("OlderSibling", "Page", target);
866+
867+
_topicRepository.Move(topic, target, sibling);
868+
869+
Assert.Equal<Topic?>(target, topic.Parent);
870+
Assert.Equal<int>(0, target.Children.IndexOf(sibling));
871+
Assert.Equal<int>(1, target.Children.IndexOf(topic));
872+
Assert.Equal<int>(2, target.Children.IndexOf(olderSibling));
873+
874+
}
875+
852876
/*==========================================================================================================================
853877
| TEST: MOVE: CONTENT TYPE DESCRIPTOR: UPDATES CONTENT TYPE CACHE
854878
\-------------------------------------------------------------------------------------------------------------------------*/
@@ -1105,5 +1129,30 @@ public void Move_TopicMovedEvent_IsRaised() {
11051129

11061130
}
11071131

1132+
/*==========================================================================================================================
1133+
| TEST: MOVE: SAME LOCATION: EVENT NOT RAISED
1134+
\-------------------------------------------------------------------------------------------------------------------------*/
1135+
/// <summary>
1136+
/// Creates a <see cref="Topic"/> and then moves it to the exact same location in the tree. Ensures that the <see cref="
1137+
/// ITopicRepository.TopicMoved"/> event is not raised.
1138+
/// </summary>
1139+
[Fact]
1140+
public void Move_SameLocation_EventNotRaised() {
1141+
1142+
var parent = new Topic("Parent", "Page", null, 1);
1143+
var sibling = new Topic("Sibling", "Page", parent, 2);
1144+
var topic = new Topic("Test", "Page", parent, 3);
1145+
var hasFired = false;
1146+
1147+
_cachedTopicRepository.TopicMoved += eventHandler;
1148+
_cachedTopicRepository.Move(topic, parent, sibling);
1149+
_cachedTopicRepository.TopicMoved -= eventHandler;
1150+
1151+
Assert.False(hasFired);
1152+
1153+
void eventHandler(object? sender, TopicMoveEventArgs eventArgs) => hasFired = true;
1154+
1155+
}
1156+
11081157
} //Class
11091158
} //Namespace

OnTopic/Repositories/TopicRepository.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,7 @@ public override sealed void Move([ValidatedNotNull]Topic topic, [ValidatedNotNul
492492
if (
493493
sibling is not null &&
494494
topic.Parent is not null &&
495+
topic.Parent == target &&
495496
topic.Parent.Children.IndexOf(sibling) == topic.Parent.Children.IndexOf(topic)-1) {
496497
return;
497498
}

0 commit comments

Comments
 (0)