As a first step to testing, a new user on the dev site with email notification seemingly switched off on the account (as default) because of the reset here and what was shown in the registration page of the user:
-changed the default value of the field at: admin/config/people/accounts/fields/message_subscribe_email

A comment was created on the page /groups/connections/connections-faq.

All users, including the new one received an email despite having their notifications switched off.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

simon.j.c’s picture

Just to add some more information- in talking with a colleague he informed me that this patch was tested, and failed in the above environment:
http://drupal.org/node/1916206

A big thanks to the team.

ezra-g’s picture

Issue tags: +Commons 7.x-3.3 radar

Adding to the 3.3 radar.

Devin Carlson’s picture

Assigned: Unassigned » Devin Carlson
Status: Active » Needs review
FileSize
905 bytes

When viewing the notification settings for a certain user at "user/%uid/notification-settings" the notification settings form currently *always* uses the ID of the current user instead of using the ID of the user's profile that is being viewed.

Devin Carlson’s picture

Status: Needs review » Reviewed & tested by the community

Tested #3 by creating a variety of users, switching between user accounts and viewing the current user's profile/other user's profiles.

Devin Carlson’s picture

Status: Reviewed & tested by the community » Fixed
garyconroy’s picture

Does not fix the original issue. Applied the patch but all site users are still notified whenever a comment is made on a piece of content (even when their notification settings are unticked).

garyconroy’s picture

Version: » 7.x-3.0
Status: Fixed » Active
japerry’s picture

Title: Changed the message_subscribe_email field on user profiles -no email. Seems to work when creating a new user- but is incorrect. » Message Subscribe sends emails regardless of flag checkmarks in notification-settings

I believe there are a few issues that have been mentioned in this queue.

a) The default email notification checkbox on the notification-settings page is just for turning on a flag automatically when a user follows or makes a post in a group. It does not go back and disable sending emails.

This functions as it should. Maybe we need to work on the wording of this function.

b) Users are getting sent emails regardless of the checkbox for emails. More specifically, if a user follows a node or other user, emails will get sent.

This is a bug.

c) Devin fixed #3, which was causing checkmarks to disappear 'randomly' from users notification pages. In fact, it was changing because the current users' account was changed, rather than the users page.

Fixed.

We just need to fix (b) and we'll be good here.

japerry’s picture

The issue in #8b is the message_subscribe_flag_prefix setting, which is currently overridden by the commons_follow module.

Because of this, anytime a user follows another, it will shoot off an email. We need to enable the message_subscribe flags and remove this variable from commons_follow. This should allow users to select whether they want to follow a post, group, or user independently of sending an email every time.

japerry’s picture

Version: 7.x-3.0 » 7.x-3.x-dev
Status: Active » Needs review
FileSize
620 bytes

I think we're fixed! try this patch.

This removes 'email' as a default notifier... which essentially was overriding message_subscribe_email.

ezra-g’s picture

Status: Needs review » Needs work

If #10 is a patch to Message Subscribe, it should be in the Message Subscribe issue queue.

However, the drupal-org.make file for Commons already includes:

; Remove "email" as default notifier.
; http://drupal.org/node/1828184#comment-7081868
projects[message_subscribe][patch][] = "http://drupal.org/files/email-notifiers-1828184-15.patch"

The referenced issue and patch includes the fix proposed in #10 as well as additional code changes.

If that issue isn't sufficient to fix the problem, let's re-roll the patch at that issue.

ezra-g’s picture

I believe the issue and proposed patch in #3 is still valid, though it match the issue title since it was retitled.

garyconroy’s picture

Applied those two patches #10 and #11 successfully but still find that users are getting notified when they shouldn't be.

I tested on two pieces of content created by the same author, 'Gary', in two separate Groups. One piece of content was a Post content type in the 'Connections' group and another was a Poll content type in the 'SOG' group. No emails were sent for comments on the post (hurray!) but emails were sent by comments on the poll.

I thought that perhaps emails were just being sent (erroneously) to Contributors but it looks like this isn't the case.

For instance:

  • Alison and David received a poll comment email notifications and both are contributors to 'SOG'
  • Bob also received a poll comment email and is not a contributor to any group
  • David is a contributor to 'Connections' (and received no email for the post comment).

