[geo] JS logic crashes if geometry of a location is None

[nemesifier]

How to replicate: create a Location with no geometry set (don't place a point on the map).
Then create a device and select the location just created and check the javascript console.

[strang1ato]

Start working on it

[strang1ato]

Does that error occurred when openwisp-controller was using oldest version of djangorestframework-gis?

EDIT:
In the whole history of openwisp-controller the minimum version of djangorestframework-gis was 0.12 and with this version of djangorestframework-gis is giving the same bug too.

[nemesifier]

I'm not sure, have you found any interesting info about this problem?

[strang1ato]

@nemesisdesign Not really, but @atb00ker found something openwisp/django-rest-framework-gis#168 (comment)

[NoumbissiValere]

Please, i wish to know if it is normal to create a location without a geometry. I thought every location should have a geometry. because it is the geometry which makes the location an actual location 🤔

[nemesifier]

@NoumbissiValere OpenWISP gives the possibility of flagging a location as "mobile", which means an object moves, think about a car, a bus, a train, a boat, a plane.

In this case, the location can be created with an empty geometry.

Alternatively, what we could do, which may simplify things (I only thought about this now) is to change the API which receives location data from devices to automatically create the location when the data is received, not sure if we already do that.. I'm checking, maybe I will send a PR for this myself

[nemesifier]

I checked. Changing the model would require to change django-loci too, which is a bit tricky right now because django-loci has been migrated to channels 2, while openwisp-controller has not.

Better continue with the initial idea, I think it's safer to assume a location can be created without a geometry set only when flagged as is_mobile == True, because we assume we don't know the location yet

[NoumbissiValere]

@nemesisdesign i found that this problem is caused by this function in django_loci/base/admin.py/AbstractLocationAdmin

def json_view(self, request, pk):
        instance = get_object_or_404(self.model, pk=pk)
        return JsonResponse({
            'name': instance.name,
            'type': instance.type,
            'is_mobile': instance.is_mobile,
            'address': instance.address,
            'geometry': json.loads(instance.geometry.json)
        })

When location has is_mobile=True the geometry is None. this means instance.geometry will return a None and None.json is an error. Hence the 500 error in the javascript console

right now am thinking two possibilities:

1. updating the above function in django_loci such that when geometry is None, it is not included in the JsonResponse since it is not useful (but am wondering if this will still be a problem because django_loci has been moved to channel 2)
   2)setting a default geometry when location is_mobile=True. (but this doesn't make sense in real life)

Please which of the above solutions is best to implement so that i can implement it and send a PR? or should i look for another better solution?

Thanks

[nemesifier]

@NoumbissiValere I'd fix django-loci.

[NoumbissiValere]

Ok @nemesisdesign let me move to another issue then 😄

[nemesifier]

@NoumbissiValere what about fixing it in django-loci?

[NoumbissiValere]

@NoumbissiValere I'd fix django-loci.

Yes @nemesisdesign that's the best option i see and wanted to fix it but you said you will fix na.

[nemesifier]

I meant that I suggest to fix it in django-loci. I'm busy on other fronts right now.