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

Support nested relations #17

Closed
ldiebold opened this issue Dec 14, 2020 · 25 comments
Closed

Support nested relations #17

ldiebold opened this issue Dec 14, 2020 · 25 comments
Labels
enhancement New feature or request

Comments

@ldiebold
Copy link

Would love to be able to include nested relations.
Something like this:

protected function includes(): array
{
  return ['business.manager'];
}

would be able to save a few calls to the API!

Thanks a million for this package, I absolutely love it. So beautifully written 😃

@alexzarbn
Copy link
Member

Hi @ldiebold,

First of all, thank you for the kind words!

Honestly, I thought this is already supported 🤔
Let me check, and I will get back to you after running a few tests.

But either way, this is definitely something that will be introduced, if it is not supported.

@alexzarbn alexzarbn added the enhancement New feature or request label Dec 14, 2020
@alexzarbn
Copy link
Member

@ldiebold,

Found the cause - it is RelationsResolver@guardRelations that wipes out eager loaded nested relations, because there is no "business.manager" relation on the model itself.

Will include the fix in v1.2 👌

@PascalHesselink
Copy link

@ldiebold,

Found the cause - it is RelationsResolver@guardRelations that wipes out eager loaded nested relations, because there is no "business.manager" relation on the model itself.

Will include the fix in v1.2 👌

I'm also facing this problem, would love to see this fix! Amazing work @alexzarbn.

@ldiebold
Copy link
Author

@alexzarbn thankyou!!!

Also want to point out how beautiful your code is. Simple comments, easy to navigate... A true joy 💓

@alexzarbn
Copy link
Member

@PascalHesselink

Thank you ~

Got it, will plan to implement the fix for this weekend.

@alexzarbn
Copy link
Member

alexzarbn commented Dec 16, 2020

@ldiebold

It took a lot of refactoring to get to its current state 😄
Thank you, will keep it that way ~

@PascalHesselink
Copy link

@PascalHesselink

Thank you ~

Got it, will plan to implement the fix for this weekend.

I noticed it works when you list them up, in my case:

        return [
                'forumCategories',
                'forumCategories.threads',
                'forumCategories.threads.user',
        ];

But it doesn't recognise the resources, so I wanted to exclude the email from the 'user' relationship, but that didn't work through the resources. I hope you can also look to that!

@alexzarbn
Copy link
Member

@PascalHesselink,

Have you tried explicitly setting the resource class?
If resources are located in non-conventional folder, indeed, it may not find them.

@PascalHesselink
Copy link

@PascalHesselink,

Have you tried explicitly setting the resource class?
If resources are located in non-conventional folder, indeed, it may not find them.

I'm using the default namespace/conventional way of creating resources. I've noticed if you want to manually 'register' them, they have to been put in a controller. In my case the 'user' relationship doesn't have a controller because it is a nested relationship.

@alexzarbn
Copy link
Member

@PascalHesselink,
Have you tried explicitly setting the resource class?
If resources are located in non-conventional folder, indeed, it may not find them.

I'm using the default namespace/conventional way of creating resources. I've noticed if you want to manually 'register' them, they have to been put in a controller. In my case the 'user' relationship doesn't have a controller because it is a nested relationship.

In that case you need to do the following:

  • Have a resource for ForumCategory
  • In ForumCategory resource you need to define the threads relation like Laravel documentation suggests (meaning you also need to have a resource for Thread)
  • In Thread resource you need to define the user relation, again the same way.
  • In User resource you can then exclude the email property

@PascalHesselink
Copy link

@PascalHesselink,
Have you tried explicitly setting the resource class?
If resources are located in non-conventional folder, indeed, it may not find them.

I'm using the default namespace/conventional way of creating resources. I've noticed if you want to manually 'register' them, they have to been put in a controller. In my case the 'user' relationship doesn't have a controller because it is a nested relationship.

