Skip to content

Map product_id for a Product #761

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.

Sign up for GitHub

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

Merged
niallzato merged 2 commits into from
Apr 7, 2021
Merged

Map product_id for a Product #761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d80199
map product_id
Mar 30, 2021
f1e071c
retrigger checks
Apr 7, 2021
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
3 changes: 2 additions & 1 deletion analytics/src/main/java/com/segment/analytics/Properties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ public boolean isRepeatCustomer() {
public static class Product extends ValueMap {

private static final String ID_KEY = "id";
private static final String PRODUCT_ID_KEY = "product_id";
private static final String SKU_KEY = "sku";
private static final String NAME_KEY = "name";
private static final String PRICE_KEY = "price";
Expand Down Expand Up @@ -422,7 +423,7 @@ public String name() {
}

public String id() {
return getString(ID_KEY);
return getString(ID_KEY) != null ? getString(ID_KEY) : getString(PRODUCT_ID_KEY);
Copy link
Contributor

@prayansh prayansh Mar 30, 2021

Choose a reason for hiding this comment

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

question: do u think we should give first preference to product_id and then fall back to id or vice versa?

Copy link
Contributor Author

@niallzato niallzato Mar 30, 2021

Choose a reason for hiding this comment

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

My thinking was to give preference to id over product_id. This should not introduce any changes if a customer is sending both id and product_id. If a customer is sending both values you'd expect those values to be the same but you never know!

Copy link
Contributor

@prayansh prayansh Mar 30, 2021

Choose a reason for hiding this comment

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

sounds good

}

public String sku() {
Expand Down
4 changes: 4 additions & 0 deletions analytics/src/test/java/com/segment/analytics/PropertiesTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,5 +197,9 @@ class PropertiesTest {
assertThat(product.name()).isNull()
product.putName("name")
assertThat(product.name()).isEqualTo("name")

product.remove("id")
Copy link
Contributor

@prayansh prayansh Mar 30, 2021

Choose a reason for hiding this comment

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

🔥

product.putValue("product_id", "id")
assertThat(product.id()).isEqualTo("id")
}
}