Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The tabs on notification settings show wrong number of content.
Seems that status and node_access is ignored.
(active tab shows a count of 1, but no content)
Responsible seems to be:
function commons_follow_ui_tab_title($account, $flag_name = NULL) {
global $user;
if (!$flag_name) {
// We are inside /message-subscribe so get the first flag.
$flag_name = key(message_subscribe_flag_get_flags());
}
$flag = flag_get_flag($flag_name);
$flaggings = flag_get_user_flags($flag->content_type, NULL, $account->uid);
$count = !empty($flaggings[$flag_name]) ? count($flaggings[$flag_name]) : 0;
$params = array(
'@flag' => $flag->get_title(),
'@count' => $count,
'@name' => format_username($account),
);
return $account->uid == $user->uid ? t('@flag you follow <span class="user-follow-count">@count</span>', $params) : t('@flag @name follows <span class="user-follow-count">@count</span>', $params);
}| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 2201909-follow_ui_group_count_views-3.patch | 2.96 KB | japerry |
| #9 | 2201909-follow_ui_group_count-9.patch | 1.42 KB | WebSinPat |
| #7 | follow_ui_people_count_mismatch_after_patch6.png | 5.4 KB | WebSinPat |
| #7 | follow_ui_group_count_mismatch_after_patch6.png | 22.94 KB | WebSinPat |
| #3 | commons_follow_ui.patch | 1.35 KB | slowflyer |
Comments
Comment #1
WebSinPat CreditAttribution: WebSinPat commentedI too have been getting this same behavior (have posted an issue about it somewhere a while back). Problem seems to be that it's still counting unpublished nodes in the count, iirc. As noted by the OP.
Comment #2
japerryComment #3
slowflyer CreditAttribution: slowflyer commentedThe attached patch is a quick solution to the fix the issue.
Comment #4
ezra-g CreditAttribution: ezra-g commentedThanks for the patch, slowflyer! Marking as, "Needs review."
Comment #5
japerrySo the crux of the issue is flag_get_user_flags call doesn't do any checks for whether content is published or not.
While the patch does work, it performs another node_load. I'm hoping we can make it a bit more efficient than that, and it probably needs to be a patch to flag.
Comment #6
japerryI realized something this morning -- we're only doing the call once, therefore its not as bad that we're doing a node (entity) load.
Unfortunately flag 2 has limitations on its architecture for us to add reliable access checks, so it does need to happen within commons_follow_ui for now.
However, we are trying to make sure we make this entity generic. The patch below should allow for that.
Comment #7
WebSinPat CreditAttribution: WebSinPat commentedThe patch in #6 still doesn't seem to account for all cases.
1) For one of my users it works great for "groups" and nodes/"other content" count and as expected does not affect "topics" count, but has done something weird for "people" count.
Though I follow 3 people and all of whose status is active, the count is only 1. (Prior to the patch that count was correct at 3)
2) for another of my users, I'm still getting a mismatch between the count and the number of groups shown. (the patch is making some adjustment: before the patch the count is 10; after the patch the count is 9; the number of groups shown in the list is: 8). I'm not sure yet what else is accounting for the mismatch after the patch, now that it's checking for unpublished nodes. If other folks don't have ideas, I can try to dig in and do some more debugging.
Comment #8
japerryThere is something possibly wrong with the views as well. I'm really thinking we need to figure out a way to get a count of the results instead of trying to re-generate the results on our own.
PS. I haven't been able to re-produce the errors in count after the patch with authenticate or admins. =/
Comment #9
WebSinPat CreditAttribution: WebSinPat commentedI agree it's a bit odd and error-prone to calculate the count different than how the view is generated...
That said, here is a patch that takes care of the final mis-matches I am seeing.
1. if the entity is a user, do not run it through $access_func = user_access. Not sure if there's a need to check that the logged in user has permissions to follow the users they are following? But as the code is written, user_access syntax is different than node_access, so that part of the code is buggy and always returns FALSE unless the user being followed is user1.
2. For some reason, I have in my $flagging array an invalid node. Not sure if it's a node that was deleted and the flag was not deleted properly or what. Adding an else in the case the entity_load fails takes care of it.
Comment #10
japerryOkay time to make this super simple...
#2206157: Add an API function to grab the view object of a message subscribe page, fix page count
With that and this patch, we shouldn't see the counts be inconsistent again.
Comment #11
japerryThis patch coincides with the patch in #2206157: Add an API function to grab the view object of a message subscribe page, fix page count Comment 4.
Comment #12
japerryUgh. This is a problem with message subscribe.
I mean it is -also- a commons issue, but thats because we copied the code from message subscribe because the API is hardcoded to output text in its title.
However, the underlying code is the same, and the bug exists there.
Therefore, we should make two changes here.
1) Fix the count in message subscribe to grab the view output
2) Create APIs inside message subscribe so users (not just commons) who implement message subscribe ui don't have to copy code.
This issue will take care of #1, while #2206157: Add an API function to grab the view object of a message subscribe page, fix page count takes care of #2.
Comment #13
japerryBringing back to the commons queue, because commons will have to do its own fix for this (because we're in a quicktab and not a page title).
The patch for fixing the count in Message subscribe is in #2206157: Add an API function to grab the view object of a message subscribe page, fix page count -- its fairly annoying to split these patches apart.
And here is a patch for follow_ui, requires the patch in 2206157 to work.
Comment #14
japerryMessage subscribe is fixed, and I've changed the make file to point to that specific commit.
http://drupalcode.org/project/commons.git/commit/780d64d2c9cdcbb80c05c08...