From 2fe6b7714caa5559cfe84e163ee9b8b38b2af5fb Mon Sep 17 00:00:00 2001 From: Gord Currie Date: Mon, 13 Apr 2020 10:49:04 -0500 Subject: [PATCH] Create custom unmarshaller for orders (#100) * Create custom unmarshaller for orders - We noticed an issue where some older orders were being sent with line items that had their properties field set an empty JSON object rather than the expected array. This was causing an error when attempting to unmarshal. * cleanup tests Co-authored-by: Gord Currie --- .../properties_empty_object.json | 3 + .../orderlineitems/properties_invalid0.json | 3 + .../orderlineitems/properties_invalid1.json | 5 + .../orderlineitems/properties_invalid2.json | 7 + .../orderlineitems/properties_object.json | 6 + fixtures/orderlineitems/valid.json | 88 ++++ order.go | 39 ++ order_test.go | 382 ++++++++++++++++++ 8 files changed, 533 insertions(+) create mode 100644 fixtures/orderlineitems/properties_empty_object.json create mode 100644 fixtures/orderlineitems/properties_invalid0.json create mode 100644 fixtures/orderlineitems/properties_invalid1.json create mode 100644 fixtures/orderlineitems/properties_invalid2.json create mode 100644 fixtures/orderlineitems/properties_object.json create mode 100644 fixtures/orderlineitems/valid.json diff --git a/fixtures/orderlineitems/properties_empty_object.json b/fixtures/orderlineitems/properties_empty_object.json new file mode 100644 index 00000000..fee87577 --- /dev/null +++ b/fixtures/orderlineitems/properties_empty_object.json @@ -0,0 +1,3 @@ +{ + "properties": {} +} diff --git a/fixtures/orderlineitems/properties_invalid0.json b/fixtures/orderlineitems/properties_invalid0.json new file mode 100644 index 00000000..e8c1b2ae --- /dev/null +++ b/fixtures/orderlineitems/properties_invalid0.json @@ -0,0 +1,3 @@ +{ + "properties": { +} diff --git a/fixtures/orderlineitems/properties_invalid1.json b/fixtures/orderlineitems/properties_invalid1.json new file mode 100644 index 00000000..f17aa5fe --- /dev/null +++ b/fixtures/orderlineitems/properties_invalid1.json @@ -0,0 +1,5 @@ +{ + "properties": { + "name": 2 + } +} diff --git a/fixtures/orderlineitems/properties_invalid2.json b/fixtures/orderlineitems/properties_invalid2.json new file mode 100644 index 00000000..d25c7e5e --- /dev/null +++ b/fixtures/orderlineitems/properties_invalid2.json @@ -0,0 +1,7 @@ +{ + "properties": [ + { + "name": 2 + } + ] +} diff --git a/fixtures/orderlineitems/properties_object.json b/fixtures/orderlineitems/properties_object.json new file mode 100644 index 00000000..f2237c33 --- /dev/null +++ b/fixtures/orderlineitems/properties_object.json @@ -0,0 +1,6 @@ +{ + "properties": { + "name": "property 1", + "value": 3 + } +} diff --git a/fixtures/orderlineitems/valid.json b/fixtures/orderlineitems/valid.json new file mode 100644 index 00000000..490d3ea6 --- /dev/null +++ b/fixtures/orderlineitems/valid.json @@ -0,0 +1,88 @@ +{ + "fulfillable_quantity": 1, + "fulfillment_service": "manual", + "fulfillment_status": "partial", + "gift_card": true, + "grams": 100, + "id": 254721536, + "name": "Soda", + "pre_tax_price": "9.00", + "price": "12.34", + "product_exists": true, + "product_id": 111475476, + "properties": [ + { + "name": "note 1", + "value": "one" + }, + { + "name": "note 2", + "value": 2 + } + ], + "quantity": 1, + "requires_shipping": true, + "sku": "sku-123", + "tax_lines": [ + { + "price": 13.50, + "rate": 0.06, + "title": "State tax" + }, + { + "price": 12.50, + "rate": 0.05, + "title": "Federal tax" + } + ], + "taxable": true, + "title": "Soda Title", + "total_discount": "1.23", + "variant_id": 1234, + "variant_inventory_management": "shopify", + "variant_title": "Test Variant", + "vendor": "Test Vendor", + "origin_location": { + "id": 123, + "address1": "100 some street", + "address2": "", + "city": "Winnipeg", + "company": "Acme Corporation", + "country": "Canada", + "country_code": "CA", + "first_name": "Bob", + "last_name": "Smith", + "latitude": 49.811550, + "longitude": -97.189480, + "name": "test address", + "phone": "8675309", + "province": "Manitoba", + "province_code": "MB", + "zip": "R3Y 0L6" + }, + "destination_location": { + "id": 124, + "address1": "200 some street", + "address2": "", + "city": "Winnipeg", + "company": "Acme Corporation", + "country": "Canada", + "country_code": "CA", + "first_name": "Bob", + "last_name": "Smith", + "latitude": 49.811550, + "longitude": -97.189480, + "name": "test address", + "phone": "8675309", + "province": "Manitoba", + "province_code": "MB", + "zip": "R3Y 0L6" + }, + "applied_discount": { + "applied_discount": "test discount", + "description": "my test discount", + "value": "0.05", + "value_type": "percent", + "amount": "25.00" + } +} diff --git a/order.go b/order.go index 61084cea..0cf75822 100644 --- a/order.go +++ b/order.go @@ -1,6 +1,7 @@ package goshopify import ( + "encoding/json" "fmt" "net/http" "time" @@ -184,6 +185,43 @@ type LineItem struct { AppliedDiscount *AppliedDiscount `json:"applied_discount,omitempty"` } +// UnmarshalJSON custom unmarsaller for LineItem required to mitigate some older orders having LineItem.Properies +// which are empty JSON objects rather than the expected array. +func (li *LineItem) UnmarshalJSON(data []byte) error { + type alias LineItem + aux := &struct { + Properties json.RawMessage `json:"properties"` + *alias + }{alias: (*alias)(li)} + + err := json.Unmarshal(data, &aux) + if err != nil { + return err + } + // if the first character is a '[' we unmarshal into an array + if len(aux.Properties) > 0 && aux.Properties[0] == '[' { + var p []NoteAttribute + err = json.Unmarshal(aux.Properties, &p) + if err != nil { + return err + } + li.Properties = p + } else { // else we unmarshal it into a struct + var p NoteAttribute + err = json.Unmarshal(aux.Properties, &p) + if err != nil { + return err + } + if p.Name == "" && p.Value == nil { // if the struct is empty we set properties to nil + li.Properties = nil + } else { + li.Properties = []NoteAttribute{p} // else we set them to an array with the property nested + } + } + + return nil +} + type LineItemProperty struct { Message string `json:"message"` } @@ -310,6 +348,7 @@ func (s *OrderServiceOp) ListWithPagination(options interface{}) ([]Order, *Pagi return resource.Orders, pagination, nil } + // Count orders func (s *OrderServiceOp) Count(options interface{}) (int, error) { path := fmt.Sprintf("%s/count.json", ordersBasePath) diff --git a/order_test.go b/order_test.go index 379116f0..25d5b2ac 100644 --- a/order_test.go +++ b/order_test.go @@ -6,6 +6,7 @@ import ( "net/http" "reflect" "runtime" + "strings" "testing" "time" @@ -696,3 +697,384 @@ func TestOrderCancelFulfillment(t *testing.T) { FulfillmentTests(t, *returnedFulfillment) } + +// TestLineItemUnmarshalJSON tests unmarsalling a LineItem from json +func TestLineItemUnmarshalJSON(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/valid.json")) + if err != nil { + t.Errorf("LineItem.UnmarshalJSON returned error: %v", err) + } + + expected := validLineItem() + + testLineItem(t, expected, actual) +} + +// TestLineItemUnmarshalJSONInvalid0 tests unmarsalling a LineItem from invalid json +func TestLineItemUnmarshalJSONInvalid0(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/properties_invalid0.json")) + if err == nil || !strings.Contains(err.Error(), "unexpected end of JSON input") { + t.Errorf("LineItem.UnmarshalJSON expected unexpected end of JSON input error got %v", err) + } +} + +// TestLineItemUnmarshalJSONInvalid1 tests unmarsalling a LineItem with properties that are a struct with invalid +// values +func TestLineItemUnmarshalJSONInvalid1(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/properties_invalid1.json")) + if err == nil || !strings.Contains(err.Error(), "cannot unmarshal number") { + t.Errorf("LineItem.UnmarshalJSON expected cannot unmarshal number into string error got %v", err) + } +} + +// TestLineItemUnmarshalJSONInvalid2 tests unmarsalling a LineItem with properties that are an array with invalid +// values +func TestLineItemUnmarshalJSONInvalid2(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/properties_invalid2.json")) + if err == nil || !strings.Contains(err.Error(), "cannot unmarshal number") { + t.Errorf("LineItem.UnmarshalJSON expected cannot unmarshal number into string error got %v", err) + } +} + +// TestLineItemUnmarshalJSONPropertiesEmptyObject tests unmarsalling a LineItem from json which has properties as an empty json object +func TestLineItemUnmarshalJSONPropertiesEmptyObject(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/properties_empty_object.json")) + if err != nil { + t.Errorf("LineItem.UnmarshalJSON returned error: %v", err) + } + + expected := propertiesEmptyStructLientItem() + + testLineItem(t, expected, actual) +} + +// TestLineItemUnmarshalJSONPropertiesObject tests unmarsalling a LineItem from json which has properties as an json object +func TestLineItemUnmarshalJSONPropertiesObject(t *testing.T) { + setup() + defer teardown() + + actual := LineItem{} + + err := actual.UnmarshalJSON(loadFixture("orderlineitems/properties_object.json")) + if err != nil { + t.Errorf("LineItem.UnmarshalJSON returned error: %v", err) + } + + expected := propertiesStructLientItem() + + testLineItem(t, expected, actual) +} + +func testLineItem(t *testing.T, expected, actual LineItem) { + if actual.ID != expected.ID { + t.Errorf("LineItem.ID should be (%v), was (%v)", expected.ID, actual.ID) + } + + if actual.ProductID != expected.ProductID { + t.Errorf("LineItem.ProductID should be (%v), was (%v)", expected.ProductID, actual.ProductID) + } + + if actual.VariantID != expected.VariantID { + t.Errorf("LineItem.VariantID should be (%v), was (%v)", expected.VariantID, actual.VariantID) + } + + if actual.Quantity != expected.Quantity { + t.Errorf("LineItem.Quantity should be (%v), was (%v)", expected.Quantity, actual.Quantity) + } + + if actual.Price == nil { + if actual.Price != expected.Price { + t.Errorf("LineItem.Price should be (%s), was (%s)", expected.Price, actual.Price) + } + } else { + if !actual.Price.Equals(*expected.Price) { + t.Errorf("LineItem.Price should be (%s), was (%s)", expected.Price, actual.Price) + } + } + + if actual.TotalDiscount == nil { + if actual.TotalDiscount != expected.TotalDiscount { + t.Errorf("LineItem.TotalDiscount should be (%s), was (%s)", expected.TotalDiscount, actual.TotalDiscount) + } + } else { + if !actual.TotalDiscount.Equals(*expected.TotalDiscount) { + t.Errorf("LineItem.TotalDiscount should be (%s), was (%s)", expected.TotalDiscount, actual.TotalDiscount) + } + } + + if actual.Title != expected.Title { + t.Errorf("LineItem.Title should be (%v), was (%v)", expected.Title, actual.Title) + } + + if actual.VariantTitle != expected.VariantTitle { + t.Errorf("LineItem.VariantTitle should be (%v), was (%v)", expected.VariantTitle, actual.VariantTitle) + } + + if actual.Name != expected.Name { + t.Errorf("LineItem.Name should be (%v), was (%v)", expected.Name, actual.Name) + } + + if actual.SKU != expected.SKU { + t.Errorf("LineItem.SKU should be (%v), was (%v)", expected.SKU, actual.SKU) + } + + if actual.Vendor != expected.Vendor { + t.Errorf("LineItem.Vendor should be (%v), was (%v)", expected.Vendor, actual.Vendor) + } + + if actual.GiftCard != expected.GiftCard { + t.Errorf("LineItem.GiftCard should be (%v), was (%v)", expected.GiftCard, actual.GiftCard) + } + + if actual.Taxable != expected.Taxable { + t.Errorf("LineItem.Taxable should be (%v), was (%v)", expected.Taxable, actual.Taxable) + } + + if actual.FulfillmentService != expected.FulfillmentService { + t.Errorf("LineItem.FulfillmentService should be (%v), was (%v)", expected.FulfillmentService, actual.FulfillmentService) + } + + if actual.RequiresShipping != expected.RequiresShipping { + t.Errorf("LineItem.RequiresShipping should be (%v), was (%v)", expected.RequiresShipping, actual.RequiresShipping) + } + + if actual.VariantInventoryManagement != expected.VariantInventoryManagement { + t.Errorf("LineItem.VariantInventoryManagement should be (%v), was (%v)", expected.VariantInventoryManagement, actual.VariantInventoryManagement) + } + + if actual.PreTaxPrice == nil { + if actual.PreTaxPrice != expected.PreTaxPrice { + t.Errorf("LineItem.PreTaxPrice should be (%v), was (%v)", expected.PreTaxPrice, actual.PreTaxPrice) + } + } else { + if !actual.PreTaxPrice.Equals(*expected.PreTaxPrice) { + t.Errorf("LineItem.PreTaxPrice should be (%v), was (%v)", expected.PreTaxPrice, actual.PreTaxPrice) + } + } + + testProperties(t, expected.Properties, actual.Properties) + + if actual.ProductExists != expected.ProductExists { + t.Errorf("LineItem.ProductExists should be (%v), was (%v)", expected.ProductExists, actual.ProductExists) + } + + if actual.FulfillableQuantity != expected.FulfillableQuantity { + t.Errorf("LineItem.FulfillableQuantity should be (%v), was (%v)", expected.FulfillableQuantity, actual.FulfillableQuantity) + } + + if actual.Grams != expected.Grams { + t.Errorf("LineItem.Grams should be (%v), was (%v)", expected.Grams, actual.Grams) + } + + if actual.FulfillmentStatus != expected.FulfillmentStatus { + t.Errorf("LineItem.FulfillmentStatus should be (%v), was (%v)", expected.FulfillmentStatus, actual.FulfillmentStatus) + } + + testTaxLines(t, expected.TaxLines, actual.TaxLines) + + if actual.OriginLocation == nil { + if actual.OriginLocation != expected.OriginLocation { + t.Errorf("LineItem.OriginLocation should be (%v), was (%v)", expected.OriginLocation, actual.OriginLocation) + } + } else { + if *actual.OriginLocation != *expected.OriginLocation { + t.Errorf("LineItem.OriginLocation should be (%v), was (%v)", expected.OriginLocation, actual.OriginLocation) + } + } + + if actual.DestinationLocation == nil { + if actual.DestinationLocation != expected.DestinationLocation { + t.Errorf("LineItem.DestinationLocation should be (%v), was (%v)", expected.DestinationLocation, actual.DestinationLocation) + } + } else { + if *actual.DestinationLocation != *expected.DestinationLocation { + t.Errorf("LineItem.DestinationLocation should be (%v), was (%v)", expected.DestinationLocation, actual.DestinationLocation) + } + } + + if actual.AppliedDiscount == nil { + if actual.AppliedDiscount != expected.AppliedDiscount { + t.Errorf("LineItem.AppliedDiscount should be (%v), was (%v)", expected.AppliedDiscount, actual.AppliedDiscount) + } + } else { + if *actual.AppliedDiscount != *expected.AppliedDiscount { + t.Errorf("LineItem.AppliedDiscount should be (%v), was (%v)", expected.AppliedDiscount, actual.AppliedDiscount) + } + } +} + +func testProperties(t *testing.T, expected, actual []NoteAttribute) { + if len(expected) != len(actual) { + t.Errorf("LineItem.Properties expected len (%d) actual (%d)", len(expected), len(actual)) + } else { + for i := 0; i < len(actual); i++ { + a := actual[i] + if a.Name != expected[i].Name { + t.Errorf("LineItem.Properties[%d].Name should be (%s), was (%s)", i, expected[i].Name, a.Name) + } + if a.Value != expected[i].Value { + t.Errorf("LineItem.Properties[%d].Value should be (%s), was (%s)", i, expected[i].Value, a.Value) + } + } + } +} + +func testTaxLines(t *testing.T, expected, actual []TaxLine) { + if len(expected) != len(actual) { + t.Errorf("LineItem.TaxLines expected len (%d) actual (%d)", len(expected), len(actual)) + } else { + for i := 0; i < len(actual); i++ { + a := actual[i] + e := expected[i] + if !a.Price.Equals(*e.Price) { + t.Errorf("LineItem.TaxLine[%d].Price should be (%s), was (%s)", i, e.Price, a.Price) + } + if !a.Rate.Equals(*e.Rate) { + t.Errorf("LineItem.TaxLine[%d].Rate should be (%s), was (%s)", i, e.Rate, a.Rate) + } + if a.Title != e.Title { + t.Errorf("LineItem.TaxLine[%d].Title should be (%s), was (%s)", i, e.Title, a.Title) + } + } + } +} + +func propertiesEmptyStructLientItem() LineItem { + return LineItem{ + Properties: []NoteAttribute{}, + } +} + +func propertiesStructLientItem() LineItem { + return LineItem{ + Properties: []NoteAttribute{ + NoteAttribute{ + Name: "property 1", + Value: float64(3), + }, + }, + } +} + +func validLineItem() LineItem { + price := decimal.New(1234, -2) + totalDiscount := decimal.New(123, -2) + preTaxPrice := decimal.New(900, -2) + tl1Price := decimal.New(1350, -2) + tl1Rate := decimal.New(6, -2) + tl2Price := decimal.New(1250, -2) + tl2Rate := decimal.New(5, -2) + return LineItem{ + ID: int64(254721536), + ProductID: int64(111475476), + VariantID: int64(1234), + Quantity: 1, + Price: &price, + TotalDiscount: &totalDiscount, + Title: "Soda Title", + VariantTitle: "Test Variant", + Name: "Soda", + SKU: "sku-123", + Vendor: "Test Vendor", + GiftCard: true, + Taxable: true, + FulfillmentService: "manual", + RequiresShipping: true, + VariantInventoryManagement: "shopify", + PreTaxPrice: &preTaxPrice, + Properties: []NoteAttribute{ + NoteAttribute{ + Name: "note 1", + Value: "one", + }, + NoteAttribute{ + Name: "note 2", + Value: float64(2), + }, + }, + ProductExists: true, + FulfillableQuantity: 1, + Grams: 100, + FulfillmentStatus: "partial", + TaxLines: []TaxLine{ + TaxLine{ + Title: "State tax", + Price: &tl1Price, + Rate: &tl1Rate, + }, + TaxLine{ + Title: "Federal tax", + Price: &tl2Price, + Rate: &tl2Rate, + }, + }, + OriginLocation: &Address{ + ID: 123, + Address1: "100 some street", + Address2: "", + City: "Winnipeg", + Company: "Acme Corporation", + Country: "Canada", + CountryCode: "CA", + FirstName: "Bob", + LastName: "Smith", + Latitude: 49.811550, + Longitude: -97.189480, + Name: "test address", + Phone: "8675309", + Province: "Manitoba", + ProvinceCode: "MB", + Zip: "R3Y 0L6", + }, + DestinationLocation: &Address{ + ID: 124, + Address1: "200 some street", + Address2: "", + City: "Winnipeg", + Company: "Acme Corporation", + Country: "Canada", + CountryCode: "CA", + FirstName: "Bob", + LastName: "Smith", + Latitude: 49.811550, + Longitude: -97.189480, + Name: "test address", + Phone: "8675309", + Province: "Manitoba", + ProvinceCode: "MB", + Zip: "R3Y 0L6", + }, + AppliedDiscount: &AppliedDiscount{ + Title: "test discount", + Description: "my test discount", + Value: "0.05", + ValueType: "percent", + Amount: "25.00", + }, + } +}