fastruby / harvesting

Ruby wrapper for the Harvest API v2
MIT License
28 stars 30 forks source link

Fetch by id methods #5

Closed lthurston closed 5 years ago

lthurston commented 6 years ago

There's currently no way to fetch Harvest records by id, unless I'm missing something. I'm happy to implement this quickly for the existing models (as well as an invoice model, which I can put together as well), but wanted to be sure I was doing it in the right way. In Harvesting::Client there's a fetch all method for each model which is the same as the model name pluralized (eg clients(), contacts(), projects()). For fetching by id, do you want to a new method that singular and accepts the id as the argument?

A naive implementation, and likely my initial stab, would look like this example:

    def client(id)
      result = get("clients/#{id}")
      Harvesting::Models::Client.new(result, client: self)
    end

How does this sound?

etagwerker commented 6 years ago

@lthurston I think you are right: There is no way to fetch one record by id.

I think it might be useful to have an interface like this one:

Harvesting::Models::Client.get(id) 

You can take a stab at it starting with Client. Later on I think we can refactor it (expand it) so that we can quickly add it to all the models in the library.

What do you think?

lthurston commented 6 years ago

Yes, sounds good. I'll give it a shot. It might be a couple weeks before I'm able to submit a PR.

benphelps commented 6 years ago

A gave this a shot, check it out, #14

etagwerker commented 5 years ago

@benphelps Awesome, thanks!