-
Notifications
You must be signed in to change notification settings - Fork 52
FEAT: Store identity map to persist for subsequent calls #1409
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
Conversation
carterworks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is huge and a great add. I have two small questions, but no real blockers.
| event.setUserXdm(xdm); | ||
| event.setUserData(data); | ||
|
|
||
| if (xdm && xdm.identityMap) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if (xdm && xdm.identityMap) { | |
| if (xdm?.identityMap) { |
|
|
||
| if (xdm && xdm.identityMap) { | ||
| identityMapStorage.store(xdm.identityMap); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about something like this, to be able to clear the stored identity map? @jonsnyder & @dompuiu let me know your thoughts, too
if (xdm?.identityMap === null) {
identityMapStorage.clear();
}
jonsnyder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tricky feature. Consider this use-case:
- sendEvent1 is sent with identityMapA
- sendEvent2 is send with identityMapB
- sendEvent1 returns, renders and sends a display notification
- sendEvent2 returns, renders and sends a display notification
Ideally, 3 would contain identityMapA, but with this solution it has identityMapB.
I think to solve this, you would need to add the identityMap to the lifecycle onBeforeEvent call (at the top level or part of one of the params) and pass it through to the collect call.
carterworks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As-is, this attaches the identity map to a proposition display event (given that an identity map is provided when fetching/rendering the proposition) but it doesn't include identity map for automatic click tracking.
Requested changes have been implemented
Description
To help track identity for users that are not leveraging ECID, this feature stores the identity map made on send event calls so that it can be populated on follow-up calls (not originated directly by user, eg proposition display).
Related Issue
Jira
Motivation and Context
(see description)
Screenshots (if appropriate):
N/A
Checklist: