Information about '%count reads' in node's footer is shown to unprivileged users. I suggest that function statistics_link in statistics.module should be corrected in this way:

// Original

function statistics_link($type, $node = 0, $main = 0) {
global $id;

$links = array();

if ($type != 'comment' && variable_get('statistics_display_counter', 0)) {
$statistics = statistics_get($node->nid);
if ($statistics) {
$links[] = format_plural($statistics['totalcount'], '1 read', '%count reads');
}
}
return $links;
}

// Fixed

function statistics_link($type, $node = 0, $main = 0) {
global $id;

$links = array();

if ($type != 'comment' && variable_get('statistics_display_counter', 0) && user_access('display statistics')) {
$statistics = statistics_get($node->nid);
if ($statistics) {
$links[] = format_plural($statistics['totalcount'], '1 read', '%count reads');
}
}
return $links;
}

The only change is in "&& user_access('display statistics')" on line 98.

Comments

RobRoy’s picture

Should read user_access('access statistics') not user_access('display statistics').

robin monks’s picture

Assigned: Unassigned » robin monks
StatusFileSize
new730 bytes

And here that is in patch form.

Robin

Steven’s picture

I'm not sure about this patch: often, read counts are shown directly on the site. But if the permission for viewing the counts is the same as the permission for accessing the administrator's detailed logs, then you wouldn't give that to everyone.

There is already an option to choose whether counts are displayed. Perhaps we could change that to "No" "For priviledged users" "For everyone". In last case it acts like it is now, it the second case it requires "access statistics" permission.

What do you think?

robin monks’s picture

Sounds good to me. I'll try to code something up for this.

Robin

robin monks’s picture

Version: 4.6.0 »
StatusFileSize
new2.26 KB

Here is the patch. Uses a switch to choose between signed in users, all users, users with permissions or noone.

Robin

robin monks’s picture

I tested this patch with various settings on my local install and it worked fine.

Robin

Bèr Kessels’s picture

Is there a reason why you check for $user->uid?
Whaen someone has "access statistics" set to anonymous users, your check for $user->uid will override taht settings. Not good IMO.

$group .= form_radios(t('Display counter values'), 'statistics_display_counter', variable_get('statistics_display_counter', 0), array('1' => t('For all users'), '2' => t('For authenticated users'), '3' => t('For priviledged users'), '0' => t('Disabled')), t('Display how many times given content has been viewed.'));

is very inconsistent. please use *only* the permissions page to set permissions, and do not create new permissions-alike settings in any configuration pages.

I would say a simple check for user_access('access statistics') will do the trick

Bèr Kessels’s picture

sorry, i meant to say user_access('access statistics counter'), not user_access('access statistics').

We already have "access statistics'" an additional "access statistics counter" for showing users the counter should work

Ber

robin monks’s picture

StatusFileSize
new23.1 KB

OK, here is a patch to that end...

Robin

robin monks’s picture

StatusFileSize
new1 KB

Um, let's just pretend I didn't just upload the entire stats module. OK? OK!

Robin

robin monks’s picture

StatusFileSize
new1.06 KB

Hotfix.

Robin

robin monks’s picture

StatusFileSize
new1.11 KB

Hopefuly the final version. Thanks to chx and berkes for pulling it apart ;-)

Robin

Bèr Kessels’s picture

+1 for this patch. it adds functionality, but does not add clutter nor any config options.

frjo’s picture

+1 I have implemented the patch authstats_2.patch on my site running Drupal 4.6.1 and it works well.

mfb’s picture

+1

jose reyero’s picture

+1
I tested the patch, works fine, and I'm always for "more options" ;-)

junyor’s picture

Status: Needs review » Reviewed & tested by the community

+1. Using this on my site now.

robin monks’s picture

StatusFileSize
new2.97 KB

This patch revision removes the duplicate show counter option from the settings.

Thanks UnConeD!

Robin

dries’s picture

-1, the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name. Is this different from the 'Count content views' setting?

robin monks’s picture

the term "access statistics counter" doesn't tell me anything. The counters are for posts -- I think that should be reflected in the permission's name.

Suggestions:
view node access counter
view node view counter
access node view counter
view node hit counter

I rather like "view node access counter". Dries, would this satisify you?

Is this different from the 'Count content views' setting?

Yes, on two counts.

  1. These permissions are for users, not for admins. The count content views setting changes both
  2. This setting lets the administrator choose specific groups to see the counter (eg, moderators, or only registered users)

Robin

robin monks’s picture

StatusFileSize
new3.04 KB

New version of patch that renames the permission to "view node access counter".

Tested on latest HEAD. Ready to commit!

Robin

Stefan Nagtegaal’s picture

Very nice patch! One option less...

Works flawelessly on my localhost... :-)

Steven’s picture

Status: Reviewed & tested by the community » Fixed

This patch did not apply anymore, but it was easy to fix.

Committed to HEAD.

dries’s picture

Status: Fixed » Closed (fixed)