taxjar / taxjar-go

Sales Tax API Client for Go
https://developers.taxjar.com/api/reference/?go
MIT License
15 stars 5 forks source link

Optional Parameters Are Not Optional #7

Closed calvinanderson closed 4 years ago

calvinanderson commented 4 years ago

Calling CreateOrder() with CreateOrderParams{}:

// CreateOrderParams should be passed to `CreateOrder` to create an order․
type CreateOrderParams struct {
    ...
    Amount          float64         `json:"amount"`
    Shipping        float64         `json:"shipping"`
    SalesTax        float64         `json:"sales_tax"`
    ...
}

Amount is an optional field in the API, however this will always be the zero value of the struct member when posting to the endpoint (so, 0)

This could possibly be solved by making this a pointer to float64 and checking for nil, as I am now required to work out this before calling CreateOrder() or I will get an error.

ScottRudiger commented 4 years ago

@calvinanderson Thanks for the issue!

Amount is actually a required field per the API reference:

Screen Shot 2020-01-17 at 3 21 25 PM

If it helps work out the logic, there are (at least) two different cases with the client as-is:

  1. Amount & LineItems add up correctly

    201 Order created

    taxjar.CreateOrderParams{
    // ...
    Amount:          36.21,
    Shipping: 5,
    SalesTax: 0,
    LineItems: []taxjar.OrderLineItem{
    {
    ID:                "1",
    Quantity:          1,
    ProductIdentifier: "12-34243-9",
    Description:       "Fuzzy Sweater",
    ProductTaxCode:    "20010",
    UnitPrice:         36.72,
    Discount:          5.51,
    SalesTax:          0,
    },
    },
    }
  2. Amount is empty or zero and LineItems includes items

    400 Bad Request - Order amount must be equal to the sum of line items and shipping

    taxjar.CreateOrderParams{
    // ...
    // Amount:          36.21,
    Shipping: 5,
    SalesTax: 0,
    LineItems: []taxjar.OrderLineItem{
    {
    ID:                "1",
    Quantity:          1,
    ProductIdentifier: "12-34243-9",
    Description:       "Fuzzy Sweater",
    ProductTaxCode:    "20010",
    UnitPrice:         36.72,
    Discount:          5.51,
    SalesTax:          0,
    },
    },
    }

If we pass nil for an empty Amount instead of 0, a different error would be returned since Amount is required:

406 Not Acceptable - amount is missing, amount is empty

type CreateOrderParams struct {
// ...
Amount          float64         `json:"amount,omitempty"`
Shipping        float64         `json:"shipping"`
SalesTax        float64         `json:"sales_tax"`
...
}

Since Amount is required, our inclination is to pass 0 when omitted.

calvinanderson commented 4 years ago

Thanks, sorry, I had thought that is was an optional param like in the calculate sales tax endpoint.

ScottRudiger commented 4 years ago

No worries! We know this is confusing since Amount excludes shipping when calculating tax, but includes shipping when creating an order as well. We hope to offer more consistency in a future version of the API.