From irc, litwol had the idea to have an extra column in the pm_index table - for the role.

Then to create a message index, it will be a simple matter of having a "WHERE pmi.uid = %d or pmi.rid IN '%s'" or something.

On the display side, the recipient/participant theming will also need to somehow be expanded to cover roles.

And on the sending side, there is still the question of *how* to send to a specific role. Another select box? different place?

A third question is one of threading - making sure the index table is not overloaded when replying to an existing conversation.

(A final issue is of not being able to see group messages once you leave the group - is that how it is supposed to work? or when people leave a group, will their threads need to have the index populated in the normal manner?)

Files: 
CommentFileSizeAuthor
#114 privatemsg_recipient_types13.patch90.75 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 1,131 pass(es). View
#113 privatemsg_recipient_types12.patch90.2 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 1,131 pass(es). View
#110 privatemsg_recipient_types10.patch85.78 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 1,031 pass(es), 79 fail(s), and 81 exception(es). View
#108 privatemsg_recipient_types9.patch85.94 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_recipient_types9.patch. View
#96 filetopatch.png10.59 KBkriskd
#84 privatemsg_recipient_types8.patch81.15 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 1,053 pass(es). View
#74 privatemsg_recipient_types7.patch80.17 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 1,019 pass(es). View
#69 1.png55.81 KBBilmar
#69 2.png33.66 KBBilmar
#64 privatemsg_recipient_types6.patch80.67 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_recipient_types6.patch. View
#63 privatemsg_recipient_types5.patch65.44 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 1,020 pass(es). View
#59 privatemsg_recipient_types4.patch64.82 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 444 pass(es). View
#44 privatemsg_recipient_types2.patch48.73 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 1,005 pass(es), 8 fail(s), and 0 exception(es). View
#41 privatemsg_recipient_types.patch47.22 KBBerdir
FAILED: [[SimpleTest]]: [MySQL] 1,008 pass(es), 6 fail(s), and 0 exception(es). View
#32 privatemsg_roles.patch34.12 KBBerdir
PASSED: [[SimpleTest]]: [MySQL] 779 pass(es). View
#15 pm_by_role.tar_.gz2.21 KBarchard

Comments

Berdir’s picture

Hm.

I am not sure. The thing is, how do you then track who has seen/deleted a message? The only way I see right now is some sort of batch processing to add more users.

Another issue is with replying. Should you be allowed to reply to an group/role/all thread if you can't start one? Probably not... but how to track that and of course, every reply needs batch processing too.

litwol’s picture

Group-wide replies can very quickly thrash the whole system. What if we do this communication as being one way only ? Some one of privilage can send a message to a group but replies will be disabled, until message is forwarded to other user(s).

As for tracking who read it, we could insert a row in pm_index upon reading a mesage that was sent to a group, so now you have 1 row for the group and 1 row for me. < This is just speculation.

Berdir’s picture

Hm. we could add another permission. For example. if you, as an admin send a message to a group, I consider it to be useful when you can send an update at a later point.

Your idea could work, we still end up with the same amount of entries in pm_index (actually, even one more), but it is not filled at once. However, there is one *huge* limitation. It would just work for roles. However, I'd like to see a way to do this for organic groups, your friends/fans, all users of a site and we would then provide two things:

a) An easy way to provide such a mass-sending facility.
b) A default implementation for roles and all users of a site

jaron’s picture

I may not be fully grasping everything you guys are suggesting here though, so forgive me if my comments are off point at all. I know you are all deep in the privatemsg development and thank you for that. More than anything, I just wanted to point out my support for this idea...

Just wanted to comment that #2 above would work great and to add my support for the functionality of messaging to an entire role. The main reason I can see for using a pm to an entire role would be notificaiton, and one way messaging is fine in that instance.

In terms of tracking, I'm a little confused, however. Are you all suggesting that you would be able to see each person who has read the pm? If you are talking about sending to an entire role, that could be a very large number of users. And if that is listed somewhere in the messaging interface, it could potentially be overwhelming (ie listing each user who has read the message).

Also, is this something that can be started by using roles so that the functionality is available (even if somewhat limited), and then down the road add on support for organic groups, friends/fans, etc?

Berdir’s picture

1. No, the tracking is for the recipient. To mark messages as new, privatemsg.module needs to know if you have read it before, and it needs to do that for every user. Exactly the same for delete.

2. Well, imho, adding a role column would not make it easier to add support for other groups later on, as the functions, db column etc. couldn't be re-used. I would like to do this in a more generic way but I can't work on that any time soon, maybe for rc4.

jaron’s picture

I can see the benefit of it being a more flexible function, especially with sites trying to create a more hierarchical user structure. I'll be looking forward to it and will be happy to do testing when patches are offered.

stephenhendry’s picture

subscribe

Liliplanet’s picture

subscribe, thx!

Yuki’s picture

subscribe

Agogo’s picture

subscribing

Bilmar’s picture

subscribing - I find this an interesting/powerful feature and will be first to help testing =)

jwilloughby’s picture

subscribing ! thanks for your help on this !

rwohleb’s picture

In response to #5, how about adding a 'module' column. This will be set to the module generating the PM. The core PM code can then call the appropriate hooks in the matching module on view, etc.

BenK’s picture

Subscribing.... Would love this feature!

archard’s picture

FileSize
2.21 KB

I developed this module for my own site, but thought it might be useful for the purposes discussed in this thread. So, I am offering it for use by the privatemsg maintainers if they would like. It does not do things exactly like what has been discussed here so far, so if you don't want to use it, no big deal. If you do, I'm very happy to contribute to this great module.

This module simply adds a group of checkboxes of roles on the new private message form, and if any are checked, it sneaks in before privatemsg's validate and submit functions and modifies the recipient list to include all users from those roles. Essentially it acts as if the user typed in all the users into the "To" field manually. The permissions are pretty granular, so you can limit which users can send mass PMs to certain roles on a per-role basis.

Berdir’s picture

Thanks for contributing, I'll try it and will give you some feedback. We will most probably not include it as-is, but it might help some users until we have a "better" solution.

Mackee’s picture