In that case you need to do the following:

  • Have a resource for ForumCategory
  • In ForumCategory resource you need to define the threads relation like Laravel documentation suggests (meaning you also need to have a resource for Thread)
  • In Thread resource you need to define the user relation, again the same way.
  • In User resource you can then exclude the email property

Got it! I made a fault in my resource nesting, you're awesome!

@alexzarbn
Copy link
Member

This is fixed in 1.1.3 release.

In 1.2 release I am planning to add the ability to specify wildcard nested relations. It will remove the need for listing all nested relations, if all of them should be available for inclusion:

protected function includes(): array
{
    return ['user', 'posts.*']; // this is NOT released yet
}

@joshuahill1609
Copy link

I was listing out my relations four levels deep like @PascalHesselink mentioned in their comment. The new 1.1.3 breaks this for some reason, I have not been able to dive into why but wanted to leave an update in case anyone else is having the same issue. I went back to 1.1.1 and my relations load again.
This may be the intended functionality and I just need to change how I eager load relations.

@alexzarbn
Copy link
Member

alexzarbn commented Dec 30, 2020

Hi @joshuahill1609,

Thank you for bringing that up. Could you please share the whitelisted includes array from your controller and the request url (with ?include= query parameter)?

@alexzarbn alexzarbn reopened this Dec 30, 2020
@joshuahill1609
Copy link

joshuahill1609 commented Jan 4, 2021

I am using the alwaysIncludes() function in the controller to always load these nested resources.

/**
    * The relations that are loaded by default together with a resource.
    *
    * @return array
    */
    protected function alwaysIncludes() : array
    {
        return [
            'divisions', 
            'divisions.cost_codes', 
            'divisions.cost_codes.tasks', 
            'divisions.cost_codes.production_codes', 
            'divisions.cost_codes.tasks.area',
            'divisions.cost_codes.tasks.nodes',
            'divisions.cost_codes.tasks.nodes.production_codes',
            'divisions.cost_codes.tasks.manifest',
            'divisions.cost_codes.tasks.manifest.materials',
        ];
    }

@alexzarbn
Copy link
Member

alexzarbn commented Jan 5, 2021

@joshuahill1609,

Does the Devision model follow the same naming convention for defining relation methods (e.g. cost_codes) as it is listed in alwaysIncludes method or it is defined as costCodes?

@joshuahill1609
Copy link

@alexzarbn Yes, the relation method is cost_codes().

This is working for me now and broke when I upgraded to the new version.

@alexzarbn
Copy link
Member

@joshuahill1609,

Okay, I will dig more into this issue.

@alexzarbn
Copy link
Member

Relations whitelisting using wildcards is added in 1.2 release.

@alireza2281
Copy link

Hello, i have updated to version 1.2 but when i use user.* in always include i get this error:
Call to undefined relationship [*] on model [Modules\\User\\Entities\\User].
and when i put these relations ['user.profile', 'user.assignedCategory'] it just fetch the last relation

@alexzarbn
Copy link
Member

Hi @alireza2281,

Whitelisting of relations using wildcards is only supported in the includes method.
Please upgrade to v1.2.1 - it contains the fix to make ['user.profile', 'user.assignedCategory'] whitelisting work.

@alexzarbn
Copy link
Member

alexzarbn commented Jan 10, 2021

@joshuahill1609,

Could you please upgrade to v1.2.2 and see, if it works? I was not able to replicate the issue though..

@alireza2281
Copy link

alireza2281 commented Jan 11, 2021

@alexzarbn hi again,
there is one more thing that if i try to access relation from one level below the example i mentioned earlier would cause the same problem.
i mean if i always include related.relatedProduct.primaryImage and then related.relatedProduct.variation
it would return just variation

@alexzarbn
Copy link
Member

Hi @alireza2281,

Thank you, I will look into the issue.

@alexzarbn
Copy link
Member

alexzarbn commented Jan 14, 2021

@alireza2281,

Recursive stuff is so confusing sometimes 😅

Please update to v1.2.2 - it will solve the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants