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

[Meta Issue] JSON-API support #2905

Closed
4 of 10 tasks
wecc opened this issue Mar 19, 2015 · 64 comments
Closed
4 of 10 tasks

[Meta Issue] JSON-API support #2905

wecc opened this issue Mar 19, 2015 · 64 comments

Comments

@wecc
Copy link
Contributor

wecc commented Mar 19, 2015

Let's use this issue to keep track of status for the JSON-API adapter.

Improve metadata handling

Features

  • Pagination (collections, hasMany relationships)
  • Filtering
  • Includes

Links

  • Support self links for relationships (reloading, saving, deleting)
  • Support self links for records (to override urlFor-methods)

Ping @kurko

@igorT igorT mentioned this issue Mar 19, 2015
28 tasks
@kurko
Copy link
Contributor

kurko commented Mar 19, 2015

👏 😄

@btecu
Copy link
Contributor

btecu commented Mar 19, 2015

I thought the RESTAdapter was going to follow JSON AP, which currently does for the most part.
So we're going to have a new different adapter?

@Panman82
Copy link

Where is this at? I'm working on my backend API and wondering if I should follow the JSON-API spec or just follow the RESTAdapter for now. Thanks!

@IanVS
Copy link

IanVS commented May 6, 2015

+1, What is the status of JSON-API in Ember Data? According to jsonapi.org:

JSON API is at a third release candidate state. This means that it is not yet stable, however, libraries intended to work with 1.0 should implement this version of the specification. We may make very small tweaks, but everything is basically in place.

@seawatts
Copy link

seawatts commented May 6, 2015

May 21st 2015 is the release for json-api 1.0
https://twitter.com/dgeb/status/588179514817130497

@IanVS
Copy link

IanVS commented May 6, 2015

@seawatts That's great, but it's only about two weeks from now. Will Ember Data be ready to use with a json-api compliant API at that time? What's left to be done? This meta-issue doesn't seem to have been updated in quite some time. Does that mean no progress has been made, or the progress has just been silent?

@bmac
Copy link
Member

bmac commented May 6, 2015

There is work in progress on making Ember Data json-api compliant in this branch: #2904

@kottenator
Copy link

Hi!

I'm new to Ember.js and I've found out in official documentation that ember-data supports JSON API.

I've created some mock data using JSON API v1.0 but RESTAdapter doesn't parse top-level "data" property into an array - it throws an error instead.

I use Ember.js v1.12.0

Here is my JSON API data sample:

{
    "data": [
        {
            "type": "profile",
            "id": "ffa13f788fd34751a218767689342d63",
            "attributes": {
                "first_name": "Irene",
                "last_name": "Dickson",
            },
            "links": {
                "self": "/assets/fixtures/individual-jsonapi.json"
            }
        }
    ],
    "links": {
        "self": "/assets/fixtures/individuals-jsonapi.json",
        "next": "/assets/fixtures/individuals-jsonapi-page-2.json",
        "last": "/assets/fixtures/individuals-jsonapi-page-10.json"
    }
}

Am I doing something wrong or RESTAdapter doesn't support this kind of data structure?

@pgasiorowski
Copy link

RESTAdapter is here to deal with simpler resources like:
http://guides.emberjs.com/v1.12.0/models/the-rest-adapter/

JSON-API adapter has just been merged (Yay!) into ember-data and should be publicly available on or after 26th June as far as I'm concerned. You might also try using the nightly version of ember-data to try it sooner.

@EmergentBehavior
Copy link

Is there any documentation on working with this? I've set it up and tweaked settings so there are no more deprecations, but none of the records loaded from my JSON-API compliant API are being loaded properly.

I was using a 1.0-spec compatible fork of @kurko's lib before and it was working fine (for loading, at least).

@EmergentBehavior
Copy link

Nevermind -- got it :)

@wwwdata
Copy link

wwwdata commented Jun 18, 2015

I tried the jsonapi implementation now. Thanks a lot for the work so far! What I was noticing is that when you have a relationship with a related url but include the related entity inside the included object, the request for the related url will be made, although it is not necessary anymore because the referenced entity was already included in the first request.

So if there is a data field with id(s) for relations and the entities for these ids are included in the included field, we should avoid loading again from the url.

Is this still under work an the Improve link handling topic in your list?

Is there already a separate ticket for this? Can I maybe help with that? It should be straight forward to create a scenario for this now in the tests.

@wecc
Copy link
Contributor Author

