Hi, I've put some love in the testcase. It does simple checking of email submission and account blocking/deleting operations, but doesn't goes really far. Email content is not checked, neither if {inactive_user} table entries are clean, just testing the basics for regular operation.

I've fixed some minors generating warnings, and a language issue in the email generation part. The hardest changes are the fixes for the block/delete operations, they were not working in my setup, neither in my tests. Please, review this part carefully, because I've modified some db_query conditions. The tests only work for me with these modifications, otherwise emails are not sent, or the account is not blocked/deleted.

Crell was right, code scares the first time! ;)

Hope this one gets committed, so I can put more love into the tests, with more focused checks.

CommentFileSizeAuthor
inactive_user.test.patch21.57 KBilo

Comments

bryancasler’s picture

bryancasler commented

subscribe

ilo’s picture

ilo commented
Priority: Normal » Major

go to top of the list again! Some issues in the queue could be solved at once:

#77835: Inactive_user does not delete users
#85668: Nothing happens, nothing written to database
#504038: Users are NEVER removed from inactivity list
#850776: No effect

deekayen’s picture

deekayen commented
Status: Needs review » Fixed

committed to HEAD

alliax’s picture

alliax commented

HEAD doesn't work with Drupal latest stable version 6.19 (drupal puts a red cross in modules, doesn't even let me activate the module), so it would be good to have a new release of the module, no? Or at least a 6.x DEV version that would work with Drupal 6. I guess HEAD doesn't work because you started to do the D7 upgrade.

In the meantime I will put the only stable version of the module, I just want to send reminders, I don't need to delete users now, although it would be a nice feature, but I know eventually you'll update the stable version, so I'll update it.

ps: I don't change the status of this issue in order not to bother you, but in reality it is still an active issue since HEAD version cannot be used with drupal 6.

deekayen’s picture

deekayen commented

#661718: Create DRUPAL-6--1 Branch for D6 development rolled 6.x-1.x-dev into the DRUPAL-6--1 branch. I just created a 6.x-1.x-dev nightly tarball release and switched the HEAD tarball to show as 7.x compatible.

alliax’s picture

alliax commented

Thank you for the fast reaction, it's a pity we have to wait for the nightly build in order to download the new 6 dev, when we don't know how to do it another way, I'm sure it's possible to get it with tools like cvs, but I never used. I can wait it's not so time-sensitive :-)

Status: Fixed » Closed (fixed)

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

jkarlsson’s picture

jkarlsson commented
Status: Closed (fixed) » Needs work

I just noticed this, great work Ilo!

However, there's one possibly evil gotcha in this patch: the interpretation of the warn users before block/delete variable has changed.

The queries for warning users before blocking/deleting has been changed from

db_query("... access < (%d - %d - %d) ...", time(), $warn_time, $block_time)

to

db_query("... access < (%d - %d) ...", time(), $warn_time)

Before this change, a value of 1 week for 'inactive_users_auto_block_warn' meant "Warn users 1 week before they are blocked" whereas it now means "Warn users after one week of inactivity".

One can argue that this new interpretation makes more sense and I could probably agree. However, this kind of change needs to be highlighted to upgrading users.