Role expiration isn't set during admin user add/create, only during edit.

Confirming bug. Subscribing.

subscribe.

Per the api documentation for hook_user(), I moved the db modification code from the 'submit' case to the 'insert' case (http://api.drupal.org/api/function/hook_user/6).

This patch also:

• Sets the respective fields in $edit to NULL per the api docs.
• Refactors the variables to optimize the code a bit. Didn't see a good reason to assign the $edit values to new variables only to delete them later.

Note that this patch does not enable the default expiration feature because:

• The feature wasn't represented in the submit code in the first place.
• In the update code and the UI input tips the meaning of an empty role expiration value is ambiguous, so the specifications need clarification.

So, you must manually specify role expiration values when creating the user. In order to see the role expiration fields on the create user form, you may need to apply this patch first: #647866: Expire textfields do not display on admin create user .

I am using this module to create a "demo" user. The user registers, and is automatically assigned a "demo user" role (autoassignrole).

If you pre-populated the role expiration date with a date based on a rule created in the admin area - would this not negate the ambiguous nature of the input field? (ie null = never expires/valid date == expiry date). This would then allow for users to have "demo" periods.

This would also apply to modules that automatically batch create users.

Allowing admins to properly pre-populate default expiry lengths would massively increase the modules target audience. As would creating a finalised port to Drupal 5.x

If you pre-populated the role expiration date with a date based on a rule created in the admin area - would this not negate the ambiguous nature of the input field? (ie null = never expires/valid date == expiry date). This would then allow for users to have "demo" periods.

This would also apply to modules that automatically batch create users.

Allowing admins to properly pre-populate default expiry lengths would massively increase the modules target audience. As would creating a finalised port to Drupal 5.x

Yeah, there's already a "default value" feature, but it doesn't engage in all cases. See #649256: Default expiration when creating new users.

subsequently I have removed the "autoassignrole" module and have set up Workflow-NG rules that will add the user role upon registering. This has sorted my problem here (ie the default expiry date is used.)

Hi there,

With this patch, does it mean that when a new user registers, and there is a function already in place to auto assign a role (I'm using Registration Codes http://drupal.org/project/regcode ), the default expiry for that role will now be applied? I had originally thought it was a bug with that module, but a developer there pointed me to this patch. ( http://drupal.org/node/681860 )

Adam Farden
adam@farden.cz
www.farden.cz

@adfad666:

With this patch, does it mean that when a new user registers, and there is a function already in place to auto assign a role (I'm using Registration Codes http://drupal.org/project/regcode ), the default expiry for that role will now be applied?

If I recall correctly, this patch only changes what happens when an admin creates a user. Maybe what you want is as described in #7.

I don't like the way the module is written, it shouldn't worry about who or why the role is being assigned ... on hook_user, it should just check if the roles have changed and act accordingly.

@aidanlis Of course you are free to use other modules.

I'd love to hear how else you would implement this module and of course we accept patches. I currently think that using hook_user is the most sensible way to see if anything has changed the roles for any user. Any other module might do user_load() or user_save() after manipulating the roles. If role_expire didn't have a hook for those other modules then it would miss changes.

You say "it should just check if the roles have changed and act accordingly." How else would you approach it?

@stewsnooze Your comments are not particularly constructive. This is the only module offering role expiration, and there's no reason to duplicate efforts. That said, to get around the shortcomings of the role_expire module I've released a new version of regcode module, which now provides the functionality to expire roles automatically assigned by the module, and added functionality for a fixed date, rather than just a duration. I'd love to be able to remove this extra code from regcode at some point when role_expire matures.

Your point about whether or not to use hook_user is moot: there's no other way to do it, and I don't understand why you'd want to. The point is, rather than checking the edit variable for role_expire fields, the algorithm should be:

if roles_have_changed
for each new_role
if expiry_value_in_edit_variable_exists then assign_role_expiry
else default_value_exists then assign_role_expiry
end
end

Can anyone foresee any problems with this? Have I missed something?

@aidanlis:

I looked at the code to try to understand how it works compared to your suggestion, but didn't get a good feel for the way it is currently.

I don't fully understand what what problem you're trying to solve.

kentr, that's half the problem my algorithm would solve - simplicity.

The way it works currently is scanning the edit array for the presence of any role_expire fields - this is just unnecessary.

Thanks for the clarification. It appears that your suggestion is independent of this issue. If so, would you please open another issue with your suggestion so we can get this one resolved more quickly?

Thanks,

Kent

kentr, my suggestion would solve this bug ... In fact I believe my suggestion is the only way this bug should be solved!

Do you find something wrong with the proposed solution such that you wish to block it?

If so, please do submit a patch. Let's just get it resolved as quickly as we can...

Thanks,

Kent

The proposed solution in #4 doesn't fix all the other issues regarding integration with other modules, and the module authors still haven't weighed in on my proposal. I'm happy for them to decide which way they want to solve this problem, e.g. incrementally via your patch and rewriting later, or rewriting now. I agree with you that getting this resolved quickly is optimal.

In the patch you have said...

+ // We go over all existing roles, because use might have disabled a role.

Should "use" be "user"?

Otherwise this patch looks good. I am going to do a little more testing on it. For instance with migrate module migrating users in, as I can migrate a few thousand users with roles in.

In the patch you have said...

+ // We go over all existing roles, because use might have disabled a role.

Should "use" be "user"?

That line came from another spot in the file, so I am uncertain. But from looking at the code the line precedes, I think it's safe to say it should be "user".

Thanks to all for your attention to this.

Subscribe.

Your patch, after a little tweaking for notices goes in and roles with explicit dates set now get their expiry date. However roles with default duration don't. I am working on that now.

This is in 6.x.1.6

eosrei & kentr.

It would be good if you could test e.t.c. I'll mark this as fixed for now

so for version 6 , I should use the rules module - http://drupal.org/project/rules- instead of autoassign to assign the roles and then the role expire will work (add the expiration date automatically when the people register ) ?

but I cant find an example of how to use the rules module to assign roles upon registration...

When I auto assign the role upon registration with autoassign it doesnt have the role expiration automatically put in .

I have to delete the role, save it, then edit again, add the role and then it automatically puts in the role expiration.

has anyone come up with a solution for this? maybe a daily cron that checks for people with that role who dont have the role expiration set yet?

I made a cron script to fix this:
http://drupal.org/node/1420070#comment-6635516

I modified the role_expire_module so now it adds the expiration date for the role that has been auto assigned everytime the cron runs
http://drupal.org/node/649256#comment-6640094

Drupal 6 version is unsupported!