amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

Documentation unclear about updateEndpoint method

Open BartoszKlonowski opened this issue 3 years ago • 5 comments

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Analytics

Amplify Categories

analytics

Environment information

# Put output below this line
  System:
    OS: macOS 12.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 10.68 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.14.0/bin/yarn
    npm: 7.19.1 - ~/Desktop/Projects/Aaqua-Mobile/node_modules/.bin/npm
    Watchman: 2022.01.31.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 98.0.4758.102
    Firefox: 97.0
    Safari: 15.2
  npmPackages:
    @typescript-eslint/eslint-plugin: 5.9.0 => 5.9.0 
    aws-amplify: ^4.2.10 => 4.3.14 
    eslint: ^7.32.0 => 7.32.0 (4.19.1)
    eslint-config-prettier: 8.3.0 => 8.3.0 
    eslint-plugin-prettier: 4.0.0 => 4.0.0 
    eslint-plugin-react: 7.28.0 => 7.28.0 
    eslint-plugin-react-hooks: 4.3.0 => 4.3.0 
    eslint-plugin-react-native: 4.0.0 => 4.0.0 
    graphql: 15.5.0 => 15.5.0 (15.8.0)
    jest: 26.6.3 => 26.6.3 
    jest-react-native: 18.0.0 => 18.0.0 
    metro: 0.66.2 => 0.66.2 
    metro-react-native-babel-preset: ^0.66.2 => 0.66.2 (0.66.0)
    react: 17.0.2 => 17.0.2 (16.14.0)
    react-dom: ^17.0.2 => 17.0.2 (16.14.0)
    react-native: 0.67.2 => 0.67.2 
  npmGlobalPackages:
    corepack: 0.10.0
    npm: 8.3.1
    yarn: 1.22.17

Describe the bug

In general: How to correctly use the Analytics.updateEndpoint method from AnalyticsClass: { Analytics } imported from '@aws-amplify/analytics'?

Documentation does not clearly specify how should the updated object look like that we are sending.

Let's say, that my example is: I have the standard endpoint with all the required information, but I also have the

endpoint: {
  attributes: {
    hobbies: ['piano', 'hiking'],
  },
}

and I want to extend the attributes with the user's pupils: pupils: ['dog', 'cat']. So what I do is:

import { Analytics } from '@aws-amplify/analytics';

Analytics.updateEndpoint({
  attributes: {
    pupils: ['dog', 'cat']
  },
})
.then(response => {
  console.log("Success with response: ", response);
})
.catch(error => {
  console.log("Error with response: ", error);
});

My questions are:

  • I keep on getting the ERROR [ERROR] 58:48.118 AWSPinpointProvider - updateEndpoint failed [TypeError: undefined is not a function] error, so I must be doing something in an incorrect way - what is it?
  • Should I update the endpoint's additional attributes by giving all of them:
Analytics.updateEndpoint({
  attributes: {
    hobbies: ['piano', 'hiking'],
    pupils: ['dog', 'cat']
  },
})

or just give the data that I want to update the endpoint with: pupils only?

  • Should I specify the address field during updating the endpoint? Like so?
Analytics.updateEndpoint({
  address: '12345678',
  attributes: {
    pupils: ['dog', 'cat']
  },
})
  • Should I give the whole configuration object or just give the object with data I want to update the endpoint with?

I'm really hoping for as much explanation as you can provide me with - I'm struggling with this a lot so I would use some help.

Expected behavior

Documentation should precisely describe what arguments and fields are required and optional, what should be passed as an arguments to updateEndpoint method.

Moreover: updateEndpoint should pass and, if returning error, the error should clearly state what is wrong. Currenlty it looks like some internal error I can't debug.

Reproduction steps

  1. Configure your endpoint
  2. Check the docs about the updateEndpoint method.
  3. Update it sometime during the app runtime
  4. Check for Promise.catch

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line
[ERROR] 58:48.118 AWSPinpointProvider - updateEndpoint failed [TypeError: undefined is not a function]

aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

BartoszKlonowski avatar Feb 17 '22 11:02 BartoszKlonowski

Thanks for this feedback @BartoszKlonowski, I will have someone from the team take a look at this and address the feedback you provided.

sammartinez avatar Feb 17 '22 16:02 sammartinez

