tapajos/highrise — pull request #70 by basvodde (2014-05-26 14:01:12+00:00)
Title: Changed tags to return ActiveResource objects rather than arrays

This pull request is a bit harder than the previous ones (actually delayed this one for a while to think about the specs). The essence is simple. The tags method should return ActiveResource objects (in this case Highrise::Tag) rather than Hash. If it doesn't, then it causes different behavior between dealing with an existing Person or with a new Person. (It was actually quite hard to figure this one out when I got a failure thrown at me when running in production).

Earlier version used get(:tags) which returns an unprocessed Hash. It was hard (stackoverflow said impossible) to change that in a Highrise::Tag object. Now it uses the normal find interface to find 'itself' when the tags attribute do not exist yet and then it will copy the tags from that object into itself. This ensures the objects are of the right type.

Initially, I had only fixed this for Person and removed it out of the Taggable mixin. I was able to generate the implementation to also work for Company (using self.class instead of Highrise::Person) but the specs are more difficult to generalize, so I left them in Person for now.

The pull request is important as without it I can't run my code in production. However, would be open for discussion on solving this in a different way. It works but I'm not incredibly happy with it. (in fact, I'm not happy that Highrise does as if people and companies are the same thing! but that is a different story)