Add support for customs information on a parcel #4
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ package sendcloud | |
| import ( | ||
| "encoding/json" | ||
| "time" | ||
|
|
||
| "github.com/shopspring/decimal" | ||
| ) | ||
|
|
||
| type LabelData []byte | ||
|
|
@@ -33,45 +35,52 @@ type ParcelParams struct { | |
| PhoneNumber string | ||
| ExternalID string | ||
| ToServicePointID 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 | ||
|
|
||
| CustomsInvoiceNr string | ||
| CustomsShipmentType int64 | ||
|
Contributor
Author
There was a problem hiding this comment. Maybe this package should define the possible values as a typed const (enum-esque construction)?
Contributor
There was a problem hiding this comment. I agree, good idea! |
||
| ParcelItems []ParcelParamsItem | ||
| TotalOrderValue decimal.Decimal | ||
| TotalOrderValueCurrency string | ||
| // Shipping method name selected by buyer during the checkout | ||
| ShippingMethodCheckoutName *string | ||
| // When set to true configured shipping rules will be applied before creating the label and announcing the Parcel | ||
| ApplyShippingRules *bool | ||
| } | ||
|
|
||
| type ParcelParamsItem struct { | ||
| Description string | ||
| Quantity uint64 | ||
| Weight decimal.Decimal | ||
|
Contributor
There was a problem hiding this comment. These could be float64
Contributor
Author
There was a problem hiding this comment. 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 CreateParcelItemRequest struct { | ||
| // Harmonized System Code Wikipedia Link. Providing a complete HS code with 8 characters increases the delivery rate. | ||
| HsCode string `json:"hs_code"` | ||
| // Weight of a single item in kilograms. | ||
| Weight decimal.Decimal `json:"weight"` | ||
| // Quantity of items shipped. | ||
| Quantity int `json:"quantity"` | ||
| // Description of the item. | ||
| Description string `json:"description"` | ||
| // ISO-2 code of the country where the items were originally produced. External Link. | ||
| OriginCountry string `json:"origin_country,omitempty"` | ||
| // Value of a single item. | ||
| Value decimal.Decimal `json:"value"` | ||
| // The SKU of the product. | ||
| SKU string `json:"sku,omitempty"` | ||
| // External ID of the item generated by a shop system or similar. | ||
| ItemId string `json:"item_id,omitempty"` | ||
| // The list of properties of the product. Used as a JSON object with {'key': 'value'}. | ||
| Properties map[string]interface{} `json:"properties,omitempty"` | ||
| } | ||
|
|
||
|
|
@@ -131,15 +140,25 @@ type ParcelRequest struct { | |
| ExternalID *string `json:"external_reference,omitempty"` | ||
| SenderID *int64 `json:"sender_address,omitempty"` | ||
| Shipment *CreateParcelShipmentRequest `json:"shipment,omitempty"` | ||
| 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"` | ||
| ShippingMethodCheckoutName *string `json:"shipping_method_checkout_name,omitempty"` | ||
| ApplyShippingRules *bool `json:"apply_shipping_rules,omitempty"` | ||
| } | ||
|
|
||
| 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 { | ||
| Label LabelResponse `json:"label"` | ||
| } | ||
|
|
@@ -230,14 +249,16 @@ func (p *ParcelParams) GetPayload() interface{} { | |
| Telephone: p.PhoneNumber, | ||
| Email: p.EmailAddress, | ||
| RequestLabel: p.IsLabelRequested, | ||
| CustomsInvoiceNr: p.CustomsInvoiceNr, | ||
| CustomsShipmentType: p.CustomsShipmentType, | ||
| TotalOrderValue: p.TotalOrderValue.String(), | ||
| TotalOrderValueCurrency: p.TotalOrderValueCurrency, | ||
| ShippingMethodCheckoutName: p.ShippingMethodCheckoutName, | ||
| ApplyShippingRules: p.ApplyShippingRules, | ||
| } | ||
| if !p.Weight.Equals(decimal.Zero) { | ||
| parcel.Weight = p.Weight.String() | ||
| } | ||
| if p.Method != 0 { | ||
| parcel.Shipment = &CreateParcelShipmentRequest{ID: p.Method} | ||
| } | ||
|
|
@@ -254,8 +275,18 @@ func (p *ParcelParams) GetPayload() interface{} { | |
| if p.ToServicePointID != 0 { | ||
| 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} | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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:
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.