Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

Allow sending and retrieval of additionalContext within authentication flow #937

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Shinsina
Copy link
Member

@Shinsina Shinsina commented Jul 29, 2024

REQUIRES parameter1/identity-x#54 TO BE MERGED AND DEPLOYED

From parameter1/identity-x#54: "This allows for the passthrough of a JSON object called additionalContext, this will allow for the newsletterSignupType (amongst other potential points of data) to be specified and returned namely for the login-link-click event tracking within P1 Events in order to enable correct event recording for reporting purposes."

Screenshot from 2024-07-29 14-12-42
image

@Shinsina Shinsina requested a review from solocommand as a code owner July 29, 2024 19:19
@Shinsina Shinsina requested review from brandonbk and B77Mills July 29, 2024 19:19
@Shinsina
Copy link
Member Author

Audience Acquisition Tab

Copy link
Member

@solocommand solocommand left a comment

Choose a reason for hiding this comment

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

This looks fine as-is, barring changes from the IdX PR. It doesn't have any P1E interaction, so can you remove those screenshots and update the description please!

Comment on lines 250 to 254
...(
data.additionalEventData
&& data.additionalEventData.newsletterSignupType
&& { newsletterSignupType: data.additionalEventData.newsletterSignupType }
),
Copy link
Member

Choose a reason for hiding this comment

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

Remove this, this will be present under additionalEventData.newsletterSignupType below

Copy link
Member Author

Choose a reason for hiding this comment

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

@solocommand This is how P1 Events expects it to be sent in order to send it accordingly via this: https://github.com/parameter1/base-cms/blob/master/packages/marko-web-p1-events/browser/index.js#L69

@Shinsina Shinsina marked this pull request as draft July 30, 2024 15:56
@Shinsina
Copy link
Member Author

Shinsina commented Jul 30, 2024

This looks fine as-is, barring changes from the IdX PR. It doesn't have any P1E interaction, so can you remove those screenshots and update the description please!

@solocommand

The IdX PR has no interaction with P1E, all that PR is intended to do is allow the retrieval of additional event data from the login link sent event in order for the corresponding click event to correlate to the correct sent location.

This PR is ultimately what interacts with P1E which is why those screenshots were included as they are.

@Shinsina Shinsina requested a review from solocommand July 30, 2024 18:21
@Shinsina Shinsina changed the title Allow sending and retrieval of additionalEventData within authentication flow Allow sending and retrieval of additionalContext within authentication flow Jul 30, 2024
@Shinsina Shinsina marked this pull request as ready for review July 30, 2024 18:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants