Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit introduced breaking changes #77

Open
tasinttttttt opened this issue Jun 27, 2024 · 2 comments
Open

Commit introduced breaking changes #77

tasinttttttt opened this issue Jun 27, 2024 · 2 comments

Comments

@tasinttttttt
Copy link
Contributor

Versions >4.4 have a merged commit which introduced a breaking change (with tests not passing with the breaking change).

An example is found in the Feature test post_can_revert_to_target_version

In the tests, there's a json field called extends which is cast as an array. With the change introduced here, the cast is not taken into account:

Failed asserting that '{"foo":"bar"}' is identical to Array &0 [
    'foo' => 'bar',
].

Makes me think of the merge process, I don't think tests are run before merging right?

Originally posted by @tasinttttttt in #50 (comment)

@overtrue
Copy link
Owner

overtrue commented Jun 27, 2024

@tasinttttttt Yes, but I think it's wrong before:

Before

$post = Post::create(['title' => 'version1', 'content' => 'version1 content']);
$post->update(['title' => 'version2', 'extends' => ['foo' => 'bar']]);

dd($post->versions[1]->contents['extends']);

# output
array:1 [
  "foo" => "bar"
]

when we revert the version, it will cause the error:

dump($post->versions[1]->revert(), $post);
// TypeError: json_decode(): Argument #1 ($json) must be of type string, array given

After

$post = Post::create(['title' => 'version1', 'content' => 'version1 content']);
$post->update(['title' => 'version2', 'extends' => ['foo' => 'bar']]);

dd($post->versions[1]->contents['extends']);

# output
"{"foo":"bar"}"
dump($post->versions[1]->revert(), $post->extends);

# output 
array:1 [
  "foo" => "bar"
]

So, I think it would be ok if you have any questions please let me know, and I will try my best to fix it.

@tasinttttttt
Copy link
Contributor Author

@tasinttttttt Yes, but I think it's wrong before:

Before

$post = Post::create(['title' => 'version1', 'content' => 'version1 content']);
$post->update(['title' => 'version2', 'extends' => ['foo' => 'bar']]);

dd($post->versions[1]->contents['extends']);

# output
array:1 [
  "foo" => "bar"
]

when we revert the version, it will cause the error:

dump($post->versions[1]->revert(), $post);
// TypeError: json_decode(): Argument #1 ($json) must be of type string, array given

By "Before", do you mean v4.4.0 or versions before 4.4.0 ?
(Either way I wasn't able to reproduce the json_decode issue.)

Also, don't you think the mentioned failing test in 4.x should at least be updated to show the expected behavior?

overtrue added a commit that referenced this issue Jul 2, 2024
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

No branches or pull requests

2 participants