Allow multiple relationships on same table

[faeby]

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

1. Does your submission pass tests and did you add any new tests needed for your feature?
2. Did you update all templates (if applicable)?
3. Did you add the relevant documentation (if applicable)?
4. Did you test locally to make sure your feature works as intended?

Changes to Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your core changes, as applicable?
• Have you successfully ran tests with your changes locally?

---

Introduction

This pull request is an attempt to introduce the possibility to embed two relationships linked to the same table.

The problem is described in the issue #688. When we try to display two relationships linked to the same table, the first relation will be shown as the second relation too. As there is no alias, the second join is not applied because a join already exists on the target table.

As far as I know, the current workaround consists in configuring the column with the foreign key, and then formatting the column with the label or format column helper. For me, this has two problems:

• it is not possible to search in the column
• for the second relation, the workaround has to be used, so we have to use two differents methods to display the same thing

Proposed solution

This PR fix the problem by generating and adding an alias to the tables that are joined. The alias for the relations joins are generated by imploding the relation parts together and glueing them with an underscore.

Some examples:
I have a document class, with a sender and a producer, where both are a Sender

class Document {
    public function sender(): BelongsTo
    {
        return $this->belongsTo(Sender::class, 'sender_id');
    }

    public function producer(): BelongsTo
    {
        return $this->belongsTo(Sender::class, 'producer_id');
    }
}

Table columns configuration:

    public function columns(): array
    {
        return [
            Column::make('sender_id', "sender_id"),
            Column::make('sender.id', "sender.id"),
            Column::make('producer_id', "producer_id"),
            Column::make('producer.id', "producer.id"),
        ];
    }

Before the fix:

• first line: producer_id is empty, because the document has no producer. But the table incorrectly link the sender 1 as a producer
• second line: producer_id is 2, but the table shows 1, because it is based on the first and unique join

Generated query:

select 
    `documents`.`sender_id` as `sender_id`,
    `senders`.`id` as `sender.id`,
    `documents`.`producer_id` as `producer_id`,
    `senders`.`id` as `producer.id` 
from 
    `documents` 
left join 
    `senders` on `documents`.`sender_id` = `senders`.`id` -- only 1 join, should have 2 (producer_id is missing)

limit 10 offset 0

After the fix

Generated query:

select
    `documents`.`sender_id` as `sender_id`,
    `sender`.`id` as `sender.id`,
    `documents`.`producer_id` as `producer_id`,
    `producer`.`id` as `producer.id` 
from
    `documents` 
left join
    `senders` as `sender` on `documents`.`sender_id` = `sender`.`id` -- alias based on the relationship name
left join
    `senders` as `producer` on `documents`.`producer_id` = `producer`.`id` -- missing join is now present

limit 10 offset 0

Because the tables are now aliased, the joins are correctly handled by the performJoin method (at line 112).

Breaking changes

If you provide a custom callback to Column searchable function and you are specifying a table, you have to replace it by the alias.

Example:
In the tests/Http/Livewire/PetsTable.php file at line 55, before the PR is merged:

    ->searchable(
        fn (Builder $query, $searchTerm) => $query->orWhere('breeds.name', $searchTerm)
    ),

After the merge:

    ->searchable(
        fn (Builder $query, $searchTerm) => $query->orWhere('breed.name', $searchTerm)
    ),

breeds.name was replaced by breed.name

TODO

• document somewhere the breaking change ?
• add some tests ? Currently there is none for the WithDataTrait

Discussion

This is my first PR proposal, please let me know if I should change or improve something.

I'd appreciate some feedback about this feature, thank you.

[lrljoe]

In terms of the issue raised in your PR for searching/ordering when using a label/format, I do have a fix ready for a future release (not the pending one), which resolves the problem:

As far as I know, the current workaround consists in configuring the column with the foreign key, and then formatting the column with the label or format column helper. For me, this has two problems:

• it is not possible to search in the column
• for the second relation, the workaround has to be used, so we have to use two differents methods to display the same thing

But for the PR in general, it looks reasonable to me at first glance!
However the test cases are going to need to be extended to cater for this type of use case.

For testing in general, I have been working on extending the test cases, and working on getting them to match the demo repo. This way, it'll all be a little more straight forward! I obviously haven't included this type of example yet!

If you are willing to take a look at the Demo repo which is here: https://github.com/rappasoft/laravel-livewire-tables-demo, and are able come up with the model/relationships to demonstrate this, then I'll get it all incorporated into the next iteration of the test cases. Otherwise there'll be a slightly longer delay while I come up with it (currently I'm working on some AlpineJS functional migrations)

I really, genuinely appreciate any/all contributions, and Anthony definitely does too!

[faeby]

Hello @lrljoe,

I've made a draft PR in the demo repo: rappasoft/laravel-livewire-tables-demo#30

I hope it is was what your were asking me to do. Please let me know if it was something else.

---

On another subject, I was thinking about an edge case that should not be supported with this PR. I think we could have a colision with the aliases if we have:

• on the base model, a relation named with an underscore, for example: my_relation, the alias will be my_relation
• on the base model, a relation named my, and this relation has a relation named relation, if we try to display a property of the relation, its alias will be my_relation too

[lrljoe]

Thanks, your PR looks fine at first glance, although it may cause some issues with existing components.

I'll take a look this weekend and look at what needs doing to get it into a future release. It should make it into the release after the imminent one (which is just clean up and fixes).l

Once I've had a proper look, I'll inevitably hassle you to produce some tests proving any backwards compatibility too, if you haven't already, or any docs adjustment explaining how it now works.

For that edge case, it'll probably be simplest to shove the aliases into an array and append something to them if they already exist. But I'll have a play and see how it behaves.

It may be the case that we stick in an option/configure method to enable this behaviour, but keep the current as default until v3, when we can make more sweeping changes to behaviour

I'll let you know if that needs to be the case.

Really appreciate the contribution!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lrljoe]

So on reviewing this, my main concern is that it'll be a breaking change for any existing components, as anyone using a relationship column will end up with it failing.

What I'm definitely going to do however is introduce this change into the V3 upgrade which I've just started working on, which is an interim upgrade to make use of the Livewire V3 elements.

[lrljoe]

@rappasoft - thoughts on adding this to v3 while we're at it?

[rappasoft]

@rappasoft - thoughts on adding this to v3 while we're at it?

Lets give it a try

[lrljoe]

Currently testing in v3.0a

[lrljoe]

Just to make sure we don't merge this into v 2.0! It seems to work in v3.0, but would be a breaking change in 2.0

Will continue testing in 3.0 tho

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lrljoe]

This is in v3. and is on my backports radar l