new no identity available event #184
Conversation
| @@ -52,7 +53,6 @@ export class CallbackManager { | |||
| public runCallbacks(event: EventType, payload: CallbackPayload) { | |||
| if (event === EventType.InitCompleted) this._sentInit = true; | |||
| if (event === EventType.SdkLoaded) CallbackManager._sentSdkLoaded[this._productName] = true; | |||
| if (!this._sentInit && event !== EventType.SdkLoaded) return; | |||
There was a problem hiding this comment.
With everything that can be done without init, I dont think it makes sense anymore to not run callbacks if init is not complete
src/callbackManager.ts
Outdated
|
|
||
| const enrichedPayload = { | ||
| ...payload, | ||
| identity: this._getIdentity() ?? null, | ||
| }; | ||
| if (event === EventType.NoIdentityAvailable && enrichedPayload.identity) { | ||
| enrichedPayload.identity = null; | ||
| } |
There was a problem hiding this comment.
Added this because it was sending the expired or opted out identity back, but I thought this may be confusing if we are sending a "no identity available" event, but then attaching an identity.
There was a problem hiding this comment.
I'm not sure about this. It seems possible that someone could be expecting an expired identity and then acting on it in some way. this could be a breaking change?
There was a problem hiding this comment.
Took this check out. Will add info to the docs to denote the two types of identities that can be returned with this event (null or expired)
src/sdkBase.ts
Outdated
| return identity; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Callback uses getIdentity for its payload but once I added the possibility of the no identity available event, it ended up in an endless loop of callback events. So callback needed its own getIdentity function that does not run an event - getIdentityForCallback() is identical to the old getIdentity() function. Set it to private since its just internal
src/sdkBase.ts
Outdated
| return this.isIdentityValid() || this._apiClient?.hasActiveRequests(); | ||
| const identity = this._identity ?? this.getIdentityNoInit(); | ||
| const identityAvailable = | ||
| (this.isIdentityValid(identity) && !this.temporarilyUnavailable(identity)) || |
There was a problem hiding this comment.
added this extra check because identity can be valid, but unavailable
| !previousOpts.identity || | ||
| opts.identity.identity_expires > previousOpts.identity.identity_expires; | ||
| } | ||
|
|
There was a problem hiding this comment.
found in testing that we didnt have the possibility that opts.identity just doesn't exist, which would automatically make useNewIdentity true
src/sdkBase.ts
Outdated
| } | ||
|
|
||
| public getIdentity(): Identity | null { | ||
| const identity = this._identity ?? this.getIdentityNoInit(); | ||
| const isValid = | ||
| identity && !this.temporarilyUnavailable(identity) && !isOptoutIdentity(identity); |
There was a problem hiding this comment.
an Optout Identity if valid so I don't think we want that in this condition.
There was a problem hiding this comment.
This code was here before - and when I remove OptoutIdentity, it does change the return value of getIdentity. If I update the return value, it then errors on getAdvertisingToken(). I think we need to make we (and the docs) are clear if an opted out identity is valid
There was a problem hiding this comment.
seems the code always checks for optout separately so it assumes a valid identity will have a particular shape. So this is fine.
No description provided.