fix: checking the state of a database with a binary field. - #240
fix: checking the state of a database with a binary field.#240AZabolotnikov wants to merge 94 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes an issue where binary fields were displaying as null when checking database state. The fix adds support for detecting and properly serializing binary data fields by converting them to hexadecimal representation.
Changes:
- Added binary field handling logic that detects binary data using UTF-8 encoding validation
- Created test infrastructure including a BinaryCast class and test cases to verify binary field state tracking
- Updated test expectations to include the new binary_field in the TestModel
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Testing/TableTestState.php | Core fix that detects binary data and converts it to hex representation before comparison |
| tests/support/Mock/Casts/BinaryCast.php | New custom cast implementation for testing binary field behavior |
| tests/support/Mock/Models/TestModel.php | Added binary_field to the test model for testing purposes |
| tests/ModelTestStateTest.php | Added test case for binary field changes and updated expectations |
| tests/BaseRequestTest.php | Updated test expectations to include binary_field in orderable fields |
| tests/fixtures/ModelTestStateTest/db_changes/test_models/assert_changes_binary_string.json | Test fixture defining expected state changes for binary field updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…y-field' into azabolotnikov/fix-db-check-binary-field
|
@DenTray I did a small research and found the following problem with the binary field: But I think this case is related only to the database test, because we get data from the DB facade , we should use bin2hex to cast the field inside test for model state |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…y-field' into azabolotnikov/fix-db-check-binary-field
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| $builderMock | ||
| ->method('select') | ||
| ->with('column_name') | ||
| ->willReturnSelf(); | ||
|
|
||
| $builderMock | ||
| ->method('where') | ||
| ->with('table_name', 'test_models') | ||
| ->willReturnSelf(); |
There was a problem hiding this comment.
could we simplify this method and use mockSelect method to assert that script made expected select query?
| { | ||
| "updated": [], | ||
| "created": [], | ||
| "deleted": [ |
There was a problem hiding this comment.
looks strange, it should contains only updated
Co-authored-by: DenTray <dpankratov@ronasit.com>
Co-authored-by: DenTray <dpankratov@ronasit.com>
Co-authored-by: DenTray <dpankratov@ronasit.com>
Co-authored-by: DenTray <dpankratov@ronasit.com>
…y-field' into azabolotnikov/fix-db-check-binary-field
|
@AZabolotnikov please resolve conflicts |
# Conflicts: # tests/TableTestStateTest.php
If a DB table has a binary field, the field displays as null when checking the database state.