Hi archard, i tried to use the module, i got an error when creating message.

warning: array_unshift() [function.array-unshift]: The first argument should be an array in /****/********/public_html/*****/sites/all/modules/pm_by_role/pm_by_role.module on line 98.

Thanks!

archard’s picture

Hi neriweaver. What version of Privatemsg are you using?

Mackee’s picture

Oh, im using 6.x-1.0-rc4. And Drupal 6.14. Thanks for the quick reply!

Mackee’s picture

Hi archard. Just an update, I update my module to privatemsg 6.x-1.1. The error is gone, but I'm not seeing any checkbox for roles. (Im the user1/admin). Thanks!

archard’s picture

Hi neriweaver,

I looked through the privatemsg code, and indeed the module will produce errors for versions less than 6.x-1.0.

However, I can't figure out why the checkboxes wouldn't show up for you if you're using privatemsg 6.x-1.1, because that's the version I wrote the module for. Maybe there's a difference between Drupal 6.14 and 6.15 that I'm unaware of, as I developed the module on 6.15 and you say you're using 6.14. Try doing things like uninstalling/reinstalling the module, clearing Drupal's cache and clearing your browser's cache and let me know if the problem persists. User 1 should see all roles on the new pm form no matter what.

Mackee’s picture

I update my drupal to 6.15, uninstall the pm_by_role, reinstall, clear drupal cache and browser cache and I still didn't see the checkbox... So far I haven't seen any errors, it's just that I didn't have the checkbox when creating message.

Thanks and I appreciate the help archard. :)

archard’s picture

neriweaver, I really don't have any idea why the checkboxes wouldn't show up. Is anyone else having this problem?

I don't suppose there's any chance that you missed the collapsible fieldset that the checkboxes are wrapped up in?

Mackee’s picture

Hi archard, I got it... I think it's having a problem with my custom theme. I tried switching to the default theme(garland) and the collapsible fieldset appeared. I find out that it's because of my theme's css, I didn't know that the checkboxes will appear on the collapsible fieldset, I hide it through css for some reasons, long ago when I installed privatemsg, because it is not supported with better formats. Maybe I'll find a better way of hiding it on privatemsg.

I haven't tried sending messages yet. Thanks for the assistance!

Mackee’s picture

It's working good, i tried it to a few roles... But haven't tried it to all authenticated users. Just a quick question, is it alright to use it to all users? Since I have nearly 200 users in all. Thanks for this great feature!

Berdir’s picture

Try it, I suppose nobody does know how well that it will work yet.

However, there will certainly be issues. One issue will most probably be that the message view page will list all 200 users.

Liliplanet’s picture

Very interested in this module. I've only tested on a local build, but my live site has over 10K+ per role. If anyhow has perhaps tried sending to a large group .. look forward to the feedback :)

Berdir’s picture

Now, 10k users is a totally different thing than 200 ;). The current module loads all user names into an array, this will make your server explode. And even if your server had unlimited ressources, displaying 10k themed users on the privatemsg view page will explode your browser ;)

Liliplanet’s picture

Ok, totally understand :) Thank you for the reply. Is there any other module that can send messages to a role, and not necessarily via Private Message?

archard’s picture

Performance definitely is an issue when sending to an extremely large group of people, and something this module doesn't really have an answer for. I wrote the module for my site as a collaboration tool for moderators and admins (~20 users), rather than a 'newsletter' type of deal. For true mass PMs by role, I think we have to continue with the original idea, which is something I would be glad to help out with as well.

Berdir’s picture

I am working on something but it will take some time...

Will upload something as soon as it's alteast partly working.

Berdir’s picture

FileSize
34.12 KB
PASSED: [[SimpleTest]]: [MySQL] 779 pass(es). View

Ok..

First, two important warnings...

The following patch changes the database structure that will make it impossible to use privatemsg without that patch. Atleast not without manual intervention in the database.

This patch contains *many* changes, do not use this on a production environment there might be serious bugs and maybe even security issues.

However, you are very welcome to help testing the patch in a test/development environment.

Features
- Makes it (theoretically) possible to write messages to every possible type of user groups, for example users of a specific role, og, your friends/fans/whatever and so on.
- The patch currently implements it for roles
- Everything is currently still in privatemsg.module, I'm not sure if it makes sense to add a privatemsg_roles module
- It does not matter how big a group is (well, more or less)
- It does work through the existing To: field, including autocomplete support.
- You can mix recipients, so it is possible to write a message to two different roles and three normal users.
- When a user reads a message he recieved as member of a group, a new entry is inserted which can then be marked as read/deleted.

There are still several flaws, some are by design and others are simply bugs or things I've not yet figured out. Incomplete list:
- Currently, if you have a user and a role with the same name, you won't be able to write that user a message anymore. Possible fix: Instead of just "rolename", require something like "rolename [role]".
- When users of a group start reading a message, they are displayed explicitly as recipient but before that, just the role was displayed. A possible fix would be to either add a new recipient type (user_hidden or so) or a new column so that they can be marked as hidden and are excluded from the display.
- I'm not sure if the unread message systems works under all circumstances, it was quite tricky to get right (as right as it is now)
- The list query is getting more and more complex, we have to find ways to make it simpler and faster. That's not something for this issue, however.
- It does not yet work for PostgreSQL.
- Users that are added to a group will see messages that have been sent to this group before they joined.

I also have to add some tests (the existing tests all pass, but they only cover pieces of the functionality of privatemsg) and document the new hooks.

nbz’s picture

Not haivng studied the patch properly, just the post above...

- When a user reads a message he recieved as member of a group, a new entry is inserted which can then be marked as read/deleted.

This could get awful big on large sites...

I would consider an option where new rows are not added upon read and the messages are always marked as "read"?

and then in the messages/ instead of going to the inbox, we can have a dashboard or something with different tables for each object type?

(we should IMO keep the inbox/all messages as they are for all messages etc, but also have object specific messages pages.)

litwol’s picture

#33 if i'm not mistaken this functionality is how most forums and commenting systems track whether you've read something or not. The good part above above solution is that the table wont grow more than it needs to, meaning it degrades performance/db size only as much as normally using pmsg would.... or so i think.

