Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
The definition of the {users_data}
table in user_schema()
includes this for the foreign_keys
key:
'foreign keys' => [
'uid' => ['users' => 'uid'],
]
This is not consistent with the structure of that key as described in the Schema API page in the API docs.
Proposed resolution
Change those lines to be consistent with the API docs. Several other modules define foreign_keys
correctly. The {node_access}
table, defined in node_schema()
is a good model.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#7 | fix-foreign-keys-definition-3011237-7.patch | 469 bytes | apaderno |
#5 | fix-foreign-keys-definition-3011237-5.patch | 596 bytes | apaderno |
Comments
Comment #2
benjifisherThe current patch on #2883260: Replace the schema example with one actually used from a module uses the
{users_data}
table as the example indatabase.api.php
, which is the source of the Schema API documentation. I am recommending a similar change there.Comment #3
apadernoComment #4
apadernoComment #5
apadernoComment #6
benjifisher@kiamlaluno:
Thanks for looking at this, and for updating the issue title!
I know that @alexpott, in [#2883260.11], says to keep that comment in the sample code in database.api.php, but I still think it is redundant. In this context, I can also argue for consistency: none of the other core modules have such a comment in their code.
I guess that the key can be anything, but why "data_owner"? Do you mean that this is the
users_data
table, and this key identifies the user that "owns" the data in this table?Looking at the other core modules,
foreign_keys
array; here, that would beusers
. I do not like that much.set_user
, so maybedata_user
would follow the same pattern.sessions
table in the System module usessession_user
.node_access
table usesaffected_node
, soaffected_user
would follow that pattern.I suggest
data_user
oraffected_user
, but you can leave it asdata_owner
if you think that is clearest.Comment #7
apadernoaffected_node makes sense for the node access table, but for the user_data table I see preferable the pattern used for the sessions table.
I left the comment because it was already present in the example given for the schema API. It probably doesn't make sense to repeat it for the module, where it doesn't have the purpose of "teaching" about the foreign keys being used only for documentation.
Comment #8
benjifisher@kiamlaluno:
Thanks for the update. This looks good to me.
Comment #9
apaderno@benjifisher Thank you for the suggestions.
Comment #10
catchCommitted 125e9f4 and pushed to 8.7.x. Thanks!