-
Notifications
You must be signed in to change notification settings - Fork 9
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
Add mobile screen engagement calculation using the screen summary context (close #16) #17
Add mobile screen engagement calculation using the screen summary context (close #16) #17
Conversation
9d7f97e
to
7a812e4
Compare
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.
A few things to change if we can. Also you'll need to add the same logic (have fun :D) to the sessions table
integration_tests/data/expected/snowplow_unified_views_mobile_screen_engagement_expected.csv
Outdated
Show resolved
Hide resolved
integration_tests/models/source/default/snowplow_unified_screen_summary_context_stg.sql
Outdated
Show resolved
Hide resolved
models/views/scratch/snowplow_unified_screen_summary_metrics.sql
Outdated
Show resolved
Hide resolved
Also can you add a changelog entry, and we'll need to update the docs to reflect this but that's not required to merge this PR. |
4f7fc00
to
3b11265
Compare
...ation_tests/models/expected/snowplow_unified_views_mobile_screen_engagement_expected_stg.sql
Show resolved
Hide resolved
models/sessions/scratch/snowplow_unified_session_screen_summary_metrics.sql
Outdated
Show resolved
Hide resolved
models/views/scratch/snowplow_unified_screen_summary_metrics.sql
Outdated
Show resolved
Hide resolved
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.
Just a few questions really, otherwise I think it looks good (assuming you tested on redshift)!
cd06569
to
eeeba0c
Compare
integration_tests/models/actual/snowplow_unified_views_mobile_screen_engagement_actual.sql
Outdated
Show resolved
Hide resolved
26309ac
to
60112c4
Compare
# Conflicts: # CHANGELOG
…depth scratch tables
…our of absolute_time_in_s in sessions and users table
…doc_width and doc_height
60112c4
to
0928227
Compare
7c817a4
into
release/snowplow-unified/0.2.0
Description
Issue #16
This makes use of the information in the
screen_summary
context entity to calculate mobile app engagement. The new schemas that this builds on are in review: snowplow/iglu-central#1358The following columns in the
views
table were updated/added:engaged_time_in_s
foreground_sec
.absolute_time_in_s
foreground_sec
+background_sec
.horizontal_pixels_scrolled
max_x_offset
.vertical_pixels_scrolled
max_y_offset
.horizontal_percentage_scrolled
max_x_offset
/content_width
* 100.vertical_percentage_scrolled
max_y_offset
/content_width
* 100.last_list_item_index
last_item_index
.list_items_count
items_count
.list_items_percentage_scrolled
last_item_index
+ 1) /items_count
* 100.New configurations:
snowplow__enable_screen_summary_context
snowplow__screen_summary_context
What type of PR is this?
Checklist
Added tests?
Added to documentation?