5046: Use recommendation service instead of crafted search #143
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xendk
force-pushed
the
5046-use-papps
branch
3 times, most recently
from
63498b5
to
0c04428
Compare
vibjerg
reviewed
Sep 7, 2021
Comment on lines
+16
to
+17
| * @param {string} The name of the search index to query. | ||
| * @param {string[]} The values to query for. |
There was a problem hiding this comment.
JSDoc misses the name of the parameter.
Suggested change
| * @param {string} The name of the search index to query. | |
| * @param {string[]} The values to query for. | |
| * @param {string} index The name of the search index to query. | |
| * @param {string[]} values The values to query for. |
Comment on lines
+68
to
+74
| // Get the current slice of recommendations. This causes repeated | ||
| // calls to the recommendation service, but it's the simplest way to | ||
| // implement it in the current code. Leave optimization for later. | ||
| const relatedFausts = (await relatedClient.getRecommendations({ id })).slice( | ||
| offset, | ||
| limit, | ||
| fields, | ||
| sort | ||
| }); | ||
| offset + limit | ||
| ); |
There was a problem hiding this comment.
I guess offset and limit will be parameters on the recommendation service over time. Would it make sense to move the slice logic into the client, and just pass them as params.
const relatedFausts = await relatedClient.getRecommendations({ id, offset, limit });
Comment on lines
+10
to
+15
| /** | ||
| * Creates an instance of LibryRecommendationService. | ||
| * | ||
| * @param {object} options | ||
| * @param {string} options.baseUrl | ||
| */ |
There was a problem hiding this comment.
ClientId is missing in doc
Suggested change
| /** | |
| * Creates an instance of LibryRecommendationService. | |
| * | |
| * @param {object} options | |
| * @param {string} options.baseUrl | |
| */ | |
| /** | |
| * Creates an instance of LibryRecommendationService. | |
| * | |
| * @param {object} options | |
| * @param {string} options.baseUrl | |
| * @param {string} options.clientId | |
| */ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The search link has been removed as it doesn't make any sense anymore, as we can't make the search return the same materials in the same order.