mikebronner / nova-map-marker-field

Provides an visual interface for editing latitude and longitude coordinates.
MIT License
131 stars 36 forks source link

Support for storing point in geo point field #53

Closed nshontz closed 3 years ago

nshontz commented 3 years ago

PR to fix issue referenced: https://github.com/GeneaLabs/nova-map-marker-field/issues/51

Allows the user to select field type (currently only implements 'point') uses ST_POINTFROMTEXT which should work with MySQL, PostGIS, and other geospatial database engines Allows the user to set a custom SRID or use a sensible default (4326)

nshontz commented 3 years ago

@mikebronner Are there any changes you'd like me to make on this PR?

mikebronner commented 3 years ago

@nshontz Was just waiting for the go-ahead that you were finished. Will review over the weekend. :) Thanks!

mikebronner commented 3 years ago

@nshontz I'm working through the code changes in your PR. Could you explain why the ActionResource class is necessary?

nickshontz-ych commented 3 years ago

Laravel Nova logs all of the events in action event, there is an issue when your model contains non-utf8 values, in this case the spatial column.

The ActionResource let's you implement your own ActionEvent which handles the encoding of the model.

mikebronner commented 3 years ago

@nshontz Thanks for all your work on this PR. Unfortunately it appears to currently only support MySQL. In order to be considered, it will need to support Postgres and SQLite without breaking. (Meaning for SQLite it would need to default to something that won't break, even if it isn't a geospatial field.

mikebronner commented 3 years ago

Laravel Nova logs all of the events in action event, there is an issue when your model contains non-utf8 values, in this case the spatial column.

The ActionResource let's you implement your own ActionEvent which handles the encoding of the model.

Thanks for the clarification. I didn't realize that the spatial data was not stored in UTF-8 format.

nickshontz-ych commented 3 years ago

Unfortunately it appears to currently only support MySQL. In order to be considered, it will need to support Postgres and SQLite without breaking.

It should support Postgres (with the PostGIS extension) if you're seeing an error, I can give it a look but the ST_POINTFROMTEXT function is available in PostGIS.

I've not worked with SQLite spatial columns but I will spin something up and get it working.

mikebronner commented 3 years ago

@nshontz SQLite doesn't support spatial columns that I know of. I think the workaround there would be to store both coordinates separated by a comma in a single field, then parse them out when reading them? What do you think?

Regarding spatial handling in Postgres, I see what you are saying now. I was trying to read up on it, and saw usage of the point() method, without the use of ST_POINTFROMTEXT.

mikebronner commented 3 years ago

Laravel Nova logs all of the events in action event, there is an issue when your model contains non-utf8 values, in this case the spatial column.

The ActionResource let's you implement your own ActionEvent which handles the encoding of the model.

Is this documented anywhere? I would love to read up on this limitation in Nova. Thanks! :)

nshontz commented 3 years ago

SQLite doesn't support spatial columns that I know of. I think the workaround there would be to store both coordinates separated by a comma in a single field, then parse them out when reading them?

I'm hesitant to blend or muddy support between supporting a true geospatial column type for any DB engine that implements the OpenGIS® spec (which this PR aims to do) and simply supporting the ability to support a single column with comma-separated-values, which might be useful but seems separate from this implementation.

So, if SQLite doesn't implement this geospatial standard, I would be hesitant to try and shoehorn faux support for a single column vs separate columns for lat/long.

I will look for some examples/documentation of Nova and UTF8 issues.