Refactor relationship creation

[pxpm]

WHY

BEFORE - What was wrong? What was happening before this PR?

We created relations "separatelly" so it would be difficult to have chained relations working.

AFTER - What is happening after this PR?

Relations are created in a closed loop so they could be chained.

HOW

How did you achieve that, in technical terms?

refactor code, since we created relations separatelly it would be very difficult to achieve what is proposed here.

there is no sense in having the item relations created in separate functions. this refactor aims to have ALL relationships be dealt in the same place, so they can be looped and chained if needed.

Is it a breaking change or non-breaking change?

ohh yeah

How can we test the before & after?

Trying to create a nested ENTRY -> HasOne (Something) -> BelongsToMany (I've added this example in the tests)

[tabacitu]

I love this. I was skeptical at first because it's a big change to the core. But it clears up how relationships are created SO MUCH. So let's do it. I have questions and suggestions, see below.

[tabacitu]

@pxpm you said this is a breaking change, but breaking how? From what I can see the only "breaking" changes are that we remove two public methods: createRelations() and syncPivot(), but those methods were actually never documented, and only used by the Backpack core, so they should have been "private".

So I think we're good, this should be non-breaking to 4.2. Am I wrong?

[pxpm]

@tabacitu pushed the changes to address your concerns.

Let me know if anything sticks out for you!

Pedro

[tabacitu]

Thank you @pxpm . Still have a few changes in terms of naming and docs to one function. We're spending WAY too much time on such a small function but hopefully it will help us work together better in the future. Also, asked a question to be sure we're not breaking anything. Thanks!

[tabacitu]

Much MUCH cleaner @pxpm , thank you.

I've tested everything that used to work, aaand... it still works 🎉🎉🎉

I've tried to test the new use case though... and couldn't make it work. But maybe it's me, I've been working for 14 hours already so I'm a bit tired. I'll try again tomorrow. I've moved the description of the bug I found here (separate issue).

[pxpm]

I've stumbled on this while refactoring demo.

I am thinking we can completely make it work as the other fields.

Note that I am defining the field like this at the moment:

[
                'name' => 'wish',
                'label' => 'HasOne when the relation is optional',
                'subfields' => [
                    [ //this is the country relation in Wish model
                        'name' => 'country_id',
                        'entity' => 'wish.country',
                        'type' => 'relationship',
                    ],
                    [
                        'name' => 'body'
                    ],
                ],
                'tab'   => 'Relationships',
            ],

I think we refactored so much the stuff that it pretty much works rigth now, but I am also sure that trying to add "auto-infering" for the subfields is not so simple as it seems because of the "different" subfields kinds. This that I showed you is the minimum definition for it to work properly.

When I finish the refactoring of demo I take a look at this.

[tabacitu]

I've moved the bug I found above in a separate issue, because... I think it may be unrelated to this branch. It happens both on 4.2 and on this branch. And I don't know if this branch is supposed to fix that because... it doesn't. And I no longer understand what this branch is supposed to do 😅 I thought I did. But apparently I don't.

---

Note that I've also tried what you said in the initial text for this PR (how to test this):

Trying to create a nested ENTRY -> HasOne (Something) -> BelongsToMany (I've added this example in the tests)

So I've added category_id on the Avatar model:

    public function category()
    {
        return $this->belongsTo('Backpack\NewsCRUD\app\Models\Category', 'category_id');
    }

And the category field on the OwnerCrudController:

        CRUD::field('avatar.category')->label('Category');

And... this works fine. The chained relationship gets saved fine. BUUUT. This also works fine on the 4.2 branch. So it's not this PR making it work, it was working before.

---

I guess my question here is... what is this PR fixing? How can I test this PR? I don't understand your "wish" example, sorry. Please give me baby steps on what to do on our current demo, to test what this PR is fixing.

[tabacitu]

Answer: we know what works extra - belongsTo as a subfield.

So I'm going to merge this into 4.2. The rest of the relationships as subfields will come as a different PR.