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.
Currently, the module is set up only for nodes.
Comment | File | Size | Author |
---|---|---|---|
#81 | serial-generic_entity_support-1333032-82.patch | 27.01 KB | BR0kEN |
#80 | interdiff-79-80.txt | 620 bytes | BR0kEN |
Comments
Comment #1
colanMarked #1261548: Add support for serial in field_collection_item entity as a duplicate of this issue.
Comment #2
colanMarked #1337408: Not compatible with 'Field Collection' as a duplicate of this issue.
Comment #3
colanMarked #1352270: Support for Field Collections as a duplicate of this issue.
Comment #4
colanBumping this up to major, as it's collecting a lot of duplicates. Here's another: #1469412: serial field does not work in user table
Comment #5
kaizerking CreditAttribution: kaizerking commentedwaiting for this feature where i can assign serial number to user
Comment #6
drvdt CreditAttribution: drvdt commentedWhy 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,
Comment #7
colan@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.
Comment #8
MM10 CreditAttribution: MM10 commentedColan-- Any broad suggestions or pointers on how to best generalize it?
Comment #9
colanThis 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.
Comment #10
drvdt CreditAttribution: drvdt commentedDid any body try #9?
Comment #11
andyhu CreditAttribution: andyhu commented+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 :)
Comment #12
j4 CreditAttribution: j4 commentedI 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
Comment #13
kaizerking CreditAttribution: kaizerking commented@j4, What you did?tried on entities?, please post it here we will test
Comment #14
j4 CreditAttribution: j4 commentedI 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
Comment #15
kaizerking CreditAttribution: kaizerking commentedI 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.
Comment #16
j4 CreditAttribution: j4 commentedWill report back if i succeed..thanks
J
Comment #17
lukusI'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?
Comment #18
colan@lukus: It's open for you to assign to yourself, and start working on it. Thanks! Looking forward to your patch. :)
Comment #19
lukus@colan - I'm going to give this a go, I'll report back later today with any update.
Comment #20
lukus@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.
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?
Comment #21
lukusComment #22
lukusHere'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:
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.
Comment #23
lukusComment #24
colanFirst of all, thanks so much for taking this on.
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.
Comment #25
lukusCheers 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.
Comment #26
lukusHi
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?
Comment #27
kaizerking CreditAttribution: kaizerking commented@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
Comment #28
lukus@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'.
Comment #29
kaizerking CreditAttribution: kaizerking commentedI 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
Comment #30
lukus@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.
Comment #31
lukusHere'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.
Comment #32
kaizerking CreditAttribution: kaizerking commentedNice,patch @ #31 worked for me, thanks
Comment #33
lukusThanks kaizerking.
Can anyone else provide any feedback?
Comment #34
PipB CreditAttribution: PipB commentedWith 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).
Comment #35
PipB CreditAttribution: PipB commentedI 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
Comment #36
ytsejam CreditAttribution: ytsejam commentedApplied 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.
Comment #37
lukusHi .. I'll be working on this later today.
Just to confirm, the error occurs on creation of a new user account?
Comment #38
ytsejam CreditAttribution: ytsejam commentedYes. And after the error, a new user is not created. Looking forward to an update, thanks in advance!
Comment #39
serguitus CreditAttribution: serguitus commentedNo new results yet?
Comment #40
HLopes CreditAttribution: HLopes commentedThere'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.
Comment #41
ikeigenwijs CreditAttribution: ikeigenwijs commentedIs there progress? is this rolled in the .dev?
Comment #42
hoporr CreditAttribution: hoporr commentedMarked https://drupal.org/node/1552280
Profile Serial field - Undefined property: Profile::$nidas duplicate of this.Comment #43
avalot CreditAttribution: avalot commentedVery 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?
Comment #44
tien.xuan.vo CreditAttribution: tien.xuan.vo commentedHere is my patch, base on #31 's patch.
Changes:
Some notes:
Tested with:
Please review my patch. Sorry for my English!
Comment #45
tien.xuan.vo CreditAttribution: tien.xuan.vo commentedChanges:
Comment #46
PROMESIt works for me.
Thanks
Comment #47
joachim CreditAttribution: joachim commentedThis 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!
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_'.)
Use entity_load() to avoid a dependency on entity module.
This didn't make that much sense beforehand, but it makes even less now.
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.
That looks like an unrelated fix; should be tackled in its own issue.
That comment doesn't look right. 'weights'?
Comment #48
ron_s CreditAttribution: ron_s commentedThis 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:
With this:
... 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:
The result is no change to the entity's path. When looking at the
$string
pushed intopathauto_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 toserial_tokens
to skip processing the code if possible conflicting modules exist.Comment #49
ron_s CreditAttribution: ron_s commentedAttached is a new patch for review. The old patch can no longer be applied to the current -dev. Updates from the previous version:
entity_load
preg_replace
inserial_update_7130
to remove the unneeded extra stepdrupal_goto
included inserial_form_alter
serial_field_attach_rename_bundle
serial_tokens
to check if the serial field is used in a pathauto pattern. If so, skip the token replacement.@joachim, one point I didn't understand from your previous comments was this:
Can you explain further? This is related to the
_serial_get_all_fields
function inserial.inc
. The only place I see this being referenced is inhook_schema
inserial.install
. Are you suggesting theserial_schema
code should be removed along with_serial_get_all_fields
?I look forward to feedback so we can make any further adjustments and get this committed.
Comment #50
ron_s CreditAttribution: ron_s commentedChanging status.
Comment #55
ron_s CreditAttribution: ron_s commentedSorry, I had forgot to update a function reference in the previous version that did not match the original patch. Needed to change
_serial_init_old_nodes
to_serial_init_old_entities
in theserial_field_create_instance
function.Let's try this one.
Comment #56
ron_s CreditAttribution: ron_s commentedOk, one more update. I realized the newest -dev is using
LANGUAGE_NONE
rather than'und'
in_serial_init_old_nodes
, which is what the patch should have too.Have updated to include this.
Comment #61
jsibley CreditAttribution: jsibley commentedHi. Is there any chance of getting the most recent patch, which seems to have passed, committed?
Thanks.
Comment #62
colanAs soon as folks have a chance to review the code and test it (and the state is RTBC), I'll happily commit this.
Comment #63
BR0kENMaybe "Rename serial table(s) when an entity bundle was renamed."?
Missed param type.
Missed param type.
Missed param type.
Missed dot at the end.
Missed param type.
Missed dot at the end.
Missed param type.
Missed dot at the end.
Remove this line.
Comment #64
BR0kENWill not work without "Entity API" module (undefined function).
Comment #65
BR0kENWill not work with "comment" entity that have "node_type" value of "bundle" entity key and it's does not exist in schema.
Comment #66
BR0kEN$data[$entity_type]->type
- will not work with "comment" entity that have "node_type" property instead of type.Comment #67
BR0kENComment #68
BR0kENComment #69
joachim CreditAttribution: joachim commentedYou can use entity_extract_ids() here instead. That gets you the bundle too.
Comment #70
BR0kENComment #71
BR0kENMaybe anybody leave some comments?
Comment #72
ron_s CreditAttribution: ron_s commented@BR0kEN, I haven't had a chance to test the update yet. Hoping that's something I can do in the next week or two, and will let you know if we identify any issues.
We have been running patch #56 for a while. The one issue we've recently noticed is every few weeks, there will be a non-unique serial ID. For example there will be an entity created with serial ID of 382, and then the very next entity will also have a serial ID of 382. Happens extremely rarely, but is certainly an issue that needs to be resolved. Seems like it might be a situation where the first entity is being edited while the second is created. Any thoughts? I haven't yet investigated it any further.
Thank you for your efforts.
Comment #73
colanMaybe we need to wrap that code in a transaction?
Comment #74
ron_s CreditAttribution: ron_s commented@colan, I think so, would make sense.
Comment #75
BR0kEN@colan, "that code"? Which?
Comment #76
colanI guess whichever code that can't be running more than once at the same time that causes the same ID to show up multiple times. I haven't looked at this in a while, so not sure. Just making a suggestion for whomever's working on this.
It could be outside the scope this issue. Maybe we need to create another ticket for it? Does it happen without this patch as well? These two things could be unrelated. So we need to solve the race condition, but maybe not in this ticket. If that's the case, we can move ahead with this one if there are no other problems.
@ron_s: Were you having the multiple ID problem before this patch?
Can anyone else (dis)confirm?
Comment #77
ron_s CreditAttribution: ron_s commented@colan, we're exclusively using the Serial module with entities. Therefore our only baseline is use of Serial with this patch.
You could be right that the race condition is outside the scope of this issue. Unfortunately I don't have data to confirm or deny. We'd need someone else to respond to know for sure.
Comment #78
colan@ron_s: I just found #2455677: Getting Duplicate numbers and gaps in the sequence order so I'm assuming it's not related to this. Please head over there for that.
Let's wait for another positive review or two here before committing.
Comment #79
BR0kENLet's try this one. But, want to say that these improvements must not be committed in this thread.
Comment #80
BR0kENJust code style improvements.
Comment #81
BR0kENJust merge with upstream. Unable to provide an interdiff, because file is reconstructed according to #2144389: Doesn't work with node_clone.
Comment #83
MustangGB CreditAttribution: MustangGB commentedSo this is fixed now right?
Comment #84
ron_s CreditAttribution: ron_s commentedYes.
Comment #85
colan@BR0kEN: How do you feel about cutting a new release? If we get this one published, we can stop talking about it on the project page.
Comment #86
BR0kEN@colan, I guess we should update the project page. As for me - everything is fine.
Comment #87
colanLooks like you already updated the project page. I just cut 7.x-1.6 to match.
Comment #88
kaizerking CreditAttribution: kaizerking commentedlukus @27 , i am sorry for being away for too long. Do you still think i should open each point as separate feature reuest , please let me know i would do so
Comment #89
MustangGB CreditAttribution: MustangGB commented@colan
The release is broken, see: #2710301: 7.x-1.6 and 7.x-1.7 updates do not work with prefixed tables.