When a user has an expiration it is not possible to remove it - the checkbox stays checked.

I have confirmed the bug on a local install and additionally on simplytest.me. The problem is that in the function user_expire_set_expiration() there is a test on $account->is_new ("if (!isset($account->is_new))"). On my installs the value is always set to true or false, so the user expire delete code is never executed.

CommentFileSizeAuthor
#4 2377207_user_expire_delete_existing_values_if_unset.patch983 bytesgreggles
#1 user_expire-remove_expiration_time-2377207-1.patch424 bytesvurt

Comments

vurt’s picture

vurt commented
StatusFileSize
new user_expire-remove_expiration_time-2377207-1.patch424 bytes

I have attached a simple patch - maybe it should be a little more complex to avoid PHP warnings in the case that the filed is not set (which I could not reproduce).

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented 
+++ b/user_expire.module
@@ -209,7 +209,7 @@ function user_expire_set_expiration($acc
-    if (!isset($account->is_new)) {

This will throw an warning if the property doesn't exist. It is probably better to doe something like:

(!isset($account->is_new) || !$account->is_new)

vurt’s picture

vurt commented

In in my tests $account->is_new was always set...
But you are correct - your way it is better to avoid potential problems. Thank you.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Needs review
StatusFileSize
new 2377207_user_expire_delete_existing_values_if_unset.patch983 bytes

Here's a patch that combines the work of vurt in #1 and seanB's review in #2.

I've tested it and basically agree that looks right. Would love to get confirmation of that.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Needs review » Fixed

I walked through the scenarios one more time and this seems to perform properly.

Committed with credit for both of you as well. Thanks!

  • greggles committed bd1221b on 7.x-1.x 
    Issue #2377207 by vurt, greggles, seanB: Unable to remove expiration

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.