fix: encodeURIComponent filters #342
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 60b35ee:
|
| eventType: "click", | ||
| eventName: "my-event", | ||
| index: "my-index", | ||
| filters: ["brand:Cool Brand"] |
There was a problem hiding this comment.
A more interesting example could be filters: ["discount:20%"] because this would break with the current implementation, and is fixed with the changes.
lib/_sendEvent.ts
Outdated
| if (filter.indexOf(":") >= 0) { | ||
| const parts = filter.split(":"); | ||
| return [parts[0], encodeURIComponent(parts[1])].join(":") | ||
| } else { | ||
| filter; | ||
| } |
There was a problem hiding this comment.
Actually, looking at the behaviour on the insights API side, it seems that we do a decodeURIComponent on the entirety of the filter string, so this could probably be simplified with a filters.map(encodeURIComponent)
Code in the go monorepo: https://github.com/algolia/go/blob/master/pkg/searchapi/filter.go#L122
Here's a test:
curl -X POST \
https://insights.algolia.io/1/events \
-H 'x-algolia-api-key: ${API_KEY}' --referer 'http://example.com/debug' \
-H 'x-algolia-application-id: ${APP_ID}' -H 'x-algolia-agent: insights-js (2.1.0); insights-gtm (1.2.1)' \
-H "Content-Type: application/json" \
-d '{
"events": [
{
"eventType": "click",
"eventName": "Product Clicked",
"index": "products",
"userToken": "user-123456",
"timestamp": 1639129812691,
"filters": ["brand%3ACool%25%20Brand","promotion:20%25"],
"queryID": "43b15df305339e827f0ac0bdc5ebcaa7"
}
]
}'
{"status":200,"message":"OK"}
There was a problem hiding this comment.
I advised that we encoded only the filter value but it's indeed more complex. I overlooked pathological facet names.
Sorry Eunjae, I think that as Jon says, we should just urlencode the all filter.
There was a problem hiding this comment.
Yeah I think you're right. Last confirmation from @marchelbling ?
There was a problem hiding this comment.
Yes (not sure if you saw my previous message), I overlooked the facet name; let's protect the full filter string.
There was a problem hiding this comment.
@marchelbling Oh I didn't see your previous message 👍🏼
Summary
This PR encodeURIComponent filter values.