[CI 4555] Support assistant pdp functions #381
Conversation
src/modules/tracker.js
Outdated
| * @param {object} parameters - Additional parameters to be sent with request | ||
| * @param {array.<{question: string}>} parameters.questions - List of pre-defined questions shown to the user | ||
| * @param {string} parameters.itemId - Product id whose page we are on | ||
| * @param {string} parameters.itemName - Product name whose page we are one |
src/modules/tracker.js
Outdated
| * }, | ||
| * ); | ||
| */ | ||
| trackAssistantPdpView(parameters, networkParameters = {}) { |
There was a problem hiding this comment.
Should we add some sort of label mentioning this will be deprecated in the future?
src/modules/tracker.js
Outdated
| * }, | ||
| * ); | ||
| */ | ||
| trackAssistantPdpOutOfView(parameters, networkParameters = {}) { |
There was a problem hiding this comment.
Similar to above, it looks like this function will be deprecated in the future
src/modules/tracker.js
Outdated
| * @param {object} [networkParameters] - Parameters relevant to the network request | ||
| * @param {number} [networkParameters.timeout] - Request timeout (in milliseconds) | ||
| * @returns {(true|Error)} | ||
| * @description The PDP Q&A element was focused on |
There was a problem hiding this comment.
Should we be more specific here and say input element?
Separately, this is just a nitpick, but could we update the descriptions for the functions to be consistent with others? (i.e. User focused on the PDP Q&A input element)
src/modules/tracker.js
Outdated
| * @param {string} parameters.itemName - Product name whose page we are one | ||
| * @param {array.<{start: string | undefined, | ||
| * end: string | undefined}>} parameters.viewTimespans - List of timestamp pairs in ISO_8601 format | ||
| * @param {string} [parameters.variationId] - Variation id whose page we are one |
There was a problem hiding this comment.
typo one -> on. looks like this typo exists on most/all of the functions. I'll leave this note here so I don't spam the comments.
|
Forgot to mention in original review message, but let's check in with the ASA team on whether or not they plan to change the names of the endpoints so we can make the change while these changes are still in progress. We'll also want to make the switch from |
jjl014
left a comment
jjl014
left a comment
There was a problem hiding this comment.
Changes are looking good from what I can tell. I did leave a couple more comments on descriptions/consistency.
src/modules/tracker.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Send product insights agent out of view event |
There was a problem hiding this comment.
I believe this should be "Send product insights agent input focus event"
src/modules/tracker.js
Outdated
| * @param {object} [networkParameters] - Parameters relevant to the network request | ||
| * @param {number} [networkParameters.timeout] - Request timeout (in milliseconds) | ||
| * @returns {(true|Error)} | ||
| * @description The product insights agent question that was clicked on |
There was a problem hiding this comment.
For consistency, should we change this to be "User clicked on a question within the product insights agent"?
src/modules/tracker.js
Outdated
| * @param {object} [networkParameters] - Parameters relevant to the network request | ||
| * @param {number} [networkParameters.timeout] - Request timeout (in milliseconds) | ||
| * @returns {(true|Error)} | ||
| * @description The product insights agent question was submitted |
There was a problem hiding this comment.
For consistency, should we change this to be "User submitted a question to the product insights agent"?
src/modules/tracker.js
Outdated
| * @param {object} [networkParameters] - Parameters relevant to the network request | ||
| * @param {number} [networkParameters.timeout] - Request timeout (in milliseconds) | ||
| * @returns {(true|Error)} | ||
| * @description The product insights agent answer was shown to the user |
There was a problem hiding this comment.
For consistency, should we change this to be "User viewed the answer provided by the product insights agent"?
src/modules/tracker.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Send product insights agent question submit |
There was a problem hiding this comment.
I missed this in my previous review, but let's add "event" to the end of this to keep things consistent
src/modules/tracker.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Send product insights agent answer view |
There was a problem hiding this comment.
Let's add "event" to the end of this
src/modules/tracker.js
Outdated
| } | ||
|
|
||
| /** | ||
| * Send product insights agent answer feedback |
There was a problem hiding this comment.
Let's add "event" to the end of this
Implement the assistant PDP tracking endpoints following this doc
https://constructor.slab.com/posts/bl-new-asa-item-questions-events-17qvw7ri#hyml0-event-assistant-pdp-views