Fix #463: added back special case (single PK)

[cjrf]

Q	A
Is bugfix?	yes
New feature?	no
Breaks BC?	no
Tests pass?	yes
Fixed issues	#463

[rob006]

Wouldn't be better to use actual param name here?

[samdark]

Fixed another way as @WinterSilence suggested. Please check if it works now.

[cjrf]

Still breaks grid/ActionColumn.

However, grid/ActionColumn probably shouldn't force "id" as the PK name.

[WinterSilence]

@cjrf can you post more details?

[cjrf]

For years now, there have been four snippets of code that "rename a single PK to id". So I assume this is a feature.

Three of those snippets where in Gii, one in yii/grid/ActionColumn::createUrl.

Fix #453 removed just 2/3 of the Gii snippets, breaking the feature and Gii itself.

My suggestion (#466) was to simply put those snippets back.

Fix #467 removed the third Gii snippet, fixing Gii itself, but not the feature.

However, grid/ActionColumn probably shouldn't force "id" as the PK name.

By this, I mean that one solution could be to change yii/grid/ActiveColumn and remove completely this renaming feature from Yii2. I advise against this, since it will break back compatibility.

[WinterSilence]

@cjrf ok, now I'm understand you, but rollback does not solve current problem - primary key can consist of several columns. We need further proceedings. If you have any ideas how fix it without BC, then please post it in new issue.

[rob006]

@cjrf We can generate urlCreator by Gii and overwrite default ActionColumn::createUrl() - in that case old code will work as before, and new templates will use new param schema everywhere.

I'm still not sure if it is worth the trouble. It should be much simpler to just revert #467 and apply this PR instead. Also, using id as param name may be more practical from UrlManager perspective - you can use single standard <controller>/<action>/<id> rule for all CRUDs, even if you name your PK as post_id, thread_id, etc.

@WinterSilence I don't think that any of recent changes affected composite private keys.

[WinterSilence]

@rob006

We can generate urlCreator by Gii and overwrite default ActionColumn::createUrl()

I suggested same thing to @samdark

[WinterSilence]

@rob006

I'm still not sure if it is worth the trouble. It should be much simpler to just revert #467 and apply this PR instead

As I'm already said

but rollback does not solve current problem - primary key can consist of several columns

[rob006]

@WinterSilence Did you actually tested that this problem exists if you revert #467 and apply #466? Or this is just your speculations?

[WinterSilence]

@rob006 https://github.com/yiisoft/yii2/blob/1d7f9d8d2da7ae3497d7c18b215b1cb569305ee2/framework/grid/ActionColumn.php#L222 ActionColumn not support it

[rob006]

For me is_array($key) check indicates the opposite. But I also do not get how does it matter in terms of #467 and #466 - both PRs affects only simple PK and do not change behavior for composite PK.

[WinterSilence]

@rob006 wrong link, I mean https://github.com/yiisoft/yii2-gii/blob/master/src/generators/crud/default/views/index.php#L36
https://github.com/yiisoft/yii2/blob/1d7f9d8d2da7ae3497d7c18b215b1cb569305ee2/framework/grid/ActionColumn.php#L222 is worst solution in any case because we don't know how pk column(s) called in route, need bind route placeholder(s) to pk column(s)

[cjrf]

rollback does not solve current problem - primary key can consist of several columns

The current implementation of ActionColumn does support composite primary keys (at least to the extent I've tested). In that case, no renaming takes place.

For me is_array($key) check indicates the opposite.

And you're correct. I wasn't sure but tested it and confirmed ActionColumn does support composite primary keys as it is.

using id as param name may be more practical from UrlManager perspective - you can use single standard <controller>/<action>/<id> rule for all CRUDs, even if you name your PK as post_id, thread_id, etc.

Exactly. That's probably why this renaming feature was added a long time ago. And it's not just urlManager, but controllers and views are much easier to write if you don't need to worry about what the PK was called in the database (as long as it's not a composite PK).

[WinterSilence]

@rob006 at least something in this world remains unchanged (: