Comments

colan’s picture

colan’s picture

Title:Add general entity support» Add generic entity support

Marked #1337408: Not compatible with 'Field Collection' as a duplicate of this issue.

colan’s picture

Marked #1352270: Support for Field Collections as a duplicate of this issue.

colan’s picture

Priority:Normal» Major

Bumping this up to major, as it's collecting a lot of duplicates. Here's another: #1469412: serial field does not work in user table

kaizerking’s picture

waiting for this feature where i can assign serial number to user

drvdt’s picture

Assigned:Unassigned» drvdt

Why only for node?
This is a great module that needs for many entities. Please improve it.
Is any body know an other same module?
Many thanks,

colan’s picture

Assigned:drvdt» Unassigned

@drvdt: Please do not assign the ticket to yourself unless you plan on working on it.

As the current maintainer, I do not have the time to work on this right now. I only needed it for nodes, but yes, this needs to be generalized. Someone else will have to provide a patch, or pay someone to work on it. Once it's sufficiently tested, I'll commit it.

MM10’s picture

Colan-- Any broad suggestions or pointers on how to best generalize it?

colan’s picture

This is really broad, but search for "node" or "nid" in the code, and then replace with "entity" or "entity_id" respectively. ;) You may need to store the entity type and the entity ID in each table as opposed to just the node ID, but I haven't looked at it that closely in a while. Also, take a look at the field API.

The code isn't that dense, so it shouldn't be too hard to take a look and see what needs doing. Play with it, and see what happens. Using other field modules (that have generic entity support) as examples would be extremely beneficial here.

drvdt’s picture

Did any body try #9?

andyhu’s picture

+1 for this feature, I think in order to get it working for all entities, we need to identify the entity type and find the base table using entity_get_info(), it shouldn't be too hard :)

j4’s picture

I did. But I am a total newbie to php, so am afraid i did not make any progress. But would certainly love to have this feature.

Thanks
Jaya

kaizerking’s picture

@j4, What you did?tried on entities?, please post it here we will test

j4’s picture

I did precisely what was offered as a tip in #9, but it threw up all kinds of errors. Sorry i did not document the errors!

Jaya

kaizerking’s picture

I suggest go through the entity API to see how an entity behaves on entity_create,entity_save,entity_update,entity_delete,I am not coder, so I cannot code, I can understand a bit on entity_hooks.

j4’s picture

Will report back if i succeed..thanks

J

lukus’s picture

I'm interested in using the serial field with the field collection module.

I'd be willing to attempt to convert the module to work with entities if nobody else is working on it?

colan’s picture

@lukus: It's open for you to assign to yourself, and start working on it. Thanks! Looking forward to your patch. :)

lukus’s picture

Assigned:Unassigned» lukus

@colan - I'm going to give this a go, I'll report back later today with any update.

lukus’s picture

Assigned:lukus» Unassigned

@colan

Okay I've started working on this. I've looked over the code, and have produced an overview of functionality. I think I understand what's going on.

It doesn't seem like a huge job - as much of the code already works with the Field API. The main issue occurs when a serial field is applied to an entity with pre-existing content; so I'm going to try to provide a generalised function that takes the place of _serial_init_old_nodes() to generate sids for any entity with pre-existing content.

One other point I've considered;
Field collection entities are generally used with a 'host entity'. Bearing this in mind, eventually I think we might need to be able to choose whether serials are allocated locally or globally. I think this could possibly be done via field UI settings.

ie.

  • locally: sid is unique to each new instance of a field collection (e.g. '1... n' for each host node/entity).
  • globally: sid spans all instances of a field collection bundle (e.g. '1... n' across all host node/entity).

For now I'm going to just work on the latter case, as that's what I currently require.

Does is sound like I'm going in the right direction?

lukus’s picture

Assigned:Unassigned» lukus
lukus’s picture

Here's my patch.

I've tested it with nodes, field collections, users and taxonomy vocabularies - both empty and pre-populated bundles

It's functional, but there's still one thing left to do:

  • if an entity's machine_name is changed the table name isn't updated automatically - e.g. a taxonomy vocabulary. Need to find the relevant hook for this.

I'd strongly recommend not using this on any production site yet - it's very much alpha. If someone could review the code for me, and try it out on a test site, I'd be most appreciative.

I'm going to look over it again tomorrow morning and test it more thoroughly.

lukus’s picture

Status:Active» Needs review
colan’s picture

First of all, thanks so much for taking this on.

if an entity's machine_name is changed the table name isn't updated automatically - e.g. a taxonomy vocabulary. Need to find the relevant hook for this.

How about hook_field_attach_rename_bundle?

The basics of what you've got make sense to me.

There are several coding standards violations, but I'm assuming that these will get cleaned up when you're done. It seems like you're not done because some lines are commented out - these need to disappear in the final version. Also, we need to figure out if that drupal_goto() should stay or go. I'm not sure myself.

I'm not in a good position to test this right now so let's wait and see if anyone else can test and/or comment on the above.

lukus’s picture

Cheers Colan,

I'll work on the generic rename function later today, and I'll also clean up the code so it passes review standards. Will add a new patch once it's done.

lukus’s picture

Hi

I've refactored the code to meet drupal.og coding conventions and I've also added the new generic hook to deal with renaming entity machine names / serial table names.

Could someone please review?

kaizerking’s picture

@lukus nice work

also it would be nice if we have the following
1.a specific number range for each entity type
2.assigning the number range for specific period
3.after expiration of the specific period the assigned number range cannot be used
4.allow prefix to number range ex:AA-12345 or AA/12345
5.allow suffix to the number range 1234/2012
6.Lock:if the same type of entity is being created by another user ? so the serial number shouldn't be assigned unless saved
to achieve the above we need a configuration settings for creating number ranges
hope some one will patch

lukus’s picture

@kaizerking

Thanks for the feedback. Can you confirm whether the patch functions as expected?

Re. the feature additions, I'd recommend adding a new issue/s and to mark them as 'feature requests'.

kaizerking’s picture

I am getting this error when i am trying to create serial field on user accounts
http://localhost/xxxxxx/#overlay=admin/config/people/accounts/fields

There was a problem creating field test: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 'name'
How ever Serial number is created when i create user. the number is not wrong indexing
before creating the user i have total 4 user including user 1
after enabiling serial module i have created one user. but the user number is generated as 2

lukus’s picture

@kaizerking

Thanks for for trying out the patch - much appreciated.

With the User entity, user '0' (anonymous) has no element for the name key by default and this is causing problems in the _serial_init_old_entities() function. For this specific case I think the count needs to begin from the first non-anonymous user.

I'll refine the code and release a new patch later today.

lukus’s picture

Here's the new patch. I've added an exception for the anonymous user - the anonymous user isn't given a serial.

@kaizerking - could you please apply the new patch and delete your serial field (and recreate it). Should be okay now.

kaizerking’s picture

Nice,patch @ #31 worked for me, thanks

lukus’s picture

Thanks kaizerking.

Can anyone else provide any feedback?

PipB’s picture

With patch #31 I still have this error:
Notice: Undefined property: stdClass::$nid in serial_field_insert() (line 83 of /home/xxxxx/domains/xxxxx.xx/public_html/sites/all/modules/serial/serial.module).
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'nid' cannot be null: INSERT INTO {serial_user_field_lidnr_} (nid) VALUES (:db_insert_placeholder_0); Array ( [:db_insert_placeholder_0] => ) in _serial_generate_value() (line 155 of /home/xxxxxxx/domains/xxxxx.xx/public_html/sites/all/modules/serial/serial.inc).

PipB’s picture

I implement the patch from #31 and on another website I insert the serial field into an account, I got this error:
Fatal error: Call to undefined function _serial_init_old_nodes() in /home/xxxxx/domains/xxxxxx.xx/public_html/sites/all/modules/serial/serial.module on line 37

ytsejam’s picture

Applied the patch and got the same error while trying to make a new user account with a serial field:

Notice: Undefined property: stdClass::$nid in serial_field_insert() (line 80 of /xxx/sites/all/modules/serial/serial.module).
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'nid' cannot be null: INSERT INTO {serial_user_field_usernumber} (nid) VALUES (:db_insert_placeholder_0); Array ( [:db_insert_placeholder_0] => ) in _serial_generate_value() (line 155 of /xxx/sites/all/modules/serial/serial.inc).

Any ideas? Thanks in advance.

lukus’s picture

Hi .. I'll be working on this later today.

Just to confirm, the error occurs on creation of a new user account?

ytsejam’s picture

Yes. And after the error, a new user is not created. Looking forward to an update, thanks in advance!

serguitus’s picture

No new results yet?

HLopes’s picture

There's an hardcoded $entity->nid, that obviously isn't set on a user object.

Should work if you use entity_extract_ids to get the entity id instead.

function serial_field_insert($entity_type, $entity, $field, $instance, $langcode, &$items) {
    module_load_include('inc', 'serial');
    $ids = entity_extract_ids($entity_type, $entity);
    $sid = _serial_generate_value($ids[0], $instance['bundle'], $field['field_name']);
    $items = array(array('value' => $sid));
    $entity->$field['field_name'] = $items;
}
ikeigenwijs’s picture

Is there progress? is this rolled in the .dev?

hoporr’s picture

