Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating from 0.2.2 to 0.2.3 breaks entities? #304

Closed
brandonweiss opened this issue Jan 3, 2013 · 10 comments
Closed

Updating from 0.2.2 to 0.2.3 breaks entities? #304

brandonweiss opened this issue Jan 3, 2013 · 10 comments

Comments

@brandonweiss
Copy link
Contributor

I just updated from 0.2.2 to 0.2.3, removed error_format because I see in the changelog that that was removed, ran my tests, and about 10% of them failed. Most of the failures look something like this:

--- expected
+++ actual
@@ -1 +1 @@
-{"id"=>"50e4e8c5b392d462f0000008", "name"=>"Foobar", "practitioners"=>[], "offices"=>[], "open_appointments_for_the_next_week"=>[]}
+"#<PracticeEntity:0xXXXXXX>"

So I'm guessing the entities aren't being serialized for some reason? I took a look through the diff between v0.2.2 and v0.2.3 and didn't see anything glaring, but I'm not really familiar with the codebase. What am I missing?

@dblock
Copy link
Member

dblock commented Jan 3, 2013

Do you have a format entry in your API or any content_type entries?

@brandonweiss
Copy link
Contributor Author

Yeah, I have format :json set.

@dblock
Copy link
Member

dblock commented Jan 3, 2013

It might need some debugging. I'd appreciate a repro.

What should be called is to_json via this code.

@brandonweiss
Copy link
Contributor Author

Sure. I'm not sure what I could tell you that might reproduce it. I'm not really doing anything out of the ordinary.

The endpoint looks like this:

class API < Grape::API

  format :json

  mount PracticesController

end

class PracticesController < Grape::API

  get "/practices" do
    practices = PracticeRepository.all.sort { |a,b| a.name <=> b.name }
    present practices, with: PracticeEntity
  end

end

The PracticeEntity looks like this:

class PracticeEntity < Grape::Entity

  root "practices", "practice"

  expose :id, :name
  expose :practitioners, using: PractitionerEntity
  expose :offices,       using: OfficeEntity

  expose :open_appointments_for_the_next_week, using: AppointmentEntity

end

How can I help?

@dblock
Copy link
Member

dblock commented Jan 4, 2013

The most helpful would be a spec that reproduces this problem inside Grape.

@brandonweiss
Copy link
Contributor Author

I might be able to manage that, although it would be helpful for me to have a better understanding of what kind of state the library and test coverage is in for entities. Is everything largely working and really well-covered? Because I consistently have issues getting basic functionality from the documentation to work. For example, I wanted to remove the repetitive with: PracticeEntity from each present call. So I removed them and put represent Practice, with: PracticeEntity at the top of the controller, and I get a bunch of test failures. And this happens pretty often for me. It actually took me some time to figure out a way of using entities that actually did work. So is coverage really good and I've just found some weird combination of libraries or usage that is breaking it, or is everyone having similar problems?

@dblock
Copy link
Member

dblock commented Jan 4, 2013

I think coverage is very good, but Entities are a very recent feature, so probably not as good. Either way, with every person working on it we get closer to where it needs to be.

@brandonweiss
Copy link
Contributor Author

For sure. OK, I don't use RSpec, but I'll see if I can hack out some failing tests.

@dblock dblock closed this as completed in e0d48ad Jan 5, 2013
@dblock
Copy link
Member

dblock commented Jan 5, 2013

Fixed. The issue with with format :json which invokes to_json on the entity which doesn't have that defined. In 0.2.3 the formatters were detangled and split, so that the odd serializable hash could be taken out of the main usage scenario.

You should try HEAD. I also think that you should be able to work around this in 0.2.3 by using format :serializable_hash instead of format :json.

@brandonweiss
Copy link
Contributor Author

Interesting. Thanks for sorting that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants