Use Model::findById() instead of Model::findByPk()

[leofeyer]

As suggested in #6912 (comment).

[Toflar]

Certainly for 5.4 and not 5.3.

[fritzmg]

We have added deprecations in bugfix versions in the past, haven't we?

[Toflar]

If there was a need - where is it here?

[fritzmg]

The earlier deprecations are added the better I thought, no?

[Toflar]

It's just a super useless change affecting 123 files in a bugfix release that wouldn't harm merging only in 5.4 - that's all I'm saying.

[leofeyer]

Certainly for 5.4 and not 5.3.

No, 5.3 is correct. We have added #6912 to the 5.3 branch and this is just an alternative approach to fix the same inconsistency. There is no reason why one should be correct for Contao 5.3 and the other not.

Also, we certainly don't want the LTS version to use findByPk() and future versions to use findById(). This would make upstream merging unnecessarily complicated.

[leofeyer]

But the issue is up for discussion. We might as well leave things as they are.

[ausi]

We’d have to deprecate Model::$strPk as well then, right?

Same with $GLOBALS['TL_DCA'][…]['config']['sql']['keys']['id'] = 'primary'; for any column that is not named id?
Although we probably shouldn’t, as we already use it for DCAs that don’t have a Model like tl_search_index for example.

[leofeyer]

We’d have to deprecate Model::$strPk as well then, right?

Yes, we would.

[leofeyer]

As discussed in the Contao call, we want to use findById() instead of findByPk() but we do not want to deprecate the primary key logic.