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

Links generation should be optional when using json-api adapter with kaminari or will_paginate. #1549

Open
youroff opened this issue Mar 2, 2016 · 17 comments

Comments

@youroff
Copy link
Contributor

youroff commented Mar 2, 2016

The links just automatically showed up with one of recent updates. Correct me if I'm wrong, but I haven't found how to turn them off right away. Shouldn't it be optional?

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

@youroff Hi! Thanks for writing! You're right that it's not documented. I just took a look at the code ant it looks like you can:

class NonPaginatedCollectionSerializer < ActiveModel::Serializer::CollectionSerializer
  def paginated?
    false
  end
end
ActiveModelSerializers.config.collection_serializer = NonPaginatedCollectionSerializer

ref:

Also, we've recently added issue templates and I haven't seen anyone using them. Are they perhaps not showing up?

@youroff
Copy link
Contributor Author

youroff commented Mar 3, 2016

That will work, but I think that global config would be better (probably, something like autopaginate: true), unless it's mandatory part of the response declared by json-api.

Yeah, I've noticed that, sorry. I thought as long as it wasn't an issue precisely (no trace, etc), I could write as is. Next will use it.

Just ran into another problem, again not sure if that's a bug or feature. So when decalred an association belongs_to and it doesn't listed in include option, serializer triggers load of relationship. Which is unnecessary as long as we have foreign keys on serializable model. Is that worth documenting as actual issue?

@victor95pc
Copy link

I have this same problem, I am making Rails + Ember integration, and I don't use this links.

I support a global option

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

@youroff

unless it's mandatory part of the response declared by json-api.

It's not mandatory, but having a pagination plugin makes it a good assumption you want to paginate... I'd accept a PR to disable it, with on being the default.

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

just ran into another problem, again not sure if that's a bug or feature. So when decalred an association belongs_to and it doesn't listed in include option, serializer triggers load of relationship. Which is unnecessary as long as we have foreign keys on serializable model. Is that worth documenting as actual issue?

Please create a new issue so we can discuss there.

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

Can we consider this resolved?

@victor95pc
Copy link

Thank you, for me this is not resolved until we make a PR, I want to do it, but I have some stuff(work) to do right now, maybe I can do this later.

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

@victor95pc Awesome. That gives me some room to work on getting more prs and issues resolved (when I'm not working :).

@youroff
Copy link
Contributor Author

youroff commented Mar 3, 2016

@bf4 not sure if using of pagination implies the need of links. I just use meta to pass total_pages and increment page when needed in the client app. Besides that ember-data generates urls on its own, not even sure if there's a way to pass link explicitly there.

@bf4
Copy link
Member

bf4 commented Mar 3, 2016

@youroff

not sure if using of pagination implies the need of links. I just use meta to pass total_pages

So, your use case is that you're using the JSON API adapter and a paginated resource, but your client prefers pagination links to be in meta, though the spec for pagination uses links, http://jsonapi.org/examples/#pagination ? I understand your need, but it's not per spec, so anything more than letting you say "I want to handle pagination myself", which is already possible, isn't really a good feature for AMS. Does that make sense?

@youroff
Copy link
Contributor Author

youroff commented Mar 3, 2016

@bf4 I would say that ember-data json-api adapter doesn't use links at all. Or at least I didn't hear about it. Yeah, option would work for me, but I see that many people would use the same pair of rails and ember and they simply wouldn't need links out of the box too.

@groyoh
Copy link
Member

groyoh commented Mar 3, 2016

I agree that the best option here is to provide a configuration. I ran into the same issue and using a special serializer for that felt counter intuitive to me. @victor95pc did you already start working on a PR here? I would like to start with one if it's not the case.

@victor95pc
Copy link

@groyoh You probably will make it more fast than me, I am kind of busy, Ember and Rails are killing me LoL.

@bf4
Copy link
Member

bf4 commented Mar 4, 2016

@groyoh see #1549 (comment)

@richmolj
Copy link
Contributor

richmolj commented Mar 4, 2016

I would say that ember-data json-api adapter doesn't use links at all

It will honor links for async relationships FWIW, and I believe it is on the roadmap to use links for pagination - unclear if it will be links or meta, depends on the implementation.

So, your use case is that you're using the JSON API adapter and a paginated resource, but your client prefers pagination links to be in meta, though the spec for pagination uses links, http://jsonapi.org/examples/#pagination ? I understand your need, but it's not per spec

AFAIK the current ember way is to use meta. At the very least, imagine a standard bootstrap pagination component that needs to display the total number of pages...this requires more than just links. Putting pagination data in meta isn't against spec, it's in addition to.

but I see that many people would use the same pair of rails and ember and they simply wouldn't need links out of the box too.

Yup - I think the JSONAPI links work for something like a command-line client, or maybe infinite scrolling, but not your standard Ember/Angular/etc app.

@pixelhandler
Copy link

@richmolj Yeah pagination can live in multiple places, top level meta or links like so:

  "meta": {
    "page": {
      "total": 69,
      "sort": [
        {
          "field": "date",
          "direction": "desc"
        }
      ],
      "offset": 0,
      "limit": 5
    }
  },
  "links": {
    "first": "http://api.pixelhandler.com/api/v1/posts?page%5Blimit%5D=5&page%5Boffset%5D=0&sort=-date",
    "next": "http://api.pixelhandler.com/api/v1/posts?page%5Blimit%5D=5&page%5Boffset%5D=5&sort=-date",
    "last": "http://api.pixelhandler.com/api/v1/posts?page%5Blimit%5D=5&page%5Boffset%5D=64&sort=-date"
  }

@bf4
Copy link
Member

bf4 commented Mar 4, 2016

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

6 participants