schmorrison / Zoho

Golang API for Zoho services
MIT License
35 stars 34 forks source link

Add subscriptions #25

Closed bondar-pavel closed 3 years ago

bondar-pavel commented 3 years ago

Initial attempt to implement subscriptions API. For now only ListSubscriptions and GetSubscription methods are implemented (and seems to be working). Plan to continue work on other methods for subscriptions and invoices.

Please review if overall structure fits into the project.

schmorrison commented 3 years ago

Looks good @bondar-pavel . I like how you have put the relevant scopes in the same file as the related functions. I also notice the naming collision with 'crm/crm.go' containing the initialization stuff, where here 'subscriptions/subscriptions.go' contains an actual endpoint. I may make a note in the future to follow your example for starting a new package.

I haven't tried the functions, but if they seem to work I have no reason to think otherwise. It all seems to follow the design style of the existing code very well. I will note that if you decide to implement any 'CREATE or UPDATE' functions, you will want to pay attention to the json:"" tag and include "omitempty" where you DO NOT want to OVERWRITE existing values with empty values.

bondar-pavel commented 3 years ago

@schmorrison thanks for you feed back! I glad that it fits into project structure.

Looks like it make sense to add "omitempty" to most of the fields, will update the code accordingly. Also plan to add Get and List methods for Invoices soon (after I test they are working correctly).

bondar-pavel commented 3 years ago

Added Create/Update/Delete methods. Started to test Update but fails so far, investigating with it.

schmorrison commented 3 years ago

Added Create/Update/Delete methods. Started to test Update but fails so far, investigating with it.

Great additions @bondar-pavel !! Sometimes with create/update it is necessary to include a specific field in the data for it to succeed. If you discover that is the case you may add a check for that field in your functions and return an error before sending to Zoho.

Zoho has a bad habit of uninformative error codes sometimes too, so be aware of that.

I can review stuff tomorrow when I have some time.

bondar-pavel commented 3 years ago

Thanks for detailed review. Initially I tried to make structs more universal, but looks like it just does not work right with zoho. So yeah, will switch to separate structs for request/response as well as for different methods.

bondar-pavel commented 3 years ago

Added separate structs for Subscription Create/Update as well as missed fields. But have not tested yet create/update

bondar-pavel commented 3 years ago

Tested Subscription Update, worked fine for me, the only required field was Plan.PlanCode.

Subscription create also worked for me, but not exactly match documentation.

When I pass customer_id I get this error: "Invalid value passed for Email"

When I pass customer_id and customer.Email it successfully creates new subscription.

When I pass just customer.Email, I get this error: "Enter a valid Customer Name"

When I pass customer.Email and customer.DisplayName, subscription is created fine.

So basically difference between observed and expected behaviour is that customer.Email is required even if customer_id is passed.

bondar-pavel commented 3 years ago

@schmorrison Hello! I would like to clarify how is it best to proceed with this merge request. In background I am working on adding more methods to subscriptions and invoices, but don't know if it make sense to put everything into single merge request or it is better address all issues with this request, merge it and proceed with reviewing next chunk of functionality.

schmorrison commented 3 years ago

@bondar-pavel okey dokey. Sounds like you have it working, I'll review and merge in the next couple days.

As far as additional PRs, it'd be hard for me to commit to review code more than a couple times a month. So it really depends on how quickly you want to see your work end up in master. Realistically I need to be comfortable merging 1 endpoint at a time if it is free work the community is providing, just know it may take as many as 2 weeks to review/merge a PR and a very complex PR will require more focus & time.

bondar-pavel commented 3 years ago

@schmorrison actually no hurry in getting it merged into the master.

So I will continue to add new endpoints in my fork up to a point to satisfy our project needs. Then will test it with the project and submit second merge request with all additional endpoints. I expect it to be ready by the end of next week.

bondar-pavel commented 3 years ago

Hi! I have stuck a bit while trying to add this endpoint: https://www.zoho.com/subscriptions/api/v1/#Invoices_Add_attachment_to_an_invoice

