Skip to content
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

fix(specs): recommend api #2958

Merged
merged 12 commits into from
Apr 8, 2024
Merged

fix(specs): recommend api #2958

merged 12 commits into from
Apr 8, 2024

Conversation

kai687
Copy link
Contributor

@kai687 kai687 commented Apr 4, 2024

🧭 What and Why

Update the spec for the Recommend API to make it suitable as API reference

Changes included:

  • Added Looking Similar model to the "Get recommendations" response and remove "Recommended for you" (didn't find any reference anywhere for this).
  • Recommend Rules != Search Rules, so I typed the schema, trying to reuse what I can
  • Small changes to common parameters:
    • made x-algolia... headers lowercase. HTTP headers are case-insensitive anyway. It just looks better IMO.
    • changed the description of the Search filters parameter to make it usable for recommendations as well.

Things that will still be wrong

Fixing this requires some remodeling in the "common" parameters/Search API response,
which might break things unrelated to Recommend.

  • Recommendation API response != Search API response.
  • queryParameters for getting recommendations does not accept hitsPerPage, page, offset, length.

🧪 Test

TODO: fix tests for the new responses.

@algolia-bot
Copy link
Collaborator

algolia-bot commented Apr 4, 2024

✗ The generated branch has been deleted.

If the PR has been merged, you can check the generated code on the main branch.
You can still access the code generated on main via this commit.

@kai687
Copy link
Contributor Author

kai687 commented Apr 5, 2024

No idea why Dart fails, but Java, Kotlin, Scala fails because threshold is tested with a 42, but the threshold parameter can be a float.
What's the right way of fixing this? Testing with a float?

@kai687 kai687 marked this pull request as ready for review April 5, 2024 13:25
@kai687 kai687 requested a review from a team as a code owner April 5, 2024 13:25
@kai687 kai687 requested review from morganleroi and shortcuts April 5, 2024 13:25
@shortcuts
Copy link
Member

No idea why Dart fails, but Java, Kotlin, Scala fails because threshold is tested with a 42, but the threshold parameter can be a float. What's the right way of fixing this? Testing with a float?

I guess so yes if it's expected to have this format, but it might be an issue with the CTS template

Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe @raed667 @marialungu @francoischalifour can help for the review on this one

for the recommended for you, I know it's available in the JavaScript client https://github.com/search?q=repo%3Aalgolia%2Falgoliasearch-client-javascript%20recommended-for-you&type=code

@kai687
Copy link
Contributor Author

kai687 commented Apr 5, 2024

Changed the integers in the test to a float to make the test pass, but the API also accepts integers...

Python fails because of a test for generatedSecuredAPIkeys(), which is unrelated to this PR.
No idea what's the issue with Dart.

And indeed, recommended-for-you seems to be some (private) beta model. The Recommend team can decide if that should be documented here or not.

@shortcuts
Copy link
Member

And indeed, recommended-for-you seems to be some (private) beta model. The Recommend team can decide if that should be documented here or not.

since it has been added by the recommend team on the javascript client we should keep it for now and let them remove it if necessary

Copy link

github-actions bot commented Apr 7, 2024

@github-actions github-actions bot temporarily deployed to pull request April 7, 2024 12:41 Inactive
Copy link
Member

@shortcuts shortcuts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

purrrrfect

@github-actions github-actions bot temporarily deployed to pull request April 8, 2024 12:38 Inactive
@shortcuts shortcuts merged commit dabdd02 into main Apr 8, 2024
25 checks passed
@shortcuts shortcuts deleted the docs/recommend-api branch April 8, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants