podio / podio-rb

The official Ruby wrapper for the Podio API used and maintained by the Podio team
https://podio.com
MIT License
66 stars 53 forks source link

"Active Podio" & custom attributes #47

Closed elsurudo closed 7 years ago

elsurudo commented 7 years ago

From the README: "... all find methods return model instances with attributes cast to the expected type (string, integer, boolean, datetime, etc.)"

I have a Podio::Item instance, and am trying to get at custom attributes (for example location), but I get NoMethodError: undefined method 'location' for #<Podio::Item:0x007f03db573010>. Using a default attribute such as title seems to work.

What can the problem be?

PavloBasiuk commented 7 years ago

The only default attribute available is title, mainly because this is pseudo-field, and any first field that Podio could treat as title will be treated as title. If you want to access fields, you need to explore fields attribute. And then dig a bit to find actual value (e.g. getting text label from category field with label 'Final layout uploaded' and external_id 'final-layout-uploaded' might be bit not intuitive: item.fields.find {|h| h['external_id']=='category'}['values'][0]['value']['text'] )

elsurudo commented 7 years ago

I don't get it... so what does the text I quoted above from the README even mean? It's not like ActiveRecord at all! Here it is again: "... all find methods return model instances with attributes cast to the expected type (string, integer, boolean, datetime, etc.)"

Yes, I did find how to get the value... eventually. Why it's so complicated, I have no idea.

Sorry to sound so dismissive. I actually quite enjoyed using the library up until now, when I had to "get" an object (quite a common operation, right?), and the complexity of something so common as accessing an attribute... wow.

PavloBasiuk commented 7 years ago

Item's fields are not item's attributes. And that's main idea. You may think that this is overcomplicated, but there is no other and simple way of handling great flexibility that Podio is offering. Let's review it on location field example. Location as field has configuration (e.g. it could be simple line, or multi-line) and value(s). Values of location field are also not just a piece of text, it could be text + map + latitude+longitude, it can also be 'use my current location' option if you set it from browser.

Regarding your quote "It's not like ActiveRecord at all!", Podio::Application.get() is giving back Podio::Application object, and Podio::Item.find() is giving back Podio::Item object, which has for example comments property which is an array of Podio::Comment objects. There is no method Item.get_field() and as I wrote before, app_fields are not item's attributes. You can find full list of Item's attributes here: https://github.com/podio/podio-rb/blob/master/lib/podio/models/item.rb

If you think this could be improved and there is some other easy and intuitive way of handling such cases, please feel free to offer PR with your fixes and changes.

elsurudo commented 7 years ago

I do understand that there is a lot of underlying complexity, as fields have a whole configuration etc.

But, wouldn't you agree that getting at an item's attributes is core functionality, and should be abstracted away as best as possible? It wouldn't work for every case of course, but the whole idea of Podio is that everything is an item with attributes. When I fetch an item, I want to get its attributes, 99% of the time.

Even offering a simplified view of the attributes would go a long way. Maybe not for more complex fields, but if I have a text field called "note" on my Podio::Item, why can't I call #note on it, and get the actual text? Looking at the field types, this can work quite simply with text, category, date, phone, email, number, link, money, progress, and probably duration fields. As far as I am concerned, all attribute values should map to some Ruby class, even if it's a custom one that is included with podio-rb (for more complex cases).

As for making a PR, I would love to if I had the time. Instead, I will probably have to resort to an incomplete hack that works for just my needs, at least for the time being.

But honestly I am paying for Podio, and this is functionality that is so obvious that I can't believe it's not already included with the ruby wrapper.

elsurudo commented 7 years ago

Or instead of custom classes, just return hashes with symbolized keys, as is standard practice in a lot of ruby libs. Even that would go a long way.