Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analytics - Data mismatch between CustomProperties and Pinpoint Attributes #2862

Closed
abdallahshaban557 opened this issue Apr 12, 2023 · 6 comments
Labels
analytics Issues related to the Analytics category feature-parity feature-request Request a new feature

Comments

@abdallahshaban557
Copy link

Is your feature request related to a problem? Please describe.

We need to allow the update of user attributes to accept an array of values, similar to what is mentioned in this Github issue aws-amplify/amplify-flutter#2850

Describe the solution you'd like

Proposal:

Allow the Pinpoint Endpoint attributes field to be set as a Map<String, List> in UserProfile by:

  • Removing the UserProfile.customProperties field
  • Adding UserProfile.attributes of type Map<String, List>
  • Adding UserProfile.metrics of type Map<String, double>

Describe alternatives you've considered

N/A

Is the feature request related to any of the existing Amplify categories?

Analytics

Additional context

No response

@abdallahshaban557 abdallahshaban557 added analytics Issues related to the Analytics category feature-parity labels Apr 12, 2023
@ruisebas
Copy link
Member

ruisebas commented Apr 19, 2023

This is supported in the new UserProfile protocol that is used on the Push Notifications category, as UserProfilePropertyValue supports being an array of Strings.

So this is perfectly valid:

let userProfile = BasicUserProfile(
    name: "Name",
    customProperties: [
        "colours": ["red", "green", "blue"]
    ]
)

We just need to update Analytics's identifyUser API signature to take the new protocol instead of the old AnalyticsUserProfile

This however would be a breaking change, so what we could do instead is to define a new API and deprecate the old one:

public protocol AnalyticsCategoryBehavior {
    func identifyUser(userId: String, userProfile: UserProfile?)

    @available(*, deprecated, message: "AnalyticsUserProfile is deprecated, please use UserProfile instead")
    func identifyUser(userId: String, userProfile: AnalyticsUserProfile?)
}

@ruisebas
Copy link
Member

@abdallahshaban557 regarding your proposal, one issue I see is that UserProfile is a category-level structure, while what you are proposing is very Pinpoint-specific (i.e. attributes being restricted to array of strings).

We aim to keep the Categories and their APIs as generic as possible, and the idea behind having just customProperties with a generic type was to encapsulate the many different types that can be accepted by the underlying service.
It is then up to the plugin to map them to their corresponding values.

For example, the existing Pinpoint plugins take care of mapping numeric properties to metrics, and boolean, string and array of strings to attributes.

@abdallahshaban557
Copy link
Author

abdallahshaban557 commented Apr 25, 2023

Understood - the request was mainly around making sure the attributes can accept an array of values as you've mentioned in your first response here. For the concerns you have on the UserProfile being a category level concept, that makes sense.

@phantumcode phantumcode added the feature-request Request a new feature label Jul 24, 2023
@github-actions
Copy link
Contributor

This has been identified as a feature request. If this feature is important to you, we strongly encourage you to give a 👍 reaction on the request. This helps us prioritize new features most important to you. Thank you!

@ruisebas
Copy link
Member

Tracked in #1416

Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analytics Issues related to the Analytics category feature-parity feature-request Request a new feature
Projects
None yet
Development

No branches or pull requests

3 participants