wecc commented Jun 18, 2015

@wwwdata if there's both data and links:related in a relationship, data should take precedence. If it's not I'd consider it a bug. Would you mind opening a new issue for that?

@wwwdata
Copy link

wwwdata commented Jun 18, 2015

I opened an issue #3380

@igorT
Copy link
Member

igorT commented Jun 18, 2015

there is an existing issue for that, should be easier to fix now. we just prioritized backwards compatible incompatible changes.

@SeyZ
Copy link

SeyZ commented Jul 13, 2015

Any updates about "sugars"?

@davidlormor
Copy link

Per the JSON API spec: "An endpoint MAY also support an include request parameter to allow the client to customize which related resources should be returned." (i.e. /articles/1?include=comments). I've been putting the JSONAPIAdapter through its paces, and I can't seem to find a way to build this query using the built in findRecord method (i.e. this.store.findRecord('article', 1, {include: 'comments'}).

Note: I'm testing this against a backend built with Rails+jsonapi-resources. I'm able to hit the endpoints without issue via Postman, but can't seem to get an Ember client properly implemented without having to customize this adapter (which seems like it should support the full breadth of the spec...).

@wecc
Copy link
Contributor Author

wecc commented Jul 13, 2015

@SeyZ unfortunately not yet

@davidlormor that's what the "Sugar for includes" part is about

@SeyZ
Copy link

SeyZ commented Jul 14, 2015

Ok thanks @wecc ! I'm watching this issue to be notified when updates are available 👍

@JohnQuaresma
Copy link

Can we expect single record metadata in the next version of ember data? Having no recourse for the metadataFor deprecation message on my single record routes is frustrating.

Please let us know what to expect and when! :-) Thanks for the hard work!

@wecc
Copy link
Contributor Author

wecc commented Jul 17, 2015

@JohnQuaresma I understand your frustration :( per-record meta will probably be available in 2.1

@JohnQuaresma
Copy link

Ok. If it looks like it's going to slip beyond that, would be great to remove the deprecation message for now until we're at most one minor version away.

Sorry to be a pest and thanks again. Looking forward to upcoming improvements!

@kbjorklid
Copy link

Hi all,

I'm wondering about the 'sugar for includes'. It seems to me that it'd be very important to have an easy way of avoiding 1+n REST calls (for a list of things with dependencies) when just 1 would work.

@benkingcode
Copy link

This issue is now 2 years old and Ember Data still does not support using self links for relationship saving. I can only assume this has been abandoned? It seems like a pretty important part of the JSON:API spec.

@matthewvalimaki
Copy link

@dbbk complete support for JSONAPI support does seem like abandoned. But take a look at https://github.com/pangratz/ember-data-meta-links-improvements as it implements some of the missing features as an addon.

@ef4
Copy link
Contributor

ef4 commented Mar 23, 2017

For people interested in per-resource meta, I just published an addon that makes it possible: https://github.com/ef4/ember-resource-metadata

@escobera
Copy link

If complete support for JSONAPI is abandoned why is the project still defaulting to it?

@ef4
Copy link
Contributor

ef4 commented Aug 16, 2017

It's not abandoned. Somebody who wants these features just needs to step up and implement them, or sponsor somebody else to implement them. This is open source.

I suspect the reason it hasn't happened yet is actually because the vast majority of apps are working fine already. Many many ember apps are using jsonapi every day happily, it makes no sense to say we shouldn't use it just because we haven't hit 100% of the spec yet.

@escobera
Copy link

escobera commented Aug 16, 2017

I wasn't implying it shouldn't be used, but since it is incomplete and doesn't seem to have a developer involved in completing it probably shouldn't be kept as the default adapter.

All of the articles and issues about dealing with embedded records (which made me arrive here in the first place) should be enough evidence.

@ef4
Copy link
Contributor

ef4 commented Aug 16, 2017 via email

@lvjames
Copy link

lvjames commented Aug 16, 2017

As long as those other apps are working fine it's alright then. Nevermind the people still checking back two years after; clearly they're doing something wrong as this is functionality they shouldn't need. We are talking about the most official ember addon in existence and one of the biggest drives behind json-api spec. I can't for the life of me understand how you can keep a straight face when you say it's fine doing spotty coverage of the spec just cause this is the stuff core team members only ever needed in their personal projects, or that's how far they were paid to implement. And I know I'm only shooting the messenger here, so {{/rant}}

By the way, you've already implemented some much appreciated missing functionality in a separate addon. Why not a PR?

@aprescott
Copy link

I can't for the life of me understand how you can keep a straight face when you say it's fine doing spotty coverage of the spec just cause this is the stuff core team members only ever needed in their personal projects, or that's how far they were paid to implement.

I know you used {{/rant}}, but I struggle to see how this is in good faith. I don't think the maintainers see it as "fine" that there's spotty coverage. The issue exists to track the gaps, which says to me that they don't think it's fine.

Claiming the default adapter is in place "just because" the core team only ever needed a subset of the spec for personal projects is uncharitable, and pretty unreasonable. As someone totally unrelated to the project, the JSON API adapter and the spec are immediately useful even if I don't use all the corner cases of the spec. In comparison to a home-rolled half-baked media type of my own making, it's better to have something well-defined, even if the implementation doesn't cover 100% of the spec.

I understand the frustration with the implementation gaps. I share some of them, which is why I'm subscribed to this issue. But attacking the core team isn't going to get features shipped faster.

@lvjames
Copy link

lvjames commented Aug 17, 2017

I know you used {{/rant}}, but I struggle to see how this is in good faith. I don't think the maintainers see it as "fine" that there's spotty coverage. The issue exists to track the gaps, which says to me that they don't think it's fine.

I struggle to see how sitting on this issue for 2.5 years translates to "active development", and that's not turning a blind eye to the good work thrown into this addon over the years.

In comparison to a home-rolled half-baked media type of my own making, it's better to have something well-defined, even if the implementation doesn't cover 100% of the spec.

It doesn't even cover 80% of the spec, but that's not the point. Ember + Ember Data is supposed to be batteries-included, with all the tough decisions made for you so you can focus on building your ambitious web app. If you've followed this issue, how do you feel about the ugly hacks people had to resort to due to the holes in the spec, and how does that fit in with the previous sentence?

But attacking the core team isn't going to get features shipped faster.

Looking the other way doesn't exactly help ship them faster either.

If I wanted to "attack" the core team, you'd see a much lengthier response with simple points like: "please name what ember-data features have been implemented over the past year". Also, I think you failed to see my response in the context of the ongoing discussion. Yes, I'm frustrated and it reflects, but I'm not blindly accusing and would certainly never take it out on a 3rd party addon developer who works on her addon in her free time out of the kindness of her heart. Sure there's core devs doing the very same thing, but this isn't a 3rd party addon. I hate to repeat myself, but once again:

We are talking about the most official ember addon in existence and one of the biggest drives behind json-api spec.

This should be The reference implementation, not a sparse adaptation of the standard.

@ef4
Copy link
Contributor

ef4 commented Aug 17, 2017

Please refrain from adding comments that add no new information. That just spams all subscribers. Everybody already agrees we want these features. If anyone wants to allocate some of their own time to working on any of them, but doesn't know where to get started, please email me and I will help you figure out where to begin.

If unproductive complaining continues I will have to lock this thread, and that would make it harder for everyone to collaborate on actually solving these problems.

If you simply must register your feelings, consider adding a 👎 reaction to my comment and I promise I will feel appropriately chastened, while helpfully avoiding spam for all the other poor souls who are subscribed to this issue.

@lvjames
Copy link

lvjames commented Aug 18, 2017

@ef4 no need for that I'm done arguing. I was only expressing what I believe is a justified concern in the back of every subscriber's mind (only wish more people would overcome this herd mentality and speak up). If that was "spam" to you and everyone else, I apologize, but next time please stick to your rules and spare us the pr speak.

I would also appreciate an answer to my original question if that's not too much to ask:

By the way, you've already implemented some much appreciated missing functionality in a separate addon. Why not a PR?

@benkingcode
Copy link

benkingcode commented Nov 4, 2017

@wecc @ef4 there is an RFC that has been open for 16 months and ignored. How can we get this pushed through? Or is the RFC process not the right way to move forward?

@ef4
Copy link
Contributor

ef4 commented Nov 7, 2017

@dbbk that RFC is incomplete and certainly doesn't address all the points in this meta issue. And it's not being ignored, its author stopped working on it. Nothing moves forward magically, it takes actual humans.

The main blocker here is people who prioritize this highly enough to implement it. Until somebody steps up and implements, people can complain here all they want and it won't matter at all. Everyone on this thread cares about this issue, but each of us is clearly prioritizing other things more highly.

Nobody needs permission to implement this.
But what if I implement and then people don't like my PR?
Then you still have the feature you wanted! Use it in your apps. Share it with other people who like the PR.
But I don't want to have to support the code.
Ah, you have just identified why this is harder than it looks. You're asking other people to do what you're not willing to do.
But I don't know how.
Neither do I, neither do any of us the first time we look. There's no shortcut.

@runspired
Copy link
Contributor

I recently landed first-class support for links in all relationship situations, and did a bug-sweep for meta on relationships as well.

This RFC will additionally expose links and meta for single Records.

@uhrohraggy
Copy link

@runspired any thoughts or updates to whether this RFC is making any headway?

@mansona
Copy link
Member

mansona commented Apr 1, 2019

@runspired In relation to @uhrohraggy's question, I am assuming that this "Meta Issue" to improve JSON-API support now actually tracks the record links & meta RFC instead of the previously closed Links and meta improvements RFC?

If so then for people wanting to keep track of any progress or stuff that they might be able to help with should be following the RFC tracking issue, is this correct? I know it's not 100% filled out yet but I am curious where I should be looking 😂 Also I have a need to help improve the link support for pagination so I would be interested in contributing (if that actually is included in the RFC)

@runspired
Copy link
Contributor

@mansona correct that this now effectively tracks the links & meta RFC, which would not on its own enable things like pagination but would at least improve access to the information needed to do so.

kevinansfield added a commit to TryGhost/Admin that referenced this issue Apr 13, 2022
refs TryGhost/Product#584
refs TryGhost/Product#1498

- updated newsletter save routine in `edit-newsletter` modal to open an email confirmation modal if the API indicates one was sent
  - modal indicates that the previously set or default email will continue to be used until verified
  - response from API when saving looks like `{newsletters: [{...}], meta: {sent_email_verification: ['sender_name]}}`
  - added custom newsletter serializer and updated model so that the `meta` property returned in the response when saving posts is exposed
    - Ember Data only exposes meta on array-response find/query methods
    - emberjs/data#2905
- added `/settings/members-email-labs/?verifyEmail=xyz` query param handling
  - opens email verification modal if param is set and instantly clears the query param to avoid problems with sticky params
  - when the modal opens it makes a `PUT /newsletters/verify-email/` request with the token in the body params, on the API side this works the same as a newsletter update request returning the fully updated newsletter record which is then pushed into the store
- removed unused from/reply address code from `<Settings::MembersEmailLabs>` component and controller
  - setting the values now handled per-newsletter in the edit-newsletter modal
  - verifying email change is handled in the members-email-labs controller
- fixed mirage not outputting pluralized root for "singular" endpoints such as POST/PUT requests to better match our API behaviour
tigefa4u pushed a commit to tigefa4u/Ghost that referenced this issue Aug 3, 2022
refs TryGhost/Product#584
refs TryGhost/Product#1498

- updated newsletter save routine in `edit-newsletter` modal to open an email confirmation modal if the API indicates one was sent
  - modal indicates that the previously set or default email will continue to be used until verified
  - response from API when saving looks like `{newsletters: [{...}], meta: {sent_email_verification: ['sender_name]}}`
  - added custom newsletter serializer and updated model so that the `meta` property returned in the response when saving posts is exposed
    - Ember Data only exposes meta on array-response find/query methods
    - emberjs/data#2905
- added `/settings/members-email-labs/?verifyEmail=xyz` query param handling
  - opens email verification modal if param is set and instantly clears the query param to avoid problems with sticky params
  - when the modal opens it makes a `PUT /newsletters/verify-email/` request with the token in the body params, on the API side this works the same as a newsletter update request returning the fully updated newsletter record which is then pushed into the store
- removed unused from/reply address code from `<Settings::MembersEmailLabs>` component and controller
  - setting the values now handled per-newsletter in the edit-newsletter modal
  - verifying email change is handled in the members-email-labs controller
- fixed mirage not outputting pluralized root for "singular" endpoints such as POST/PUT requests to better match our API behaviour
@runspired
Copy link
Contributor

With the new Cache and RequestManager RFCs which bring support for document-level information caching (and thereby pagination/filtering etc.) the only piece of JSON:API that isn't well supported at this point is resource links & meta. emberjs/rfcs#332 was intended to address that but the PR #7855 has sat stale. I'm going to close this issue as resource links & meta is already tracked elsewhere.

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

No branches or pull requests