Only update user once we have the flags for that user. #3
Conversation
| @@ -407,6 +407,23 @@ describe('LDClient', () => { | |||
| }); | |||
|
|
|||
| describe('identify', () => { | |||
| it('does not set user until the flag config has been updated', async () => { | |||
There was a problem hiding this comment.
Probably need a better test. What we really want to assert is that the user and the flags are updated synchronously at the same time.
There was a problem hiding this comment.
I don't think "synchronously" is a meaningful term in this context— I get what you mean, but there really isn't any way to verify that they're updated on the same pass through the event loop rather than one tick apart or whatever. I'm not super happy with the arbitrary 200ms sleep but right now I can't think of a better way.
| const flagValueMap = utils.transformVersionedValuesToValues(requestedFlags); | ||
| ident.setUser(realUser); |
There was a problem hiding this comment.
This would of course be easier if we could write it as async code (some day), but I believe you could skip the extra .then step by nesting the next one, like this:
.then(realUser =>
requestor.fetchFlagSettings(realUser, hash)
.then(requestedFlags => (
const flagValueMap = utils.transformVersionedValuesToValues(requestedFlags);
ident.setUser(realUser);
Update babel config to work in `test` without `useBuiltIns`
|
Hello @eli-darkly, thank you for the quick response! Edvin will not be available for some time, and he asked me to step in. If you consider the change you mentioned to be necessary for this pull request to be merged, would you mind making it yourself and pushing it to this branch? Alternatively, I could open another pull request from my account, and you could close this one. Taking a step back, do you confirm that the concern raised here is valid and has to be addressed? |
|
@IvanUkhov Yes, we believe that this is a valid fix. I'm not sure I have the ability to push to this branch, but I can make any necessary minor changes after merging the PR. |
Requirements
Related issues
I am mostly opening this to have a repro case for a problem we have with the js client.
We use the data export feature to get feature events into our DWH. We have enabled the option to include the whole user object in these events in order to get a better picture of why a certain variation got picked etc. However it seems that the user that is being included don't necessarily have to be the user that fetched the flag settings. In particular when we call
identify, the sdk will update the user before the flag settings have started to take effect. This means that our events will have the new user but the old flag config.Describe the solution you've provided
I changed so that the user is only updated when the flag settings have been updated.
Describe alternatives you've considered
None, I am happy to hear your thoughts here.