Marked https://drupal.org/node/1552280 Profile Serial field - Undefined property: Profile::$nid as duplicate of this.

avalot’s picture

Very interested in progress on this issue. I need a "member number" field on the user entity, and it's not working right now. How can I help?

tien.xuan.vo’s picture

Here is my patch, base on #31 's patch.

Changes:

  1. Add entity_type to serial tables.
  2. Use entity_get_info and db_query instead of EntityFieldQuery.

Some notes:

  • I can't find serial_field_insert(). I think it was removed from latest code.

Tested with:

  • Node.
  • Entity types from ECK
  • User.

Please review my patch. Sorry for my English!

tien.xuan.vo’s picture

StatusFileSize
new13.03 KB
new2.17 KB

Changes:

  1. Add tokens support for generic entity.
PROMES’s picture

It works for me.
Thanks

joachim’s picture

Status:Needs review» Needs work

This would be a really great improvement!

Patch needs a bit of work though -- mostly just tweaks, though the big think I'm not sure of is adding the dependency on entity module. That's quite a big change!

  1. +++ b/serial.inc
    @@ -170,48 +174,67 @@ function _serial_generate_value($bundle, $field_name, $delete = TRUE) {
    +  $bundle_key = $entity_info['entity keys']['bundle'];

    +++ b/serial.install
    @@ -84,3 +84,15 @@ function serial_update_7130() {
    +    $tablename_without_serial = preg_replace('/^serial_/', '', $tablename);
    +    db_rename_table($tablename, 'serial_node_' . $tablename_without_serial);

    I don't really follow why you're stripping the initial 'serial_' here. If you're using preg_replace() anyway, just do '/^serial_/' to 'serial_node_'.

    (Or be ultra-swanky and use a lookbehind: '/(?<=^serial_)/' to 'node_'.)

  2. +++ b/serial.inc
    @@ -170,48 +174,67 @@ function _serial_generate_value($bundle, $field_name, $delete = TRUE) {
    +      $entity = entity_load_single($entity_type, $id);

    Use entity_load() to avoid a dependency on entity module.

  3. +++ b/serial.inc
    @@ -170,48 +174,67 @@ function _serial_generate_value($bundle, $field_name, $delete = TRUE) {
    - * @return result set containing pairs of (node type name, field name).
    + * @return result set containing of (entity type, entity bundle, field name).

    This didn't make that much sense beforehand, but it makes even less now.

  4. +++ b/serial.info
    @@ -2,6 +2,7 @@ name = Serial
    +dependencies[] = entity

    Do we need to add this dependency? I've only found one use of a function from that module -- entity_load_single(), which can be dispensed with.

  5. +++ b/serial.module
    @@ -68,10 +68,6 @@ function serial_form_alter(&$form, $form_state, $form_id) {
    -
    -    // Go back to Managed Fields:
    -    $type = $form['#bundle'];
    -    drupal_goto("admin/structure/types/manage/$type/fields");
       }

    That looks like an unrelated fix; should be tackled in its own issue.

  6. +++ b/serial.module
    @@ -106,45 +102,67 @@ function serial_node_presave($node) {
    +  // Update the extra weights variable with new information.

    That comment doesn't look right. 'weights'?

ron_s’s picture

This would definitely be great to get incorporated into the module.

I've reviewed the patch and agree with @joachim on his feedback. I don't see a reason why the functionality needs to be dependent on entity. Just replace this line:

$entity = entity_load_single($entity_type, $id);

With this:

$entity = entity_load($entity_type, array($id));

... and should be able to remove the dependency. Also I agree we should just handle the preg_replace as part of one statement rather than two.

One issue I did uncover in my testing is with the serial_tokens function. It's running into a conflict because Serial and another module are both attempting to process the same token. I haven't yet figured out which module is causing the conflict, but my guess is its either Entityform or Pathauto Entity.

When saving the entity in this scenario, we see the following error messages:

Warning: Illegal offset type in isset or empty in pathauto_cleanstring() (line 180 of /sites/all/modules/pathauto/pathauto.inc).
Warning: html_entity_decode() expects parameter 1 to be string, array given in decode_entities() (line 463 of /includes/unicode.inc).
Warning: Illegal offset type in pathauto_cleanstring() (line 223 of /sites/all/modules/pathauto/pathauto.inc).

The result is no change to the entity's path. When looking at the $string pushed into pathauto_cleanstring(), I noticed it is actually an array. This is the same issue that has been documented before as part of Pathauto:

https://www.drupal.org/node/1604766
https://www.drupal.org/node/1246074
https://www.drupal.org/node/1792436

I was able to resolve this by commenting out the serial_tokens function in the patch. My recommendation would be to add some logic to serial_tokens to skip processing the code if possible conflicting modules exist.