@BartoszKlonowski I attempted to reproduce this issue, but was unsuccessful. Could you take a look at the sample source code, and see if there is something you are doing differently to reproduce the issue? I also have a working demo under the "Analytics Test" which allows you to update the endpoint, as well as send an analytics event with the updated payload.

Also, regarding updating the docs here, I believe no update is necessary. All of the fields that are listed in the updateEndpoint function are optional. As can be seen in the sample, the function even accepts an empty object, and the request succeeds.

david-mcafee avatar Feb 17 '22 22:02 david-mcafee

Thank you @david-mcafee for taking the time! Two things I need to discuss then:

  1. If all fields are optional, they should be marked as OPTIONAL just like in the Setting up the existing analytics backend example. Having them not marked as OPTIONAL confuses (at least me) as it is not certain if they really are optional comparing to other examples in the same docs page.
  2. Thank you for giving me the link to those samples, very helpful 👍 I checked one thing: I'm debugging my solution while having the disabled field in the Analytics.configure({}) method set to false but when I set the configuration as:
Amplify.configure({
  // ...
  Analytics: {
    disabled: __DEV__,
    // ...
  }
})

the Analytics.updateEndpoint method succeeds, but the response is undefined <- should it work like that?

BartoszKlonowski avatar Feb 18 '22 09:02 BartoszKlonowski

I got some findings about the structure and fields of updateEndpoint method's arguments. I described them in the PR, so this issue can be closed by aws-amplify/docs#4054

BartoszKlonowski avatar Feb 21 '22 11:02 BartoszKlonowski

Thank you @david-mcafee for taking the time! Two things I need to discuss then:

  1. If all fields are optional, they should be marked as OPTIONAL just like in the Setting up the existing analytics backend example. Having them not marked as OPTIONAL confuses (at least me) as it is not certain if they really are optional comparing to other examples in the same docs page.
  2. Thank you for giving me the link to those samples, very helpful 👍 I checked one thing: I'm debugging my solution while having the disabled field in the Analytics.configure({}) method set to false but when I set the configuration as:
Amplify.configure({
  // ...
  Analytics: {
    disabled: __DEV__,
    // ...
  }
})

the Analytics.updateEndpoint method succeeds, but the response is undefined <- should it work like that?

I was definitely confused regarding what properties are required and which are optional. The documentation was good, but still leaves a lot of open questions for a dev consuming this package.

For example, my team has our analytics module broken into separate events.

There is an initialize event called on app startup where we setup PushNotification.onRegister

PushNotification.onRegister(async (token: any) => {
  console.log('in app registration', token);

  // set pinpoint push endpoint
  Analytics.updateEndpoint({
    address: token,
    channelType: Platform.OS === 'android' ? 'GCM' : 'APNS',
    optOut: 'NONE', // TODO: use 'ALL' if push disabled
    demographic: {
      appVersion: await getVersion(),
      platform: Platform.OS,
    },
  })
    .then(console.log)
    .catch(console.error);
});

There is also a setAccount event which can happen anytime later as the user changes their account and this is where we apply userAttributes to the endpoint

if (account) {
  Analytics.updateEndpoint({
    userId,
    userAttributes: {
      firstName: this.cleanArray([account.firstName]),
      storeName: this.cleanArray([account.store?.mall]),
      storeId: this.cleanArray([account.store?.id]),
      storeZip: this.cleanArray([account.store?.address.zipCode]),
      membershipStatus: this.cleanArray([isPro ? 'Pro' : 'Player']),
      membershipId: this.cleanArray([account.rewardsProfile?.id]),
      rewardsCurrentPoints: this.cleanArray([
        account.rewardsProfile?.points,
      ]),
      rewardsMembershipValue: this.cleanArray([
        account.rewardsProfile?.lifetimePoints,
      ]),
      rewardsPointsExpirationDate: this.cleanArray([
        account.rewardsProfile?.expirationDate,
      ]),
    },
  })
    .then(console.log)
    .catch(console.error);
}

What happens in this case is unclear with the current documentation.

One of my questions was do I need to pass the address parameter still in which case we'd have to store it in localstorage.

In my experience, all endpoints with the same userId seem to have the userAttributes applied to them which may work fine for some use cases but not for others.

It's questions like this that are unclear on the surface at least.

ChrisLFieldsII avatar Sep 13 '22 16:09 ChrisLFieldsII