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.
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.
Comments
Comment #1
simon.j.c CreditAttribution: simon.j.c commentedJust 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.
Comment #2
ezra-g CreditAttribution: ezra-g commentedAdding to the 3.3 radar.
Comment #3
Devin Carlson CreditAttribution: Devin Carlson commentedWhen 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.
Comment #4
Devin Carlson CreditAttribution: Devin Carlson commentedTested #3 by creating a variety of users, switching between user accounts and viewing the current user's profile/other user's profiles.
Comment #5
Devin Carlson CreditAttribution: Devin Carlson commentedCommitted #3 to Commons Follow 7.x-3.x.
http://drupalcode.org/project/commons_follow.git/commit/0135216
Comment #6
garyconroy CreditAttribution: garyconroy commentedDoes 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).
Comment #7
garyconroy CreditAttribution: garyconroy commentedComment #8
japerryI 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.
Comment #9
japerryThe 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.
Comment #10
japerryI think we're fixed! try this patch.
This removes 'email' as a default notifier... which essentially was overriding message_subscribe_email.
Comment #11
ezra-g CreditAttribution: ezra-g commentedIf #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:
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.
Comment #12
ezra-g CreditAttribution: ezra-g commentedI believe the issue and proposed patch in #3 is still valid, though it match the issue title since it was retitled.
Comment #13
garyconroy CreditAttribution: garyconroy commentedApplied 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:
One difference between SOG and Connections is that the former (misbehaving) group is set to be private (Commons Group Privacy module)
Comment #14
japerryOkay 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.
Comment #15
ezra-g CreditAttribution: ezra-g commentedThanks, 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!
Comment #16
ezra-g CreditAttribution: ezra-g commentedI think japerry meant to reference #2046283: Message subscribe email adds a notifier regardless of the email flag that is selected.
Comment #17
ezra-g CreditAttribution: ezra-g commented#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."
Comment #18
ezra-g CreditAttribution: ezra-g commentedGiven 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.
Comment #19
ezra-g CreditAttribution: ezra-g commentedSome 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!
Comment #20
ezra-g CreditAttribution: ezra-g commented#2052431: Undefined index errors with latest Flag stable 2.x is committed upstream and now removed from the Commons make file: http://drupalcode.org/project/commons.git/commit/9a97fbd.
Comment #21
ezra-g CreditAttribution: ezra-g commentedI updated the make file to use the latest patch #1828184: Message Subscribe sends emails regardless of context: http://drupalcode.org/project/commons.git/commit/1162bc8751baf7e697cfb04... .
Comment #22
ezra-g CreditAttribution: ezra-g commentedOur 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
Comment #23
japerryPart 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.
Comment #24
ezra-g CreditAttribution: ezra-g commentedLooks 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!
Comment #25
japerryRe-assigning this to Devin, as he is taking on the testing and reproduction of the second part of this issue.
Comment #26
ezra-g CreditAttribution: ezra-g commentedIn 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.
Comment #27
japerryTested and passes my tests.
Comment #28
ezra-g CreditAttribution: ezra-g commented#26 is committed. http://drupalcode.org/project/commons_notify.git/commit/1d1b0df2396aad48...
Comment #29
Tim Clifford CreditAttribution: Tim Clifford commentedIn 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?
Comment #30
Tim Clifford CreditAttribution: Tim Clifford commentedJust fixed my version by rolling back to flag 2x instead of the latest 3x. In case anyone else has this problem.