Berdir’s picture

Status: Active » Needs review

Big data sets shouldn't be an issue with databases, that's what they are made for. An issue might be that our queries are slow, but I'm working on that.
On big sites, the pm_index table will grow anyway, with or without that. As litwol said, most forums use such a system to track who has read a message/post.

Also, I don't think what you suggest will lead to good usability, as there will be no visual indication if a message is new or not.

Your dashboard idea is a nice one, but I don't think that's something for this issue, the patch is already huge.

YK85’s picture

+1 subscribing

Status: Needs review » Needs work

The last submitted patch, privatemsg_roles.patch, failed testing.

Berdir’s picture

Version: 6.x-1.x-dev »

Wrong version...

Berdir’s picture

Status: Needs work » Needs review

#32: privatemsg_roles.patch queued for re-testing.

Feet’s picture

+1 subscribing

Berdir’s picture

FileSize
47.22 KB
FAILED: [[SimpleTest]]: [MySQL] 1,008 pass(es), 6 fail(s), and 0 exception(es). View

Ok, updated the patch above...

Lot's of new stuff....

From the above todo list:
- Currently, if you have a user and a role with the same name, you won't be able to write that user a message anymore. Possible fix: Instead of just "rolename", require something like "rolename [role]".
- Roles now require that you use "rolename [role]", which is also automatically suggested with the autocomplete. Translation is a bit tricky here, not sure if there is a better way than what I used in the patch...

- When users of a group start reading a message, they are displayed explicitly as recipient but before that, just the role was displayed. A possible fix would be to either add a new recipient type (user_hidden or so) or a new column so that they can be marked as hidden and are excluded from the display.
- Automatically added entries for delete/read marking now use the type hidden and aren't displayed anymore.

- I'm not sure if the unread message systems works under all circumstances, it was quite tricky to get right (as right as it is now)
It actually was quite buggy and requires SELECT SUM(is_new) FROM (...) subquery syntax now because PostgreSQL wants to use GROUP BY
- The list query is getting more and more complex, we have to find ways to make it simpler and faster. That's not something for this issue, however.
Not solved here, but I'm working on that in other issues.

- It does not yet work for PostgreSQL.
- Tested and confirmed to work with PostgreSQL

- Users that are added to a group will see messages that have been sent to this group before they joined. This is still true and I can't see a way around this. There is no information available about when a user was granted a role, so we can't filter out these messages based on such a date.

Another things done in this update
- Actually added the privatemsg_roles.module, moved the hooks to that
- Changed a few hook implementations slightly
- Documented all new hooks and added a documentation page to the apidocs
- Added some basic tests (writing a message to a group, response from normal user and admin, who can see what)

At this point, it should be possible to implement this for other "group providers" such as OG or UR.

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review

Reminder: I need to fix #729326: Unread static cache broken when there are no new messages on the next re-roll.

Berdir’s picture

FileSize
48.73 KB
FAILED: [[SimpleTest]]: [MySQL] 1,005 pass(es), 8 fail(s), and 0 exception(es). View

I introduced a few blocking related bugs with a hook_privatemsg_block_message() change. Fixed. Also fixed the static cache issue.

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types2.patch, failed testing.

Feet’s picture

Status: Needs work » Needs review

#44: privatemsg_recipient_types2.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types2.patch, failed testing.

YK85’s picture

I have a quick question on:
- Users that are added to a group will see messages that have been sent to this group before they joined. This is still true and I can't see a way around this. There is no information available about when a user was granted a role, so we can't filter out these messages based on such a date.

Does this mean:
1) if a user joins a group, and say the website has been around for 5 years, the user will receive 5 years worth of messages to that group?
2) i also understood group messages as a type of subscription, so you only get the group messages from the minute you join the group. are the group messages in a shared inbox that all users in the group view? or are users receiving the group messages and the user is able to delete/tag/etc the way they please?

I understand this whole thing is a work in progress but wanted to throw some questions out there.
Thanks!

okday’s picture

subscribing

nbz’s picture

If we are going to eventually create a new row per user anyway, is there a reason why we are not doing that in the first place? Just create all the rows when creating the message.

2) i also understood group messages as a type of subscription, so you only get the group messages from the minute you join the group. are the group messages in a shared inbox that all users in the group view? or are users receiving the group messages and the user is able to delete/tag/etc the way they please?

It is a mixture - for the end user, the currently proposed approach will mean that the user will see all the previus group messages and once a user has viewed a particular message, it will behave like any other message (meaning view permissions for a viewed message cannot be revoked.)

YK85’s picture

ah I see, thanks for the explanation!

It would be nice to have an option to allow/disallow access to 'historical' group messages for new joiners into a group. Also, if a user is given access to the 'historical' group messages then maybe all messages before the current moment should not be marked as *new* as they are past messages?

I am just trying to imagine a user joining a group on a site that has been around for multiple years and upon joining their inbox could show thousands and thousands of new group messages since inception.

Really great to see this develop in Privatemsg module and I look forward to any help needed with testing!

nbz’s picture

I do not think drupal stores data for exactly when a role was granted. More, i do not think we should be trying to replicate forum module - that with forum_access will cover the seeing messages from before the user joined the role. We should focus on pm-like functionality IMO

Berdir’s picture

I'm unsure right now. Let me explain...

I had two reasons for going with the current way:
- Even on sites with hundreds of thousands of users, I suspect that more than a few thousand will actually read a message. So in these cases, we would save a huge amount of entries to the pm_index table.
=> The problem of that is now that the current patch makes the queries, especially the list query, extremely complex. So a faster query on more data would probably be faster than a slow query on less data. Additionally, the list query still does not work correctly, for example, for deleted messages.

- If you send a message to say, 1000 users, we don't want to display all of them in the thread view *nor* do we want to allow normal users to reply to all of these.
=> The current patch allows that with the concept of "hidden" recipients, that are not displayed nor do they get replies (directly). That solution does not care if all recipients (members of a role, for example) are added explicitly, or just a few of those. So this isn't an argument against inserting all recipients initially.

I'm now thinking about the following (as an example for role, should work for all others too):

