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

new no identity available event #184

Merged
merged 28 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a34660a
first examples of no identity available
ashleysmithTTD Feb 13, 2025
8460640
added more tests where identity could be unavailable
ashleysmithTTD Feb 13, 2025
bacfbf6
no identity available on set identity
ashleysmithTTD Feb 13, 2025
59856db
more set identity examples
ashleysmithTTD Feb 13, 2025
f8b39dd
no identity available new event
ashleysmithTTD Feb 14, 2025
d9b53b0
callback changes no need to init
ashleysmithTTD Feb 14, 2025
d549c07
Merge branch 'main' into ans-UID2-2295-no-identity-available-event
ashleysmithTTD Feb 14, 2025
d8cc05d
not needed from prev branch
ashleysmithTTD Feb 14, 2025
75b4952
wrote separate identity function for callback instead
ashleysmithTTD Feb 14, 2025
ea6f0e4
added tests for cstg and refresh timer
ashleysmithTTD Feb 14, 2025
1879ddc
added test for local storage
ashleysmithTTD Feb 14, 2025
f0d0bed
added test without init
ashleysmithTTD Feb 14, 2025
b991a98
identity should always be null
ashleysmithTTD Feb 14, 2025
7aa9c23
organizing tests
ashleysmithTTD Feb 14, 2025
dc38026
added cookie example also
ashleysmithTTD Feb 14, 2025
2a7f821
cleaned up valid identity test
ashleysmithTTD Feb 14, 2025
50d0cea
added sdk loaded check in run callbacks
ashleysmithTTD Feb 18, 2025
3c0a13a
removed identity forced to be null
ashleysmithTTD Feb 18, 2025
ccd68ae
fixed tests to expect expired identity sometimes
ashleysmithTTD Feb 18, 2025
6fcb0d3
cleaned up get identity to be more consistent with isvalididentity fu…
ashleysmithTTD Feb 18, 2025
6bea2a5
the check iwll already happen in islogin required
ashleysmithTTD Feb 18, 2025
c16234b
unavailable check still needed
ashleysmithTTD Feb 19, 2025
f1d89c5
removed any no identity available event reference
ashleysmithTTD Feb 25, 2025
12ae483
dont need separate get identity functions anymore
ashleysmithTTD Feb 25, 2025
1c4167c
no need to call is identity available everywhere
ashleysmithTTD Feb 25, 2025
4053cb7
clean up get identity
ashleysmithTTD Feb 25, 2025
c4061fe
get identity return
ashleysmithTTD Feb 25, 2025
86501e8
fixed is identity available
ashleysmithTTD Feb 25, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/callbackManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export enum EventType {
IdentityUpdated = 'IdentityUpdated',
SdkLoaded = 'SdkLoaded',
OptoutReceived = 'OptoutReceived',
NoIdentityAvailable = 'NoIdentityAvailable',
}

export type CallbackPayload = SdkLoadedPayload | PayloadWithIdentity;
Expand Down Expand Up @@ -52,7 +53,8 @@ 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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With everything that can be done without init, I dont think it makes sense anymore to not run callbacks if init is not complete

// if SDK has not been loaded yet, callbacks should not run
if (!CallbackManager._sentSdkLoaded[this._productName]) return;

const enrichedPayload = {
...payload,
Expand Down
146 changes: 145 additions & 1 deletion src/integrationTests/options.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterEach, beforeEach, describe, expect, jest, test } from '@jest/globals';

import * as mocks from '../mocks';
import { SdkOptions, sdkWindow, UID2 } from '../uid2Sdk';
import { EventType, SdkOptions, sdkWindow, UID2 } from '../uid2Sdk';
import { loadConfig, removeConfig } from '../configManager';
import { ProductDetails } from '../product';

Expand Down Expand Up @@ -556,3 +556,147 @@ describe('Store config UID2', () => {
});
});
});

describe('calls the NoIdentityAvailable event', () => {
let handler: ReturnType<typeof jest.fn>;
let expiredIdentity = makeIdentity({
identity_expires: Date.now() - 100000,
refresh_expires: Date.now() - 100000,
});

beforeEach(() => {
handler = jest.fn();
uid2.callbacks.push(handler);
});

test('when init is called for the first time with no identity', () => {
uid2.init({});

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when init is already complete and called again with no identity', () => {
uid2.init({});
uid2.init({});

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when init is already complete and called again with an expired identity', () => {
uid2.init({});
uid2.init({
identity: expiredIdentity,
});

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, {
identity: expiredIdentity,
});
});
test('when init is already complete but the existing identity is expired', () => {
uid2.init({
identity: expiredIdentity,
});
uid2.init({});

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, {
identity: expiredIdentity,
});
});
test('when identity is expired but refreshable', () => {
let expiredRefreshableIdentity = makeIdentity({
identity_expires: Date.now() - 10000,
refresh_expires: Date.now() + 10000,
});
uid2.init({ identity: expiredRefreshableIdentity });

// in this case, identity is temporarily unavailable but still unavailable
expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when login is required', () => {
uid2.isLoginRequired();

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when get identity returns null', () => {
uid2.getIdentity();

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when there is no advertising token', () => {
uid2.getAdvertisingToken();

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when cstg does not succeed', () => {
uid2.init({});

expect(uid2.setIdentityFromEmail('a', mocks.makeUid2CstgOption())).rejects.toThrow(
'Invalid email address'
);
expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when identity was valid on init but has since expired', () => {
const originalIdentity = makeIdentity({
advertising_token: 'original_advertising_token',
identity_expires: Date.now() + 100,
});
uid2.init({ identity: originalIdentity });

expect(handler).not.toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, {
identity: null,
});

// set time to an expired date for this identity
jest.setSystemTime(originalIdentity.refresh_expires * 1000 + 1);

uid2.isIdentityAvailable();

expect(handler).toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, {
identity: originalIdentity,
});
});
});

