Skip to content
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

Add support for customs information on a parcel #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for customs information on a parcel #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,7 @@ module github.com/afosto/sendcloud-go

go 1.15

require golang.org/x/text v0.3.5
require (
github.com/shopspring/decimal v1.2.0
golang.org/x/text v0.3.5
)
3 changes: 3 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
github.com/shopspring/decimal v1.2.0 h1:abSATXmQEYyShuxI4/vyW3tV1MrKAJzCZ/0zLUXYbsQ=
github.com/shopspring/decimal v1.2.0/go.mod h1:DKyhrW/HYNuLGql+MJL6WCR6knT2jwCFRcu2hWCYk4o=
golang.org/x/text v0.3.5 h1:i6eZZ+zk0SOf0xgBpEpPD18qWcJda6q1sxt3S0kzyUQ=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e h1:FDhOuMEY4JVRztM/gsbk+IKUQ8kj74bxZrgw87eMMVc=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
59 changes: 58 additions & 1 deletion parcel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"encoding/json"
"strconv"
"time"

"github.com/shopspring/decimal"
)

type LabelData []byte
Expand All @@ -24,9 +26,28 @@ type ParcelParams struct {
PhoneNumber string
ExternalID string
ToServicePointID int64
Weight int64
OrderNumber string
SenderID int64

// Weight contains the weight in Kilograms. Supports up to 3 decimals
// (grams). For example: decimal.New(20, -3) specifies 20 grams.
Weight decimal.Decimal
Copy link
Contributor Author

@GeertJohan GeertJohan Jun 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This incurs a breaking change in the API of this package. The Weight field was typed as int64. I don't like breaking changes so initially tried to make it backwards compatible but found that it was impossible to do that because previous behavior was unspecified.

The Weight field here was not sent to SendCloud. It was just ignored. There is no documentation on the field that specifies whether it is expected to have the weight in Kilograms, grams, pounds, whatever. So if we were to silently start sending this value to SendCloud it might cause issues with existing users of the package that use a different unit from whatever we choose to convert/send (e.g. they set it in grammes, we send it in Kilos).
Therefore I propose to explicitly break existing users of this package (if they set the Weight field), forcing them to validate their code and explicitly pass a decimal.Decimal value in Kilograms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch about the in64! We'd like to keep the dependencies of this package to a minimum, so I don't think using an external package to do something a float64 could do is what we're looking for.

You're right that there's no documentation in this package about the type of weight we're expecting. However, the Sendcloud documentation specifies that they need the weight in kilograms (https://docs.sendcloud.sc/api/v2/shipping/#create-a-parcel). It's a great idea to document it in this package like you did here.

Keeping in mind that Sendcloud does specify they want the weight in kilograms, it would be a mistake on the consumer side to send grams in the first place.

Let me know if you have a case against using float64 types rather than decimal.Decimal in this case.

Copy link
Contributor Author

@GeertJohan GeertJohan Aug 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I agree that external dependencies is something to avoid. In this case I needed it because the Sendcloud API places a constraint on the number of decimal places that can be sent. Otherwise it returns this error:

2021/08/03 17:01:02 failed to create a new parcel at sendcloud: request api/v2/parcels resulted in error code 400: weight: "Ensure that there are no more than 3 decimal places."

Also, a float is not an accurate decimal type. It shouldn't be used for things like weight and money/value.

So I would still suggest to include the github.com/shopspring/decimal package as a dependency, it is a popular and well-maintained package.


CustomsInvoiceNr string
CustomsShipmentType int64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this package should define the possible values as a typed const (enum-esque construction)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, good idea!

ParcelItems []ParcelParamsItem
TotalOrderValue decimal.Decimal
TotalOrderValueCurrency string
}

type ParcelParamsItem struct {
Description string
Quantity uint64
Weight decimal.Decimal
Copy link
Contributor

Choose a reason for hiding this comment

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

These could be float64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @roelofjan-elsinga , sorry for the late reply!

I tried float64 but some API calls failed because four decimal places are not supported (8.1234 kg). This is very easy to occur with floats64's, as they don't enforce the number of decimal-places. The shopsprig/decimal package is very popular, imported by over 30k public Go packages (and probably even more private ones) it's the de-factor decimal package for Golang. I don't think users of the sendcloud package would mind having it as indirect dependency.

Value decimal.Decimal
HsCode string
OriginCountry string
SKU string
}

type Parcel struct {
Expand Down Expand Up @@ -80,6 +101,22 @@ type ParcelRequest struct {
Shipment struct {
ID int64 `json:"id"`
} `json:"shipment"`
Weight string `json:"weight"`
CustomsInvoiceNr string `json:"customs_invoice_nr"`
CustomsShipmentType int64 `json:"customs_shipment_type"`
ParcelItems []ParcelRequestItem `json:"parcel_items"`
TotalOrderValue string `json:"total_order_value"`
TotalOrderValueCurrency string `json:"total_order_value_currency"`
}

type ParcelRequestItem struct {
Description string `json:"description"`
Quantity uint64 `json:"quantity"`
Weight string `json:"weight"`
Value json.RawMessage `json:"value"` // decimal (as json-number, not as json-string), but we can't go through float64.
HsCode string `json:"hs_code"`
OriginCountry string `json:"origin_country"`
SKU string `json:"sku"`
}

type LabelResponseContainer struct {
Expand Down Expand Up @@ -176,6 +213,13 @@ func (p *ParcelParams) GetPayload() interface{} {
}{
ID: p.Method,
},
CustomsInvoiceNr: p.CustomsInvoiceNr,
CustomsShipmentType: p.CustomsShipmentType,
TotalOrderValue: p.TotalOrderValue.String(),
TotalOrderValueCurrency: p.TotalOrderValueCurrency,
}
if !p.Weight.Equals(decimal.Zero) {
parcel.Weight = p.Weight.String()
}
if p.SenderID != 0 {
parcel.SenderID = &p.SenderID
Expand All @@ -190,6 +234,19 @@ func (p *ParcelParams) GetPayload() interface{} {
parcel.ToServicePointID = &p.ToServicePointID
}

parcel.ParcelItems = make([]ParcelRequestItem, len(p.ParcelItems))
for i, item := range p.ParcelItems {
parcel.ParcelItems[i] = ParcelRequestItem{
Description: item.Description,
Quantity: item.Quantity,
Weight: item.Weight.String(),
Value: json.RawMessage(item.Value.String()),
HsCode: item.HsCode,
OriginCountry: item.OriginCountry,
SKU: item.SKU,
}
}

ar := ParcelRequestContainer{Parcel: parcel}
return ar
}
Expand Down