> Initially, we only insert the role recipient entry, as we do know.
> Then we automatically insert all members of that role as a hidden recipient, there are several ways to do this and open questions..
- > How much can we handle in privatemsg.module, what needs to be repeated for every module that provides recipient types.
- > When are we doing that? For messages sent through the form, we could use Batch API, but that doesn't work when the message is sent through the API. Then we would have to use cron. If there are no more than X recipients in that group, we could do it directly when sending the initial message.
- > That means it can take a while until all users get the message.
> Once all recipients are inserted, we can mark the initial role recipient entry as "read" (Note: we can't delete that, see below)
> When looking for messages, we just look for the recipient types user and hidden. Everything else is ignored. This means that a lot of the complex situations of the current way don't exist anymore (check if message is read/deleted is the most important one). The query also doesn't get more complex when you add more recipient types
> But we still look for all recipient types *except* hidden when looking for recipients and display these.

This means that there is still a timeframe when newly added users will recieve a message sent to a large group of users, but it's relatively small. But once a recipient group is processed, new recipients will not be added. They will get replies though, if a user is allowed to reply/write to that recipient group but I don't think we can work around that.

nbz’s picture

If you send a message to say, 1000 users, we don't want to display all of them in the thread view *nor* do we want to allow normal users to reply to all of these.

How about a Bcc/hidden feature? Blind carbon copy/hidden column in the index where if it is set, the usernames are not displayed (and replies are also either only to sender or disabled totally).

Berdir’s picture

What is the difference to my proposal? type = hidden would be that flag.

But such a flag isn't enough. Imho, we need to display to whom the messages where sent, and we need a way to send messages to groups of any size through the UI and the API functions.

sotiris’s picture

subscribing

Hierba’s picture

subscribing

hkvd’s picture

subscribing

Berdir’s picture

Status: Needs work » Needs review
FileSize
64.82 KB
PASSED: [[SimpleTest]]: [MySQL] 444 pass(es). View

Ok, attaching a new patch that implements what I proposed in #53.

The patch is quite big but all tests are working now (including lots of new roles specific tests)

The following happens now when a new message is sent:

- First, all recipients are added to pm_index as before
- Then, they are checked and if they are non-user recipients, we check if there are more than 100 (configurable) users that "belong" to that recipient (We assume that all recipients types "contains" one or many users) and if that's the case, they are directly processed
- If sending a message through the UI and one or multiple recipients has more than 100 users, they are added to a batch run and processed directly
- If the message is sent through an API function (new_thread/repy), recipient types with more than 100 users are processed during cron runs. Cron processed no more than 10 recipients or 1000 new hidden recipients (configurable, needs fine-tuning)

Now, processing means that a hidden entry for each user which belongs to a recipient type is added to pm_index and when searching for messages etc., we only look for user/hidden entries, but display all recipients except hidden.

The patch also contains a few other things, for example a privatemsg_user_load() that implements both a static cache and adds the required type and recipient properties to every user object.

Testing/Reviews would be very welcome but again, make sure to only test this on test sites and make backups before doing so!

litwol’s picture

haven't groked the patch so i may be saying something redundant but plz use negative values for non-user recipients to ensure avoiding possible future conflict when user recipient uid.

Berdir’s picture

The patch uses a new column called type to separate between user ids and role ids (and other things later on).

negative values don't work for the "other things" part, you can't use them to separate between between roles, groups, friends and more.

Berdir’s picture

#59: privatemsg_recipient_types4.patch queued for re-testing.

Berdir’s picture

FileSize
65.44 KB
PASSED: [[SimpleTest]]: [MySQL] 1,020 pass(es). View

Fixed a few bugs, all tests should now pass.

Berdir’s picture

FileSize
80.67 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_recipient_types6.patch. View

Ups, it looks like all my patches missed the new privatemsg_roles module :)

Berdir’s picture

Note that due to a bug in PIFR, the new roles tests aren't picked up by the test bot, see #763852: Use test.review.directory argument instead of list of modules

nilsja’s picture

subscribe

Berdir’s picture

Interesting, it seems that dozens want this feature but not a single test report in more than two weeks since I posted the updated patch, which pretty much only needs exactly that... field testing (field as in development environments but with possibly real data) :)

brst t’s picture

I'll test it!

Some time next week I will. Maybe someone will beat me to being the first before then.

Bilmar’s picture

FileSize
33.66 KB
55.81 KB

Hello,

