fix: Move ready queue processing after identity request #933
Conversation
alexs-mparticle
force-pushed
the
fix/SQDSDKS-6835-event-logging-wo-identities
branch
from
8b5ffb4
to
6478269
Compare
src/mp-instance.js
Outdated
| // If for some reason the identity call inside of Session Manager does not run | ||
| // we should clear out the ready queue. |
There was a problem hiding this comment.
Your PR description appropriately states when this is the case, rather than "for some reason", so I'd turn that into a comment.
"Will also continue to clear out the ready queue as part of the initial init flow if an identify request is unnecessary, such as if there is an existing session".
| const functionSpy = jest.fn(); | ||
| (window.mParticle as any) = { | ||
| fakeFunction: functionSpy, | ||
| args: () => {}, |
There was a problem hiding this comment.
Is this args function necessary? I haven't pulled this code down, but as I read through the processReadyQueue function, the only times I see args is where it references the item in the array, as opposed to it being a key on window.mParticle.
There was a problem hiding this comment.
I'm open to discussing this further. Adding args here was the only way I could get it to hit that specific codepath in my tests.
There was a problem hiding this comment.
let's hop on a zoom about this to walk through
There was a problem hiding this comment.
hm. when i comment out line 32, it passes for me. see screenshot below:
There was a problem hiding this comment.
It's weird but commenting out arg goes to a different code path that has the same result. It's likely a redundant conditional and could use some clean up. We should definitely sync up on the use cases to make sure nothing is missed.
|
## [2.30.4](v2.30.3...v2.30.4) (2024-11-05) ### Bug Fixes * Move ready queue processing after identity request ([#933](#933)) ([678ccb7](678ccb7))
|
🎉 This PR is included in version 2.30.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Instructions
developmentSummary
Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)