Consider screen view ID from the screen view context (close #14) #15
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses issue #14 to consider the screen ID in the screen context entity, not just the one in the screen view event.
This is important to be able to attribute events other than the screen view event to the screen view (it's mainly necessary to enable screen engagement tracking).
Updates to integration tests
Unfortunately, the integration test data seems to be buggy as the ID in the screen context entity does not reflect the screen view events – all events have the same ID
4e8c2289-b1cd-4915-90de-2d87e1976a58
.After making this change and considering that ID, the test results changed slightly – there is a higher number of views per session as all non-screen view events have the extra
4e8c2289-b1cd-4915-90de-2d87e1976a58
ID.I think we would need to collect new mobile test data to address this (which we probably should). But that seems like a bigger task so in this PR I just updated the expected test output. Let me know if you have some other ideas how to address this.
What type of PR is this? (check all applicable)
Checklist
Added tests?
Added to documentation?