Below is the result of applying the patch (Hunk #1 FAILED at 315):

patching file privatemsg.api.php
patching file privatemsg.install
patching file privatemsg.module
Hunk #15 succeeded at 869 with fuzz 1.
Hunk #16 succeeded at 896 (offset 1 line).
Hunk #17 succeeded at 924 (offset 1 line).
Hunk #18 succeeded at 961 (offset 1 line).
Hunk #19 succeeded at 1108 (offset 1 line).
Hunk #20 succeeded at 1211 (offset 1 line).
Hunk #21 succeeded at 1265 (offset 1 line).
Hunk #22 succeeded at 1289 (offset 1 line).
Hunk #23 succeeded at 1365 (offset 1 line).
Hunk #24 succeeded at 1438 (offset 1 line).
Hunk #25 succeeded at 1485 (offset 1 line).
Hunk #26 succeeded at 1518 (offset 1 line).
Hunk #27 succeeded at 1527 (offset 1 line).
Hunk #28 succeeded at 1578 (offset 1 line).
Hunk #29 succeeded at 1631 (offset 1 line).
Hunk #30 succeeded at 1807 (offset 1 line).
Hunk #31 succeeded at 1912 (offset 1 line).
Hunk #32 succeeded at 1991 (offset 1 line).
Hunk #33 succeeded at 2017 (offset 1 line).
patching file privatemsg.pages.inc
patching file privatemsg.test
Hunk #1 FAILED at 315.
Hunk #2 succeeded at 460 (offset -1 lines).
Hunk #3 succeeded at 475 (offset -1 lines).
1 out of 3 hunks FAILED -- saving rejects to file privatemsg.test.rej
patching file pm_email_notify/pm_email_notify.module
patching file privatemsg_filter/privatemsg_filter.admin.inc
patching file privatemsg_filter/privatemsg_filter.module
patching file privatemsg_rules/privatemsg_rules.module
patching file views/views_handler_field_privatemsg_link.inc
Hunk #1 succeeded at 150 (offset 19 lines).
patching file pm_block_user/pm_block_user.module
Hunk #1 succeeded at 161 (offset 4 lines).
Hunk #2 succeeded at 216 (offset 4 lines).
patching file pm_block_user/pm_block_user.pages.inc
patching file pm_block_user/pm_block_user.test
patching file privatemsg_roles/privatemsg_roles.info
patching file privatemsg_roles/privatemsg_roles.module
patching file privatemsg_roles/privatemsg_roles.test

After applying the patch and running update.php, I enabled the privatemsg role module.
I sent a message to users with testrole and the To: input field autocompleted to testrole [role].
Message was sent successfully and all users with role testrole received the message.

However, viewing the message results in the below user warning (screengrabs attached)

user warning: Not unique table/alias: 'r' query: SELECT pmi.recipient, u.name, pmi.type, r.name AS role_name, r.realname FROM pm_index pmi LEFT JOIN users u ON (u.uid = pmi.recipient AND pmi.type IN ('user', 'hidden')) LEFT JOIN role r ON (r.rid = pmi.recipient AND pmi.type = 'role') LEFT JOIN realname r ON (r.uid = u.uid) WHERE (pmi.thread_id = 189) AND ((pmi.type <> 'hidden') OR (pmi.type = 'hidden' AND pmi.recipient = 1)) GROUP BY pmi.recipient, u.name, pmi.type, role_name in /srv/www/example/public_html/beta/sites/all/modules/privatemsg/privatemsg.module on line 368.

Screengrab 1.png: reading the test message sent to role as admin (admin has role testrole)
Screengrab 2.png: reading the test message sent to role as regular user with role testrole

Please let me know if I may be doing something wrong in testing or if there is something else I can do to make troubleshooting easier.

Thanks!

Berdir’s picture

Status: Needs review » Needs work

Thanks for testing.

Usually, you can stop testing if a patch does not fully apply (if there are any failed hunks). In this case however, the failed hunk was in a .test file, which is not needed for the actually functionality, only for automated tests. A re-roll is

I assume you are using the privatemsg_realname.module from the other thread? This is the problem I guess, both modules try to use the alias "r" for their tables, which is not allowed.

I will re-roll this with a different alias so that the two modules work together, but if you want to continue testing, you could disable privatemsg_realname.module for now.

Berdir’s picture

Status: Needs work » Needs review

#64: privatemsg_recipient_types6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types6.patch, failed testing.

nilsja’s picture

the pm_by_role module does not work anymore with the current beta...

"Disallowed to send a message without at least one valid recipient"

Berdir’s picture

Status: Needs work » Needs review
FileSize
80.17 KB
PASSED: [[SimpleTest]]: [MySQL] 1,019 pass(es). View

Re-roll, fixed the failing hunk and used a different alias.

Bilmar’s picture

Hello Berdir,

The patch version 7 applied smoothly and testing went great!
I tested the patch version 6 with disabling realname as suggested and it worked without the errors.
Thanks for adding the privatemsg_realname compatibility =)

One thought I had during the testing was that it would be nice to expand permissions somehow for each role.

For example, a site may have role A, role B, role non-authenticated and role administrator.
-We may want to restrict role A to only be able to send to role A, but not role B, role non-authenticated and role administrator
-We may want to restrict role B to only be able to send to role A and role B, but not role non-authenticated and role administrator
-Not allow non-authenticated to send any messages to role (already possible with current permission [x] write privatemsg to roles)
-And of course, role administrator can send to any role =)

Just a thought, but hope it is something can may be considered.
I hope I didn't miss anything in the settings pages that might already allow this functionality.

Regards

Berdir’s picture

I guess you meant role authenticated, since you can't send message to non-authenticated, but I get the point.

I am not yet sure how to best implement that, possibly through pm_block_user module, since that already allows role based sending permissions, which are currently only applied to the users which have these roles but can be extended to the roles too. Will probably add a third blocking type, since you still want to allow sending messages to users with role B.

Berdir’s picture

#74: privatemsg_recipient_types7.patch queued for re-testing.

Bilmar’s picture

sorry I didn't clarify in my comment at #75, non-authenticated is a role I created which is temporarily assigned to a user on registration via LoginToboggan module and unassigned once they validate their account by clicking a link sent to their email (so different from anonymous or authenticated role). Nothing important but wanted to clarify. Not allowing users to send to this role would be one use-case.

Just to give an example with users:

user1 has role administrator, role A, role B
user2 has role A
user3 has role A, role B

role administrator allowed to send to role administrator, role A and role B
role A allowed to send to role A / not allowed to send to role administrator and role B
role B allowed to send to role A and role B / not allowed to send to role administrator

If user2 sends a message to role A, user1 and user3 should still receive this message even though user3 has role B and user1 has role administrator which user2 is not allowed to send a message to (and an error shouldn't appear for user2 when sending). I think this is one situation where it becomes complex as users can have multiple roles and essentially be allowed/be blocked to send/receive.

I look forward to following your work! =)

igorik’s picture

subscribing

Berdir’s picture

I created a site that can be used to test specific patches: http://test.worldempire.ch

This patch is the first one that will use that page, so everyone can now test this there!

Looking forward to many test reports, you don't have an excuse left for not testing this :)

Just like the demo site, the test site will reset itself every two hours, so you can safely play around with all the settings!

Important: Do not report anything through that site (comments, private messages), only do that here since all these messages would be deleted too.

davisjan’s picture

subscribing

gold-dupe’s picture

Hi berdir

great module btw!