describe('does not call NoIdentityAvailable event', () => {
let validIdentity = makeIdentity();
let handler: ReturnType<typeof jest.fn>;
beforeEach(() => {
handler = jest.fn();
uid2.callbacks.push(handler);
});

test('when setIdentity is run with a valid identity, should not call NoIdentityAvailable on set or get', () => {
uid2.init({});
handler = jest.fn();

uid2.setIdentity(validIdentity);
uid2.getIdentity();
uid2.getAdvertisingToken();

expect(handler).not.toHaveBeenCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when identity is set with opted out identity', () => {
uid2.init({ identity: makeIdentity({ status: 'optout' }) });

expect(handler).not.toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when cstg is successful', async () => {
uid2.init({});
handler = jest.fn();

expect(async () => {
await uid2.setIdentityFromEmail('[email protected]', mocks.makeUid2CstgOption());
}).not.toThrow();
expect(handler).not.toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when identity is set with local storage and init has never been called', () => {
mocks.setUid2LocalStorage(validIdentity);
uid2.isIdentityAvailable();

expect(handler).not.toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
test('when identity is set with cookie and init has never been called', () => {
mocks.setUid2Cookie(validIdentity);
uid2.isIdentityAvailable();

expect(handler).not.toHaveBeenLastCalledWith(EventType.NoIdentityAvailable, { identity: null });
});
});
50 changes: 42 additions & 8 deletions src/sdkBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export abstract class SdkBase {
this._callbackManager = new CallbackManager(
this,
this._product.name,
() => this.getIdentity(),
() => this.getIdentityForCallback(),
this._logger
);
}
Expand Down Expand Up @@ -159,9 +159,22 @@ export abstract class SdkBase {
}
this._callbackManager.runCallbacks(EventType.IdentityUpdated, {});
}
this.isIdentityAvailable();
}

public getIdentity(): Identity | null {
const identity = this._identity ?? this.getIdentityNoInit();
const isValid =
identity && !this.temporarilyUnavailable(identity) && !isOptoutIdentity(identity);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an Optout Identity if valid so I don't think we want that in this condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems the code always checks for optout separately so it assumes a valid identity will have a particular shape. So this is fine.

if (!isValid) {
this._callbackManager.runCallbacks(EventType.NoIdentityAvailable, {});
return null;
} else {
return identity;
}
}

Copy link
Contributor Author

@ashleysmithTTD ashleysmithTTD Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

private getIdentityForCallback(): Identity | null {
const identity = this._identity ?? this.getIdentityNoInit();
return identity && !this.temporarilyUnavailable(identity) && !isOptoutIdentity(identity)
? identity
Expand All @@ -181,11 +194,27 @@ export abstract class SdkBase {
}

public isLoginRequired() {
const identity = this._identity ?? this.getIdentityNoInit();
// if identity temporarily unavailable, login is not required
if (this.temporarilyUnavailable(identity)) {
return false;
}
return !this.isIdentityAvailable();
}

public isIdentityAvailable() {
return this.isIdentityValid() || this._apiClient?.hasActiveRequests();
const identity = this._identity ?? this.getIdentityNoInit();
const identityAvailable =
(this.isIdentityValid(identity) && !this.temporarilyUnavailable(identity)) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added this extra check because identity can be valid, but unavailable

this._apiClient?.hasActiveRequests();

if (!identityAvailable) {
if (this._callbackManager) {
this._callbackManager.runCallbacks(EventType.NoIdentityAvailable, {});
}
return false;
}
return true;
}

public hasOptedOut() {
Expand Down Expand Up @@ -230,10 +259,14 @@ export abstract class SdkBase {
this._initCallbackManager?.addInitCallback(opts.callback);
}

const useNewIdentity =
opts.identity &&
(!previousOpts.identity ||
opts.identity.identity_expires > previousOpts.identity.identity_expires);
let useNewIdentity;
if (!opts.identity) useNewIdentity = true;
else {
useNewIdentity =
!previousOpts.identity ||
opts.identity.identity_expires > previousOpts.identity.identity_expires;
}

Copy link
Contributor Author

@ashleysmithTTD ashleysmithTTD Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found in testing that we didnt have the possibility that opts.identity just doesn't exist, which would automatically make useNewIdentity true

if (useNewIdentity || opts.callback) {
let identity = useNewIdentity ? opts.identity : previousOpts.identity ?? null;
if (identity) {
Expand Down Expand Up @@ -277,11 +310,11 @@ export abstract class SdkBase {

this.setInitComplete(true);
this._callbackManager?.runCallbacks(EventType.InitCompleted, {});
this.isIdentityAvailable();
if (this.hasOptedOut()) this._callbackManager.runCallbacks(EventType.OptoutReceived, {});
}

private isIdentityValid() {
const identity = this._identity ?? this.getIdentityNoInit();
private isIdentityValid(identity: Identity | OptoutIdentity | null | undefined) {
return identity && !hasExpired(identity.refresh_expires);
}

Expand Down Expand Up @@ -499,6 +532,7 @@ export abstract class SdkBase {
} else {
const errorText = 'Unexpected status received from CSTG endpoint.';
this._logger.warn(errorText);
this.isIdentityAvailable();
throw new Error(errorText);
}
}
Expand Down