Looks like zoho documentation is incorrect or at least incomplete for this endpoint. Here is a few strange points I noted:

  1. Example curl command does not actually passes a file for upload:
    $ curl https://subscriptions.zoho.com/api/v1 /invoices/{invoice_id}/attachment
    -X POST
    -H "Authorization: Zoho-oauthtoken 1000.41d9f2cfbd1b7a8f9e314b7aff7bc2d1.8fcc9810810a216793f385b9dd6e125f"
    -H "X-com-zoho-subscriptions-organizationid: 10234695"
    -H "Content-Type: application/json;charset=UTF-8"
    -d '{
    "can_send_in_mail": true
    }'
  2. In example curl there is a space in the middle of URL https://subscriptions.zoho.com/api/v1 /invoices/{invoice_id}/attachment
  3. It states that file should be uploaded with Content-Type: application/json, but most examples of file upload I meet uses Content-Type: multipart/form-data
  4. While it is actually possible to upload file using json, it has to be base64 encoded, but it will not include metadata like filename. And reply for this request indicates that zoho backend receives filename:
    {
    "code": 0,
    "message": "Your file has been successfully attached to the invoice.",
    "documents": [
        {
            "file_name": "Rental Agreement.pdf",
            "file_type": "pdf",
            "file_size": 5447,
            "file_size_formatted": "5.3 KB",
            "document_id": "903000005689",
            "attachment_order": 1
        }
    ]
    }

Points 3 and 4 makes me think that file has to be sent as a Content-Type: multipart/form-data, but then I have a question if there is a way to make such request using HTTPRequest method? https://github.com/schmorrison/Zoho/blob/master/http.go#L31

bondar-pavel commented 3 years ago

Added invoices related commits to review. Currently most of the methods that we need for our functionality is implemented. One exception is AddAttachment method which is partly implemented as per documentation it is not clear how it should work (so stuck with it for a while).

So most likely I stop here in terms of adding new methods and we can start work on polishing what we have here to make it good enough to be merged.

schmorrison commented 3 years ago

Hey @bondar-pavel , i meant to review things last weekend but the spring weather got the best of me. Ill have some time between now and Friday to take a look. looks good from my cursory glance at things.

On the particulars of the AddAttachment endpoint; I expect multi-part form the way to encode. We already need to manage this partly for requests to Invoice or Expense, some of their endpoints expect the payload to be multi-part form in the format JSONString='{"key": value}'. So making this change will require some minor refactoring to the HTTPRequest method and the Endpoint struct.

I can suggest a definitive way to go once I review things, otherwise Im just guessing based on my experience with Zoho. Sometimes their weird documentation is accurate.

Best regards, schmorrison

bondar-pavel commented 3 years ago

Hi @schmorrison,

I got reply from zoho support and (as we expected) multi-part form is the way to go:

Kindly use the below CURL command so that you could get the attachment linked to the invoice. 

curl https://subscriptions.localzoho.com/api/v1/invoices/{invoiceID}/attachment?can_send_in_mail=true -X POST -H "Authorization: Zoho-oauthtoken 1000.XXXXX" -H "X-com-zoho-subscriptions-organizationid: 102346" -H "Content-Type : multipart/form-data" -F "attachment=@attachment_file_location"

Try this and let us know if you face trouble in working with it further. 

I'll investigate how to update the HTTPRequest method and the Endpoint struct and will keep you posted once I figure it out or stuck.

bondar-pavel commented 3 years ago

@schmorrison thanks for the review! Plan to work on addressing comments early next week.

bondar-pavel commented 3 years ago

I have addressed comments from previous iteration. Also file upload finally worked out for me, but I just hooked it up into JSONString part, which could be less than ideal, so I might try to rework it in a better way on next iteration. Also I am not sure if CanSendInEmail flag was parsed correctly while passed via JSONString. When I tested email send functionality attachments were not included into the email I received, so will investigate it deeper.

Example from zoho support suggests to pass it as url parameter:

curl https://subscriptions.localzoho.com/api/v1/invoices/{invoiceID}/attachment?can_send_in_mail=true -X POST -H "Authorization: Zoho-oauthtoken 1000.XXXXX" -H "X-com-zoho-subscriptions-organizationid: 102346" -H "Content-Type : multipart/form-data" -F "attachment=@attachment_file_location"

But the value is bool, and in endpoint URLParameters is defined as map[string]Parameter, which is string, so not quite sure if it would work out with "true" passed as a string. Will try to test tomorrow.

schmorrison commented 3 years ago

I have addressed comments from previous iteration. Also file upload finally worked out for me, but I just hooked it up into JSONString part, which could be less than ideal, so I might try to rework it in a better way on next iteration. Also I am not sure if CanSendInEmail flag was parsed correctly while passed via JSONString. When I tested email send functionality attachments were not included into the email I received, so will investigate it deeper.

