In Google Maps/OSM after saving a few points with manual location recording in GeoShape widget and returning to View, the number of points is reduced.

[srujner]

Software and hardware versions All Android devices

Problem description

https://user-images.githubusercontent.com/85225519/178740246-0e4b22f6-2ee6-4542-aeec-88675acaecf5.mp4

Steps to reproduce:

• Go to Project Settings -> Maps and Select OSM/Google Maps as Source;
• Download all Widgets form;
• Open GeoShape widget;
• Select Manual location recording and click start;
• Record 4 points and click save button;
• Click on "View or change GeoShape" button;
• There will be "-1" recorded points.

Expected behavior

• Number of recorded points should remain the same.

[seadowg]

@srujner this is present in v2022.2.3 as well as current master right?

[srujner]

@seadowg Yes, I just checked it.

[grzesiek2010]

This only happens in geoshape if the first and last points are the same (the video shows that all points are in the same location). In geoshape we usually add an extra point (the first one) to close polygon. However if the last point already equals the first one we don't do that: https://github.com/getodk/collect/blob/master/geo/src/main/java/org/odk/collect/geo/GeoUtils.java#L24 then when we open the map that last point is removed (if it's the same like the first one): https://github.com/getodk/collect/blob/master/geo/src/main/java/org/odk/collect/geo/geopoly/GeoPolyActivity.java#L337 there is no easy way to determine if the last point was added by us to close polygon or it was added by user manually so we always remove it. In the example all points are equal so they are removed one by one. We could try to fix this but I think it could make more harm than good. I would mark it as won't fix. What do you think @seadowg @lognaturel

[seadowg]

I would mark it as won't fix. What do you think @seadowg @lognaturel

I don't think I agree. It definitely feels to me that this is less scary than it was before (it only happens in a fairly weird case where the shape is recorded on the same spot), but it still feels like the behaviour is dangerous as it mutates data the user might have saved - potentially with fake GPS or by manually placing points. I think we could maintain the current behaviour, without removing the points with some logic like this:

• Save the extra point when saving (returning an activity result) a new shape
• If editing an existing shape, ignore the last point that comes from the existing answer rather than removing it
• Stop ignoring the last point when editing an existing shape if a change is made (we'll add the last point in again on save)

Alternatively, we could disallow points that are equal to another point in the shape (we could throw up a toast or just ignore the point). This would prevent any uses that would cause the issue (rather than fixing it itself).

[grzesiek2010]

I will think about it once again but I'm sure at least that there is no need to add it to hotfix. It has always worked like that.

[grzesiek2010]

Now as I'm digging deeper I can see that there are more places where we add additional point to close polygon. That means I need to spend more time on it to make sure there are no other bugs. It would be good to simplify the code a bit too.

[grzesiek2010]

Alternatively, we could disallow points that are equal to another point in the shape (we could throw up a toast or just ignore the point). This would prevent any uses that would cause the issue (rather than fixing it itself).

I think this is the best solution here and I've implemented it in https://github.com/getodk/collect/pull/5275