-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs(realtime-perso): add new realtime-perso api #4613
base: main
Are you sure you want to change the base?
Conversation
🪓 The generated code will be pushed at the end of the CI.Action triggered by commit Please do not push any generated code to this pull request. |
specs/realtime-personalization/common/schemas/personalizationFilters.yml
Outdated
Show resolved
Hide resolved
$ref: '#/searchFilters' | ||
|
||
searchFilters: | ||
type: object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The attribute additionalProperties
allows us to tell the generator that the key of the object can be dynamic but that the format of the value should be of type searchFilters
.
I'm not fond of this method but we don't have too much choice to respect the response we want.
We could still challenge the response format to replace the objects by arrays which would make the specs clearer imo like:
...
"search": [
{ "aliases": "abc", "strategy": "def", "filters": {...} }
]
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the current specs the generated exemples looks like this :
{
"version": "string",
"userID": "string",
"search": {
"additionalProp1": {
"indices": [
"storefront",
"storefront_price_asc",
"storefront_price_desc"
],
"strategy": "session",
"filters": {
"session": [
"brand:Dyson<score=12>"
],
"additionalProp1": {}
},
"additionalProp1": {}
},
"additionalProp2": {
"indices": [
"storefront",
"storefront_price_asc",
"storefront_price_desc"
],
"strategy": "session",
"filters": {
"session": [
"brand:Dyson<score=12>"
],
"additionalProp1": {}
},
"additionalProp1": {}
},
"additionalProp3": {
"indices": [
"storefront",
"storefront_price_asc",
"storefront_price_desc"
],
"strategy": "session",
"filters": {
"session": [
"brand:Dyson<score=12>"
],
"additionalProp1": {}
},
"additionalProp1": {}
}
},
"additionalProp1": {}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the reason for this structure was so that the FE could retrieve the required attributes as quick as possible (it's more time consuming to filter an array based on a key than it is to get a specific key from a map).
This looks good for now, but if it causes issues we can rethink it
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/PRED-3652
Changes included:
TODO
🧪 Test