search

urbanairship / android-library

Urban Airship Android SDK
Other
109 stars 123 forks source link

PrivacyManager documentation issues & obscurity #193

Closed G00fY2 closed 2 years ago

G00fY2 commented 2 years ago

Hi everyone,

Preliminary Info

What Airship dependencies are you using?

we are currently using com.urbanairship.android:urbanairship-fcm in the latest version 15.0.0.

Report

We migrated to the new PrivacyManager while updating to 14.5.0 before. Everything seems to be working fine so far. But we have discussions about some public APIs of the PrivacyManager with our Android team as well as our colleagues maintaining the iOS part. I will only reference the Android APIs in this issue. But you should consider checking if there are similar obscurity in your iOS SDK.

What unexpected behavior are you seeing? / What is the expected behavior?

We use the following 3 methods of the PrivacyManager to enable or disable Airship in the user settings of our app:

  1. public void enable(@Feature int... features) This one is pretty clear. It enables additional features without changing other features. Thats also what the documentation in the source code says:

    Enables features.
Params: features – The features to enable.

  2. public void disable(@Feature int... features) This one is also pretty clear. It should disable the passed features without changing other features. But the documentation is wrong for this one. It also says:

    Enables features.
Params: features – The features to enable.

  3. public void setEnabledFeatures(@Feature int... features) The method name and the documentation aren't really helpful. If the user disables Airship we want to disable all features but FEATURE_IN_APP_AUTOMATION, FEATURE_MESSAGE_CENTER, FEATURE_PUSH. From going through the source code setEnabledFeatures will first use FEATURE_NONE flag as a base and combine the features to it. So it basically disables all features except the features passed in, right? So calling disable(FEATURE_ALL) before is not required? Maybe you could make this clear in the documentation.

What are the steps to reproduce the unexpected behavior?

N/A

Do you have logging for the issue?

N/A

Thanks in advance.

rlepinski commented 2 years ago

setEnabledFeatures will do a set operation (replace with what you pass in) while enable and disable will mutate your current set of features. Anything calls before setEnabledFeatures should not be needed. We will update the docs on this. Thanks for the feedback.

G00fY2 commented 2 years ago

The changes from 25bfcd6 really help making the usage clear. Thanks for updating the javadocs! 👍