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

JSON-API + new serializers #283

Merged
merged 11 commits into from
Jul 18, 2015
Merged

JSON-API + new serializers #283

merged 11 commits into from
Jul 18, 2015

Conversation

tstirrat
Copy link
Contributor

Minimizes the footprint of code by using as much of the underlying JSONSerializer methods as possible.

The main point of difference between the inherited code is how hasMany relationships are stored.

Also uses EmbeddedRecordsMixin for embedded record processing. This will mean that setting the embedded option in the models/* files will be deprecated. The new way forward will be to use a per type serializer with embedded: 'always' as seen here

Merging #282 into this one while we iterate.

tmayr and others added 2 commits July 11, 2015 13:35
Minimizes the footprint of code by using as much of the underlying
JSONSerializer methods as possible.

The main point of difference is how `hasMany` relationships are stored.

Now uses `EmbeddedRecordsMixin` for embedded record processing
@tstirrat tstirrat added this to the 1.13.0 Compat milestone Jul 11, 2015
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

Tim Stirrat added 3 commits July 11, 2015 14:46
- Serializer does not actually push into the store anymore so the integration parts are unnecessary
- Moved the empty/undefined hasMany relationship code into the serializer
- Cleaned up the unit test so it doesn't rely on the fixture data anymore
@tstirrat
Copy link
Contributor Author

Even 2.0.0-canary passes ✅

Just needs the embedded records stuff fixed up and we're good to go.

@tmayr
Copy link
Contributor

tmayr commented Jul 12, 2015

Nice! I've tried looking at the embedded records but it goes way over my head, will keep looking!

@tstirrat
Copy link
Contributor Author

I've decided to make new models specifically for the embedding, trying to override embedding stuff inside each test was too cryptic and problematic.

Should have something working by end of day :)

@tstirrat tstirrat mentioned this pull request Jul 15, 2015
9 tasks
@tstirrat
Copy link
Contributor Author

Waiting on emberjs/data#3550

@tmayr
Copy link
Contributor

tmayr commented Jul 16, 2015

Oh wow, that was way more complex than I thought, glad that it's being resolved soon!

Is there any other change left for either #267 or 2.0 compat milestone?

@tstirrat
Copy link
Contributor Author

Yeah, the way the new serializer stuff works you don't have access to the records anymore (it's for the best) .. but it sure made things a little crazy in the adapter :/

@tstirrat
Copy link
Contributor Author

This will get us compatible with both 1.13.x and 2.0-canary

If you want to help out any more, we could do with some more testing to work out if there are any edge cases in the embedded records stuff. I have tested the "basic" example for embedded records, but there could be things like when there are relationships defined in both directions i.e. parent hasMany children where children define child belongsTo parent.

Or just check out this branch, pair it with branch in emberjs/data#3550 and try a few things out with it (not necessarily embedded stuff)

Tests in general will always be helpful in getting us over the line when 2.0 stable launches. Some of the new methods I wrote in the adapter need some checks and balances.

And finally I do want to eventually move the recordWillDelete code into the ember data standard deleteRecord handler.

@tstirrat
Copy link
Contributor Author

Disabling embedded tests so that I can make an early release. the embedded stuff will need to wait for emberjs/data#3550

@tmayr
Copy link
Contributor

tmayr commented Jul 17, 2015

Might give a try to updating to Ember Data 1.13 later using this branch, will let you know if everything goes smoothly.

tstirrat added a commit that referenced this pull request Jul 18, 2015
@tstirrat tstirrat merged commit f6f20c0 into master Jul 18, 2015
@tstirrat tstirrat deleted the ts-jsonapi-serializers branch July 18, 2015 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants