Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
EntityTypeInterface::getKeys() documents that entity IDs must be numeric. This is not true, however, we explicitly support string IDs, see in particular the EntityTestStringId test entity.
Steps to reproduce
-
Proposed resolution
Update the documentation.
Remaining tasks
User interface changes
-
Introduced terminology
-
API changes
-
Data model changes
-
Release notes snippet
-
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | issues_3467540_7.png | 191.51 KB | kleiton_rodrigues |
Issue fork drupal-3467540
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3467540-entity-type-interface--get-keys--docs
changes, plain diff MR !9177
Comments
Comment #3
tstoecklerCreated a quick MR.
Since this was directly in scope of the lines being touched, I opted to touch up two other things:
- Removed the note about the ID key being required. It is not, it is only required if you actually want to save the entity. That of course means in 99% of cases you do want to specify an ID key, but with contact messages in core we actually have an example of not having an ID, so it doesn't make much sense to claim it's required.
- Removed the usage of the term "Field API". It is a remnant of D7 times. In current times it is actually in correct because
getKeys()is used for config entities as well and they have nothing to do with Field API in its current form. The more correct/current term would be "Entity API", but instead of replacing that I just removed the explicit mentioning of that as that would have felt somewhat out of place there.I also noted that we use the term "property" there, to describe the values of the keys array. That's also somewhat incorrect, as for content entities, those are actually field names and properties are something different, but for config entities we do tend to use the term properties (for example
ConfigEntityTypeInterface::getPropertiesToExport(), so I think it might be more clear to consistently use "field or property" instead of just "property". That would need to be changed a bunch of times, though, in that documentation paragraphs, so I anticipate that being flagged as too broad in scope for this issue.Comment #4
prem suthar commented@tstoeckler you used twice from at the @return array description .
Comment #5
tstoecklerNice catch, thanks! 👍
Comment #6
smustgrave commentedSeems pretty straightforward
Comment #7
kleiton_rodrigues commented@prem-suthar RTBC +1
Comment #8
quietone commented@kleiton_rodrigues, There are contributor tasks on Drupal.org that provide the steps for making useful additions to an issue. You may find How is credit granted for Drupal core issues useful as well.
Comment #9
catchOne comment on the MR.
Comment #10
tstoecklerFixed
Comment #11
smustgrave commentedAlso the MR is 250 commits back :) but you're right the comments would not cause those failures. Feedback from @catch appears to be addressed.
Comment #15
quietone commentedCommitted to 11.x and cherry-picked to 11.1.x
Thanks!