rikas / zoho_hub

Zoho CRM API V2 Wrapper
MIT License
25 stars 30 forks source link

Remove capitalization of API names #29

Closed pkayokay closed 2 years ago

pkayokay commented 5 years ago

Zoho module API names are always capitalized: https://github.com/rikas/zoho_hub/blob/master/lib/zoho_hub/with_attributes.rb#L48

Example: Account Name -> Account_Name will work with

Account.find_by(account_name: 'name')
Account.find_by(account_Name: 'name')
Account.find_by(Account_name: 'name')

But what if we have a custom field (or many which is my case), that are named as Account Valid -> Account_valid (instead of Account_Valid on our custom API name)

None of the formats above will work unless written exactly as is. I would rather not assume that custom field API names are always formatted with caps.

Errors because it can't find a valid custom field due to capitalization.

Account.find_by(Account_valid: true)

{:criteria=>"Account_Valid:equals:Thomas P's Sports Bar and Patio"}
body {:code=>"INVALID_QUERY", :details=>{:reason=>"the field is not available for search", :api_name=>"Account_Valid"}, :message=>"invalid query formed", :status=>"error"

We should leave it to the user making the API call to match the API name 🤔

I can do a PR to remove it or if anyone thinks there's a better solution let me know.

artsyca commented 5 years ago

The idiomatic ruby way is snake_case and the defaults at Zoho are Class_Camel_Snake_Case or what have you, which is super annoying, especially when you add abbreviations to the mix -- is it pdf_url or Pdf_Url or PDF_URL?

I have created a huge entry in attribute_translations spelling out my bindings manually and agree we need a better way to control the default output here -- at a minimum I'd want access to the attr_to_zoho_key on a per-class basis that can be overridden or disabled --

rikas commented 5 years ago

So you say you would like a method to override the default key case translation and keep whatever comes from Zoho?

pkayokay commented 5 years ago

Yea another method would be nice, or a parameter we can pass to the method to make this optional. I have hundreds of api names I would need to change otherwise...

rikas commented 5 years ago

@pkayokay can you purpose a way so solve this? Otherwise I'll see what I can do.

artsyca commented 5 years ago

I base a lot of my opinions on the architecture of https://github.com/emberjs/data -- in their paradigm they've got a pipeline that goes something like api -> adapter -> serializer -> transform -> model and along the way they offer hooks for one to override the default behaviour -- a lot of it is focused on getting camelCase attributes into the model layer no matter what the API layer actually looks like --

In this paradigm, they offer a method called pathForType and another called keyForAttribute, among oh-so-many others to cover the myriad of edge and corner cases one can encounter with JSON and REST apis

I'd say in terms of a data pipeline, this albeit Javascript layer (which was originally designed by members of the Rails core team) has every hook one would possibly want when dealing with data between API <-> Model concerns, which is the reason I'm suggesting making the attr_to_zoho_key protected or public, as it carries the same function as keyForAttribute and pathForType mentioned above

rikas commented 2 years ago

@pkayokay for now I don't see an easy solution for this, so you can use the attribute_translation method in your class to solve this.

You have an example here: https://github.com/rikas/zoho_hub/blob/master/examples/models/potential.rb