Admin viewing another user's notification settings tab shows admin's notification settings

Great feedback - Thanks! Retitling slightly and moving to "normal" priority since this seems common enough that it's worth fixing.

I think the expected behavior for an admin viewing another user's notification settings tab would be to see the settings for that user, or to just not show the tab.

Additional observation: When an admin edits their own notification settings via another user's profile, as made possible through this bug, changing the "Send email notifications by default." setting on this screen has no effect for the admin. The changes to this setting are persistent though, just not reflected when looking at the same screen in the admin's own profile. So I guess they affect the other user to whom the profile belongs, but I did not verify that.

I was doing some research and found that the associated views (nodes a user follows, groups a user follows, topics a user follows, users a user follows) would all need to be updated to allow a contextual filter for the user ID of the user page being viewed. Would also have to change all the relationships for the Flags would have to be changed from "current user" to "any user", and add a new relationship for the Flag's user. We would also need to change the callback to remove the follow button and disable the send email checkbox, for administrators viewing another user's profile. This would be the long fix that would accomodate the first option.

Option 2 requires only a slight change, as the current access callback is 'message_subscribe_ui_tab_access', and that is already doing a check for making sure the flag's user is the currently logged in user, or if the user has the 'administer message subscribe' permission. We would only need to remove the check for the administer permission.

The second options (of just not showing this tab) would probably be sufficient until somebody finds time for the first. Because, as an admin one can always use the masquerade module to view and modify a user's notification settings.

The second options (of just not showing this tab) would probably be sufficient until somebody finds time for the first. Because, as an admin one can always use the masquerade module to view and modify a user's notification settings.

Agreed.

Patch attached.

I've also added a filter on node type (not empty) to make sure we only see nodes. This fixes another error for me too where the list shows nodes by uid instead of nid.

Please review, this should really make it into Commons 3.4

Bump

@BarisW, I am interested in trying your patch. I achieved the similar desired result via views_ui to make the changes as described in #2108377: User profile "notifications" page always shows data of logged in user, regardless of uid in the url, so can test out the patch and see if it also works for me.

First though a question. it seems like the patch only targets commons_follow_node, but I think it would also need to do the same thing for commons_follow_taxonomy and commons_follow_user. thoughts?

Also, i've never worked with a .inc file before. does patching that work the same as a .module file? Just patch and then reload the page, or are there any additional steps needed in order for the changes to go into effect?

@WebSinPat: patching works the same indeed. And I believe you are correct regarding the commons_follow_taxonomy and commons_follow_user. Can you let me know whether my patch works for you? If so, I'll add the same logic for the other two tabs as well.

(shouldn't this be in the email notifications queue?)

OK @BarisW, i've given your patch in #12 a try and it seems to work fine, with one exception and one question.

1) The exception is that I believe we need to keep this line:
$handler->display->display_options['relationships']['flag_content_rel_1']['required'] = 0;
otherwise only nodes that have the email_node flag set are displayed, meaning a user only sees nodes they are already subscribed to and not given the option to subscribe to other nodes they follow.

2) The question is that I had set up my user uid relationships/contexts slightly differently than your patch, and both seems to be working fine. I'm hazy on how views relationships should work, so I don't know if one is more or less correct. I'm attaching a patch with the method I used for comparison.

If we can put the line in (1) back in, and resolve the relationship question in (2), then everything else seems to be working fine and I think this fix will be good.

EDIT: Actually, note that none of these patches address this portion of the fix suggested by @jpontani in comment #4 . "We would also need to change the callback to remove the follow button and disable the send email checkbox, for administrators viewing another user's profile."

and possibly off topic: as i was investigating issues related to these patches, I noticed another discrepancy in the total number of nodes in the tab title and the number of nodes displayed in the view, which I have tracked down to the title counting nodes that have been deleted or unpublished, whereas the view filters these out. In this case I believe the view is correct.

@BarisW or others following this, has anyone run into this issue? I haven't found an existing issue about it, so next I will dig in here and try to debug this, unless someone can save me the trouble.

Hi @WebSinPat, yes I have observed a discrepancy between numbers in tab titles vs data in the tab itself. That was actually the original bug I reported in https://drupal.org/node/2108377 before that issue morphed, merged into this one, and got closed as a duplicate. I have no insights on why this bug occurs or how to fix it, unfortunately.

