Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,17 @@ public String recommendationsAsJSON(RecommendationsRequest req, UserInfo userInf
}
}

if (StringUtils.isNotBlank(req.getVariationId())) {
if (req.getItemIds() == null || req.getItemIds().size() != 1) {
throw new IllegalArgumentException(
"variationId requires exactly one itemId to be specified");
}
url =
url.newBuilder()
.addQueryParameter("variation_id", req.getVariationId())
.build();
}
Comment on lines +1814 to +1823
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

variation_id is documented as usable only when exactly one item_id is specified, but the request builder will currently add variation_id even when itemIds is null or contains multiple entries. Consider validating this combination (e.g., throw an IllegalArgumentException/ConstructorException when variationId is set and itemIds == null || itemIds.size() != 1) to prevent sending invalid requests.

Copilot uses AI. Check for mistakes.

if (StringUtils.isNotBlank(req.getTerm())) {
url = url.newBuilder().addQueryParameter("term", req.getTerm()).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public class RecommendationsRequest {
private String term;
private int numResults;
private List<String> itemIds;
private String variationId;
private Map<String, List<String>> facets;
private String section;
private String preFilterExpression;
Expand All @@ -32,6 +33,7 @@ public RecommendationsRequest(String podId) throws IllegalArgumentException {
this.podId = podId;
this.numResults = 10;
this.itemIds = null;
this.variationId = null;
this.term = null;
this.section = "Products";
this.variationsMap = null;
Expand Down Expand Up @@ -97,6 +99,22 @@ public List<String> getItemIds() {
return itemIds;
}

/**
* @param variationId the variation id to set. Can be used with exactly one item_id specified in
* the request. Applicable for alternative_items, complementary_items, and bundles pod
* types.
*/
public void setVariationId(String variationId) {
this.variationId = variationId;
}

/**
* @return the variation id
*/
public String getVariationId() {
return variationId;
}

/**
* @param section the section to set
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,47 @@ public void getRecommendationsShouldReturnAResultWithSingleItemId() throws Excep
assertTrue("recommendation result id exists", response.getResultId() != null);
}

@Test
public void getRecommendationsShouldReturnAResultWithItemIdAndVariationId() throws Exception {
ConstructorIO constructor = new ConstructorIO("", apiKey, true, null);
UserInfo userInfo = new UserInfo(3, "c62a-2a09-faie");
RecommendationsRequest request = new RecommendationsRequest("item_page_1");
request.setItemIds(Arrays.asList("power_drill"));
request.setVariationId("power_drill_variation");
RecommendationsResponse response = constructor.recommendations(request, userInfo);
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This test sets variationId, but it doesn’t assert that the client actually sent variation_id (or that the server understood it). Since RecommendationsResponse exposes the parsed request map, add an assertion that response.getRequest().get("variation_id") matches the value you set so the test validates the new behavior.

Suggested change
RecommendationsResponse response = constructor.recommendations(request, userInfo);
RecommendationsResponse response = constructor.recommendations(request, userInfo);
assertEquals(
"variation_id in request should match the variationId set",
"power_drill_variation",
response.getRequest().get("variation_id"));

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@quiteeasy I think this makes sense. What do you think?

assertEquals(
"variation_id in request should match the variationId set",
"power_drill_variation",
response.getRequest().get("variation_id"));
assertTrue("recommendation results exist", response.getResponse().getResults().size() >= 0);
assertTrue("recommendation result id exists", response.getResultId() != null);
Comment on lines +39 to +45
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

The new test’s assertion response.getResponse().getResults().size() >= 0 is always true for any non-null list, so it won’t catch regressions. Prefer asserting that the results list is non-null (and/or size() > 0 if non-empty results are expected) and/or asserting the server-understood request includes the expected variation_id value.

Copilot uses AI. Check for mistakes.
}

@Test
public void getRecommendationsShouldErrorWithVariationIdAndNoItemIds() throws Exception {
ConstructorIO constructor = new ConstructorIO("", apiKey, true, null);
UserInfo userInfo = new UserInfo(3, "c62a-2a09-faie");
RecommendationsRequest request = new RecommendationsRequest("item_page_1");
request.setVariationId("power_drill_variation");

thrown.expect(ConstructorException.class);
thrown.expectMessage("variationId requires exactly one itemId to be specified");
constructor.recommendations(request, userInfo);
}

@Test
public void getRecommendationsShouldErrorWithVariationIdAndMultipleItemIds() throws Exception {
ConstructorIO constructor = new ConstructorIO("", apiKey, true, null);
UserInfo userInfo = new UserInfo(3, "c62a-2a09-faie");
RecommendationsRequest request = new RecommendationsRequest("item_page_1");
request.setItemIds(Arrays.asList("power_drill", "drill"));
request.setVariationId("power_drill_variation");

thrown.expect(ConstructorException.class);
thrown.expectMessage("variationId requires exactly one itemId to be specified");
constructor.recommendations(request, userInfo);
}

@Test
public void getRecommendationsShouldReturnAResultWithMultipleItemIds() throws Exception {
ConstructorIO constructor = new ConstructorIO("", apiKey, true, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public void newShouldReturnDefaultProperties() throws Exception {
assertEquals(request.getPodId(), podId);
assertEquals(request.getNumResults(), 10);
assertNull(request.getItemIds());
assertNull(request.getVariationId());
assertNull(request.getTerm());
assertEquals(request.getSection(), "Products");
assertEquals(request.getFacets().size(), 0);
Expand All @@ -53,7 +54,8 @@ public void settersShouldSet() throws Exception {

request.setPodId("zero_results_1");
request.setNumResults(3);
request.setItemIds(Arrays.asList("1", "2", "3"));
request.setItemIds(Arrays.asList("1"));
request.setVariationId("variation-1");
request.setSection("Search Suggestions");
request.setFacets(facets);
request.setTerm(term);
Expand All @@ -62,6 +64,8 @@ public void settersShouldSet() throws Exception {

assertEquals(request.getPodId(), "zero_results_1");
assertEquals(request.getNumResults(), 3);
assertEquals(request.getItemIds(), Arrays.asList("1"));
assertEquals(request.getVariationId(), "variation-1");
assertEquals(request.getSection(), "Search Suggestions");
assertEquals(request.getFacets(), facets);
assertEquals(request.getTerm(), term);
Expand Down
Loading