anemchinova commented 3 years ago

Requirements

[x] I have added test coverage for new or changed functionality
[x] I have followed the repository's pull request submission guidelines
[x] I have validated my changes against all supported platform versions

Related issues

33 Private User Attributes

Describe the solution you've provided

This PR adds support for built-in user attributes and also an ability to configure both the built-in user attributes and custom attributes as private.

Built-in attributes

LaunchDarkly provides set of built-in user attributes like firstName, lastName, email, etc.

The API was extended to accept a parameter of LaunchDarklyUser type in order to those attributes can be passed to LaunchDarkly SDK.

Private attributes

There're three options to configure an attribute as private:

Mark all attributes private globally in the LDClient configuration object.
1. Mark specific attributes private by name globally in the LDClient configuration object.
2. Mark specific attributes private by name for individual users when you construct LDUser objects.

This PR adds support for all three.

In order to configure private attributes globally, pass an optional parameter of LaunchDarklyConfig type to init call. LaunchDarklyConfig could be extended later for other client configuration options if required.

In order to configure specific attributes as private for individual users, use private-prefixed arguments of LaunchDarklyUser or the privateCustom parameter of init / identify methods for custom attributes.

Describe alternatives you've considered

Another solution could be to combine both built-in user attributes and custom attributes in a single free-from map.

I'm not sure the end users of the package will benefit a lot from the ability to distinguish between custom attributes and built-in attributes. So, potentially both custom and built-in user attributes could be simply passed as a free-form map:

await launchDarkly.identify(
    "userId", 
    "attributes": {
        "email": "example@example.com",
        "customAttr": "customValue",
    }
}

Then, during parsing on the platform side, built-in attributes would be recognized among others and passed to LaunchDarkly SDK using an appropriate method. This could also prevent against unintentional usage of built-in user attribute keys as custom keys leading to data loss. From LaunchDarkly Android SDK docs:

When set to one of the built-in user attribute keys, this custom attribute will be ignored.

The drawbacks of this approach vs the implemented approach:

No compile-time type safety for the built-in user attributes.
The implemented approach follows to the original LaunchDarkly documentation more precisely. So, it might be easier to adopt.
Since the map will accept both custom and built-in attributes, it can't be named custom anymore. This introduces a backward incompatible change.

Additional context

Settings user attributes

I would love to hear your feedback!

maxlapides commented 3 years ago

@andre-paraense Do you have an estimate on when you may get a chance to review this PR? Thanks!

andre-paraense commented 3 years ago

@andre-paraense Do you have an estimate on when you may get a chance to review this PR? Thanks!

Will try to review it this weekend!

anemchinova commented 3 years ago

@andre-paraense thank you for the review! apart from ios testes, it's ready for another round