Not sure how we missed this! Adding to my queue.

While the node relationships work properly, the user relationship for users you follow doesn't.

Fixed! Updated node, terms, and user views.

Commits are here:
http://drupalcode.org/project/commons.git/commit/8a24292

Thanks! Just wondering, does that also fix the checkboxes for "Send e-mail"? As that seems to go wrong as well. Will test ASAP.

Thanks @jappery. I will give this a test run as well.
1) Like @BarisW, I'm wondering if this addresses this portion of the fix suggested by @jpontani in comment #4 . "We would also need to change the callback to remove the follow button and disable the send email checkbox, for administrators viewing another user's profile."
2) And I'm wondering if this addresses the issue of unpublished nodes being counted in the count total in the tab title, as mentioned in comments #14 and #15.

Thanks, japerry!

Marking as "needs review" per #19 and 20.

3) Can someone tell me how to test changes to an .install file? I'm not sure how to get that code to run, since i already have the module(s) installed and enabled on my site. Otherwise I can revert the views by hand in order to test the new views code.
EDIT For any other newbies reading this wondering the above question, I've discovered that running update.php/drush updb causes the .install code to be run.

4) For good measure, I'll also point out that I think the email notification on/off toggle still is not working as far as i know, per #2126689: Email Notifications sent regardless of email checkboxes, revisited. bug in message_subscribe_email , but i have not seen any movement in response to my comments or patch over there.

with reverting the views by hand (not via the .install files), the new views code seems solid to me!

5) How can a user follow a topic? I don't see a follow button whenever I browse by taxonomy. (I'm still on Commons 3.3) So I haven't really tested that particular follow_term view, but I'm sure it works the same as the other 2.

@WebSinPat, my site is Commons 3.3 and it has a follow button near the top of each topic page (e.g., http://mysite.com/topics/mytopic)

Here is a patch to make the flag load the selected user instead of the current users' flags.

Okay this patch makes a bunch of changes to the notification settings

1) Checkmarks are greyed out and checked, corresponding to the users' settings
2) Following is just text, since the button is useless in this case
3) Current user notification page should still work without any changes

I've removed the rest of the patches from display, as we fixed the views already.

added screenshots to top of the issue.

Looks good so far @jappery. thanks for the fix introduced in #26; I hadn't quite realized that was another weirdness.

1) what about the overall "Send email notifications by default. " checkbox? Should that one get greyed out as well?
2) I'm still seeing discrepancies in the totals, with unpublished/deleted nodes counted in the total. (let me know if i should open a separate issue on this point. sorry if i'm being repetitive.)
3) message_subscribe_email issue

4) going back to the views code for a moment, I just noticed that commons_follow_node is the only view that got "distinct" added into the query configs. Is that by design or should the other views have it as well?

commons_follow_node_views_default_views() {
   $handler->display->display_options['access']['type'] = 'perm';
   $handler->display->display_options['cache']['type'] = 'none';
   $handler->display->display_options['query']['type'] = 'views_query';
+  $handler->display->display_options['query']['options']['distinct'] = TRUE;

Thanks everyone for the help on reviewing this! I've added the functionality to grey out the 'Send emails by default' in the committed version here:

http://drupalcode.org/project/commons.git/commit/151d778

thanks @jappery for the fixes and the commit.. However --
I'm working on upgrading to 3.6 with the committed code and now the "send emails by default" checkbox is greyed out for every user, even when the logged in user is viewing their OWN notification settings.

problem is in commons_follow_ui.module conditional

if($account != $user) {
  $form['message_subscribe_email']['#disabled'] = TRUE;
}

Changing the line to if($account->uid != $user->uid) does the trick for me.
patch attached.

just figured I'd bump this, since we're getting sooooo close thanks to current progress on #2201909: Wrong count of content followed

Oh shoot, WebSinPat, I didn't even notice #33. I committed that today, from #2204763: Disabled checkbox "for automatically sign up for email notifications" So thats fixed.

The style changed with #27 needs to be changed in the original sass file, and not the css file directly.

The CSS issue is in another issue #2215695: Resync css and sass

Otherwise, this issue is fixed.