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

feat: allow constraining ToMany query #103

Merged
merged 4 commits into from
Mar 31, 2024

Conversation

SychO9
Copy link
Contributor

@SychO9 SychO9 commented Mar 1, 2024

A better alternative to #102, generally allowing to constrain the relationship query during eager load is more flexible for use cases like the one outlined in the stale PR.

@tobyzerner
Copy link
Owner

This sounds good. I think I'd like to call this scope to be consistent with resources. Any reason to not allow it on ToOne relationships too? (Move to the base Relationship class)

@tobyzerner
Copy link
Owner

Actually, just remembered that scope is an Eloquent-specific behaviour so I'm not sure about including this functionality in the framework-agnostic Relationship classes. I'll have a think about an alternative approach, maybe like making those classes macroable or something.

@tobyzerner
Copy link
Owner

OK, I think the best approach will be to subclass the ToOne and ToMany classes under a new Tobyz\JsonApiServer\Laravel\Field namespace, and both of them can use a trait which adds a scope method. This way the concept of scoping remains entirely within the Laravel namespace.

@SychO9
Copy link
Contributor Author

SychO9 commented Mar 30, 2024

Done 👍🏼

@tobyzerner tobyzerner merged commit e7c87c7 into tobyzerner:main Mar 31, 2024
@tobyzerner
Copy link
Owner

Thank you!

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