Example from zoho support suggests to pass it as url parameter:

curl https://subscriptions.localzoho.com/api/v1/invoices/{invoiceID}/attachment?can_send_in_mail=true -X POST -H "Authorization: Zoho-oauthtoken 1000.XXXXX" -H "X-com-zoho-subscriptions-organizationid: 102346" -H "Content-Type : multipart/form-data" -F "attachment=@attachment_file_location"

But the value is bool, and in endpoint URLParameters is defined as map[string]Parameter, which is string, so not quite sure if it would work out with "true" passed as a string. Will try to test tomorrow.

Here is how I think the Endpoint struct should be modified:

type Endpoint struct {
    Method         HTTPMethod
    URL            string
    Name           string
    ResponseData   interface{}
    RequestBody    interface{}
    URLParameters  map[string]Parameter
    Headers        map[string]string
    BodyFormat          BodyFormat
    Attachment        string
}

type BodyFormat string
const (
    JSON =   ""
    JSON_STRING  =   "jsonString"
    FILE =           "file"
)

And HTTPRequest:

        var req *http.Request
    if endpoint.RequestBody == nil {
         req, err = http.NewRequest(string(endpoint.Method), fmt.Sprintf("%s?%s", endpointURL, q.Encode()), nil)
        if err != nil {
            return fmt.Errorf("Failed to create a request for %s: %s", endpoint.Name, err)
            }

            req.Header.Set("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8")
    } else {
           // JSON Marshal the body
           marshalledBody, err := json.Marshal(endpoint.RequestBody)
           if err != nil {
               return fmt.Errorf("Failed to create json from request body")
           }
           reqBody := bytes.NewReader(marshalledBody)

           // Create a multipart form just incase
           var b bytes.Buffer
           w := multipart.NewWriter(&b)

            // Check the expect BodyFormat
        if endpoint.BodyFormat == JSON_STRING {
                   // Use the form to create the proper field
            fw, err := w.CreateFormField("JSONString")
            if err != nil {
                return err
            }
                    // Copy the request body JSON into the field
                    if _, err = io.Copy(fw, reqBody); err != nil {
                        return err
                    }
          } else if endpoint.BodyFormat == FILE {
                    // Retreive the file contents (Assuming
            fileReader, err := os.Open(endpoint.Attachment)
            if err != nil {
                return err
            }
            defer fileReader.Close()
                   // Create the correct form field
            part, err := w.CreateFormFile("attachment", filepath.Base(endpoint.Attachment))
            if err != nil {
                return err
            }
                   // copy the file contents to the form
            if _, err = io.Copy(part, fileReader); err != nil {
                return err
            }
        }

        // Close the multipart writer to set the terminating boundary
        err = w.Close()
        if err != nil {
            return err
        }

            // Choose which body/content type we are sending
        var body io.Reader
            var contentType string
           if endpoint.BodyFormat == JSON_STRING || endpoint.BodyFormat == FILE {
                   body = &b
                   contentType = w.FormDataContentType()
        } else {
                  body = reqBody
                  contentType = "application/x-www-form-urlencoded; charset=UTF-8"
        }

        req, err = http.NewRequest(string(endpoint.Method), fmt.Sprintf("%s?%s", endpointURL, q.Encode()), body)
        if err != nil {
            return fmt.Errorf("Failed to create a request for %s: %s", endpoint.Name, err)
            }

            req.Header.Set("Content-Type", contentType)
    }

That should drop in directly without any errors in that file (http.go). Endpoints which are using the JSONString type of encoding will require changes in the related Endpoint struct (ie. Replace JSONString: true with BodyFormat: Zoho.JSON_STRING). Any Endpoint struct that were not using JSONString field, should NOT require any changes because the BodyFormat field has a zero value of "" which is equal to BodyFormat: Zoho.JSON.

A change like this should mean setting a new version on the go.mod file also I should think because while it shouldn't break any of the subpackages, the Endpoint struct is technically exported/public and these changes will be breaking to it.

Let me know if that makes sense, not sure if this is exceeding the scope of your PR or if I should make this change to master first then you can merge those changes into your branch? Whatever you'd prefer honestly.

Best regards, schmorrison

schmorrison commented 3 years ago

Also I am not sure if CanSendInEmail flag was parsed correctly while passed via JSONString. When I tested email send functionality attachments were not included into the email I received, so will investigate it deeper.

