Does not add subscribtions at all..[fixed ?]

Installed the same versions as you and fired it up today myself. It removed the subscriptions I already had so now I have no subscribers to my newsletter. :)

It works with a custom role (admin) that I made though. So the error is probably caused by "authenticated user" not actually being listed as a role in the users list.

I confirm this as a bug. I fortunately only had 25 users and one newsletter, and was able to manually add all of them by appending values to the simplenews_tid_snid table (or whatever it is called) - I had tried to use the role 'authenticated user' and instead of subscribing them all it removed all of them! No matter what I did, it wanted to kill all the subscriptions. I have uninstalled the module for now - probably there is a single line of code that I could hack to fix it. Too bad - this is very useful, potentially.

Hey guys, I just committed a fix for this. Can you test from DRUPAL-5 branch or when a new dev snapshot gets packaged up in a few hours and report back?

If it works for y'all, then I'll create a 1.0 release.

Haven't actually sent the newsletter, but here's what I observed.

Before enabling the fixed module, 25 entries in the simplenews_tid_snid table, and 25 users are subscribed.

Enable your patched module as of 10-18 - and go to the page which subscribes by role. By default all boxes (I have four roles) are blank. Save that state (eg, press 'submit')

This erases the subscriptions. Behavior as expected (although it could be a problem if you are not expecting it to do that)

Go back to subscribe-by-role page and now check box for 'authenticated user' and submit.

All 25 users are subscribed.

Case closed on this bug, I think. One could make a case to save the original state as a reversion point )or perhaps export the list as a CSV file.) I can see innocent users collapsing in tears if they erase all of their predecessors work and the users who are nuked off the list are not in a single role...

Thanks !

/Hound

Sorry, should have mentioned - I am using the 5.3 release version.

Yeah so this same issue is happening in 6.x-2.x-dev

I checked the role (authenticated user) that I wanted to automatically add, but nothing happens.

No error appears, but the subscriptions simply does not work, as no users were added to the list of subscribed members...

My pathetic attempt at a workaround (it works but is a pain to set up initially):

- create a new role
- duplicate the permissions of the Authenticated User role to the new role in /admin/user/permissions/
- install the Auto-Assign Roles module
- configure Auto-Assign Roles module to automatically add all new users to the new, duplicate role
- configure my newsletter (i.e. simplenews roles module) to automatically subscribe people who are in the new, duplicate role
- add all authenticated users to the new role (either manually at /admin/user/user/ or with the Views Bulk Operations module if you have a lot of users)
- run cron, and then check to see if the number of subscribers on page /admin/content/simplenews/types/ has been updated.

Hope someone will fix this someday, but as the magic 8-ball says, "outlook not so good"...

Same here. nothing happens

> I checked the role (authenticated user) that I wanted to automatically add, but nothing happens.

It may well be that the authenticated user role is excluded from the system, as that is the same as just automatically adding *every* user to your newsletter -- which can be done without this module.

Could someone perhaps check in the code to see whether this is the case?

@joachim: Yes, that's indeed the case; in the function simplenews_roles_update_subscriptions(), $query_1 removes all users not in the selected roles. As the Drupal table users_roles does not register mappings for the system role "authenticated user" (rid=2), checking that role for automatic subscription does not do anything (since according to the database table, no user has that role).

For those of you interested in having a patched version of the 6.x-2.x-dev working with "authenticated roles" then replace the function "simplenews_roles_update_subscriptions" with the attached text file in the module file.

To resume, I simply added new SQL queries if I find the "Authenticated role" has been selected in the newsletter list of roles. By choice, I only select users that have logged in at least once on the site, to avoid all those robot-faked user accounts with invalid emails.

Drop me a line if you wish to have more details.

Sounds good! Any chance you could post your changes as a patch please?

Here's the patch file.

+++ W:/sites/all/modules/simplenews_roles/simplenews_roles.module	Wed Dec 19 17:14:11 2012
@@ -289,6 +289,7 @@
+  $hasAuthenticatedRole = in_array(DRUPAL_AUTHENTICATED_RID, $rids);

This function isn't meant to work with anon users.

Which function? in_array()?

This patch does not change the logic of simplenews_roles, it will only check if the role of type "Authenticated users" has been selected for a given newletter. As far as I know, simplenews_role does not handle anonymous roles either.

> As far as I know, simplenews_role does not handle anonymous roles either.

Yup, that's what I meant....

ah yes, I see now. Sorry, got confused reading the patch over breakfast ;)
I get what you mean now -- $hasAuthenticatedRole means 'does $rids contain the authenticated user role as one of the list'.

Could you change that to $contains_authenticated_role please?

Corrected patch.

Does this patch apply to 7.x? We really need to fix the bug there first (assuming it's present on 7).

I should make a confession too -- though I'm maintainer, I don't use this module at all any more. Hence I'm not in a position to test the patch; nor do I remember enough about the monster queries in this module to give it that much of a read review either.

Given we do have several hundred reported installs of this site, I don't feel it's responsible to just commit code without it being tested -- so this patch getting in is dependent on users trying it out and reporting back. (Or someone volunteering to take over as maintainer!)

For now, this is only valid for 6.x, I haven't looked at the 7.x version yet.

The only thing I would like to remind again, is the fact that I voluntarely skipped registered users that have never logged in the site because my customer only wishes to send to active users, not bots, blocked or invalid emails. So this rule could be annoying to a brand new imported list of users.