I noticed that the count was not incrementing for me when users logged in -- it always stayed at 1. Looking at the code, it appears that the reason is that the cache isn't set, and always returned a value that evaluated to 0.

To fix this, I removed the code getting the value out of the cache, and modified the update statement as follows:

db_update('user_stats_values')
->expression('value', 'value + :value', array(':value' => 1))
->condition('name', 'login_count')
->condition('uid', $uid)
->execute();

I based it on this: http://api.drupal.org/api/drupal/includes--database--database.inc/functi...

It appears to work for me, so I'll attach the patch in case it helps others too. I didn't modify the decrement case, but you could apply the same principle if needed.

Thanks for the plugin!

CommentFileSizeAuthor
#1 login_increment-1326908.patch1.45 KBjuliakoelsch

Comments

juliakoelsch’s picture

StatusFileSize
new1.45 KB

Patch attached...

liam mcdermott’s picture

Status: Active » Needs review

Thanks very much for the patch Julia!

I added code to decrement the login count and committed your patch, it's not tested though (as I'm lacking time) so it would be great if you could test it and make sure it's incrementing at least.

A couple of things about the patch: 1) generally people generate patches for modules from the module's directory (meaning the user_stats directory, where user_stats.module is) not the Drupal root; 2) there were some random spaces added to the end of one of the lines (along with the perfectly valid indentation fixes you also did).

I fixed those two tiny things, but thought you'd like to know for when you're generating patches in future. :)

Thanks again!

juliakoelsch’s picture

I tested pretty thoroughly on a dev site, and now the updated code is on a live site, and it's incrementing correctly and without error. So I think the update should be ready to go.

Sorry about the patch issues -- I thought I followed this: http://drupal.org/node/707484, but maybe I strayed somehow. May I ask -- what about the patch made you think that I patched it from the Drupal root? I'm not even sure how one would do that. I just want to make sure I don't repeat the same mistake. Thanks!

liam mcdermott’s picture

Status: Needs review » Fixed

Sorry for not replying to this sooner, Julia. If you open the patch it contains lines like:

--- a/sites/all/modules/user_stats/user_stats.module
+++ b/sites/all/modules/user_stats/user_stats.module

Also, IIRC, it wouldn't apply from inside the module's directory, once I moved it to the Drupal root all was well. I'm not sure how you managed to do it, though it's no biggie. :)

Also marking as fixed, since I'm pretty sure I committed this. Thanks for the patch!

Status: Fixed » Closed (fixed)

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