In testing my changes for the advuser module (D-5) I notice that if I as administrator edit a user, then the Last Access column changes from Never to just a few seconds ago for that user. How important would a patch to resolve this be? I'm willing to chase it if there is enough interest. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
A quick search of the issue queue would have found these two issues (and one or two duplicates). The short answer is this was done intentionally late in the D6 release cycle to fix another bug and backported to D5. http://drupal.org/node/171117 http://drupal.org/node/202909 On Fri, May 2, 2008 at 5:04 PM, Earnie Boyd <earnie@users.sourceforge.net> wrote:
In testing my changes for the advuser module (D-5) I notice that if I as administrator edit a user, then the Last Access column changes from Never to just a few seconds ago for that user. How important would a patch to resolve this be? I'm willing to chase it if there is enough interest.
Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
This happens in user_save(), see http://api.drupal.org/api/function/user_save/5 thus, would require a patch for Drupal core
-----Original Message----- From: development-bounces@drupal.org [mailto:development-bounces@drupal.org] On Behalf Of Earnie Boyd Sent: Friday, May 02, 2008 6:04 PM To: development@drupal.org Subject: [development] User last access
In testing my changes for the advuser module (D-5) I notice that if I as administrator edit a user, then the Last Access column changes from Never to just a few seconds ago for that user. How important would a patch to resolve this be? I'm willing to chase it if there is enough interest.
Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
Quoting "Daniel F. Kudwien" <news@unleashedmind.com>:
This happens in user_save(), see http://api.drupal.org/api/function/user_save/5
thus, would require a patch for Drupal core
You mean <?php // Consider users edited by an administrator as logged in, if they haven't // already, so anonymous users can view the profile (if allowed). if (empty($array['access']) && empty($account->access) && user_access('administer users')) { $array['access'] = time(); } ?> I don't understand the logic of the comment. The same logic exists for the create. I'll do some more research. My awe with the comment is; why do I want to destroy the statistical data when I do an administrative update just so someone I don't know can view the profile if they even have permission to do so? And the same doesn't happen if the user registers himself. My inclination is to just remove this since I consider it silly but I'll do some more research. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Fri, May 2, 2008 at 1:41 PM, Earnie Boyd <earnie@users.sourceforge.net> wrote:
You mean <?php // Consider users edited by an administrator as logged in, if they haven't // already, so anonymous users can view the profile (if allowed). if (empty($array['access']) && empty($account->access) && user_access('administer users')) { $array['access'] = time(); } ?>
I don't understand the logic of the comment. The same logic exists for the create. I'll do some more research. My awe with the comment is; why do I want to destroy the statistical data when I do an administrative update just so someone I don't know can view the profile if they even have permission to do so? And the same doesn't happen if the user registers himself. My inclination is to just remove this since I consider it silly but I'll do some more research.
Users who register themselves and never log in should not have public user pages created since they are empty, useless, and possibly contains spam. Admins can view those user pages. This would be confusing for admins creating users. When admins do things, stuff should happen, those users need to show up. The way we store this is by overriding the access time. Users creating their account get a time of 0, which means the account registration is not finished and the user page does not go live. Admins creating accounts get a timestamp. A couple options: * Make admin-created users get a access time bump, but not admin-edited * Remove the column overriding and make an is_active column or somesuch. Bonus points for merging with the status column. -- Neil Drumm http://delocalizedham.com
On Fri, 2 May 2008 16:13:51 -0700, "Neil Drumm" <drumm@delocalizedham.com> wrote:
A couple options: * Make admin-created users get a access time bump, but not admin-edited
If it's determined to be a bug, that's probably the best solution for Drupal 5/6.
* Remove the column overriding and make an is_active column or somesuch. Bonus points for merging with the status column.
Amen to making the status column more useful than just binary. There's lots of other statuses a user could have. (I have a use case where I want to admin create users that are visible to other users but can't log in; they represent former members of the organization that we need to be able to reference for historical data.) Of course, then the question is whether it's better to: A) Make status have a lot of different states with int/consts. B) Make status a bitmask field (compact but harder to query). C) Break status out into a series of is_foo fields (possibly a lot of needless data). --Larry Garfield
Quoting Larry Garfield <larry@garfieldtech.com>:
A) Make status have a lot of different states with int/consts. B) Make status a bitmask field (compact but harder to query). C) Break status out into a series of is_foo fields (possibly a lot of needless data).
I like option B for this. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
Remember that MySQL cannot truly index bitmasks. Consider a series of CHAR(0) columns, which would take the same space but be indexable. -----Original Message----- From: Earnie Boyd <earnie@users.sourceforge.net> Date: Sat, 03 May 2008 09:59:43 To:development@drupal.org Subject: Re: [development] User Status [WAS: User last access] Quoting Larry Garfield <larry@garfieldtech.com>:
A) Make status have a lot of different states with int/consts. B) Make status a bitmask field (compact but harder to query). C) Break status out into a series of is_foo fields (possibly a lot of needless data).
I like option B for this. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Sat, 03 May 2008 10:59:42 -0400 Aaron Winborn <winborn@advomatic.com> wrote:
i agree here. the storage would be trivial, considering how much other data we store anyway. even if you have 500k users, that would add what? an additional 8 char(0) fields would be something like .5M, but the db would still be so large that that's a small percentage.
David Strauss wrote:Remember that MySQL cannot truly index bitmasks. Consider a series of CHAR(0) columns, which would take
char(0) wouldn't it be char(1)
the same space but be indexable.
I agree in choosing the data that best represent what you're putting in. Beware that char(1) is UTF8 char that may involve a bit of overhead and is not invariant for ordering on change of locale. The smallest int in SQL92 standard is smallint aka int2 that is 2 bytes. Many DB support aggregation of boolean data type, that mean you're not going to waste 1 byte for each boolean. Many DB have aggregate function for boolean too. Modern CPU have a strange relationship with data smaller than one word. -- Ivan Sergio Borgonovo http://www.webthatworks.it
No, char(0). MySQL uses one bit to store the NULL/not NULL state. -----Original Message----- From: Ivan Sergio Borgonovo <mail@webthatworks.it> Date: Sat, 3 May 2008 19:24:27 To:development@drupal.org Subject: Re: [development] User Status [WAS: User last access] On Sat, 03 May 2008 10:59:42 -0400 Aaron Winborn <winborn@advomatic.com> wrote:
i agree here. the storage would be trivial, considering how much other data we store anyway. even if you have 500k users, that would add what? an additional 8 char(0) fields would be something like .5M, but the db would still be so large that that's a small percentage.
David Strauss wrote:Remember that MySQL cannot truly index bitmasks. Consider a series of CHAR(0) columns, which would take
char(0) wouldn't it be char(1)
the same space but be indexable.
I agree in choosing the data that best represent what you're putting in. Beware that char(1) is UTF8 char that may involve a bit of overhead and is not invariant for ordering on change of locale. The smallest int in SQL92 standard is smallint aka int2 that is 2 bytes. Many DB support aggregation of boolean data type, that mean you're not going to waste 1 byte for each boolean. Many DB have aggregate function for boolean too. Modern CPU have a strange relationship with data smaller than one word. -- Ivan Sergio Borgonovo http://www.webthatworks.it
On Sat, 3 May 2008 18:04:33 +0000 "David Strauss" <david@fourkitchens.com> wrote:
No, char(0). MySQL uses one bit to store the NULL/not NULL state.
It is not portable. PostgreSQL and MS SQL as an example doesn't support char(0). + the other considerations I made. -- Ivan Sergio Borgonovo http://www.webthatworks.it
But they probably support another bit-based format. -----Original Message----- From: Ivan Sergio Borgonovo <mail@webthatworks.it> Date: Sat, 3 May 2008 21:05:41 To:development@drupal.org Subject: Re: [development] User Status [WAS: User last access] On Sat, 3 May 2008 18:04:33 +0000 "David Strauss" <david@fourkitchens.com> wrote:
No, char(0). MySQL uses one bit to store the NULL/not NULL state.
It is not portable. PostgreSQL and MS SQL as an example doesn't support char(0). + the other considerations I made. -- Ivan Sergio Borgonovo http://www.webthatworks.it
On Sat, 3 May 2008 19:12:46 +0000 "David Strauss" <david@fourkitchens.com> wrote:
But they probably support another bit-based format.
The smallest thing you can find in SQL92 I think is boolean. As said most DB support bit packing and some have aggregate functions over boolean fields. char doesn't look as a good choice because on UTF8 may have some overhead, it is ordered according to locale and in need it misses interesting aggregate functions. I don't know SQL standards and enough implementations to know if you can avoid both problems in most DB. Some DB let you define collation and encoding on a field base... some don't even let you define the encoding/collation on a table base. The smallest SQL92 int is 2 bytes (smallint) someone may think it is too large but it is very well handled by modern CPU and is not affected by locale nor encoding... if you've more statuses... -- Ivan Sergio Borgonovo http://www.webthatworks.it
And we could always use char(1). Oracle is the only DB that doesn't distingish "" from NULL. -----Original Message----- From: Ivan Sergio Borgonovo <mail@webthatworks.it> Date: Sat, 3 May 2008 21:05:41 To:development@drupal.org Subject: Re: [development] User Status [WAS: User last access] On Sat, 3 May 2008 18:04:33 +0000 "David Strauss" <david@fourkitchens.com> wrote:
No, char(0). MySQL uses one bit to store the NULL/not NULL state.
It is not portable. PostgreSQL and MS SQL as an example doesn't support char(0). + the other considerations I made. -- Ivan Sergio Borgonovo http://www.webthatworks.it
-----Original Message----- From: Earnie Boyd <earnie@users.sourceforge.net>
Date: Sat, 03 May 2008 09:59:43 To:development@drupal.org Subject: Re: [development] User Status [WAS: User last access]
Quoting Larry Garfield <larry@garfieldtech.com>:
A) Make status have a lot of different states with int/consts. B) Make status a bitmask field (compact but harder to query). C) Break status out into a series of is_foo fields (possibly a lot of needless data).
I like option B for this.
Quoting David Strauss <david@fourkitchens.com>:
Remember that MySQL cannot truly index bitmasks. Consider a series of CHAR(0) columns, which would take the same space but be indexable.
I was thinking more of an integer field used as a bitmask. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
Quoting Neil Drumm <drumm@delocalizedham.com>:
On Fri, May 2, 2008 at 1:41 PM, Earnie Boyd <earnie@users.sourceforge.net> wrote:
You mean <?php // Consider users edited by an administrator as logged in, if they haven't // already, so anonymous users can view the profile (if allowed). if (empty($array['access']) && empty($account->access) && user_access('administer users')) { $array['access'] = time(); } ?>
I don't understand the logic of the comment. The same logic exists for the create. I'll do some more research. My awe with the comment is; why do I want to destroy the statistical data when I do an administrative update just so someone I don't know can view the profile if they even have permission to do so? And the same doesn't happen if the user registers himself. My inclination is to just remove this since I consider it silly but I'll do some more research.
Users who register themselves and never log in should not have public user pages created since they are empty, useless, and possibly contains spam. Admins can view those user pages.
This would be confusing for admins creating users. When admins do things, stuff should happen, those users need to show up.
This isn't nearly as confusing as the access time being updated because I create or edit a user. Precious data is lost and using a column that was meant to mean last access for is active isn't a good thing.
The way we store this is by overriding the access time. Users creating their account get a time of 0, which means the account registration is not finished and the user page does not go live. Admins creating accounts get a timestamp.
Well, not before the patch. The admin creating and editing left the access NULL or 0 before the patch sometime last November.
A couple options: * Make admin-created users get a access time bump, but not admin-edited * Remove the column overriding and make an is_active column or somesuch. Bonus points for merging with the status column.
Why isn't the user module looking at the status column for its data that created the patch at node/171117 that we're discussing? The wrong fix was applied to a problem that caused issues with statistical data and upset a number of people. Why does the fact that the user has accessed the system matter to the functioning of anonymous users being able to "access user profiles"? Destroying the statistical data was a workaround and as a workaround should never have been a patch. We need to revert this behavior in D5 and D6 and then find a real solution to apply that doesn't destroy data used for statistics. I might be happy with only removing the on edit part of the patch but it still isn't giving correct data if I create a user and the access is given as the time of creation because the user hasn't accessed the system. Earnie -- http://for-my-kids.com/ -- http://give-me-an-offer.com/
On Fri, May 2, 2008 at 6:13 PM, Neil Drumm <drumm@delocalizedham.com> wrote:
Users who register themselves and never log in should not have public user pages created since they are empty, useless, and possibly contains spam. Admins can view those user pages.
Oh, thank you for this! I've had so many complaints with advanced profile because accounts where the user never logged in are only accessible to admins. Which is a pain because they show up in the user list and then people get a 403 when they try to visit the profile. I never could figure out what was causing it. So that's by design?! Damn, I guess it won't be fixed, then. :( Well, at least I can tell people to blame Drupal and not my module for the screwiness. :P Unless someone has some suggestion to get around this? It's really a bad user experience to click on a name and end up at a 403 page so it would be nice to fix it if possible. Michelle
participants (9)
-
Aaron Winborn -
catch -
Daniel F. Kudwien -
David Strauss -
Earnie Boyd -
Ivan Sergio Borgonovo -
Larry Garfield -
Michelle Cox -
Neil Drumm