chore(ios): support new property to remove note

[vijaysingh-axway]

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

[build]

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

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

Tests:

Classname	Name	Time	Error
android.emulator.Titanium.Network.HTTPClient	callbackTestForGETMethod	1.408	Error: 404 : Not Found at HTTPClient.xhr.onerror (/ti.network.httpclient.test.js:409:16)

Generated by 🚫 dangerJS against ec5e9dd

[hansemannn]

From an architecture point of view, this is the wrong API design I think. Apple will very likely add more properties to the "sensitive" ones, so probably the user should have a property to override all fields that should be fetched - with the default not containing this one anymore. And it's likely a breaking change, just like this one also is.

[vijaysingh-axway]

I can be agree with you that we have to change/extend the behavior of exiting APIs specially which are fetching use contacts data. Probably android also have to make some changes to have a parity and there may be some breaking change. For that I think we can not make it before 9.0.0.
But this new property will give developer an option to avoid fetching the 'note' in iOS otherwise they have to make some unnecessary setup. So there is no problem in having this new property. And by default this property is true, so there is no breaking change.

[hansemannn]

Without implementing this property, apps are being rejected, so it's a breaking change for them. Either adding this property or defining the properties they actually need, which will be more future proof in the first place. But it's just a community comment 🙂

[sgtcoolguy]

code change looks ok for now, but this is an odd way to work around the issue. Apps will need to set this to false on iOS 13+ or add the entitlement. Is there any way to detect entitlements and make this false on iOS 13+ if the entitlement is not declared/set? Just seems like if we're going to make a change here we should try and "default" to a working version?

[vijaysingh-axway]

code change looks ok for now, but this is an odd way to work around the issue. Apps will need to set this to false on iOS 13+ or add the entitlement. Is there any way to detect entitlements and make this false on iOS 13+ if the entitlement is not declared/set? Just seems like if we're going to make a change here we should try and "default" to a working version?

I don't think there is any way to read entitlements at runtime. In this release default value of this property is set to 'true' to avoid breaking change. We will make default value of this property to 'false' in major release, 9.0.0.

[ssekhri]

FR Passed
Fetching of contacts works without error when Ti.Contacts.includeNote is set to false.
Verified On:
Mac OS: 10.14.5
SDK: 8.2.1.v20190927141609
Appc CLI: 7.1.1
JDK: 1.8.0_162
Node: 10.5.0
Studio: 5.1.4.201909061933
Xcode: 11.0
Device: iPhone X(13.0), iPhone 7Plus(12.3.1)

[hansemannn]

This still doesn't look right, sorry. And I highly suspect that the apps will still be rejected because Apple checks their blacklisted API's from the compiled app symbols rather than manually. So if the binary still contains CNContactNoteKey (which it does, because you only added an if-statement), the app will very likely be rejected again, even with this fix. Just my 2 cents.

[vijaysingh-axway]

If you see doc https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_developer_contacts_notes, if one is fetching note without entitlement key, it will give error at runtime. Having CNContactNoteKey is not a problem, it should not be used to fetch note without entitlement.