-
Notifications
You must be signed in to change notification settings - Fork 48
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
Support Laravel Nova #28
Comments
Hi @mziraki, thanks for opening this issue. I need some context in order to understand the issue. I don't work with Nova. Can you share some code? When the error occurs? Is that really related to the package? |
@MatanYadaev hi, thanks for quick reply |
e.g: Exhibition::find(86) dd with this package: dd with grimzy package: |
@mziraki I have no idea how can I help you with that. Feel free to send a PR if you find a solution. |
@mziraki I've just run into exactly the same problem, and I've asked about it in the Nova channel on Laracasts, but no answers yet :( I wonder if it's trying to treat the WKB string as UTF-8, which will probably cause that error message, though it should not be trying to do that if the cast is being applied when it is fetched from the DB – which I have asked it to do. |
@Synchro you can fork my fork (https://github.com/mziraki/laravel-mysql-spatial) and use:
in your composer.json |
@MatanYadaev the problem is that it's not casting the raw WKB value to an object. The code to reproduce that is very simple, as @mziraki said:
and see in the output:
This should be something like:
so for some reason the cast is not being applied when creating or retrieving a point field. |
I have set breakpoints on the first line of every method in Point, Geometry, and GeometryCast classes, and none of them are called when doing find and dump operations! I have the feeling that something small but critical is missing. Also note that this issue isn't anything to do with Nova, it just happens to show up there. |
I made a fork of this repo and have written a test that shows the problem.
The first
the second one does this:
So when the data is retrieved from the DB, it it not turned back into an object, and this is apparently not covered by the tests. I spotted that
but that didn't do anything. |
Oh, and here's a weird wrinkle; this passes!
|
@Synchro No worries, we'll fix it easily. Can you please share the branch that has failing tests with me? I just need a way to reproduce the bug so I'll know when it got fixed. Just send a PR with the failing tests. |
Thanks. I've pushed a test, however, I can't figure out how to expose the failing behaviour in it. When you inspect the individual properties, they are cast correctly, but when you look at them with This assertion fails:
but that seems to be more to do with the test factory, so I have modified the factory to set more of the default properties, but it still has a few differences. |
@Synchro I looked and played with your branch. It doesn't look like a real issue. Laravel stores the original value of the column $this->assertInstanceOf(Point::class, $testPlace->point);
$this->assertEquals($testPlace->toArray(), $testPlace2->toArray());
$this->assertEquals($testPlace->point->toArray(), $testPlace2->toArray()['point']); BTW, the return type of |
Well, the problem must be somewhere else then. I don't think it's a Nova issue because nova is simply acting on the instance of my model that is passed to it. I've also seen this problem that may be related: In my model I have this constructor:
If I do a |
@Synchro Can you reproduce the issue in a non-Nova environment? Can you try to write your own cast? Try removing |
I've invited you to a private repo for this, including Nova. It shows the casting problem described here, but also (incidentally) demonstrates the issue with non-null geo fields (#32) if you try to create a new Place, which it seems I have not solved after all. I also reported this in the Nova issues project to try and attack this problem from both ends. |
Hi @MatanYadaev, did you get a chance to look at the project I set up? |
@Synchro Please create a new project from scratch that reproduce the issue without having any dependencies like Nova. |
Note that doing the equivalent thing with the Grimzy spatial library works, however, it lacks support for L9, and I prefer your API. |
|
Same problem. class GeoobjectGeometry extends AbstractEloquentModel {
// Casts
protected $casts = [
'center' => Point::class,
'coordinates' => Geometry::class,
];
// .....
public function getRawOriginal($key = null, $default = null)
{
foreach (['center', 'coordinates'] as $cast) {
$this->original[$cast] = Geometry::fromWkb(Arr::get($this->original, $cast, $default));
}
return Arr::get($this->original, $key, $default);
}
} |
FYI: |
Hi everyone. Just look at this code: Model: <?php
namespace App\Models;
use Illuminate\Database\Eloquent\Model;
use MatanYadaev\EloquentSpatial\Objects\Polygon;
class Area extends Model
{
protected $casts = [
'location' => Polygon::class,
];
} Test: # retrieve a record
$area = Area::query()->latest()->first();
# datasets
$coordinates = [['4.3876602', '51.9096641'], ['4.3869092', '51.9085389'], ['4.387038', '51.9082079'], ['4.3946554', '51.906434'], ['4.3953421', '51.906434'], ['4.3963506', '51.9066856'], ['4.3959214', '51.9078505'], ['4.393883', '51.9082212'], ['4.391072', '51.9085389'], ['4.3909647', '51.9086845'], ['4.3910505', '51.9092405'], ['4.3894627', '51.9093331'], ['4.3876602', '51.9096641']];
$coordinates = [['4.3874886', '51.9096893'], ['4.3836691', '51.9100599'], ['4.3832185', '51.9090803'], ['4.3859865', '51.908392'], ['4.3864801', '51.9083787'], ['4.3866946', '51.9084846'], ['4.3874886', '51.9096893']];
# prepare a polygon using the chosen dataset
$points = [];
foreach ($coordinates as $coordinate) {
$points[] = new Point($coordinate[0], $coordinate[1]);
}
$area->location = new Polygon([
new LineString($points)
]);
# results
dump('raw original: ', $area->getRawOriginal('location'));
dump('dirty: ', $area->getDirty());
$area->save();
dump('changes: ', $area->getChanges());
dd(); You will see these results: ❌ Get Dirty ❌ Get Changes
More Information:
|
@mostafaznv Great, thanks for the research. Can you please send a PR fixing it? |
Yes, I will work on it soon. |
Should be fixed in version 2. But, there's a catch, Laravel doesn't compare between two different geometry instances. They might have the same values, but the instances are different, so Laravel see it as dirty. You can fix it by adding this snippet to your model: public function originalIsEquivalent($key)
{
if (! array_key_exists($key, $this->original)) {
return false;
}
if (parent::originalIsEquivalent($key)) {
return true;
}
if($this->isClassCastable($key) && is_subclass_of($this->getCasts()[$key], Geometry::class)) {
$originalGeometry = $this->getOriginal($key);
$attributeWkbOrWkt = Arr::get($this->attributes, $key);
return $this->castAttribute($key, $attributeWkbOrWkt) == $originalGeometry;
}
return false;
} Please check if it works for you. If so, I'll try to add it to the core of this package. |
Hi @MatanYadaev Thanks for this update.
|
@mostafaznv Can you send a PR with a failing test? |
I can confirm this workaround works nice and I'm excited to use it in new versions 🚀 |
@mostafaznv Version 2 is already released. Please let me know how it works with Nova. |
Last week when I tried (of course I added that |
Any updates about |
@mostafaznv No. I don't think I will push this into the package's source code. |
error:
Unable to encode attribute [original] for model [Laravel\Nova\Actions\ActionEvent] to JSON: Malformed UTF-8 characters, possibly incorrectly encoded.
The text was updated successfully, but these errors were encountered: