Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore choices with invalid geometry in Select one from map widget #5369

Merged
merged 4 commits into from
Dec 8, 2022

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Nov 30, 2022

Closes #5368

What has been done to verify that this works as intended?

I've tested the fix manually and added automated tests.

Why is this the best possible solution? Were any other approaches considered?

So far we would throw an error if any of X choices had an invalid geometry. This doesn't make sense such choices should be just excluded not to block users.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Choices with invalid geometries should be ignored when used in Select one from map widget so that users can still see other valid choices. Please test all possible options: external csv attachments (like the one attached below) and geojson attachments.

Do we need any specific form for testing your changes? If so, please attach one.

I used a form with a csv file:
geoTest.xlsx
geo.csv

but please also verify geojson

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No.

Before submitting this PR, please make sure you have:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@grzesiek2010 grzesiek2010 changed the title Collect 5368 Ignore choices with invalid geometry in Select one from map widget Nov 30, 2022
@grzesiek2010 grzesiek2010 marked this pull request as ready for review November 30, 2022 14:24
@seadowg seadowg added the high priority Should be looked at before other PRs/issues label Nov 30, 2022
@dbemke
Copy link

dbemke commented Dec 7, 2022

Android version

10, 13

Device used

Redmi 9T, Pixel 6a

Problem description

Leaving a blank space in geometry values for "select place” included in xml file of a form crashes the app.

Steps to reproduce the problem

  1. Go to the form with empty geometry value.
    mapinternalselectlocalizedchanged.xml.txt
  2. Tap "select place”

Expected behavior

The app shouldn’t crash.

@grzesiek2010
Copy link
Member Author

This happens in javarosa so it's kind of different I would rather leave it as is for now and file a separate issue.

@dbemke
Copy link

dbemke commented Dec 8, 2022

Tested with Success!

Verified on Androids: 10

Verified cases:

  • issue Invalid geometry in CSV file with entities #5368 is no longer reproducing
  • forms: Select one from map, S1FromMap, Map internal select localized, Map internal select invalid
  • invalid geometry in a geojson file set to: 199.xxx, "a” instead of a number, valid number in brackets, only longitude valid, negative values
  • all points in geojson file with invalid geometry
  • in CSV file changing geometry values to: valid in brackets, 0.0, 199. xxx, a blank space
  • invalid geometry in CSV file with a dataset
  • changing geometry in xml file (map internal select localized form- geometry in xml: 199.xx, "a”, a blank space)
  • changing geometry in xlsx file (map internal select invalid)
  • submissions from different versions of a form with different invalid geometry
  • screen rotation, minimizing the app, using backward button

The issue form the PR is filed as a separate issue.

@srujner
Copy link

srujner commented Dec 8, 2022

Tested with Success!

Verified on Androids: 13

@seadowg
Copy link
Member

seadowg commented Dec 8, 2022

Leaving a blank space in geometry values for "select place” included in xml file of a form crashes the app.

@dbemke is this an existing case? i.e. is it present in v2022.4.0?

@dbemke
Copy link

dbemke commented Dec 8, 2022

@seadowg It's in the PR and v2022.4.0 but I guess we haven't tested that case of invalid geometry in xml file before

@seadowg seadowg merged commit 74d87b0 into getodk:master Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior verified high priority Should be looked at before other PRs/issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid geometry in CSV file with entities
4 participants