-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat: query-suggestions client APIC-202 #74
Conversation
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.
Looks good overall!
Some missing descriptions and mostly wondering what we should do with our error status code
'401': | ||
$ref: ../../common/responses/Unauthorized.yml | ||
'422': | ||
$ref: ../common/responses/StatusUnprocessableEntity.yml | ||
'500': | ||
$ref: ../../common/responses/InternalError.yml |
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.
Are we sure other status are not handled? I believe the engine default status error for all endpoints, but I could be wrong
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.
My guess would be that some errors are documented and some not, we could do some tests to trigger them and see if it's global for all clients
e.g. 403 is declared as MethodNotAllowed
on our clients
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 followed the doc but I'm sure some errors are missing, I'll add the common one
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.
In case you added you one, don't hesitate to add them to the common folder too. I wonder if we could have a global schema for all those responses
clients/algoliasearch-client-javascript/client-query-suggestions/model/models.ts
Outdated
Show resolved
Hide resolved
* | ||
* @param getAllConfigs - The getAllConfigs parameters. | ||
*/ |
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.
Those @param
comments are fixed in #77
specs/query-suggestions/common/schemas/QuerySuggestionsIndex.yml
Outdated
Show resolved
Hide resolved
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.
Purfect!
content: | ||
application/json: | ||
schema: | ||
type: array |
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.
Does it really return an array directly? :mad:
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'm sorry :(
'422': | ||
$ref: ../../common/responses/StatusUnprocessableEntity.yml |
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 guess we should add it to all our endpoints 🤔
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.
We should test first, not sure how to trigger this error
🧭 What and Why
🎟 JIRA Ticket: APIC-202
Create the query-suggestions client
Changes included:
qs
toquery-suggestions
because it's not worth it🧪 Test
CI