Skip to content

Conversation

@Dragomir-Ivanov
Copy link
Contributor

…eld projection for Connected resources.

This PR helps with following scenario:

  • Having this resource graph
	index := resource.NewIndex()
	aa := index.Bind("a", a, mem.NewHandler(), resource.Conf{AllowedModes: resource.ReadWrite})
	bb := aa.Bind("b", "id", b, mem.NewHandler(), resource.Conf{AllowedModes: resource.ReadWrite})
	bb.Bind("c", "id", c, mem.NewHandler(), resource.Conf{AllowedModes: resource.ReadWrite})

Being able to correctly execute queries like /a?fields=b{c}

smyrman
smyrman previously approved these changes Jan 18, 2019
rs
rs previously approved these changes Jan 18, 2019
Copy link
Owner

@rs rs left a comment

Choose a reason for hiding this comment

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

Had a quick glance and it LGTM. I'll let @smyrman do the final merge.

Sorry for the limited time I can dedicated to those PR atm.

@Dragomir-Ivanov
Copy link
Contributor Author

Thanks guys. I think I have added everything I needed to rest-layer, so this may be my last patch for now. I will keep eye on the any issues on GitHub, and will support and try to help as much as I can.
Special thanks for @smyrman for his guidance and patience.

@Dragomir-Ivanov
Copy link
Contributor Author

Dragomir-Ivanov commented Jan 19, 2019

Guy, hold off with the merge. I thing there is some problem.

@Dragomir-Ivanov Dragomir-Ivanov dismissed stale reviews from rs and smyrman via 4fecf8f January 19, 2019 23:23
@Dragomir-Ivanov
Copy link
Contributor Author

Okay, I think I fixed the issue with the latest patch.

smyrman
smyrman previously approved these changes Jan 21, 2019
@smyrman
Copy link
Collaborator

smyrman commented Jan 21, 2019

@Dragomir-Ivanov, when you are ready, please squash your commits to one and we can do a merge.

I would recommend the final commit message to follow the 50/72 rule, but won't review it in particular. This does however make the commit more readable in many tools, both CLI, web and GUI.

Quering fields on sub-resources were broken for sub-resource,
deeper than 2nd level.

Adds validation on field projection for `Connected` resources.
@Dragomir-Ivanov
Copy link
Contributor Author

Done. Thanks for the link.

@smyrman smyrman merged commit c3ebffd into rs:master Jan 21, 2019
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