Skip to content

Conversation

@Valian
Copy link
Contributor

@Valian Valian commented Jul 16, 2025

When using the project, I noticed move operation is not correctly applied when both source and target are coming from the same list. Eg this test was failing.

  # Move within the same list
  test "move element within the same list" do
    move = %Jsonpatch.Operation.Move{from: "/arr/0", path: "/arr/2"}
    target = %{"arr" => ["a", "b", "c"]}

    assert Jsonpatch.Operation.Move.apply(move, target, []) ==
             {:ok, %{"arr" => ["b", "c", "a"]}}
  end

Reason

We were first inserting and then removing. If it happened within the same list, remove operation might have targeted invalid object because indices are updated after insertion. It should first remove and then insert.

Solution

Now order of operations is correct: First, we determine which element should be inserted. Next, we remove that element, and as a last step we add previously remembered element into desired place.

@corka149 corka149 assigned corka149 and unassigned corka149 Jul 17, 2025
@corka149 corka149 added this to the 2.3 milestone Jul 18, 2025
Copy link
Owner

@corka149 corka149 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update expected error values in the tests. Else LGTM!

@corka149 corka149 self-assigned this Jul 18, 2025
@corka149 corka149 merged commit 94cb7cd into corka149:master Jul 19, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants