feat(ios): expose new APIs to use location AccuracyAuthorization #11813

vijaysingh-axway · 2020-07-10T23:36:51Z

https://jira.appcelerator.org/browse/TIMOB-27976

build · 2020-07-10T23:54:20Z

	Fails
🚫	Tests have failed, see below for more information.

	Messages
📖	💾 Here's the generated SDK zipfile.
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖	❌ 1 tests have failed There are 1 tests failing and 711 skipped out of 8154 total tests.

Tests:

Classname

Name

Time

Error

ios.ipad.Titanium.UI.iOS.CollisionBehavior.example

works (14.0)

15

Error: timeout of 15000ms exceeded
file:///Users/build/Library/Developer/CoreSimulator/Devices/5B6B25D0-3445-48E0-97D6-199BBA69C78B/data/Containers/Bundle/Application/548AAA4D-4CFD-4FCC-8FCD-A4FFA7BD3CBF/mocha.app/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against f2bda48

tests/Resources/ti.geolocation.addontest.js

iphone/Classes/GeolocationModule.m

tests/Resources/ti.geolocation.test.js

iphone/Classes/GeolocationModule.m

sgtcoolguy

LGTM. My review comments are just suggestions, not required.

apidoc/Titanium/Geolocation/Geolocation.yml

sgtcoolguy · 2020-08-27T12:59:23Z

apidoc/Titanium/Geolocation/Geolocation.yml

+    description: Will be undefined if `error` is `true`.
+    optional: true
+    type: Number
+    constants: [Titanium.Geolocation.ACCURACY_AUTHORIZATION_FULL, Titanium.Geolocation.ACCURACY_AUTHORIZATION_REDUCED]


missing newline at EOF

sgtcoolguy · 2020-08-27T13:03:01Z

tests/Resources/ti.geolocation.test.js

@@ -244,7 +246,52 @@ describe.windowsBroken('Titanium.Geolocation', function () {
 		should(Ti.Geolocation.trackSignificantLocationChange).be.true();
 	});

+	it.ios('.ACCURACY_REDUCED', function () {
+		if (OS_VERSION_MAJOR >= 14) {


So we define these special filters like it.ios, where it skips on other platforms. It may make sense to expand to add ones like it.ios14Plus because technically we'd want to skip on older versions, not have a no-op test that always passes. (Alternatively, once we upgrade from ti-mocha to the real mocha package, I think you can explicitly call this.skip() inside a test to mark it at runtime)

tests/Resources/ti.geolocation.test.js

sgtcoolguy · 2020-08-27T13:04:43Z

tests/Resources/ti.geolocation.test.js

+				should(e).have.property('code').which.is.a.Number();
+				should(e.code).be.eql(1);
+				should(e).have.property('error').which.is.a.String();
+				finish();


typically I'd move this outside the try/catch block, because if an after test hook fails this could cause the catch block to fire and call finish again.

jquick-axway · 2020-08-27T19:25:45Z

apidoc/Titanium/Geolocation/Geolocation.yml

@@ -282,6 +282,63 @@ methods:
    platforms: [iphone, ipad, android]
    since: "5.1.0"

+  - name: requestTemporaryFullAccuracyAuthorization


Do we have to create a new method? From a cross-platform standpoint, it might be nice to re-use our existing requestLocationPermissions() and hasLocationPermissions() methods and add the new "purpose" string key as a parameter... or even better add support for a settings dictionary to these methods with a new "purpose" field.

On Android, location permission can be granted temporarily by the end-user too. We can still leverage the hasLocationPermission() method to check if it's still granted. And the Ti.Location.accuracy property defines the accuracy of the permission we need to look for. On Android, the ACCESS_FINE_LOCATION sounds like the equivalent of the "accurate" permission on iOS (which means read from the GPS).
https://developer.android.com/preview/privacy/permissions#one-time

So, can we do something like this?

const permissionSettings = { authorization: Ti.Geolocation.AUTHORIZATION_ALWAYS, purpose: 'PurposeKey1', }; if (Ti.Geolocation.hasLocationPermissions(permissionSettings)) { fetchLocationInfo(); } else { Ti.Geolocation.requestLocationPermissions(permissionSettings, (e) => { if (e.success) { fetchLocationInfo(); } }); }

Question:
On iOS 14, is the "purpose" key required? Are people going to have location accessing problem if they don't update their location permission handling code?

@jquick-axway This new API is for requesting full accuracy if user requires it in app. And it get expires. It is not mandatory. e.g your app do not need full accurate location, you should simply need location access . let's say at some screen you need full accuracy to show use'r location, you have to go for it. See here for more details

I think changes in Android 11 is same as of 'allow once' permission given in iOS 13. It is location permission's life not accuracy. See here.

'purpose' key is only required if one is going to access location with full accuracy. It will not break existing implementation.

So my only beef with this is if the app developer sets Ti.Location.accuracy to ACCURACY_HIGH, then I think hasLocationPermissions() should return false if it has high accuracy has expired, because that's what the app desires. That's why I'm question the necessity of these new APIs... that and I think we're making this more complicated for app developers than it needs to be. A portable API that works between all iOS versions and Android would be the ideal.

If user has granted location permission hasLocationPermissions() will be true. ACURRACY_HIGH (kCLLocationAccuracyBest) is desirable not guaranteed.
In iOS 14+ when app request for location permission there comes a popup (see attached Image1) which have a button mentioning 'Precise: On'. If user sets it off, app will not get location data with full accuracy. If at some point it is required to get 'full accuracy', with this new API app can ask for permission to get full accuracy in location data with a valid reason (see image2 ). And it will be temporary.
Image1-

image2 -

ssekhri · 2020-09-01T23:54:59Z

FR Passed
Verified on:
Mac OS: 10.15.4
SDK: 9.2.0.v20200831113302
Appc CLI: 8.1.0
JDK: 11.0.4
Node: 10.17.0
Studio: 6.0.0.202005141803
Xcode: 12.0 Beta6
iPhone 7Plus(v14.0 Beta6)

sgtcoolguy · 2020-09-02T14:56:37Z

Holding for @vijaysingh-axway and @jquick-axway to continue discussion about the APIs before we merge this.

Fixes TIMOB-27976

…on method

sgtcoolguy · 2020-09-08T19:03:49Z

@jquick-axway Opened up https://jira.appcelerator.org/browse/TIMOB-28114 to track investigating a better cross-platform API to hide the platform specific and OS version specific codepaths.

vijaysingh-axway added feature ios work in progress 🚧 labels Jul 10, 2020

build added this to the 9.1.0 milestone Jul 10, 2020

build requested a review from a team July 10, 2020 23:54

build added the needs tests 🚨 label Jul 10, 2020

build requested a review from a team July 14, 2020 02:03

build added docs and removed needs tests 🚨 labels Jul 14, 2020

vijaysingh-axway removed the work in progress 🚧 label Jul 14, 2020

vijaysingh-axway removed this from the 9.1.0 milestone Jul 14, 2020

sgtcoolguy reviewed Jul 16, 2020

View reviewed changes

tests/Resources/ti.geolocation.addontest.js Outdated Show resolved Hide resolved

sgtcoolguy reviewed Jul 16, 2020

View reviewed changes

iphone/Classes/GeolocationModule.m Outdated Show resolved Hide resolved

build added this to the 9.1.0 milestone Jul 16, 2020

sgtcoolguy added the iOS 14 label Jul 17, 2020

vijaysingh-axway modified the milestones: 9.1.0, 9.2.0 Jul 28, 2020

sgtcoolguy reviewed Jul 28, 2020

View reviewed changes

tests/Resources/ti.geolocation.test.js Show resolved Hide resolved

sgtcoolguy reviewed Jul 28, 2020

View reviewed changes

iphone/Classes/GeolocationModule.m Outdated Show resolved Hide resolved

sgtcoolguy reviewed Jul 28, 2020

View reviewed changes

iphone/Classes/GeolocationModule.m Outdated Show resolved Hide resolved

sgtcoolguy self-requested a review August 27, 2020 12:55

sgtcoolguy approved these changes Aug 27, 2020

View reviewed changes

sgtcoolguy added backport 9_3_X in-qe-testing 🕵 labels Aug 27, 2020

jquick-axway reviewed Aug 27, 2020

View reviewed changes

vijaysingh-axway and others added 2 commits September 8, 2020 14:28

feat(ios): expose new APIs to use location AccuracyAuthorization

1e62a2d

Fixes TIMOB-27976

docs(api): add Ti.Geolocation.requestTemporaryFullAccuracyAuthorizati…

31351db

…on method

test: add unit tests for new Ti.Geolocation constants/method

f2bda48

sgtcoolguy force-pushed the TIMOB-27976 branch from 50a182d to f2bda48 Compare September 8, 2020 18:29

sgtcoolguy merged commit 39ef241 into tidev:master Sep 8, 2020

build mentioned this pull request Sep 8, 2020

[Backport 9_3_X] feat(ios): expose new APIs to use location AccuracyAuthorization #12023

Merged

build removed the backport 9_3_X label Sep 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ios): expose new APIs to use location AccuracyAuthorization #11813

feat(ios): expose new APIs to use location AccuracyAuthorization #11813

vijaysingh-axway commented Jul 10, 2020

build commented Jul 10, 2020 •

edited

sgtcoolguy left a comment

sgtcoolguy Aug 27, 2020

sgtcoolguy Aug 27, 2020

sgtcoolguy Aug 27, 2020

jquick-axway Aug 27, 2020

vijaysingh-axway Aug 31, 2020 •

edited

jquick-axway Aug 31, 2020

vijaysingh-axway Sep 1, 2020

ssekhri commented Sep 1, 2020

sgtcoolguy commented Sep 2, 2020

sgtcoolguy commented Sep 8, 2020

feat(ios): expose new APIs to use location AccuracyAuthorization #11813

feat(ios): expose new APIs to use location AccuracyAuthorization #11813

Conversation

vijaysingh-axway commented Jul 10, 2020

build commented Jul 10, 2020 • edited

Tests:

sgtcoolguy left a comment

Choose a reason for hiding this comment

sgtcoolguy Aug 27, 2020

Choose a reason for hiding this comment

sgtcoolguy Aug 27, 2020

Choose a reason for hiding this comment

sgtcoolguy Aug 27, 2020

Choose a reason for hiding this comment

jquick-axway Aug 27, 2020

Choose a reason for hiding this comment

vijaysingh-axway Aug 31, 2020 • edited

Choose a reason for hiding this comment

jquick-axway Aug 31, 2020

Choose a reason for hiding this comment

vijaysingh-axway Sep 1, 2020

Choose a reason for hiding this comment

ssekhri commented Sep 1, 2020

sgtcoolguy commented Sep 2, 2020

sgtcoolguy commented Sep 8, 2020

build commented Jul 10, 2020 •

edited

vijaysingh-axway Aug 31, 2020 •

edited