trailblazer / roar

Parse and render REST API documents using representers.
http://www.trailblazer.to/gems/roar
MIT License
1.85k stars 138 forks source link

Release 1.1.0 #202

Closed shaneog closed 7 years ago

shaneog commented 8 years ago

I see in the CHANGES file there is a version 1.1.0, but it's not on Rubygems - this is holding back Representable for me; which in turn is holding back Reform, etc.

Any chance of a release compatible with Representable 3? 😄

apotonick commented 8 years ago

I actually prepared it months ago and forgot to release! :laughing:

shaneog commented 7 years ago

Ping 😜

acaron commented 7 years ago

Bump too, holding back Representable 3 and I'm not comfortable with the use of the master branch in production! THANKS :D

carlthuringer commented 7 years ago

Normally I would just leave a reaction here, but it's been almost 30 days since the last bump with no reaction and over 3 months since the last promise to release roar with proper JSONAPI support.

Is there anything I can do to help make this release happen?

myabc commented 7 years ago

RE: JSON API support - If you can summarize the most important issues affecting you, that would be helpful.

--
Alex Coles

On 23 December 2016 at 15:35:58, Carl Thuringer (notifications@github.com(mailto:notifications@github.com)) wrote:

Normally I would just leave a reaction here, but it's been almost 30 days since the last bump with no reaction and over 3 months since the last promise to release roar with proper JSONAPI support.

Is there anything I can do to help make this release happen?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub(https://github.com/trailblazer/roar/issues/202#issuecomment-269006569), or mute the thread(https://github.com/notifications/unsubscribe-auth/AAAC82wXW88S2Stc0vwiq9SD0rsKCegIks5rK-pegaJpZM4KYhWf).

carlthuringer commented 7 years ago

My issues are covered by: roar-jsonapi#7 but since you ask, I will reiterate here.

In the current release of roar gem, the gem doesn't support the JSONAPI specification.

require "roar/json/json_api"

module Api
  module V2
    module UserRepresenter
      include Roar::JSON::JSONAPI
      type :users

      property :email
      property :name
      property :admin
      property :created_at
    end
  end
end
# controller action...
# get a user, do stuff... then render.
render json: Api::V2::UserRepresenter.prepare(user).to_json
{
  "users": {
    "admin": true,
    "created_at": "2016-12-23T09:53:54.000-06:00",
    "email": "admin@test.com"
  }
}

This is not correct JSONAPI 1.0.

A document MUST contain at least one of the following top-level members:

data: the document’s “primary data” errors: an array of error objects meta: a meta object that contains non-standard meta-information.

But the data is simply inserted at the root level under the "users" key.

Primary data MUST be either:

  • a single resource object, a single resource identifier object, or null, for requests that target single resources
  • an array of resource objects, an array of resource identifier objects, or an empty array ([]), for requests that target resource collections.

Is the json returned by Api::V2::UserRepresenter.prepare(user).to_json a resource object?

“Resource objects” appear in a JSON API document to represent resources.

A resource object MUST contain at least the following top-level members:

  • id
  • type

It is not a resource object. What JSONAPI object is it? What document? The best I can reason is that it could match the definition of the "attributes object".

However, there is a surprise when we consider the missing name field in the response.

A client MAY request that an endpoint return only specific fields in the response on a per-type basis by including a fields[TYPE] parameter.

The specification is not explicit about this, but the implicit reading of this statement is that fields are not omitted unless the client has requested a sparse fieldset. I am therefore surprised that the name field is missing from the response.


Moving along, I'm aware that there's some changes on the master branch, and that roar-jsonapi has been extracted. Let's try that.

NoMethodError: undefined method `type' for #<Class:0x007fdfe1366780>
/Users/carlthuringer/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/gems/activerecord-4.2.6/lib/active_record/dynamic_matchers.rb:26:in `method_missing'
/Users/carlthuringer/.rbenv/versions/2.3.3/lib/ruby/gems/2.3.0/bundler/gems/roar-jsonapi-40e60a96e3d5/lib/roar/json/json_api.rb:206:in `to_hash'

Oh. I guess that's broken. that Class is actually User. So self.class.type calls type on my User, which doesn't implement that method. Hmm. Well, what can I learn if I read the tests.

Looks like you're only testing the decorator format, which isn't mentioned in the readme. Does it work?

class UserRepresenter < Roar::Decorator #...
#...
end
# ...
render json: Api::V2::UserRepresenter.new(user).to_json
{
  "data": {
    "attributes": {
      "admin": true,
      "created_at": "2016-12-23T09:53:54.000-06:00",
      "email": "admin@test.com"
    },
    "id": "",
    "type": "users"
  }
}

Well, some issues are resolved! For example, the data is there, along with its members, attributes, id, and type. but the ID is missing, and the blank name is still missing from attributes, which I still consider a surprise (bug?). The README doesn't explicitly say that property :id is required for basic functionality, but I'll put it in anyways.

{
  "data": {
    "attributes": {
      "admin": true,
      "created_at": "2016-12-23T09:53:54.000-06:00",
      "email": "admin@test.com"
    },
    "id": "1",
    "type": "users"
  }
}

Quite good! Still missing that name property, but I can go to production like this. Though it would be nice not to have to rely on a master@sha release.

So, @myabc, What can I do to help get this functionality released in roar and roar-jsonapi

myabc commented 7 years ago

Looks like you're only testing the decorator format, which isn't mentioned in the readme. Does it work?

@carlthuringer correct. roar-jsonapi currently does not test Module representers, only Decorators.

(I'm also able to reproduce the NoMethodError: undefined method \type'` you mention)

The biggest problem here is documentation, which is currently badly out-of-sync with, what I consider to be, best practice. However, it is our intention to remove support for Module representers in the next major version.

I recommend Decorators for all new roar projects. In the case, roar-jsonapi only Decorators will be supported. I'm sorry for the confusion.

If you want to help move things along, then patches updating README documentation examples are welcome.

myabc commented 7 years ago

… the blank name is still missing from attributes, which I still consider a surprise (bug?)

@carlthuringer to fix this, you should be able to use the render_nil option, either on this particular property or on all properties for the representer (defaults render_nil: true).

carlthuringer commented 7 years ago

Thanks, @myabc I'll look over the documentation this weekend, and my own lengthy comment, and make some PRs that I hope you'll find helpful.

myabc commented 7 years ago

Release 1.1 is out.

shaneog commented 7 years ago

Thanks @myabc!