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 link templates #27

Merged
merged 4 commits into from
Mar 25, 2014
Merged

Conversation

kjg
Copy link
Contributor

@kjg kjg commented Mar 20, 2014

This adds support for the top level links in the JSON API

Class.new(Oat::Serializer) do
schema do
type 'users'
link_template "user.managers", "http://foo.bar.com/{user.id}/managers"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be implemented as part of the existing link DSL method? Other formats such as HAL also support link templates and it looks like this could be part of the generic link DSL. For example

link 'user.managers', href: "http://foo.bar.com/{user.id}/managers", templated: true

I don't particularly like overloading methods with hash options too much, but it looks like many adapters could benefit from link templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that makes sense over a new method. I'll get that taken care of. Probably after #26 is squared away.

@kjg kjg mentioned this pull request Mar 21, 2014
ismasan pushed a commit that referenced this pull request Mar 21, 2014
Json api attribute links

It makes sense holding off the gem release until #27 is also merged I think. Good to have a fully-implemented JsonAPI at last!
@kjg
Copy link
Contributor Author

kjg commented Mar 21, 2014

I've change this to link :templated => true What do you think?

@ismasan
Copy link
Owner

ismasan commented Mar 24, 2014

Can we add this to the Readme? After that it's good to go.

@kjg
Copy link
Contributor Author

kjg commented Mar 24, 2014

Where do you see this going? There is nothing JSON API specific in the README and your goal is to have this adapter extracted into its own gem.

On Monday, March 24, 2014 at 12:43 PM, Ismael Celis wrote:

Can we add this to the Readme? After that it's good to go.


Reply to this email directly or view it on GitHub (#27 (comment)).

@ismasan
Copy link
Owner

ismasan commented Mar 24, 2014

Yeah, I just don't like to have too many hidden undocumented features... Do you think we should wait until JsonAPI support is complete before creating that new gem?

@kjg
Copy link
Contributor Author

kjg commented Mar 24, 2014

I was waiting for the api decision needed to resolve #16 and #19 before extracting it out.

I’ll add a note about templates: true in the Defining Properties section somewhere.

@ismasan
Copy link
Owner

ismasan commented Mar 24, 2014

I've resolved conflicts and merged #16

I'm still not happy with how it forces you to have conditional checks in every adapter, but in the meantime it allows us to move forward.

I might revisit @shekibobo's idea of using a null serializer (#21) to try and make adapter's a bit nicer.

@kjg
Copy link
Contributor Author

kjg commented Mar 24, 2014

Let me know if the README update looks good. Now that you've merged #16 you'd like me to extract the json api functionality, right? Any suggestions on a name. "oat-jsonapi"?

@ismasan
Copy link
Owner

ismasan commented Mar 25, 2014

Thanks, the readme looks Ok.

oat-jsonapi makes sense I think.

@kjg
Copy link
Contributor Author

kjg commented Mar 25, 2014

Would you mind pulling this in and releasing a new version (maybe 0.3.0 due to the adapter api change)? This would allow me to continue with my project and give me time to extract the json api into a gem to maybe be released along side an oat 0.4.0.

N.B. the Travis failure it due to Rake 10.2 being release. #29 should fix those failures.

ismasan pushed a commit that referenced this pull request Mar 25, 2014
@ismasan ismasan merged commit ab3d20f into ismasan:master Mar 25, 2014
@kjg kjg deleted the json_api_link_templates branch March 25, 2014 15:56
@ismasan
Copy link
Owner

ismasan commented Mar 25, 2014

Done!

@kjg
Copy link
Contributor Author

kjg commented Mar 25, 2014

Woohoo! Thanks

On Tuesday, March 25, 2014 at 11:35 AM, Ismael Celis wrote:

Done!


Reply to this email directly or view it on GitHub (#27 (comment)).

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.

2 participants