Skip to content

Conversation

@yuhao900914
Copy link
Contributor

@yuhao900914 yuhao900914 commented Mar 29, 2024

Summary

Original Issue:
The campaign tracking fired after the session start event, so the session start event won't have the web attribution identify, which led to the Channels related to sessions being wrong.

Related Tickets:
https://amplitude.atlassian.net/browse/AMP-96571
https://amplitude.atlassian.net/browse/AMP-95820

  • move the web attribution logic into the analytics-client-common
  • create WebAttribution class and group the web attribution logic in.
  • call webAttribution track in setSessionId and in process(xx)
    - make setSessionId async and add several await to keep the order.

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from 80a754e to 4aef4a7 Compare March 29, 2024 20:30
@yuhao900914 yuhao900914 reopened this Mar 29, 2024
@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from a424fb2 to cb448e4 Compare March 29, 2024 20:43
@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from ea65423 to ff7f12a Compare April 8, 2024 08:37
@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from 1011316 to 6f25593 Compare April 8, 2024 18:01
@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from aacb90a to 45aa354 Compare April 9, 2024 01:42
@omares
Copy link

omares commented Apr 9, 2024

Hey @yuhao900914, since we're affected by this issue, I would like to understand the solution which has been applied. I would appreciate it if you could elaborate on that.
I'd also like to understand why the changes haven't been applied to the web attribution browser plugin. Instead, it seems the code has been duplicated.

@yuhao900914 yuhao900914 requested a review from a team April 9, 2024 16:34
@yuhao900914
Copy link
Contributor Author

Hey @yuhao900914, since we're affected by this issue, I would like to understand the solution which has been applied. I would appreciate it if you could elaborate on that. I'd also like to understand why the changes haven't been applied to the web attribution browser plugin. Instead, it seems the code has been duplicated.

Hi @omares , Thanks for your support. Previously, fire session end and session start events were bundled together, the web attribution was a plugin, and when to execute the web attribution plugin had a specific order. This makes it impossible to fire a web attribution to identify the call in the middle of the session end and session start event.

@justin-fiedler justin-fiedler self-requested a review April 9, 2024 17:51
@izaaz
Copy link
Contributor

izaaz commented Apr 9, 2024

@yuhao900914 on the summary, can you please also describe the problem that we are trying to solve?

@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from e948619 to ca2214d Compare April 10, 2024 20:56
@yuhao900914 yuhao900914 force-pushed the web-attribution-order branch from 98ce3d6 to b6b2ca3 Compare April 17, 2024 01:49
Copy link
Contributor

@izaaz izaaz left a comment

Choose a reason for hiding this comment

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

A couple of minor comments but the rest looks good. Thanks @yuhao900914

@yuhao900914 yuhao900914 merged commit 2f077da into main Apr 18, 2024
@yuhao900914 yuhao900914 deleted the web-attribution-order branch April 18, 2024 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants