Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ability to replace primaryKey field via config (6.x) #707

Open
wants to merge 1 commit into
base: 6.x
Choose a base branch
from

Conversation

asgraf
Copy link

@asgraf asgraf commented Oct 24, 2023

Example use case:

$this->Crud->action()->setConfig('primaryKey', 'uuid');

this change makes FriendsOfCake/crud plugin compatible with dereuromark/cakephp-expose

Example:
$this->Crud->action()->setConfig('primaryKey', 'uuid');
@ravage84
Copy link

Wouldn't it make more sense to change this in the table itself?

Or why would one change this for the CRUD component but not in the table (thus for the whole application)?

A primary key normally doesn't get changed... 🤷

@asgraf
Copy link
Author

asgraf commented Jan 24, 2024

@ravage84

A primary key normally doesn't get changed... 🤷

Yes, normally it does not (see dereuromark/cakephp-expose for details)
This change is created only to make FriendsOfCake/crud plugin compatible with dereuromark/cakephp-expose

why would one change this for the CRUD component but not in the table (thus for the whole application)?

$this->Crud->action()->setConfig('primaryKey', 'uuid'); applies this change only for SQL query generated by Crud action leaving other queries untouched

With cakephp-expose you can have urls like:
/view/SAftQtJqYjAs3XRMsWgRfk instead of /view/3 (url contains short-uuid instead numeric id)
(see expose examples)

For url like /view/SAftQtJqYjAs3XRMsWgRfk Crud would try to generate SQL like this:

SELECT [fields list] FROM articles as Articles WHERE Articles.id = 'SAftQtJqYjAs3XRMsWgRfk'

This would fail because id is autoincrement int field
After this patch with $this->Crud->action()->setConfig('primaryKey', 'uuid'); and dereuromark/cakephp-expose Crud will generate valid SQL as follows:

SELECT [fields list] FROM articles as Articles WHERE Articles.uuid = 'efbfbdef-bfbd-7d56-efbf-bdefbfbd41ef'

SAftQtJqYjAs3XRMsWgRfk is short version of efbfbdef-bfbd-7d56-efbf-bdefbfbd41ef (see keiko/uuid-shortener)

Wouldn't it make more sense to change this in the table itself?

Changing primary key at table level would break all queries with $query->contain() in whole app (foreign keys are numeric ids not uuids). See dereuromark/cakephp-expose for details

@davidyell
Copy link
Member

Seems like a test is needed to prove the functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants