clickbar / laravel-magellan

A modern PostGIS toolbox for Laravel
MIT License
202 stars 12 forks source link

Geometry fields are passed as Expression during Eloquent model event are fired. #87

Closed RomainMazB closed 2 months ago

RomainMazB commented 4 months ago

Hi!

During the insert, the library operate in three steps:

  1. It caches all the original geometries attributes as their defined type (a point as Point, and so on...) and convert all geometries as Expression.
  2. It performs insert using the eloquent performInsert method
  3. It refills the model attributes as their original states

https://github.com/clickbar/laravel-magellan/blob/main/src/Database/Eloquent/HasPostgisColumns.php#L116-L118

The problem here is that if you use the laravel eloquent model event:

class Person
{
    use \Clickbar\Magellan\Database\Eloquent\HasPostgisColumns;

    /** @var string[] */
    protected $postgisColumns = [
        'location' => [
            'type' => 'geometry',
            'srid' => 4326
        ]
    ];

    protected $dispatchesEvents = [
        'created' => \App\Events\Person\Created::class
    ];
}

The event is dispatched from the eloquent performInsert call, meaning it's called with all the geometries attributes as Expression and not as Point.

I'll investigate if there is some kind of hook inside the Eloquent model event dispatcher to see if this could be fixed properly.

saibotk commented 4 months ago

Oh interesting use case!

Good catch, and thank you for letting us know about this. We would appreciate more info on this.

The goal then would be to fix this for v1, but the upcoming v2 will switch away to just using native casts directly, because those are handled in a less hacky way and should provide a cleaner way forward :)

RomainMazB commented 4 months ago

I think I made a good PR to properly fix this without touching to the event dispatcher. I just reduced the code sections that has been override from the initial class to only care about the attributes array to be inserted/updated and not the whole performInsert and performUpdate methods

saibotk commented 2 months ago

Thanks and big sorry for the delay!