-
Notifications
You must be signed in to change notification settings - Fork 4
[CDX-255] Expose features object in request models #168
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
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.
Pull Request Overview
This PR adds support for exposing features and feature variants from search and browse API responses by adding new model classes and updating response models.
- Added new
FeatureandVariantmodel classes to represent feature data - Updated
SearchResponseInnerandBrowseResponseInnerto include features list - Added comprehensive test coverage for the new features functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Feature.java | New model class representing a feature with display name, feature name, enabled status, and variant |
| Variant.java | New model class representing a feature variant with name and display name |
| SearchResponseInner.java | Added features field and getter/setter methods |
| BrowseResponseInner.java | Added features field and getter/setter methods |
| SearchResponseTest.java | Added test case to verify features parsing in search responses |
| BrowseResponseTest.java | Added test case to verify features parsing in browse responses |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Mudaafi
left a comment
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.
LGTM, I checked the schemas in the autocomplete repo and they match the structure we have here.
| assertEquals( | ||
| "browse result feature variant [display name] exists", | ||
| response.getResponse().getFeatures().get(0).getVariant().getDisplayName(), | ||
| "Default weights"); | ||
| assertEquals( | ||
| "browse result feature variant [name] exists", | ||
| response.getResponse().getFeatures().get(0).getVariant().getName(), | ||
| "default_rules"); | ||
| } |
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.
Can we also test the case where variant is null? Something like ****
| assertEquals( | |
| "browse result feature variant [display name] exists", | |
| response.getResponse().getFeatures().get(0).getVariant().getDisplayName(), | |
| "Default weights"); | |
| assertEquals( | |
| "browse result feature variant [name] exists", | |
| response.getResponse().getFeatures().get(0).getVariant().getName(), | |
| "default_rules"); | |
| } | |
| assertEquals( | |
| "browse result feature variant [display name] exists", | |
| response.getResponse().getFeatures().get(0).getVariant().getDisplayName(), | |
| "Default weights"); | |
| assertEquals( | |
| "browse result feature variant [name] exists", | |
| response.getResponse().getFeatures().get(0).getVariant().getName(), | |
| "default_rules"); | |
| assertEquals( | |
| "browse result feature variant returns null if empty", | |
| response.getResponse().getFeatures().get(1).getVariant(), | |
| null); | |
| } |
| assertEquals( | ||
| "search result feature variant [name] exists", | ||
| response.getResponse().getFeatures().get(0).getVariant().getName(), | ||
| "default_rules"); | ||
| assertEquals( | ||
| "search result feature variant [display name] exists", | ||
| response.getResponse().getFeatures().get(0).getVariant().getDisplayName(), | ||
| "Default weights"); |
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.
Same as above for Browse Tests
|
Tests are failing due to an unrelated API updates. I've created a story internally to fix. |
features&feature_variantsthat are inside therequestobject of the response can be accessed as therequestobject is a genericMap<String, Object>Map<String, Object> features = (Map<String, Object>) response.getRequest().get("features");System.out.println(features.get("personalization"));