Implement some of the zoho invoice API handlers (v3).

[VincentK-Titandc]

Implementation based on Zoho invoice API v3: https://www.zoho.com/invoice/api/v3

Available endpoints:

• Invoice: create/list/get/update
• Recurring Invoice: create/get/list/update/stop
• Contact: create/get/list/update
• Contact Person: create/list/delete
• Item: list
• Payment: create/get/list

[schmorrison]

Amazing!! Thanks for the PR. I will review when I get a moment, but since there is so much it may be a couple days. I do already have a couple questions but maybe they will make sense when I review the code against the API docs.

Again, thanks for your help.

Best regards, Sam Morrison

[VincentK-Titandc]

Hi,

I just made the changes, re-using sub packages constants leaded to cycle dependency issues so I just moved the relevant ones into main package.

The headers X-com-zoho-*-organizationid are mandatory to target the right organisation when using both expense and invoice APIs. The header for 'expense' was already here, I just added the switch case to handle the invoice API.

However, I didn't try the expense API and I made a breaking change about how we format requests: they are now wrapped into a "JSONString" parameter (see commit 27cdded96af3ff855a7c0fa2ec1b1f40199bc095). I don't know if this is due to the v3 APIs but it's currently the only way to properly send data to zoho APIs. It would be nice to double check if expense v1 endpoints are still working and/or upgrade it to v3.

BTW I'm also thinking about using go mod to manage dependencies in order to improve testing and debugging: this will allow to import embedded packages without relying on github fullpath.

[schmorrison]

I have been going through that last commit, all the endpoints you have added!!! I have a few notes, and a few questions.

• I would like all the request objects to default to 'omitempty' in the JSON field tags. This was so when issuing an update the empty fields wouldn't overwrite the existing data. If a user has a specific case where omitempty is not appropriate then they would need to modify the struct in their own package, this is the same of custom fields and such. Response fields were set to omitempty because AFAICT Zoho really doesn't guarantee fields in responses. I suppose an empty field could indicate an error, but I don't know that the JSON enc/dec file is the appropriate place to indicate that. What do you think?
• I noticed in a few places you have added a call to refresh the token, I forget why I didn't include a method for refreshing the token automatically. There must've been a reason, anyway I believe that those calls should be moved to the main package within http.go before the HTTP request is performed.
  • I would like to see the URL parameters added to the package. I know there are alot of them, so don't worry about it on this PR. Hopefully they can be added as required. Even though making the change down the road will cause a breaking change to the function signatures, because you are the only one using this subpackage I will let it go.
  • I was wondering if it would be better for debugging if the code and message were written to stdout when an error occurs? If the message is all you need then I won't push it.
  • In places you are providing a struct which will be encoded as JSON, you should probably avoid using interface{} and instead use the request struct you had provided. Were you facing a type error, or were these signatures pulled from 'zoho/crm/records.go'? If so, you should instead model after the function signatures in 'zoho/crm/notes.go'.

I think if we address my last comment, and these few things I will be happy to merge.

Thanks and best regards, Sam Morrison

P.S. Almost all of my knowledge of Zoho API comes from interacting with the CRM v1 XML based API for years. If you check the history of this project, I started it just prior to the change to the JSON API and the field parsing was an absolute mess of reflection calls. Now I believe there are only 3 calls to reflect in this entire package. Point being, if you have some experience with the API's on other Zoho services you might have to school me a bit; I tended to parse and write the responses as safely as I could.

[schmorrison]

Hey Vince, I am not sure if you noticed but I issued a PR on your repo that should take care of most of the changes I would like to see here.

If you can go over it, test it, and tidy it up as you like, that'd be great.

[VincentK-Titandc]

Hello Sam, I missed that PR indeed, thanks for the refactoring efforts! I'll go through all of it and run the tests this week, I keep you in touch. Thanks again.

[schmorrison]

@VincentK-Titandc I would love to clean up this PR so I can build some more endpoints, but don't want to start until the new behaviour is finalized. If you merge the PR I created on your repo, I will tidy up this PR and merge it.

[VincentK-Titandc]

@schmorrison Sorry for the delay, I finally managed to free up some time to get back on this project. Your PR is now merged, the main change I made in your code is the auto-renewal of access token (see 6d20a61e53db9aae87d9f4e3ce6bccfb23d099ab). On top of that I fixed a few things in the invoice API and I switched to the go vendoring mode (go.mod, go.sum at the root of the project) to ease upgrades/testing, required go version is set to 1.13.

[schmorrison]

Awesome, that is great @VincentK-Titandc !

I will check it all out, make sure it works, and merge it later this week.

Thanks again & best regards!

[VincentK-Titandc]

Hello @schmorrison,

I found an issue in the token renewal process, I guess this is due to some changes in Zoho API. Check out 8754c4f5d76060821a1b4c8d22ccb7035cd1937f.

BR,

[schmorrison]

Hello @schmorrison,

I found an issue in the token renewal process, I guess this is due to some changes in Zoho API. Check out 8754c4f.

BR,

Interesting. Good catch! I notice that my existing tokens seem to return milliseconds, so the renewal may be happening very quickly with 'legacy' tokens. They added an option in the API console to address this potential issue, check it out:

I have done some testing here, everything seems to work good on my end! I was having an issue with pulling the branch from your fork using Go mod. But eventually I figured out how to use the commit hash to pull the PR for testing. I just did a few simple create, read, and deletes in CRM. Unfortunately I don't have any data in ZohoInvoice to properly test, so Im counting on you having tested what you needed to.

Im going to be merging very shortly. Thanks a ton for your effort, and patience!

[VincentK-Titandc]

Hello @schmorrison,

Finally! Thanks.

Interestingly, I don't have such option on the Zoho API console as you can see :

Anyway I will definitely monitor token renewals frequency on my app to infer the unit of this value.

As you may have seen, I recently changed the way data are sent to the API by using multipart requests instead of regular JSON payloads (see 421b8acd13b0928e9f681211a85ac0b822d76182). In fact, passing some '%' characters to string fields was rejected by the API and I found the multipart solution by looking at the requests generated from their dashboard.

Regarding non tested code, be assured that all the endpoints I have implemented are fully tested on my end and work just fine.

BR,

[schmorrison]

Sorry @VincentK-Titandc for the delay :( and thanks again for the PR!

I did notice the change related to multipart requests, and I believe that you organized those blocks perfectly for the situation. I've been checking the docs these past couple days, Im considering doing a sprint on adding a bunch of endpoints soon, and noticed there are a few instances where the multipart request is required across the API in CRM. Suffice it to say, when I get around to those endpoints it will need a few extra options added to the endpoint struct, or somekind of rework.

Best regards!