Add join support for tables with PK different than "id"

[abbadon1334]

related with #803

[abbadon1334]

What do you think about the naming and description of this, because that 'idField' is for the FakeModel, not the table itself :

data/src/Model/Join.php

Lines 89 to 93 in 2426c4d

	* Custom definition if $idField for of ForeignModel
	* By Default ForeignModel idField is equal to 'id'
	* To be used when the joined table doesn't have a field named 'id'
	*/
	public ?string $foreignModelIdField = null;

i decided to make it nullable because in method setDefaults($defaults) nulled values will not be set

[mvorisek]

Does this PR really solve #803? The issue describes something else, this PR adds support for PK to be different than "id". But if PK is "id" and we join based on a different column, the foreign model still needs to have the knowledge about PK to be "id", for AI (autoincrement) to work at least (not mentioning fake model will never execute proper model hooks).

[abbadon1334]

Does this PR really solve #803? The issue describes something else, this PR adds support for PK to be different than "id". But if PK is "id" and we join based on a different column, the foreign model still needs to have the knowledge about PK to be "id", for AI (autoincrement) to work at least (not mentioning fake model will never execute proper model hooks).

You are right about the hooks, it is a FakeModel so the only functionality of this PR is to build a correct query to avoid raising a DSQL Exception when an update on joined fields occurs, but no hook will be triggered.

So join can be done directly with models? ( like this $model->join($joinedModel) )

[mvorisek]

that is the direction, see #960, however Join class still relies on many hooks as we must hold a reference to joined data and that has limitations currently, imagine like same entity joined/referenced back in one model

[abbadon1334]

that is the direction, see #960, however Join class still relies on many hooks as we must hold a reference to joined data and that has limitations currently, imagine like same entity joined/referenced back in one model

Ok, I will do some tests.

[abbadon1334]

in another repo I solved it in this way:

Define a different Join Model::$_defaultSeedJoin
https://github.com/abbadon1334/atk-wordpress-plugin/blob/ccf5fb538f60e6cc513f4c148de5555bba7edd80/src/Models/PostModel.php#L26

Extend Join class
https://github.com/abbadon1334/atk-wordpress-plugin/blob/master/src/Models/Internal/WpModelJoin.php

Use it in this way:
https://github.com/abbadon1334/atk-wordpress-plugin/blob/ccf5fb538f60e6cc513f4c148de5555bba7edd80/src/Models/Internal/WpModel.php#L48-L64

It works.
What do you think if i try to rewrite the PR following this path?

[mvorisek]

The src changes LGTM.

Here is a summary of the discussion above:

• Add join support for tables with PK different than "id" #1077 (comment) - no fake model must be used for saving, this needs to reviewed before merge
• Add join support for tables with PK different than "id" #1077 (comment) - in this PR, we probably do not want to do more, but tests need to pass with every DB engine - UPDATE: fixed by Fix unique index creation for MSSQL & Oracle #1117

#1077 (comment) - @abbadon1334 what usecases does it solve vs. this PR? Some short example?

[mvorisek]

Thank you, @abbadon1334, for this contribution and sorry for letting you wait almost a year. Fixing the DBAL issues needed for proper FK creation - #1117 - was not easy.