I found following two bugs (in the test environment at http://test.worldempire.ch/):
- users that aren't in a role see the role names in the autocompletion box.
- when sending a message to a user with a role from an user without a role, I got following error message:
Fatal error: Function name must be a string in [..]/modules/privatemsg/privatemsg.module on line 2159

Berdir’s picture

If user2 sends a message to role A, user1 and user3 should still receive this message even though user3 has role B and user1 has role administrator which user2 is not allowed to send a message to (and an error shouldn't appear for user2 when sending). I think this is one situation where it becomes complex as users can have multiple roles and essentially be allowed/be blocked to send/receive.

Yes, I will add a way to separate roles and users in the pm_block_user admin page, so that you can block sending messages to a role but not the users which have that role.

Berdir’s picture

FileSize
81.15 KB
PASSED: [[SimpleTest]]: [MySQL] 1,053 pass(es). View

A few updates...

- Fixed the bugs reported in #82
- Added the author to the max and load recipients callbacks (required for user_relationship integration, I will upload a first version to #728552: Send private message to all friends soon)

(The test site has already been updated)

nilsja’s picture

privatemsg_recipient_types8.patch works great. thank you!

will it be included in the next beta?

Berdir’s picture

Yes, this will be added to the 6.x-2.x branch when it's finished. Note that it is not finished yet, I still need to polish some things a bit.

Bilmar’s picture

Berdir - currently on your test site only admin is able to send to roles.
user1 and user2 sees "Not allowed to write private messages to role members"

I'll take this situation to test for when users are not allowed to send to roles =)

Permissions
Admin (Richard Cypher) - can send to roles
User1 (Kahlan Amnell) - cannot send to roles
User2 (Zeddicus Zu'l Zorander ) - cannot send to roles

Test Flow
1. Admin sends message to recipient: role A [role]
2. User1 and User2 receive the message / shows Participants: role A [role] and Richard Cypher
3. User1 replies to message / shows Recipients: Richard Cypher
4. User1 now sees Participants: role A [role], Richard Cypher and Kahlan Amnell
5. User2 does not see User1's reply to the message but participants list updates to now show Participants: role A [role], Richard Cypher and Kahlan Amnell (!) this was unexpected
6. User2 now replies to message
7. Admin and User1 receive the message (!)

My Expected Flow
1. Admin sends message to recipient: role A [role]
2. User1 and User2 receives this message
3. User1 replies to message
4. Admin and User2 recieves the reply

I think all users that receive the message originally should all be includes in the replies thereafter, but this is just my understanding of blast/group messaging. I look forward to learning more about the direction/flow you're planning to go (your comments at #53 helped me understand better during testing)

Thanks!

nilsja’s picture

after applying the patch sending to roles works great.

but the send this user a message link under comments and in user profile produces an html line in the "To:" field.

<a href="/users/1" title="View user profile.">username</a>, instead of just the username.

edit: same problem when i directly call /messages/new/userid

wils.solutions’s picture

Hi,

So, basic I need just be able to send msgs to all users in a specific role.
I would like to know how could I have the multiple role feature. My users don't have rights to reply my msgs, I'm just going to use the Privatemsg as a kindof notifications system where I can send 1 message to all my Users.

So, please, how could I use to add the Role feature to my site?

thank you
Wils

Berdir’s picture

Wait until this has been commited and optionally help testing. This is not ready to be added to a production site.

Feet’s picture

Cheers Beldir, this patch is awesome.

Don't know if this is an issue (at all or for used modules) but Privatemsg roles doesn't register in the module 'used modules'.

(I found out when using the list to update core and then wondering why roles wasn't working, woops)

Berdir’s picture

No, it's not. used_modules only explicitly lists "official modules" which have project information in their info file. That information is automatically added to the file when a new release is made on d.o, but not when you use a CVS checkout or add an additional module as a patch. Once this is commited and you download the -dev release, it will be listed there.

Woggers’s picture

I downloaded the privatemsg 2.x dev package but there is no privatemsg_roles submodule? Where do I find the module which contains the active changes to this feature?

Berdir’s picture

In this issue, it is part of the patch.

Woggers’s picture

ah, applying the patch generates the files and applies the the code?

Thanks Berdir!

kriskd’s picture

FileSize
10.59 KB

I was prompted to enter a file to patch as noted on the attachment. I expected the patch to just run. What should I enter here?

kriskd’s picture

I was able to get the patch to apply successfully on my localhost. My attempt earlier today was on my webhost.

I ran the database update with no problem. However, I'm not seeing any difference in the private message user interface, nothing that indicates how to send to a role. What am I missing?

Berdir’s picture

Just start writing a role name in the To: field, and then select it from the autocomplete suggestions.

kriskd’s picture

Thanks, I got it working and it looks great!

I read in comment #87 that only the admin (user 0) can send to roles and that appears to be the case. In this instance, that works for my needs, but are there plans to integrate it with the permission system so users can send to group if the admin allows?

nilsja’s picture

you find it at /admin/user/permissions

Bilmar’s picture

@#99 - that was just the settings for my testing. As mentioned by nilsja, you can give permission to other roles at /admin/user/permissions

BenK’s picture

Hey Berdir,

I just completed some testing of the latest patch at test.worldempire.ch and the state of the patch looks pretty good. I did notice, however, a few possible bugs/suggestions that I've outlined below. (Note that for my use case, I'm coming from the perspective of someone who is using the module to send mostly one-way notifications to site users from the site administrator.) Here are a few points:

1. When a private message is sent to a role, the "role A [role]" is displayed as the recipient. Yet, for the use case of a site notification system, the administrator may not want recipients to be aware of their role name. (For instance, imagine a role called "Bad Users.") Would it be possible to have an option for administrators to hide the role name from recipients when sending?

2. For site notification type messages, we may not want users to be able to reply to the message because we could get thousands of replies that administrators don't have time to read and respond to. So could there be a feature for administrators to disable replies on a particular message sent to a role?

3. If user1 receives a message that was sent to "role A" and chooses to respond, then user1's name appears as a participant in the conversation even though other users can't see what user1 actually wrote. First, might user1 not want their identity to suddenly be displayed to thousands of other users? Second, might the site administrator not want this either (if user1 was disgruntled, for instance). Third, imagine thousands of people replying to a message that was originally sent to a role. Would all of their names suddenly appear as participants? That might get pretty unruly on the page.

4. It appears that if user1 blocks messages from the admin user, the admin user can still send messages to user1 so long as the message is instead addressed to a role that user1 belongs to. Is this intended? I don't think this is necessarily bad, but I could see how it would be confusing to user1 (to be receiving a message from a user that he blocked).

5. Is there any way to prevent user1 from blocking the admin user? For instance, we might want admin messages to get through to site users no matter what. Could there be any way to specify certain users that cannot be blocked?

6. Token Integration: Even though these are bulk messages to a role, a site administrator may want it to appear like a more personal message. So how difficult would it be to add token support so that a user's name (or CCK field value) could be referenced from within the message?

Anyway, none of this is major stuff... overall, it's working pretty well. Looking forward to hearing your thoughts. I'm also happy to do more testing. Just let me know.

Cheers,

Ben

kriskd’s picture

3. If user1 receives a message that was sent to "role A" and chooses to respond, then user1's name appears as a participant in the conversation even though other users can't see what user1 actually wrote. First, might user1 not want their identity to suddenly be displayed to thousands of other users? Second, might the site administrator not want this either (if user1 was disgruntled, for instance). Third, imagine thousands of people replying to a message that was originally sent to a role. Would all of their names suddenly appear as participants? That might get pretty unruly on the page.

Clarification on this patch -- when an user replies to a message sent to a role, everyone in the role receives that message? If so, can this be disabled? I want to use this to send a message to all authenticated users or to one group to primarily point them to a post in the forum that I need to bring their attention to. If a conversation was started via private message, it would defeat the purpose of my message board.

Please let me know as being able to send a PM via role was the last thing I needed to upgrade my site from D5 to D6.

Berdir’s picture

Thanks for the testing and feedback!

1. When a private message is sent to a role, the "role A [role]" is displayed as the recipient. Yet, for the use case of a site notification system, the administrator may not want recipients to be aware of their role name. (For instance, imagine a role called "Bad Users.") Would it be possible to have an option for administrators to hide the role name from recipients when sending?

The next version of this will use the theme system to display/format the recipient. This means that it is easy to override it and customize it. I'm not 100% sure how to do exactly what you are asking for, but I will try to address it.

2. For site notification type messages, we may not want users to be able to reply to the message because we could get thousands of replies that administrators don't have time to read and respond to. So could there be a feature for administrators to disable replies on a particular message sent to a role?

Not for a single message. But, you could use a separate user account, call that "System" or something like that and add that user to a role to which normal users are not allowed to write to (you can configure this with the pm_block_user module). Another idea that was asked for in a separate issue is the ability send messages as the System instead of a user, which sounds like a useful feature.

3. If user1 receives a message that was sent to "role A" and chooses to respond, then user1's name appears as a participant in the conversation even though other users can't see what user1 actually wrote. First, might user1 not want their identity to suddenly be displayed to thousands of other users? Second, might the site administrator not want this either (if user1 was disgruntled, for instance). Third, imagine thousands of people replying to a message that was originally sent to a role. Would all of their names suddenly appear as participants? That might get pretty unruly on the page.

This is a good point and is also true for normal websites. Not sure how to address this yet, need to think about it.
@#103: Only if the user that is responding to that thread is allowed to write messages to roles, if not, only the author (and other directly adressed users), will receive that message.

4. It appears that if user1 blocks messages from the admin user, the admin user can still send messages to user1 so long as the message is instead addressed to a role that user1 belongs to. Is this intended? I don't think this is necessarily bad, but I could see how it would be confusing to user1 (to be receiving a message from a user that he blocked).

Yeah, I have noticed that too already. I have an idea on how to fix this, need to try it out.

5. Is there any way to prevent user1 from blocking the admin user? For instance, we might want admin messages to get through to site users no matter what. Could there be any way to specify certain users that cannot be blocked?

Again, the pm_block_user module allows you to define role based configurations which roles can write/block which other roles.

6. Token Integration: Even though these are bulk messages to a role, a site administrator may want it to appear like a more personal message. So how difficult would it be to add token support so that a user's name (or CCK field value) could be referenced from within the message?

Useful feature, but imho something that should be handled in a separate issue, this patch is already *huge* ;) There is an issue about this already, but nothing happened so far: #511796: Support tokens in private messages. Feel free to resurrect that issue and please describe *exactly* how you'd expect that this feature worked (think: permissions, user interface, available tokens, ...)

I'm working on a new version that will hopefully address a few of your points and also some internal things I found recently.

BenK’s picture

Hey Berdir,

Thanks for the insightful comments as always. Looking forward to testing the next version... ping me (I think you already have my e-mail) if you'd like any testing before you officially post it.

And yes, I'd love to able to send messages as the System (with a config option to specify the "name" of the System) instead of a user.

As for the Token issue, I've already subscribed to the other thread you suggested and will put some thought into it.

Quick question: When this functionality is complete do you plan to commit to 7.x branch as well? I'm going to start doing much more testing on the 7.x branch....

Thanks,
Ben

Berdir’s picture

Thanks for the insightful comments as always. Looking forward to testing the next version... ping me (I think you already have my e-mail) if you'd like any testing before you officially post it.

I'm fine with releasing, but it I need to finish a few things first. Will update the patch and the test site once it's done.

And yes, I'd love to able to send messages as the System (with a config option to specify the "name" of the System) instead of a user.

Please create a new feature request then (or check the queue, maybe there is already something).

Quick question: When this functionality is complete do you plan to commit to 7.x branch as well? I'm going to start doing much more testing on the 7.x branch....

Sure. Testing of the 7.x release would be awesome, especially of this #443784: Adding new fields to Private Messages.

BenK’s picture

Hey Berdir,

Per your suggestion in #106, I found an old feature request thread to send messages as the system. I updated the thread and it can be found here:

http://drupal.org/node/97746

Note that this thread has patches on it for Drupal 4.7 and 5.x. It doesn't appear that the patches were ever reviewed. So do you think any of this code can be updated or is it better to start from scratch? And should this be developed for the 6.x or 7.x version of Private Message?

Also, I'll soon begin testing the 7.x version of Private Message. As you suggested, I'll put special attention on #443784: Adding new fields to Private Messages.

Hope all is well,
Ben

Berdir’s picture

FileSize
85.94 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_recipient_types9.patch. View

Ok finally, an update, there are quite a few changes:

- Made the [role] optional and introduced [user]. Sending a message to a role now doesn't *require* the [role] but it can be used to explicitly specify a role. When there is a user with the same name as a role, the user can be addressed by adding a [user]. Also, if the user doesn't have permission to write to roles, the user name will always be used.

- Added a description to each recipient type, the To: field now contains a list of recipient types the user is allowed to write to. This should hopefully address the UX point made in #97

- Added a way to define a write and view permission for each recipient type, this allows for example to not display the role names when sending messages to your users. This is also automatically checked now and doesn't require block_message hooks anymore. This addresses 1. in #102

- The format callback is now a theme function and can be overriden in the usual ways

- User blocking rules are now checked when a recipient is added to a message. This addresses 4. in #102 and I added a test to confirm this.

- Only recipients with visible messages are displayed now in the thread view page (/messages not yet, since that is more complicated). This at least partly addresses 3. in #102.

I will update the test site once user_relationship_privatemsg has been ported too.

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types9.patch, failed testing.

Berdir’s picture

FileSize
85.78 KB
FAILED: [[SimpleTest]]: [MySQL] 1,031 pass(es), 79 fail(s), and 81 exception(es). View

Forgot to update to latest HEAD, this should apply.

Berdir’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, privatemsg_recipient_types10.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
90.2 KB
PASSED: [[SimpleTest]]: [MySQL] 1,131 pass(es). View

Ugh. Another reason why I'm glad that we're having a pretty good test coverage.

The change to fix 3. from #102 has been added just before I uploaded the patch and introduced a gazillion bugs. The new patch should now pass all tests.

Berdir’s picture

FileSize
90.75 KB
PASSED: [[SimpleTest]]: [MySQL] 1,131 pass(es). View

Ok, another re-roll with few changes to make user_relationships_privatemsg work correctly (update will be posted to #728552: Send private message to all friends soon).

The test site http://test.worldempire.ch has now been updated, including user_relationship_privatemsg, so go ahead and test!

There is one flaw remaining which I'm not sure how to resolve:

- When sending a message to a role/relationship, one of the recipients respond and the original author then responds again, the recipient that answered is now adressed directly and therefore visible for all other recipients.

All tests pass now on MySQL and PostgreSQL, this is getting ready....

nbz’s picture

How about limiting this to a broadcast system where replying is not possible?

Berdir’s picture

@nbz: Hm.. as written above, that is optionally possible (by sending messages from an account users are not allowed to answer or the system as soon as that is possible), which might be the usual when sending messages to roles.

However, I assume that it's a whole different story for things like friends, organic groups or contact groups. There, you might (or might not, depends on the site) want a discussion between the recipients.

Also, the problem could easily by solved for a single recipient type by implementing the block message hook and "block" recipients which are addressed through the group recipient type anyway. However, I'm trying to find a more generic, easier to implement (for recipient type provider modules) way. One of the earlier patches had a "does that user belong to your recipient type" type of hook, which I could re-add.

BenK’s picture

Just did a quick test of the http://test.worldempire.ch site (which includes the path in #114) and things are working pretty well. Here are three issues I noticed:

A. If a message is sent to a roleA and user1 replies to that message, then user1's name is properly hidden at messages/view/[messageid] for other users in roleA. However, at /messages other users in roleA can still see user1's name.

B. If a message is sent to role A, then recipients in role A can see that it was sent to all users in role A. I know you said earlier that hiding this could be handled in the theme layer, but I'm wondering instead about a couple of possibilities:

Option #1: Only recipients with permission to send to role A would be able to see that it was sent to role A. (One issue with this is that maybe an administrator is some cases wants recipients to know it was sent to role A.)

Option #2: A checkbox that appears only for users who can send to role A when they are sending to a role. This checkbox would say something like "Allow all recipients to see that the message was sent to a role."

The reason I think this is an issue is that I think in some cases an administrator may want a message to appear as a personal message, not a bulk message (especially if they are using tokens in the message for each user's name). Yet, in other cases, the admin may want all users to know that it was sent to a role (so that recipients can know that everyone was informed). Other ideas?

C. When a message is sent to a role A and the "role A" name appears at messages/view/[messageid], then perhaps "role A" should be a clickable link to admin/user/user with the role A set as the filter. This would make it easier to see who was sent a message and also could be used with the tokens patch to view each user's received message with the tokens filled in. (Likewise, if a message is sent to a user's "Friends" via the User Relationships module, this should link to a user's list of Friends... but I'll post another issue thread for this.)

--Ben

Berdir’s picture

A. I'm aware of that, I just don't know how to fix it :) This might have to wait until we have a working patch here: #647212: Load participants in thread list in a single but separate query. But that is rather complicated too :)

B. See #108. There are now separate write and view permissions, so you can configure which users can see role recipients and which not. Doesn't allow to configure it for each message separatly, though...

C. Will have to check if that is actually possible, but nice idea...

BenK’s picture

Hey Berdir,

Wanted to check in to see what else needs to be done before this can be committed to 6.x-2.x-dev. More testing? Or something else? Just wanted to know what I can to do to help.

Cheers,
Ben

Berdir’s picture

Version: » 7.x-1.x-dev
Status: Needs review » Patch (to be ported)

This is blocking pretty much everything else. To be able to proceed, this needs to be commited and then we can fix reported bugs. I will try to handle A in #647212: Load participants in thread list in a single but separate query, B is solved and I don't know how to do C, you can't simply create a link to list all members of a role but feel free to open a feature request. Please report any things you find in follow-up issues.

I have commited the latest patch, thanks to everyone who helped with testing and reviewing!

No I will start with the D7 port...

BenK’s picture

Awesome! That's great news!

Berdir’s picture

Status: Patch (to be ported) » Fixed

Forgot to commit the privatemsg_roles module yesterday. Did that now and also commited the Drupal 7 port. Please report any bugs you can find in new issues to avoid pinging all subscribed users in this issue.

Status: Fixed » Closed (fixed)

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