Add location to start_configuration call #3176
Conversation
Fix sync buttons in learn and delivery progress pages
… into dv/delivery_progress_ui_fix
Delivery Progress UI fix
Solved smaller screens UI issues
Job status fix for learning
Pm/master initial merge
Fixed a bad translation (across languages) involving malformed escape XML.
…re-android into connect_qa Removed a stale (empty) override function in SqlStorage.
… into dv/token_exceptions Resolved conflicts in ApiConnectId and ConnectSsoHelper.
… into dv/token_exceptions
Handling thrown token exceptions throughout the code and reporting to UI where appropriate.
…and recovery code continue button
… in app selector is selected.
…the relevant message for each scenario.
Fixing obsolete app selector code
… into dv/token_exceptions
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
Lot of things seems wrong with the PR and don't think we have tested various scenarios this PR is trying to handle at all. This PR needs a detailed safety story along with a video on UX behaviour in different scenarios -
- Behavior when we don't have location permission and on showing location dialogue user denies the permission
- Behavior when we don't have location permission and on showing location dialogue user accepts the permission
- We already have location permission but location is turned off in device settings
- We already have location permission but device is not able to get location due to no network and no gps.
app/res/values/strings.xml
Outdated
| <string name="camera_permission_title">Permission for camera</string> | ||
| <string name="camera_permission_msg">In order to take a picture, CommCare needs permission to use your device camera.</string> | ||
| <string name="capture_photo">Capture Photo</string> | ||
| <string name="location_permission_denied">Location permission denied</string> |
There was a problem hiding this comment.
should prefix it with personalid_
|
|
||
| View.OnClickListener cancelButtonListener = v -> { | ||
| location = null; | ||
| requireActivity().finish(); |
There was a problem hiding this comment.
we should not finish but show a blocking error message using message fragment.
| locationDialog = new GeoProgressDialog(requireActivity(), | ||
| StringUtils.getStringRobust(requireActivity(), R.string.found_location), | ||
| StringUtils.getStringRobust(requireActivity(), R.string.finding_location)); | ||
| locationDialog.setImage(getResources().getDrawable(R.drawable.green_check_mark)); | ||
| locationDialog.setMessage(StringUtils.getStringRobust(requireActivity(), R.string.please_wait_long)); | ||
| locationDialog.setOKButton(StringUtils.getStringRobust(requireActivity(), R.string.accept_location), | ||
| okButtonListener); | ||
| locationDialog.setCancelButton(StringUtils.getStringRobust(requireActivity(), R.string.cancel_location), | ||
| cancelButtonListener); | ||
| } |
There was a problem hiding this comment.
Don't think we want to show a user visible dialog here, but we should directly check for requestLocationPermission instead on startup and once granted capture the location in background.
| private static final int LOCATION_SETTING_REQ = 102; | ||
| private CommCareLocationController locationController; | ||
| private static final String[] REQUIRED_PERMISSIONS = new String[]{ | ||
| Manifest.permission.ACCESS_FINE_LOCATION, |
There was a problem hiding this comment.
think we only need approximate location and using ACCESS_COARSE_LOCATION should be enough here ?
| if (locationController != null) { | ||
| locationController.stop(); | ||
| } |
There was a problem hiding this comment.
start and stop should correspond to mirroring Activitiy lifecycle methods . If we are starting in onCreate , stopping should be done in onDestroy instead.
| body.put("phone_number", phone); | ||
| body.put("application_id",requireContext().getPackageName()); | ||
| body.put("application_id", requireContext().getPackageName()); | ||
| body.put("gps_location", location.getLatitude() + " " + location.getLongitude()); |
There was a problem hiding this comment.
we should send accuracy and altitude as well here for sake of completeness and in separate fields.
| Exception exception = ((CommCareLocationListener.Failure.ApiException)failure).getException(); | ||
| if (exception instanceof ResolvableApiException) { | ||
| try { | ||
| ((ResolvableApiException)exception).startResolutionForResult(requireActivity(), | ||
| LOCATION_SETTING_REQ); | ||
| } catch (IntentSender.SendIntentException e) { | ||
| Logger.log("Location Error", e.getMessage()); | ||
| } |
There was a problem hiding this comment.
abstract as handleResolvableLocationException and resue.
|
|
||
| @Override | ||
| public void requestNeededPermissions(int requestCode) { | ||
| missingPermissions(); |
There was a problem hiding this comment.
rename to requestLocationPermission
There was a problem hiding this comment.
these functions are overidded from the CommCareLocationListener
There was a problem hiding this comment.
I meant missingPermissions -> requestLocationPermission
| integrityTokenApiRequestHelper = new IntegrityTokenApiRequestHelper(getViewLifecycleOwner()); | ||
| initializeUi(); | ||
| setupLocationDialog(); | ||
| requestLocationPermission(); |
There was a problem hiding this comment.
rename to checkLocationPermission
| ActivityCompat.requestPermissions(requireActivity(), | ||
| new String[]{Manifest.permission.ACCESS_FINE_LOCATION, | ||
| Manifest.permission.ACCESS_COARSE_LOCATION}, | ||
| LOCATION_SETTING_REQ); |
There was a problem hiding this comment.
where are we handling result from permission changes here ? Have you tested this ?
String resources cleanup
- Added translation for missing strings
- Solved french string error
…in some languages
Restore Connect buttons in beta
Fixed button spacing issue
remove secondry number handling
Connect Beta Backmerge
…android into pm_ccct_1081_fix # Conflicts: # app/res/values/strings.xml # app/src/org/commcare/fragments/personalId/PersonalIdPhoneFragment.java
|
stale pr |
Product Description
Passing GPS coordinate in the start configuration call
https://dimagi.atlassian.net/browse/CCCT-1081
Taking reference from the GeoPointActivity
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review