Example from zoho support suggests to pass it as url parameter:

curl https://subscriptions.localzoho.com/api/v1/invoices/{invoiceID}/attachment?can_send_in_mail=true -X POST -H "Authorization: Zoho-oauthtoken 1000.XXXXX" -H "X-com-zoho-subscriptions-organizationid: 102346" -H "Content-Type : multipart/form-data" -F "attachment=@attachment_file_location"

But the value is bool, and in endpoint URLParameters is defined as map[string]Parameter, which is string, so not quite sure if it would work out with "true" passed as a string. Will try to test tomorrow.

I never actually responded to the second part of your message previously.

The current implementation would create a POST body that contains the following body: attachment=[image data bytes],JSONString={"can_send_in_mail": true} so you will almost certainly want to pass can_send_in_mail as a URL Query param and exclude a Endpoint.RequestBody. Passing a "true" as a Zoho.Parameter isn't an ideal 'Go way' of doing things, its not Type safe (maybe create a const PARAM_TRUE: Parameter = "true"), but it will work because everything in a URL is a string anyways.

bondar-pavel commented 3 years ago

@schmorrison thanks for Endpoint/HTTPRequest snipped, it actually worked out. The only change I did there is updated check if endpoint.RequestBody == nil { to be if endpoint.RequestBody == nil && endpoint.BodyFormat != FILE {, since on file upload request body is nil.

Updated existent methods that used JSONString option to use BodyForma across the project, but VSCode additionally updated formatting in modified files, so it could be harder to review.

I agree it make sense to increase major version for the package, since JSONString -> BodyFormat is interface braking change. So I'll add appropriate changes to this PR, once other bits are sorted out. Have not yet updated libs to v2, so will explore topic a bit, I assume need to update go.mod to module github.com/schmorrison/Zoho/v2 and update all imports in the project to include v2.

bondar-pavel commented 3 years ago

Also I guess it would be useful to tag existent master with v1.0.0 before going to version v2. So there would be an easy way to switch to v1 for users who don't want to go to v2. Because currently import looks like this: github.com/schmorrison/Zoho v0.0.0-20201206041350-6677fd2d68ed

FYI, started to look into getting repository into v2: https://stackoverflow.com/questions/53344471/taking-a-repository-to-v2 And longer version of reading: https://research.swtch.com/vgo-module

Stackoverflow versioning suggest using v2 branch for v2 code, but as far as I see v2 code can sit in master branch as well, since tags are important for versioning, not branches.

schmorrison commented 3 years ago

Okay, thanks @bondar-pavel .

I think we can version to V1.0.0, as you noted this package is currently in v0. I think using versioning using branches is based on third-party versioning schemes, AFAIK go.mod uses the latest tagged version on master by default unless you specifiy a specific branch/commit, if there are no tagged versions it will use "psuedo-versions" based on the latest commit to master.

While the subpackages are not finished, and definitely not in V1 condition, the subpackages are technically consumers of github.com/schmorrison/Zoho, and github.com/schmorrison/Zoho is pretty polished and works nicely. By versioning the 1.0.0 we can pin the state of the subpackages to a specific version of the root package.

Thanks for all the work, I will take another look over things tonight or tomorrow.

Best regards, schmorrison

schmorrison commented 3 years ago

Here is the final amendment to http.go, again this should drop in correctly:

    var (
        req         *http.Request
        reqBody     io.Reader
        contentType string
    )

    // Has a body, likely a CRUD operation (still possibly JSONString)
    if endpoint.RequestBody != nil {
        // JSON Marshal the body
        marshalledBody, err := json.Marshal(endpoint.RequestBody)
        if err != nil {
            return fmt.Errorf("Failed to create json from request body")
        }

        reqBody = bytes.NewReader(marshalledBody)
        contentType = "application/x-www-form-urlencoded; charset=UTF-8"
    }

    if endpoint.BodyFormat != "" {
        // Create a multipart form
        var b bytes.Buffer
        w := multipart.NewWriter(&b)

        // Check the expect BodyFormat
        if endpoint.BodyFormat == JSON_STRING {
            // Use the form to create the proper field
            fw, err := w.CreateFormField("JSONString")
            if err != nil {
                return err
            }
            // Copy the request body JSON into the field
            if _, err = io.Copy(fw, reqBody); err != nil {
                return err
            }

            // Close the multipart writer to set the terminating boundary
            err = w.Close()
            if err != nil {
                return err
            }

            reqBody = &b
            contentType = w.FormDataContentType()
        }

        if endpoint.BodyFormat == FILE {
            // Retreive the file contents
            fileReader, err := os.Open(endpoint.Attachment)
            if err != nil {
                return err
            }
            defer fileReader.Close()
            // Create the correct form field
            part, err := w.CreateFormFile("attachment", filepath.Base(endpoint.Attachment))
            if err != nil {
                return err
            }
            // copy the file contents to the form
            if _, err = io.Copy(part, fileReader); err != nil {
                return err
            }

            err = w.Close()
            if err != nil {
                return err
            }

            reqBody = &b
            contentType = "application/x-www-form-urlencoded; charset=UTF-8"
        }
    }

    req, err = http.NewRequest(string(endpoint.Method), fmt.Sprintf("%s?%s", endpointURL, q.Encode()), reqBody)
    if err != nil {
        return fmt.Errorf("Failed to create a request for %s: %s", endpoint.Name, err)
    }

    req.Header.Set("Content-Type", contentType)
schmorrison commented 3 years ago

Ok, I cannot find anything problematic. I'll be happy to merge when you make the previous changes I noted.

bondar-pavel commented 3 years ago

@schmorrison Thanks for review and amendments! Plan to make the changes tomorrow.

bondar-pavel commented 3 years ago

Fixed next items:

It looks like covers all change requests I am aware of, but please let me know if I missed something. Thanks!

bondar-pavel commented 3 years ago

Yesterday I tested Invoices_Collect_charge_via_credit_card functionality in test zoho subscription account, and reply has completely different structure, comparing to what is seen in documentation.

I have replaced some fields with 'xxx' to remove company specific details. Here is reply for "https://subscriptions.zoho.%s/api/v1/invoices/%s/collect"

{
    "code": 0,
    "invoice": {
        "ach_payment_initiated": false,
        "adjustment": 0,
        "adjustment_description": "",
        "allow_partial_payments": false,
        "approver_id": "",
        "auto_reminders_configured": false,
        "balance": 0,
        "bcy_adjustment": 0,
        "bcy_discount_total": 0,
        "bcy_shipping_charge": 0,
        "bcy_sub_total": 10,
        "bcy_tax_total": 0,
        "bcy_total": 10,
        "billing_address": {
            "address": "",
            "attention": "",
            "city": "",
            "country": "",
            "fax": "",
            "phone": "",
            "state": "",
            "street": "",
            "street2": "",
            "zip": ""
        },
        "can_edit_items": false,
        "can_send_in_mail": false,
        "can_send_invoice_sms": true,
        "can_skip_payment_info": false,
        "client_viewed_time": "",
        "contactpersons": [
            {
                "contactperson_id": "xxxxx",
                "email": "",
                "mobile": "",
                "phone": "",
                "zcrm_contact_id": ""
            }
        ],
        "coupons": [],
        "created_by_id": "",
        "created_date": "2021-03-29",
        "created_time": "2021-03-29T21:42:20+0100",
        "credits": [],
        "credits_applied": 0,
        "currency_code": "",
        "currency_id": "",
        "currency_symbol": "",
        "custom_field_hash": {},
        "custom_fields": [],
        "customer_custom_field_hash": {},
        "customer_custom_fields": [],
        "customer_id": "",
        "customer_name": "",
        "date": "2021-03-29",
        "discount_percent": 0,
        "discount_total": 0,
        "documents": [],
        "due_date": "2021-03-29",
        "email": "",
        "exchange_rate": 1,
        "inprocess_transaction_present": false,
        "invoice_date": "2021-03-29",
        "invoice_id": "xxxxx",
        "invoice_items": [
            {
                "account_id": "xxxxxxx",
                "account_name": "Sales",
                "code": "",
                "description": "xxxxxxxx",
                "discount_amount": 0,
                "item_custom_fields": [],
                "item_id": "xxxxx",
                "item_total": 10,
                "name": "xxxxxx",
                "price": 10,
                "product_id": "xxxxx",
                "product_type": "none",
                "quantity": 1,
                "tags": [],
                "tax_id": "",
                "tax_name": "",
                "tax_percentage": 0,
                "tax_type": "tax",
                "unit": ""
            }
        ],
        "invoice_number": "INV-xxxx",
        "invoice_url": "xxxxxxxx",
        "is_inclusive_tax": false,
        "is_reverse_charge_applied": false,
        "is_viewed_by_client": false,
        "is_viewed_in_mail": false,
        "last_modified_by_id": "",
        "mail_first_viewed_time": "",
        "mail_last_viewed_time": "",
        "notes": "%InvoiceNumber% test ",
        "number": "INV-xxxx",
        "page_width": "8.27in",
        "payment_expected_date": "",
        "payment_gateways": [
            {
                "payment_gateway": "test_gateway"
            }
        ],
        "payment_made": 10,
        "payment_reminder_enabled": false,
        "payment_terms": 0,
        "payment_terms_label": "Due On Receipt",
        "payments": [
            {
                "amount": 10,
                "amount_refunded": 0,
                "bank_charges": 0,
                "card_type": "visa",
                "date": "2021-03-29",
                "description": "AutoPayment has been added to INV-xxx",
                "exchange_rate": 1,
                "gateway_transaction_id": "xxx",
                "invoice_payment_id": "xxxx",
                "last_four_digits": "1111",
                "payment_id": "xxx",
                "payment_mode": "test_gateway",
                "reference_number": "xxxx",
                "settlement_status": ""
            }
        ],
        "price_precision": 2,
        "pricebook_id": "",
        "reference_id": "",
        "reference_number": "",
        "salesperson_id": "",
        "salesperson_name": "",
        "shipping_address": {
            "address": "",
            "attention": "",
            "city": "",
            "country": "",
            "fax": "",
            "phone": "",
            "state": "",
            "street": "",
            "street2": "",
            "zip": ""
        },
        "shipping_charge": 0,
        "shipping_charge_exclusive_of_tax": 0,
        "shipping_charge_exclusive_of_tax_formatted": "£0.00",
        "shipping_charge_inclusive_of_tax": 0,
        "shipping_charge_inclusive_of_tax_formatted": "£0.00",
        "shipping_charge_tax": "",
        "shipping_charge_tax_formatted": "",
        "shipping_charge_tax_id": "",
        "shipping_charge_tax_name": "",
        "shipping_charge_tax_percentage": "",
        "shipping_charge_tax_type": "",
        "status": "paid",
        "stop_reminder_until_payment_expected_date": false,
        "sub_total": 10,
        "submitter_id": "",
        "subscriptions": [],
        "tax_rounding": "entity_level",
        "tax_total": 0,
        "taxes": [],
        "template_id": "xxx",
        "template_name": "xxxx",
        "template_type": "",
        "terms": "xxxx",
        "total": 10,
        "transaction_type": "new",
        "unbilled_charges_id": "",
        "unused_credits_receivable_amount": 0,
        "updated_time": "2021-03-29",
        "vat_treatment": "uk",
        "write_off_amount": 0,
        "zcrm_potential_id": ""
    },
    "message": "The payment from the customer has been recorded"
}

https://www.zoho.com/subscriptions/api/v1/#Invoices_Collect_charge_via_credit_card

Per documentation reply should have 'payment' as top level object, but in fact 'invoice' is returned as top level object. Question here is what should I do with the structure of CollectChangeViaCreditCardResponse object. Options that come to my mind are next:

bondar-pavel commented 3 years ago

Added fix for content-type in case of file upload, noticed that uploads stopped to work after latest changes, so fixed content-type to be multipart/form-data. Now file uploads works for me.

schmorrison commented 3 years ago

Hey there, not sure why that problem may have started for you, but it seems like Go's multipart form library is pretty nifty!

In regards to the CollectChargeViaCreditCard method, I don't expect there to be an issue with leaving both the top-level payment (as per docs) and invoice (as per curl) structs intact on the CollectChangeViaCreditCardResponse (<-- Note the N in Change) both with an omitempty tag and a note on the method that mentions the expectation in the docs and the real world experience.

Have a good weekend! If you make that commit I'll definitely try to take a look / merge this weekend. :)

bondar-pavel commented 3 years ago

Hi, I have fixed typos in struct names, added 'invoice' subtree to reply (with omitempty tag on to top level fields, but not sure if make sense to add it to each field down the tree as wel). Also I added one method to get customer, since already started to work in it.

FYI, I am on vacation during next 2 weeks, so can respond slowly, but will try to keep an eye on this review.

bondar-pavel commented 3 years ago

Hi @schmorrison, I am back from vacation, so if there are any feedback to address I am ready to work on it.

schmorrison commented 3 years ago

Okey dokey.

PR merged, thanks for the work @bondar-pavel !!