| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | entity system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
After looking at #1288658: user bundles I would like to submit this patch which hopefully will allow for users to be turned into entities that support multiple bundles. I believe this is currently impossible within Drupal 7 core.
In the current Drupal 7 code, if an entity is declared as having multiple bundles, the entity_extract_ids() function will throw an EntityMalformedException if the entity is passed in without the bundle name explicitly declared.
The result is that Drupal core's handling of the user module means that the only possible outcome is to throw this exception.
Consider a contrib module has been added which uses hook_entity_info_alter() to add a 'bundle' key to the users table, allowing each user account to designate a specific "user type" bundle.
Now consider visiting user/register which will go through the user_register_form() function. This will then call entity_extract_ids() with an entity that has no explicit bundle set, since the user module is not 'bundle aware'. The exception is triggered because, now the contrib module has allowed the user entity to have multiple bundles, and the user entity has no bundle set.
In my proposed solution I make the assumption that for entities with only one bundle, the bundle name is always the same as the entity name. For example, by default, the user entity has exactly one bundle called user. I believe this to be the case but if my belief is incorrect, it might affect the solution.
The patch replaces the exception with the assumption that the bundle name is the same as the entity name. That is, as in our example, if a user entity is passed to entity_extract_ids() with no bundle information, we will assume that the bundle name is 'user', even if there are multiple bundles for this entity.
This will allow 'bundle-unaware' modules that handle entities to avoid failure if they are later adapted by contrib modules to have multiple bundles.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| Drupal-allow_user_bundles.patch | 864 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch Drupal-allow_user_bundles.patch. Unable to apply patch. See the log in the details link for more information. | View details | Re-test |
Comments
#1
This is a feature request and needs to go against d8 - I don't understand why a new ticket was created rather than using the existing issue.
#2
Sorry, I should have explained that above. This is surely a bug. Why shouldn't users be able to have bundles? Everything else is set up to have bundles, but it appears that rather than there being a conscious decision to write code that says "users may not have bundles", it's a bug by omission, and should be fixed!
As a bug, this is a separate ticket from the feature request in D8.
#3
I asked on IRC about whether this could be in D7 and there isn't a definitive "no" but it sounds like it's a risky API change and unlikely unless there's some feedback from the entity experts as to whether this would cause problems making this change in D7. I'm going to leave the settings alone and just add this note but, if it's veto'd for D7, then I believe this is a duplicate of the issue for making users classed entities? Surely that would allow for bundles?
Michelle
#4
user_entity_info() does not have a bundle key defined therefore the code you are changing
if (!empty($info['entity keys']['bundle']))is not firing. If you are doing ahook_entity_info_alterto add the missing bundle key, stored another table (much like comment and taxonomy does) that's absolutely fine and then you need to populate that onhook_entity_load. So I see no bug here.#5
Entities cannot have more than one bundle in D7 at all and that cannot change, so this cannot be backported to D7.
#6
I think the real issue here is that drupal_anonymous_user() is hard-coded and is not possible to 'add' a bundle to that object. There is no way to alter that object from when it is called in user_register_form() and assigned to $form['#user'] and from when field_attach_form() is called.
#7
So the problem is that if you add a bundle key you can't add that to the anonymous user because it's hardcoded. In fact, if you do things like schema_alter and entity_info_alter then other problems might arise, so http://paste.pocoo.org/show/536796/
#8
chx in IRC:
#9
What would there be need for in order for the solution of the issue to go on?
#10
This is simple patch, no test.
#11
Bot testing…
#12
small test module:
<?php
/**
* Implements hook_menu().
*/
function userentitytest_menu() {
$items = array();
$items['admin/config/people/accounts/userentitytest'] = array(
'title' => 'User Entity Test',
'page callback' => 'userentitytest_page',
'access callback' => TRUE,
);
return $items;
}
/**
* Implements hook_entity_info_alter().
*/
function userentitytest_entity_info_alter(&$entity_info) {
$entity_info['user']['bundles']['userentitytest'] = array(
'label' => t('User Entity Test'),
'admin' => array(
'path' => 'admin/config/people/accounts/userentitytest',
)
);
}
/**
*
*/
function userentitytest_page() {
return 'foobar';
}
#13
Since user.module provides a default entity, I would propose this solution:
1) Use whatever object is specified for the user entity. Not stdclass and not a variable defined in $conf in settings.php
2) user.module provides a default bundle, 'user'.
3) To allow this bundle to be altered away, we could add a 'default bundle' key or something along those lines specifically to the user entity. Such a key might be generically useful as a way to determine what the default bundle should be on new entities created without a bundle. Even if not used generically, that would solve our problem.
4) Then we could just create a new user entity of the default bundle, and set it up appropriately.
5) Alternatively we could simply decide that the bundle cannot be altered away and require that the 'user' bundle exist on the users and completely fail if some hook_entity_info_alter tries to remove it.
While I'm not going to mark this 'needs work' because I'm proposing something completely different, I would encourage significant thought along these lines. The solution here is very hacky and I think we can solve this gracefully with just a little more work.
#14
The #11 patch is very little and dirty trick, but (theoretically) work in Drupal 7. Drupal 8 give nicer solution.
Patch and test is coming soon…
#15
We still need to fix it in D8 first. We can also apply the D7 fix to D8 first and then improve D8 later, but the issue needs to be fixed in D8 first to prevent a regression. Thanks!
#16
#17
I'm bad, this link deleted.
Drupal 8 related issue: #1361228: Make the user entity a classed object
Issue to go in Drupal 7?
#18
I think, this is issue is fixed in Drupal 8: #1361228: Make the user entity a classed object (committed: http://drupal.org/node/1361228#comment-5919814)
What is needed for this yet?
#19
i doubt that #1361228: Make the user entity a classed object will get backported..so maybe #11 could go in d7
#20
Maybe :) let's try.
#21
When I look at the drupal_anonymous_user() function in Drupal 8, I still see this:
<?phpfunction drupal_anonymous_user() {
$user = new stdClass();
...
?>
So the same patch is not in Drupal 8.
I see that the usage in user_register_form() is fixed in Drupal 8... so if we wanted a Drupal 7-only solution perhaps we could do something just targeted at that form. But if we're allowing the class to change everywhere drupal_anonymous_user() is called, that would have to go in Drupal 8 first.
In principle I don't see anything wrong with the variable_get() solution (in either location) for D7 backport though.
#22
i got simmilar project here : http://drupal.org/project/user_categories
#23
What would there be need for in order for the solution of the issue to go on?
#24
This was fixed for D8 in #1634280: drupal_anonymous_user() should return a User entity, so this issue only needs to take care of D7 now.