[CSL-3151] Update search results load to v2 api #329
Conversation
mocca102
left a comment
mocca102
left a comment
There was a problem hiding this comment.
We need to update the types in tracker.d.ts as well
mocca102
left a comment
mocca102
left a comment
There was a problem hiding this comment.
This is looking good just left small comments
jjl014
left a comment
jjl014
left a comment
There was a problem hiding this comment.
Looking pretty good to me!
Just left a couple comments and questions. Lmk what you think!
src/modules/tracker.js
Outdated
| resultCount = numResults || result_count, | ||
| customer_ids, | ||
| item_ids, | ||
| resultCount = numResults || result_count || 0, |
There was a problem hiding this comment.
Should we add in an extra layer of logic here as an extra precaution before defaulting to 0? We could probably use the length of customerIds / itemIds
resultCount = numResults || result_count || customerIds?.length || itemIds?.length || 0
What do you think?
(out of scope for this story, but maybe we can do this for browse load and rec views as well if we think it makes sense 😛)
There was a problem hiding this comment.
Did we want to move forward in making the change mentioned above?
jjl014
left a comment
jjl014
left a comment
There was a problem hiding this comment.
Nice! Thanks for working on this and making the updates!
Things are looking great from what I can tell. Just left a follow-up comment, but otherwise I think this is ready to be shipped 🔥 🚢
src/modules/tracker.js
Outdated
| resultCount = numResults || result_count, | ||
| customer_ids, | ||
| item_ids, | ||
| resultCount = numResults || result_count || 0, |
There was a problem hiding this comment.
Did we want to move forward in making the change mentioned above?
esezen
deleted the
csl-3151-search-results-load-v2-behavioral-event-migration
branch
No description provided.