Problem/Motivation

Currently when exposing a Geofield via JSON:API you get the raw property values. Since Geofield is already capable of converting the geo data to GeoJSON it would be nice if there were a field enhancer to expose the GeoJSON output of the field via JSON:API.

Steps to reproduce

-

Proposed resolution

Add a field enhancer.

Remaining tasks

User interface changes

-

API changes

-

Data model changes

-

Issue fork geofield-3576947

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

namingthings created an issue. See original summary.

nickolaj made their first commit to this issue’s fork.

nickolaj opened merge request !45

nickolaj’s picture

nickolaj
Primary language Ukrainian
commented
Status: Active » Needs review

Added a JSON:API Extras field enhancer plugin that converts Geofield WKT data to GeoJSON format on output and back to WKT on input. Uses the existing `geofield.geophp` service for geometry conversion.

itamair’s picture

itamair commented
Status: Needs review » Needs work

Thanks for providing all this.
Actually the MR !45 implementation of the GeofieldGeoJsonEnhancer looks correct, and implementing this feature request functionalities.
BUT the added Unit Tests (in geofield/tests/src/Unit/Plugin/jsonapi/FieldEnhancer/GeofieldGeoJsonEnhancerTest.php) are not passing,
... so this still look needing some refinement work.

PS: In relation to this review I created this JSON:API Extras Bug issue (and additional patch): https://www.drupal.org/project/jsonapi_extras/issues/3577834

namingthings’s picture

namingthings commented
Status: Needs work » Needs review

Thank you @itamair for reviewing this.
The Unit test should now pass the pipeline.

I noticed that when passing a geojson array that either doesnt have a type key or an invalid type key that a warning is raised that cannot be caught.
Any thoughts on that?

itamair’s picture

itamair commented
Status: Needs review » Reviewed & tested by the community

@namingthings thanks ...
I found and dedicated some time in better reviewing all this,
and provided some meaningful improvements:

Due the dependency from the from the jsonapi_extras module, etc. the Kernel test is a better way to test the GeofieldGeoJsonEnhancer.php.
This is also proven by the Kernel test from the jsonapi_extras module itself: jsonapi_extras/tests/src/Kernel/Plugin/jsonapi/FieldEnhancer/ListFieldEnhancerTest.php

Thus I removed the Unit Test (tests/src/Unit/Plugin/jsonapi/FieldEnhancer/GeofieldGeoJsonEnhancerTest.php) and your latest references / dependencies from the jsonapi_extras module in the Geofield code base,
and added the Kernel Test: tests/src/Kernel/Plugin/jsonapi/FieldEnhancer/GeofieldGeoJsonEnhancerTest.php with multiple additional tests.

Also I provided some small improvements to the GeofieldGeoJsonEnhancer FieldEnhancer definition itself.

itamair’s picture

itamair commented
Status: Reviewed & tested by the community » Needs work
itamair’s picture

itamair commented
Assigned: namingthings » itamair
itamair’s picture

itamair commented
Status: Needs work » Reviewed & tested by the community

Well, ok. This is interesting and logical. The Kernel test /tests/src/Kernel/Plugin/jsonapi/FieldEnhancer/GeofieldGeoJsonEnhancerTest.php
would fail now, due to this reported issue in the jsonapi_extras module: https://www.drupal.org/project/jsonapi_extras/issues/3577834
That is correct.
But in that case the Geofield module Kernel test on the GeofieldGeoJsonEnhancer will depend from the real functionality of the jsonapi_extras module as a whole (actually broken).

Thus, I moved back the Kernel GeofieldGeoJsonEnhancerTest tests in the Unit /tests/src/Unit/Plugin/jsonapi/FieldEnhancer/GeofieldGeoJsonEnhancerTest tests

This was a relevant headache ...
But now the coverage is wider and all test pass.

  • itamair committed aa69f6e4 on 10.3.x authored by nickolaj 
    Issue #3576947: Added a JSON:API Extras field enhancer plugin that...
itamair’s picture

itamair commented
Status: Reviewed & tested by the community » Fixed

MR !45 merged into 10.3.x dev branch.
Thanks @namingthings. This will be parto of the next incoming Geofield release ...

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.