Skip to content
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 UUID to each pageview event #74

Merged
merged 4 commits into from
Oct 6, 2023

Conversation

wzieba
Copy link
Collaborator

@wzieba wzieba commented Oct 5, 2023

Closes: #28

Description

This PR is a twin of #73 for pageview and heartbeat events. It adds a new, internal field lastPageviewUuid. See internal discussion about necessity of it: p1696417656257339-slack-C0533SEJ82H

Test

  1. Install example app, open logcat
  2. Tap on Track Url
  3. Tap on Start Engagement
  4. Wait 30 seconds until you see [Parsely] POST Data {"events": (...) log. Copy the JSON part.
  5. Wait another 30 seconds until another [Parsely] POST Data {"events": (...) log, copy it as well.
  6. Check the copied logs:
  • there should be one "action": "pageview" event, rest of them should be "action": "heartbeat"
  • every event from both logs should have "pvid" field with the same value

New payload

Click Screenshot 2023-10-05 at 18 34 23

Base automatically changed from issue/add_pvid_parameter to master October 5, 2023 16:13
To identify `pageview` events. `heartbeat` events should attach uuid of the parent `pageview` event.
@wzieba wzieba force-pushed the issue/add_pvid_parameter_to_pageview_events branch from 8e3ad70 to 9e7db50 Compare October 5, 2023 16:17
@wzieba wzieba force-pushed the issue/add_pvid_parameter_to_pageview_events branch from 9e7db50 to 68243c4 Compare October 5, 2023 16:18
@wzieba wzieba marked this pull request as ready for review October 5, 2023 16:42
@wzieba wzieba requested a review from ParaskP7 October 5, 2023 16:44
@ParaskP7 ParaskP7 self-assigned this Oct 6, 2023
Copy link
Collaborator

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 @wzieba !

I have reviewed and tested this PR as per the instructions, everything works as expected, good job! 🌟


  1. Thinking (🤔): Although I am "kind of" not a fan of having this new lastPageviewUuid state added and using it as such, depending that the clients would first trackPageview(...) and then startEngagement(...), just because it is in the docs, since I understand that this was discussed and agreed on over Slack, I am okay keeping it as is.
  2. Warning (⚠️): Let's deal with this here and remove it before merging this PR to master.

@wzieba
Copy link
Collaborator Author

wzieba commented Oct 6, 2023

Let's deal with #73 (comment) here and remove it before merging this PR to master.

Thanks for being vigilant! Addressed in 61c3db3

@wzieba wzieba merged commit faf8302 into master Oct 6, 2023
1 check passed
@wzieba wzieba deleted the issue/add_pvid_parameter_to_pageview_events branch October 6, 2023 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attach a different UUID to every request
2 participants