InTaVia / web

InTaVia web client
MIT License
2 stars 0 forks source link

GeoMap assumes that entities always have geometry #250

Closed samuelbeck closed 1 year ago

samuelbeck commented 1 year ago

This leads to the application crashing if it's not the case.

image

johannesliem commented 1 year ago

Would you have an example detail page where this happens? Where place does not has a geom (or coordinates). Does the example from above relates to a place entity?

samuelbeck commented 1 year ago

The error occurred when Angel imported an IDM-JSON file. Paris doesn't have geometry associated with it and the app crashes when opening its detail page.

We should probably just check whether geometry isn't undefined before using it.

johannesliem commented 1 year ago

Mhm ... looks like that the hasGeometry check does not work as expected...

const hasGeometry = useEntityHasGeometry(entity) || (entity.kind === 'place' && entity.geometry !== undefined);

samuelbeck commented 1 year ago

I found the error. In the knowledge graph places don't have any relations. Therefore, useEntityHasGeometry(entity) always returns false. However, as in this case, in Angels data places can have relations to other entities. If these entities have geometry the hook will return true and the map will get rendered.

This isn't a problem by itself. However, because until now we didn't expect places to have relations, we tread them differently from other entity kinds when it comes to the map. Instead of rendering the events on the map we render a marker at the coordinates of the geometry object. Since Paris doesn't have a geometry object in this case but useEntityHasGeometry(entity) returns true the map is rendered without any geometry being passed to the component.

I will fix the error in the entity-details component but it probably still makes sense to check in the GeoMap component whether an entity actually has a valid geometry before passing it to the map.

johannesliem commented 1 year ago

In the geomapwrapper component only events that have related places with valid geometries are used in the layers of the geomap. As you said, the geomap as used for places on the entity detail page is different; I think as for the example, it is the initialViewState(lat, lon) and the Marker positon (lat, lon) that would require a check. While we could check for the viewstate in the geomap and use default values otherwise; we would need to check outside of the react-map-gl marker component - as we do already.

samuelbeck commented 1 year ago

I've implemented a check in the entity-details component in branch fix/place-detail-geometry.

If a place doesn't have geometry but related events with geometry, the GeoMapWrapper component is used to render the events as for other kinds of entities. However for Angel's data example that causes another runtime error.

For now I won't look into this further as I would like to keep working on the graph exploration and visual querying instead.

image

johannesliem commented 1 year ago

I will have a look, thanks for fixing and documenting.