Return full item data instead of just ids option#1193
Draft
AbhilashVijayakumar wants to merge 13 commits intogorse-io:masterfrom
Draft
Return full item data instead of just ids option#1193AbhilashVijayakumar wants to merge 13 commits intogorse-io:masterfrom
AbhilashVijayakumar wants to merge 13 commits intogorse-io:masterfrom
Conversation
…ust ids if include-items query parameter respond with full items instead of just ids so that additional call to downstream systems to show a list to user can be avoided & make integration more efficient. Currently after getting 10 ids to recommend to user, i need to make 10 calls to downstream system to get the meta data to show user a listing of the recommendations
zhenghaoz
requested changes
Mar 10, 2026
zhenghaoz
requested changes
Mar 11, 2026
zhenghaoz
requested changes
Mar 11, 2026
|
|
||
| // RecommendResponse is the response for the recommend endpoint. | ||
| // It includes both item IDs (for backward compatibility) and full item data. | ||
| type RecommendResponse struct { |
Collaborator
There was a problem hiding this comment.
Return array and remove structure to be compatible with no items situation
|
|
||
| // ScoredItem is a scored item with optional full item data for X-Api-Version: 2. | ||
| type ScoredItem struct { | ||
| ItemId string `json:"ItemId"` |
Collaborator
There was a problem hiding this comment.
Use id to be compatible with score
| // Send result | ||
| if apiVersion == "2" { | ||
| Ok(response, scores) | ||
| if includeItems { |
Collaborator
There was a problem hiding this comment.
Change variable names as well
| itemIds := lo.Map(scores, func(item cache.Score, index int) string { | ||
| return item.Id | ||
| }) | ||
| includeItems := request.QueryParameter("return-items") == "true" |
Collaborator
There was a problem hiding this comment.
Move BatchGetItems to return item clause
|
|
||
| // marshalRecommend builds the expected JSON for the recommend endpoint. | ||
| // It fetches full item data from the test DataClient and orders them to match itemIds. | ||
| func (suite *ServerTestSuite) marshalRecommend(itemIds []string) string { |
Collaborator
There was a problem hiding this comment.
Rename to marshal scored items
| // marshalRecommend builds the expected JSON for the recommend endpoint. | ||
| // It fetches full item data from the test DataClient and orders them to match itemIds. | ||
| func (suite *ServerTestSuite) marshalRecommend(itemIds []string) string { | ||
| ctx := context.Background() |
Collaborator
There was a problem hiding this comment.
Use suite.T(). Context()
zhenghaoz
reviewed
Mar 11, 2026
| Writes([]string{})) | ||
| Param(ws.QueryParameter("return-items", "Include full item data in response").DataType("boolean")). | ||
| Returns(http.StatusOK, "OK", RecommendResponse{}). | ||
| Writes(RecommendResponse{})) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1193 +/- ##
==========================================
- Coverage 72.72% 72.68% -0.05%
==========================================
Files 88 88
Lines 16258 16289 +31
==========================================
+ Hits 11824 11840 +16
- Misses 3233 3246 +13
- Partials 1201 1203 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or 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
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.
Currently after getting 10 ids to recommend to user, i need to make 10 calls to downstream system to get the meta data to show user a listing of the recommendations
if include-items query parameter is set, respond with full items instead of just ids so that additional call to downstream systems to show a list to user can be avoided & make integration more efficient.
Backward compatible as response will change only if the include-items new query parameter is set