One difference between SOG and Connections is that the former (misbehaving) group is set to be private (Commons Group Privacy module)

japerry’s picture

Status: Needs work » Needs review
FileSize
3.68 KB
1.71 KB

Okay after doing a bit of debugging, found two issues that need correcting:

1) Message subscribe email was checking for ALL flags with the email tag on it. There are many other types of flags that use the email prefix (terms, users, groups), so if you had any of those flags set, it would send an email. I've added a check that makes it check the content type as well before adding the email notification

2) Added code in commons_notify to remove the uid from the message_subscribe stack if the email notifier isn't there. It looks like by default if the notifier isn't there, it still picks up other flags and shoots off an email.

Two patches below.

PS. I also have some coder cleanup in the commons_notify module.

ezra-g’s picture

Assigned: Devin Carlson » japerry

Thanks, japerry! Looking forward to reviewing these.

Looks like the match to Message Subscribe should be filed in that module's issue queue. Can you file and reference? Thanks!

ezra-g’s picture

ezra-g’s picture

Status: Needs review » Needs work

#2046283: Message subscribe email adds a notifier regardless of the email flag that is selected has been marked as a duplicate of a patch we already use in Commons, so marking as "needs work."

ezra-g’s picture

Priority: Normal » Critical

Given how central email notifications generally are to Commons sites and the potential to make users angry if their settings aren't respected, I believe "critical" is a reasonable priority. Marking as critical to help us prioritize this over other non-critical 3.3 radar issues.

ezra-g’s picture

Some updates on tackling this issue:

- Devin Carlson filed a patch with a new Simpletest to Message Subscribe to help demonstrate the present issue related to the message's $contexst: #1828184: Message Subscribe sends emails regardless of context to help in defining the problem and vetting solutions.
- japerry filed a patch to Message Subscribe to restore compatibility with Flag 2.x: #2052431: Undefined index errors with latest Flag stable 2.x.
- I added japerry's patch and brought us from 7.x-1.x-alpha5 to the latest nightly dev snapshot of Message Subscribe: http://drupalcode.org/project/commons.git/commitdiff/95eb705?hp=cac25874...

Thanks, japerry & Devin Carlson!

ezra-g’s picture

ezra-g’s picture

ezra-g’s picture

Our progress patch was committed to #1828184: Message Subscribe sends emails regardless of context, so I've removed it from the Commons make file to restore the build.

http://drupalcode.org/project/commons.git/commit/2751c04

japerry’s picture

Part one of this patch restores og_access functionality when creating new nodes. We add a register shutdown function to send the message after the node is created.

ezra-g’s picture

Looks good in my testing with watchdog() calls in our newly registered shutdown function. I tweaked the code comments & formatting, and added a reference to #1918666: Entity Access support so people can read the full background. #23 is committed: http://drupalcode.org/project/commons_notify.git/commit/fce6848 .

Thanks, japerry!

japerry’s picture

Assigned: japerry » Devin Carlson

Re-assigning this to Devin, as he is taking on the testing and reproduction of the second part of this issue.

ezra-g’s picture

Status: Needs work » Needs review
FileSize
3.05 KB

In combination with the latest version of Message Subscribe that includes Devin Carlson's recent commits and found that when setting the default notifiers variable to an empty array, users' email subscription preferences are properly enforced.

japerry’s picture

Status: Needs review » Reviewed & tested by the community

Tested and passes my tests.

ezra-g’s picture

Status: Reviewed & tested by the community » Fixed
Tim Clifford’s picture

Status: Fixed » Needs review

In order to get this working then from previous Commons installation (3.1), what modules/patches would you need to update? Is it only Commons_follow, commons_notify and message subscribe? Or would you need flags 7.x3 and any other modules. The reason I ask is that I've installed these patches above and I still get email notifications sent out when 'send email' link os un-ticked but content is being followed.

Is anyone else still having this problem?

Tim Clifford’s picture

Status: Needs review » Fixed

Just fixed my version by rolling back to flag 2x instead of the latest 3x. In case anyone else has this problem.

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