NEXT-00000 - Fix changeset issues #3695
Conversation
| @@ -74,6 +74,13 @@ public function hasChanged(string $property): bool | |||
| return \array_key_exists($property, $this->after) || $this->isDelete; | |||
| } | |||
|
|
|||
| public function merge(ChangeSet $changeSet): void | |||
There was a problem hiding this comment.
Hi @jasperP98,
thanks for the bug fix, changes are looking good to me, but could you please provide some tests for it?
Integration tests would also be fine.
There was a problem hiding this comment.
hi @OliverSkroblin i will do this ASAP
There was a problem hiding this comment.
I created a UT to check the constructor and merge function from the changeset
|
Hello, thank you for creating this pull request. Please use this issue to track the state of your pull request. |
|
Hi, can you please squash and rebase your commits? Otherwise i have issues to import and merge it. Thanks in advance |
Otherwise null and null will resolve in a change
jasperP98
force-pushed
the
fix-changeset-issues
branch
from
d556829
to
1f5ea48
Compare
|
I synced the branch and cherry picked the commits. It should be fine now. |
|
Hello, thank you for creating this pull request. Please use this issue to track the state of your pull request. |
1. Why is this change necessary?
When we generate a changeset and the before and after property are both
nullthis will be seen a merge since the after property is being type casted to a string. This is incorrect, or both should be type casted or none.When a JsonUpdateCommand is triggered for the same entity, this command will override the previous changeset, leading to incorrect write results.
2. What does this change do, exactly?
This change will type cast both the after and the before property.
This change will also merge the changeset from the JsonUpdateCommand and the previous command from the same entity.
3. Describe each step to reproduce the issue or behaviour.
/
4. Please link to the relevant issues (if any).
/
5. Checklist