Further improve autocomplete for different recipient types and name conflicts between them

The patch fixes autocomplete for usernames for me, but I can't seem to get autocomplete to work for roles. I've got both an "editor" user and an "editor" role... only the editor user shows up in the autocomplete.

Also, I just noticed that I don't have any permissions related to sending to roles in the D7 port. Didn't we add those in the D6 version? Could that be why the roles autocomplete isn't working?

--Ben

Oops! My fault... I didn't have the Private Message Roles module enabled in my latest install. Will test the roles autocomplete now. Sorry about that!

--Ben

I've started preliminary testing with roles and I've noticed a few things:

A. Using "[user]" to specify the user (rather than the role with the same name) doesn't seem to work. I'm getting a message that the user isn't valid.

B. I've got a user named "authenticated" and the Drupal default user role "authenticated user." Only "authenticated user" will appear in the autocomplete even though they aren't exactly the same name.

C. I've tested with a user who doesn't have the "Write private message to roles" permission, but does have the "Administer private message" permission. I've also got an "editor" user and an "editor" role. I sent a message to editor (as suggested by the autocomplete) and assumed the message would go to the editor user (since the user can't send to roles according to the permissions). But instead the message went to the editor role. I'm wondering if this a permission issue, autocomplete issue, or both.

D. I'm wondering if the "[role]" or "[user]" suffix could be shown in the autocomplete itself, but only for users with the "Write private message to roles" permission. The issue is a usability one: As a sender with "send to role" permission, it's a bit unclear if you are sending to a role or a user unless you know exactly how the system works.

This could be especially problematic in a production scenario which has many more users and roles than our test environment--plus more users who may be allowed to send to a role. For instance, if a user has a similar name to a role, it's easy to inadvertently send to an entire role when you think you are sending to just one user. Currently, you don't know that you're sending to a role until you see the confirmation message (which has "(role)" appended to it).

Thoughts?

--Ben

Also, I wasn't exactly sure what you meant by "As a hint, the description below the field should indicate what recipient types are enabled."

Regardless of whether a user has the "Write private message to roles" permission or not (I tried this with different users), I see the same "Enter the recipient, separate recipients with commas" message underneath the "To" field.

--Ben

Okay, I'll test the new patch as soon as it passes the tests... I'll open up a new issue for the rest.

--Ben

Hey Sascha,

I tested the patch in #12 and like the latest version a lot. I really like your simple solution of displaying the type [role] for non-user recipients. Enhances the usability greatly.

I did, however, find one fairly major bug: If you have a user and role of the same name (editor), the message always goes to the role, regardless of whether you select "editor" or "editor [role]".

To see this, try to send a message just to "editor" (the user). When you click "Send message," the confirmation message at the top of the screen says "A message has been sent to editor (role)."

And all users in that role are getting the message instead of just the one user.

Thanks,
Ben

Hmmm.... Good questions. A lot to think about. One thought: It seems a main challenge is the logic that if there is no type specified, then the first matching type is used. The issue with this approach is that we're not really sure who is being sent to... it's just whatever type happens to come first in the list.

How about specifying that if there is no [type], then we assume the type is "user" and just send to the user? This way, any other types we think of later (besides roles) would need to specify a type in the [type] format. This sort of makes sense given the nature of the module: Private Messages are designed to be sent to a user and if you want to send to something else (role, organic group, etc.) then you need to specify the type.

Thoughts?

--Ben

Well, here's a thought: Maybe we should just commit the patch in #12 (which works well).

In thinking about this, it seems the problem is very limited. It would only affect a sender who has the "send to roles" permission and who also has a user and role with the identical name. Furthermore, once the "[user]" suffix works (I'll test http://drupal.org/node/850546), then the sender can be extra sure by attaching "[user]" to the end of the username.

What do you think?

One other option would be to tack on "[user]" at the end of any username only when the user has "send to roles" permission. But maybe this is overkill given that the problem is so limited.

We've got so many other issues to do that we just don't want to lose too much momentum on this... :-)

--Ben

Awesome! You've been busy... ;-)

I'm swamped today, but I'll plan on test the patch tomorrow...

--Ben

Hey Berdir,

I tested the patch and it's working very nicely with in cases with one recipient. More specifically, the autocomplete is working great and messages are now being sent to the correct recipient (when there are more than one recipients with the same name). And the JS used with the name lookup is very cool and functioning very well!

Here are the bugs/issues I noticed:

1) If there is more than one recipient in the "To" field and the recipient with more than one name is not listed first, then the system is breaking down. For instance, I sent a message to "person1" and "editor" (in which "editor" is the name of both a user and a role). Upon clicking send, I received both a notice that the message was sent to "person1" as well as the "recipient not unique" error notice (in which the possible recipients were not clickable). And the message was actually sent before I could specify the unique recipient.

The system also breaks down if there are two recipients, one with more than one possible name and a second valid recipient. In this case, the message is being sent to only the valid recipient and the "recipient not unique" message is displayed after the message is already sent.

2) I'd suggest changing the following strings:

a. Change: "The provided recipient editor is not unique, choose one of the following possible recipients:". To this: "Our site has multiple recipients named editor. Please choose your intended recipient:"

b. Change: "Updated recipient field, submit the form again to send the message." To this: "The recipient field has been updated. You may now send your message." Also, this "update successful" message is displaying as an error (red in bartik). It would be better if this one just displayed as a regular notice (green in bartik) since the update action was successful.

3) Once the recipient field has been updated, the "You must include at least one valid recipient" message is also being displayed. I think this is confusing to the end user because it makes it seems as though the updated recipient value is invalid. Is there any way to hide the "You must include at least one valid recipient" in this case?

--Ben

The patch is working great and all errors are fixed. This is RTBC.

One point I did want to note: If you have two recipients in a message and both of them have multiple possible recipients, then the system only deals with one at a time (meaning you'll get the "choose a recipient" for the first one, fix it, re-send, and then get the "choose a recipient" message for the second one). This wasn't exactly what I expected (I expected to see both "choose a recipient" messages simultaneously), but I actually think this method works fine. And maybe this is better because imagine if you had 10 recipients, each with multiple recipients. That would be a lot of error messages to show at once. So I just wanted to note this (in case it was something other than you intended), but I don't think this approach needs to be changed.

Nice work on this!

--Ben