Skip to content

Commit

Permalink
tree(fix): Validate source indices correctly on moves (#21552) (#21558)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
  • Loading branch information
daesunp and taylorsw04 authored Jun 20, 2024
1 parent 3a2899b commit b69c11f
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 3 deletions.
9 changes: 6 additions & 3 deletions packages/dds/tree/src/simple-tree/arrayNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
33 changes: 33 additions & 0 deletions packages/dds/tree/src/test/simple-tree/arrayNode.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit b69c11f

Please sign in to comment.