From b69c11f50be699f30435b8d1d60a7dfd9d031eeb Mon Sep 17 00:00:00 2001
From: daesunp <138815173+daesunp@users.noreply.github.com>
Date: Thu, 20 Jun 2024 14:31:05 -0700
Subject: [PATCH] tree(fix): Validate source indices correctly on moves
 (#21552) (#21558)

## Description

This PR fixes the validation of moves when there is a specified source
array. Previously, the validation was using the destination array for
validation unconditionally instead of the source when it was passed.

Co-authored-by: Taylor Williams <60717813+taylorsw04@users.noreply.github.com>
---
 .../dds/tree/src/simple-tree/arrayNode.ts     |  9 +++--
 .../src/test/simple-tree/arrayNode.spec.ts    | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/packages/dds/tree/src/simple-tree/arrayNode.ts b/packages/dds/tree/src/simple-tree/arrayNode.ts
index fb0df12406e1..bde85fab5cc6 100644
--- a/packages/dds/tree/src/simple-tree/arrayNode.ts
+++ b/packages/dds/tree/src/simple-tree/arrayNode.ts
@@ -773,17 +773,20 @@ abstract class CustomArrayNodeBase<const T extends ImplicitAllowedTypes>
 		fieldEditor.remove(removeStart, removeEnd - removeStart);
 	}
 	public moveToStart(sourceIndex: number, source?: TreeArrayNode): void {
-		const field = getSequenceField(this);
+		const sourceArray = source ?? this;
+		const field = getSequenceField(sourceArray);
 		validateIndex(sourceIndex, field, "moveToStart");
 		this.moveRangeToIndex(0, sourceIndex, sourceIndex + 1, source);
 	}
 	public moveToEnd(sourceIndex: number, source?: TreeArrayNode): void {
-		const field = getSequenceField(this);
+		const sourceArray = source ?? this;
+		const field = getSequenceField(sourceArray);
 		validateIndex(sourceIndex, field, "moveToEnd");
 		this.moveRangeToIndex(this.length, sourceIndex, sourceIndex + 1, source);
 	}
 	public moveToIndex(index: number, sourceIndex: number, source?: TreeArrayNode): void {
-		const field = getSequenceField(this);
+		const sourceArray = source ?? this;
+		const field = getSequenceField(sourceArray);
 		validateIndex(index, field, "moveToIndex", true);
 		validateIndex(sourceIndex, field, "moveToIndex");
 		this.moveRangeToIndex(index, sourceIndex, sourceIndex + 1, source);
diff --git a/packages/dds/tree/src/test/simple-tree/arrayNode.spec.ts b/packages/dds/tree/src/test/simple-tree/arrayNode.spec.ts
index c6bed983fafe..e6751342d44f 100644
--- a/packages/dds/tree/src/test/simple-tree/arrayNode.spec.ts
+++ b/packages/dds/tree/src/test/simple-tree/arrayNode.spec.ts
@@ -110,6 +110,17 @@ describe("ArrayNode", () => {
 		});
 
 		describe("moveToStart", () => {
+			it("move element to start of empty array", () => {
+				const schema = schemaFactory.object("parent", {
+					array1: schemaFactory.array(schemaFactory.number),
+					array2: schemaFactory.array(schemaFactory.number),
+				});
+				const { array1, array2 } = hydrate(schema, { array1: [], array2: [1, 2, 3] });
+				array1.moveToStart(1, array2);
+				assert.deepEqual([...array1], [2]);
+				assert.deepEqual([...array2], [1, 3]);
+			});
+
 			it("move within field", () => {
 				const array = hydrate(schemaType, [1, 2, 3]);
 				array.moveToStart(1);
@@ -144,6 +155,17 @@ describe("ArrayNode", () => {
 		});
 
 		describe("moveToEnd", () => {
+			it("move element to end of empty array", () => {
+				const schema = schemaFactory.object("parent", {
+					array1: schemaFactory.array(schemaFactory.number),
+					array2: schemaFactory.array(schemaFactory.number),
+				});
+				const { array1, array2 } = hydrate(schema, { array1: [], array2: [1, 2, 3] });
+				array1.moveToEnd(1, array2);
+				assert.deepEqual([...array1], [2]);
+				assert.deepEqual([...array2], [1, 3]);
+			});
+
 			it("move within field", () => {
 				const array = hydrate(schemaType, [1, 2, 3]);
 				array.moveToEnd(1);
@@ -178,6 +200,17 @@ describe("ArrayNode", () => {
 		});
 
 		describe("moveToIndex", () => {
+			it("move element to start of empty array", () => {
+				const schema = schemaFactory.object("parent", {
+					array1: schemaFactory.array(schemaFactory.number),
+					array2: schemaFactory.array(schemaFactory.number),
+				});
+				const { array1, array2 } = hydrate(schema, { array1: [], array2: [1, 2, 3] });
+				array1.moveToIndex(0, 1, array2);
+				assert.deepEqual([...array1], [2]);
+				assert.deepEqual([...array2], [1, 3]);
+			});
+
 			it("move within field", () => {
 				const array = hydrate(schemaType, [1, 2, 3]);
 				array.moveToIndex(0, 1);