When selecting the action Send mail to all users of a role, a list of roles is shown, authenticated user is missing from this list. Attached is a patch to remedy this.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Chi’s picture

Status: Active » Needs work

Is it necessary to call entity_metadata_user_roles to get all user roles?

function rules_entity_metadata_user_roles() {
  $roles = user_roles();
  unset($roles[DRUPAL_ANONYMOUS_RID]);
  return $roles;
}
kristiaanvandeneynde’s picture

Why not just use
user_roles(TRUE);

See: user_roles()

perarnet’s picture

So much for reading the docs. :-) user_roles(TRUE); is much better of course.

kristiaanvandeneynde’s picture

Component: Rules Engine » Rules Core
Status: Needs work » Needs review
FileSize
913 bytes

Rerolled the patch with a more sensible approach.

elBradford’s picture

bump. This patch works great and should be committed.

elBradford’s picture

Status: Needs review » Closed (duplicate)

I'm not sure why this isn't being tested or committed, so I rolled this patch in with mine: #1111436: Emails being sent to blocked user accounts. Apologies if I went about this wrong, it's my first patch, but this fix has been included in my patch. I'm marking this as a duplicate (even though it's not) to limit confusion.

kristiaanvandeneynde’s picture

Status: Closed (duplicate) » Needs review
kristiaanvandeneynde’s picture

Status: Needs review » Reviewed & tested by the community

Marking patch in #4 RTBC.

mitchell’s picture

Status: Reviewed & tested by the community » Postponed

This is going to annoy you guys, but I'm 99% sure fago would not be a fan of this change. #4 solves this issue, but it breaks the design patterns of Rules, so it could create other problems in the process.

#925212: Include "anonymous" and "authenticated" in user roles list could be expanded to include providing the 'authenticated' role to Rules. I'll postpone this issue for that one so that if the email actions need to remove the 'anonymous' role from the list, we could implement that in Rules afterward. Thanks for your understanding.

kristiaanvandeneynde’s picture

Which patterns does it break, actually?

Seems quite strange that a workflow automating tool does not allow you to e-mail all users at once.
Keeping in mind that you can still achieve the same effect by tediously adding each role as a separate action, that is.

I'm not trying to start an argument, I'm just asking you to shed a bit more light on this.
Saying that "fago isn't a fan" and it "breaks design patterns" is a cold comfort for shutting down this issue.

mitchell’s picture

> I'm not trying to start an argument, I'm just asking you to shed a bit more light on this.
Absolutely. I'm happy to help to coordinate Rules development, and we all appreciate your patches.

> Which patterns does it break, actually?
The patches in OP and #4 substitute Rules' Entity API integration for a core data integration, however Rules 2.x uses Entity API in every case to access these objects. It may seem like valid workaround for this case, but there's virtually no chance that it will be committed. What we need is to expose the "authenticated" user role through Entity API. Then Rules will "get it for free."

> shutting down this issue
I'm not shutting it down. We'll still need to reconsider how Rules calls entity_metadata_user_roles() after #925212: Include "anonymous" and "authenticated" in user roles list is made upstream.

My guesses are:
* 'Send mail' action will need to exclude the "anonymous" role.
* 'User has role' condition would not need any changes.
* 'Add a user role' action and 'Remove a user role' action will need to be reworked slightly to exclude both "anonymous" and "authenticated" roles.

I could easily be wrong about my guesses, because I don't know how $op could be modified in entity_metadata_user_roles(). Let's discuss that #925212: Include "anonymous" and "authenticated" in user roles list. Sound good?

kristiaanvandeneynde’s picture

Now that makes more sense to me :)
Thanks for taking the time to clear this up.

elBradford’s picture

What's wrong with committing this patch until the better solution becomes available? I know it's important to do it the right way, but this solution is immediate and doesn't have any side-effects that make it unusable aside from the points made above.

mitchell’s picture

Right. So in #925212: Include "anonymous" and "authenticated" in user roles list, I just posted a patch that needs review and continued development. That'll lay the foundation for this improvement.