fix: dynamic messages - pass merchant id from cpnw callback, flush logger #2381
Conversation
src/zoid/buttons/component.jsx
Outdated
| getLogger().addTrackingBuilder(() => ({ | ||
| [FPTI_KEY.SELLER_ID]: merchantID, | ||
| })); |
There was a problem hiding this comment.
What happens when an actual merchantID is passed in as part of the SDK script (i.e. partner integration)? Will this unintentionally override that or are we still good there? Also will this impact other non-messaging events that are fired from outside the IFrame since addTrackingBuilder changes the shared logger instance?
There was a problem hiding this comment.
good callout, looks like it will override the partner integration so need to add a check to see if that exists first
src/zoid/buttons/util.js
Outdated
| @@ -396,7 +396,7 @@ export const getModal: ( | |||
|
|
|||
| return window[namespace].MessagesModal({ | |||
| account: `client-id:${clientID}`, | |||
| merchantId: merchantID?.join(",") || undefined, | |||
| merchantId: merchantID, | |||
There was a problem hiding this comment.
For the modal input here we want this value to be the SDK script merchantID value because we don't want to pass in a merchantId value unless it is a partner integration, otherwise it may impact authentication.
There was a problem hiding this comment.
good callout, ill fix this one
src/zoid/buttons/component.jsx
Outdated
| @@ -730,7 +736,7 @@ export const getButtonsComponent: () => ButtonsComponent = memoize(() => { | |||
| [FPTI_KEY.CONTEXT_ID]: buttonSessionID, | |||
| [FPTI_KEY.CONTEXT_TYPE]: "button_session_id", | |||
| [FPTI_KEY.EVENT_NAME]: "message_click", | |||
| [FPTI_KEY.SELLER_ID]: merchantID?.join(","), | |||
| [FPTI_KEY.SELLER_ID]: merchantID, | |||
There was a problem hiding this comment.
If we're including this above, do we also need it here?
There was a problem hiding this comment.
going to test this to see if that is the case
src/zoid/buttons/component.jsx
Outdated
| const merchantID = isPartnerMerchantIdEmpty | ||
| ? serverMerchantId | ||
| : partnerMerchantId?.join(); |
There was a problem hiding this comment.
This merchantID that we pass into getModal() we do not want to be polluted with the value that comes back from the server. The merchantID that is passed into getModal() should only be the props.merchantID value that is supplied by the merchant, and not any computed value otherwise it could negatively impact auth.
There was a problem hiding this comment.
i will modify this, i thought we wanted both server + partner going to it. let me change this
src/zoid/buttons/util.js
Outdated
| @@ -373,7 +373,7 @@ function buildModalBundleUrl(): string { | |||
|
|
|||
| export const getModal: ( | |||
| clientID: string, | |||
| merchantID: $ReadOnlyArray<string> | void | |||
| merchantID?: string | void | |||
There was a problem hiding this comment.
We should consider reverting this so that we more explicitly ensure we're getting the original props.merchantID value here.
src/zoid/buttons/component.jsx
Outdated
| @@ -730,7 +736,7 @@ export const getButtonsComponent: () => ButtonsComponent = memoize(() => { | |||
| [FPTI_KEY.CONTEXT_ID]: buttonSessionID, | |||
| [FPTI_KEY.CONTEXT_TYPE]: "button_session_id", | |||
| [FPTI_KEY.EVENT_NAME]: "message_click", | |||
| [FPTI_KEY.SELLER_ID]: merchantID?.join(","), | |||
| [FPTI_KEY.SELLER_ID]: merchantID, | |||
src/zoid/buttons/component.jsx
Outdated
|
|
||
| // override with server id if partner does not exist | ||
| getLogger().addTrackingBuilder(() => ({ | ||
| [FPTI_KEY.SELLER_ID]: merchantID, |
There was a problem hiding this comment.
Would something similar to props.merchantID.toString() || serverMerchantId be the same result without the need for some of the code above?
There was a problem hiding this comment.
yea that should work ill update
Description
we are passing merchantID from a callback handled in an internal service. We are also flushing the logger that way on page navigation we can send up logger events
Why are we making these changes? Include references to any related Jira tasks or GitHub Issues
Reproduction Steps (if applicable)
Screenshots (if applicable)
Dependent Changes (if applicable)
Groups who should review (if applicable)
❤️ Thank you!