add/remove somebody to/from discussion (Privatemsg forwarding)

Please elaborate on this feature more. for example:
1) when adding a new user to conversation, do can they see all previou conversation or only new messages.
2) when removing user from conversation, do they keep old messages in the inbox, do they get notified that they were removed from the thread, etc etc...

Discuss this in more detail.

I have a working forward module, that allows to to forward a thread to someone and optionally remove yourself. I will release it soon.

Ok, here is a first version.

Note: This has been tested, but only against a patched version of privatemsg.module. It *should* work with the current -dev version, but It has not been tested.

Features:
- Displays a Forward Thread form below the reply form
- Allows to forward a thread to 1-n users
- Optionally allows to remove yourself from the thread. The checkbox is currently enabled by default.
- It is also possible to write a reply and forward a thread at the same time.

Ah, I see.

To avoid a bug when a recipient is added multiple times, it does check if he can already see that message. However, that functionality depends on the changes in _privatemsg_load from #298502: Better access to another user's messages.

Marking this postponed on that issue, but you can of course apply the other patch and then try this again.

Hint: Even as it does not work currently, it should still display a confirmation message.

No, it hasn't :)

This is a new feature, so others might be looking for it too, don't set it to fixed unless it has been added to privatemsg CVS.

read all has been commited, setting this back to needs work.

Ok, updated the the module and made a few changes..

- to field is optional now, to make replying work again. Not sure if it should be allowed to remove themself from a thread without forwarding. maybe different permissions for forward/remove self?

- blocking rules are now checked when forwarding

Not that there is a small bug related to blocking, the recipients are currently validated even if no message is sent, so a warning is displayed if the thread contains participants that block you. This can be fixed with the attached patch.

with the newest 6.x-1.x-dev i'm getting the following error whenever i try to forward a thread:

Fatal error: Call to undefined function _privatemsg_load() in /Applications/MAMP/htdocs/10_27ddoMirror/sites/all/modules/privatemsg/privatemsg_forward/privatemsg_forward.module on line 113

does this have something to do with my setup or is there a new conflict with the newest dev build?

thanks.

This is a conflict. I'll re-roll this soon.

You can simply replace "_privatemsg_load" with "privatemsg_message_load" but there might be other conflicts as well.

that did solve the error. i will let you know if i see any other conflicts.

i also am noticing that if someone removes themselves from a thread when forwarding, any message they have written is not included in the forwarded message. if they are simply forwarding the thread to include a new participant, the message is included, but when the "remove myself" box is enabled, the newly typed message is not included. this is a very nuanced difference for a user to intuit. i think it would make more sense for the module to check if there is anything in the message box and include it in the thread prior to making either of the participant side changes. from a user perspective it is much simpler that way.

Updates:

- Rerolled against latest -dev release of privatemsg
- Forwarding a message could result in a strange order of the messages for the "new" user. Requires a patch.
- Added a few test cases as it seems that I'm always breaking a feature when fixing a bug :)

As said already in other places, the sub-module won't make it into the 1.0 release but I will most probably include the patch once that has been tested a bit more. So please report if everything works.

hi berdir,

the new build seems really good. i'm noticing one issue. if the sender selects to remove themselves from the thread without forwarding it to anyone (just to remove themselves from a multiple participant thread), any comment is lost. however, if the sender selects to remove themselves and selects someone to forward it to, their comment is saved into the thread. is there anyway to just have the comment always saved into the thread?
i think it will be useful for people in multiple participant threads to be able to comment and remove themselves.

everything else seems to be working great.

thanks.

Yeah, I missed those two use cases in my tests. Fixed the bug and updated the tests.

Uploading the module would help a lot.

do we still need to use the patch on privatmsg.module with the current -4 build?

Yes, the patch does fix two bugs in privatemsg.module that are only visible when using this module.

Thanks for this submodule. But why is forwarding a separate action rather than adding a person to the message when replying? Is it possible to allow for the addition of users ON the actual message or response?

you can combine add participant/remove me/reply in any way you want, just enter a reply and press "Forward thread".

Commited the patch in #19 against 6.x-1.x-dev and 7.x-1.x-dev, this module should now work correctly without the need of a patch.

Hint: The D7 Port also contained a few other bugfixes that were necessary due to changes in Drupal 7.

subscribing

What is the status on this?

And can this be used om the 6.x-2.x-dev release?

It *should* work but I haven't tested it for quite some time.

I suggest you try it. It it still works, a re-roll as a patch would be great.

Some tests would also help to get this commited :)

Re-rolled the latest version, all tests pass again.

Edit: Changes:
- Forward thread fieldset is now inside the privatemsg fieldset
- *If* there is a message, the "Send message" button will also work to forward/delete yourself. But only then.

Subscribing to help with testing...

Hey Berdir,

The functionality of this new sub-module is working great. I've tested the various buttons and options and things are functioning smoothly.

One thought that occurred to me, however, is that we may want to divide the one permission supplied by this module ('use privatemsg forward') into two different permissions ('forward a thread' and 'remove myself from thread'). This is because the "remove" functionality clutters the UI somewhat and may not be needed in many uses cases (since users can delete a thread, too). I do think the "remove"capability is useful, but maybe should just be optional for certain users who actually need it.

Also, one thing we may want to do before committing is to possibly change references from "thread" to "conversation"... I just think "thread" is not as clear to regular non-technical users who may not have used many forum-based websites. Here is how I would edit a few of the user-facing strings:

A. 'Forwarded thread %subject to !new.'

Change to >> 'This conversation has been forwarded to !new.'

B. 'Removed from thread.'

>> 'You have been removed as a participant in the conversation.'

C. 'Forward thread' (fieldset title)

>> 'Forward conversation to others'

D. 'Remove myself from thread.' (checkbox)

>> 'Remove myself as a conversation participant.'

E. 'This will remove you as a participant of that thread.' (checkbox description)

>> 'You will no longer be listed as a participant in this conversation. However, any messages you have previously posted will still appear in the conversation.'

F. 'Forward thread' (button)

>> 'Forward this conversation'

Cheers,
Ben

Thanks for the review.

I like the rename thread to conversation suggestion, we should do that too in other places where it is used.

My english is very technical, because I learned it in a technical environment. Words like thread, quota, threshold, .. are "normal" for me (see privatemsg limits issue discussion ;)), I only see how bad and technical they are when translated to german :) The german translation of Privatemsg uses "Nachrichtenstrang" or something like that, that makes my head hurt :)

Btw, if you have time, would be nice if you could join #drupal-games.

One more thing I wanted to mention: I'm not sure about the usability of putting the "Forward thread" functionality in a collapsed fieldgroup--especially right after the Input format fieldgroup. It just seems a little weird for this type of functionality to be kind of hidden like this (and to appear more like an option than a separate action).

If we wanted to change this, the possibilities include:

1) Make the "forward thread" fieldgroup a button called "Forward This Conversation". The options (recipient name, remove myself from thread) could then display on a separate page or in an overlay (D7). Not sure if the message should appear next to "Send message" or somewhere else.

2) Place a "Forward this message" link next to "Delete message" and "Block author" in the links section of each message. The only issue is that we're at the message level here rather than the thread level. One possibility would be to include a new option that allows users to forward the individual message only or forward the entire thread. This would be an added feature, too, but I'm not sure how feasible this is from a technical perspective. If the link was clicked, the options could then display on a separate page or in an overlay (D7).

3) Create a link called "Forward this conversation" and place it near where "Displaying messages 1 - 1 of 1" is currently shown in a thread. We'd be sort of creating a new place/location for thread-level options.

Thoughts?

--Ben

#38: privatemsg_forward.patch queued for re-testing.

Need to think about #42.

The problem with your idea is that even though the module is *called* privatemsg_forward, it's actually more an invite than a forward. Technically, we don't forward anything or create new messages (unless you have filled out the reply form, then a new message will be sent as well), but we *add* the recipients to the current, existing thread.

Given that, I think that for example "Forward this message" is a totally different thing then, there is simply no point in "inviting users to single messages of a thread" in my opinion, as they will get new messages of that thread as well from then on which might be completely out of context for them.

@Berdir (in #44): Instead of "Forward this message," how about one of these:

* "Add new recipient"
* "Include new recipient"
* "Add conversation participant"
* "Add person to conversation"

If we changed the action to one of these, how about placing an "Add new recipient" button (or one of the alternatives) next to the list of current recipients? So our layout would be communicating to the user, "these are the current conversation participants, want to add another?" ;-)

--Ben

love to sub

Sorry, I hope that we can add this soon ;)

Here is another re-roll.

Changes:
- Split the current permission into two separate permissions for forward/remove. No automatic conversion is done, you need to update your permissions.
- Updated strings from thread to conversation according to #40.

I haven't addressed #42 yet. We could maybe add a link beneath "Tag this conversation" but this might be easier as a follow-up after this has been commited.

Forgot to change the autocomplete path.

This should work...

There is a donation link on the project description page that you can use or click here to donate.

Thanks ;)

Ok, I decided to go ahead with this and commit it. Also added a class, but used "privatemsg-forward".

Thanks for testing. This will need to be ported to 7.x-2.x-dev once that branch has been opened.

Has this been ported to 7.x-2.x-dev now?

Is this a feature that has been ported to the D7 branch?

Subscribing. Although I'd like to see a D6 version if possible.

I'm working on a patch to make this feature available on the latest stable version. At this time i've simply commented out the perm function and all the '#access' from the hook_form_privatemsg_new_alter function just to make it work (for everyone, not just the superadmin) and see what has to be done, so I'm not going to attach anything now.

Do you think we need to create a different thread to work on this?

This issue is fine I guess.

Feel free to post a patch even if it's not finished, I'll try to do a review to give you feedback.

I tried to apply it against the latest stable and the latest dev and it's not working.
Some parts evidently still needs to be ported. Just to say: "hook_perm()" is a D6 function, we need to use "hook_permission()" to make permissions work on D7.

Well yes, it of course needs to be ported properly to Drupal 7, it will not just work.

It was much less complicated than I supposed because the only things to change were permissions and one if statement.
Attached is the source I'm testing. Since I've changed only privatemsg_forward.module I'm attaching only my modified version of that file.
This far I've not found any problems but surely this needs more tests before we can say it's "stable".

Please provide it as a patch so that I can review your changes.

This should work for 7.x-1.x

+++ b/privatemsg.module
@@ -85,16 +85,14 @@ function _privatemsg_generate_user_array($string, $slice = NULL) {
     // If it is an integer, it is a user id.
-    $user_ids = privatemsg_user_load_multiple(array($uid));
-    if ((int)$uid > 0 && ($account = array_shift($user_ids))) {
+    if ((int)$uid > 0 && ($account = array_shift(privatemsg_user_load_multiple(array($uid))))) {
       $participants[privatemsg_recipient_key($account)] = $account;
     }
     elseif (strpos($uid, '_') !== FALSE) {
       list($type, $id) = explode('_', $uid);
       $type_info = privatemsg_recipient_get_type($type);
       if ($type_info && isset($type_info['load']) && is_callable($type_info['load'])) {
-        $temp_load = $type_info['load'](array($id));
-        if ($participant = array_shift($temp_load)) {
+        if ($participant = array_shift($type_info['load'](array($id)))) {
           $participants[privatemsg_recipient_key($participant)] = $participant;
         }
       }

It looks like you diffed against a different version of Privatemsg than what you started with. All these changes in private.module look unrelated or temporary and shouldn't be there..

+++ b/privatemsg_forward/privatemsg_forward.info
+++ b/privatemsg_forward/privatemsg_forward.info
@@ -0,0 +1,6 @@

@@ -0,0 +1,6 @@
+; $Id$
+name = Privatemsg Forward

The $Id$ stuff should be removed, that's a left over from the CVS times. Same for the other files

+++ b/privatemsg_forward/privatemsg_forward.module
@@ -0,0 +1,149 @@
+    'forward a privatemsg thread' => array(
+      'title' => t('Forward a message'), ¶
+      'description' => t('Forward a privatemsg thread to one or more users.'),
+    ),
+    'remove from a privatemsg thread' => array(
+      'title' => t('Remove from thread'), ¶
+      'description' => t('Remove from a privatemsg thread after forwarding it.'),

Trailing spaces.

+++ b/privatemsg_forward/privatemsg_forward.module
@@ -0,0 +1,149 @@
+ * Implements hook_privatemsg_view_messages_alter().

Should be Implements hook_form_FORM_ID_alter().

+++ b/privatemsg_forward/privatemsg_forward.module
@@ -0,0 +1,149 @@
+  if ($form_state['values']['remove']) {
+    db_query("DELETE FROM {pm_index} WHERE thread_id = %d AND recipient = %d AND type IN ('hidden', 'user')", $form_state['values']['thread_id'], $user->uid);
+
+    // Display message and redirect to messages if user has removed himself.

This query needs to be updated to use db_delete().

+++ b/privatemsg_forward/privatemsg_forward.test
@@ -0,0 +1,165 @@
+}
+\ No newline at end of file
\ No newline at end of file

This looks strange, looks like you added the no newsline line into the file. You should remove that and leave an empty line at the end of the file (same for the other files) instead.

I must have applied all changes to the wrong version, or something... I'll check later and submit a new patch.

is this thread/mission still active?

Any news?

Re-roll of #68 in accordance with the comments in #71. I have not yet updated the SQL query because I don't understand it. Also, the changes to privatemsg.module (in #53) were not brought over into the D7 version... Does this need to be done?

You need to add the .test file to the .info file as files[] = filename so that the tests are picked up.

Oops, here it is, this time with the tests included.

Fixed the most obvious error in the test (misconfiguration of initial accounts).

+++ b/privatemsg_forward/privatemsg_forward.infoundefined
@@ -0,0 +1,8 @@
+files[]=privatemsg_forward.module

The module isn't needed here, only files with classes.

+++ b/privatemsg_forward/privatemsg_forward.moduleundefined
@@ -0,0 +1,148 @@
+    db_query("DELETE FROM {pm_index} WHERE thread_id = %d AND recipient = %d AND type IN ('hidden', 'user')", $form_state['values']['thread_id'], $user->uid);

db_delete('pm_index')
  ->condition('thread_id', ...)
  ->condition('recipient', $user->uid)
  ->condition('type', array('hidden', 'user')
  ->execute();

In the test, you have $recipient1 = $this->drupalCreateUser(...), but the createThread helper method uses $this->recipient1, needs to be that in the test method too.

Ah, ok. Then patch #78 should be the "more correct" version then, although it still needs work.

Ok, I see that the module is not needed. I put it in because I just copied from privatemsg_filter. Now that I check more closely, the only two submodules that reference the .module file are privatemsg_filter and pm_block_user.

For:

  ->condition('thread_id', ...)

What should be in the ...? I tried to look at the other delete queries but I couldn't find one with a thread_id.

$form_state['values']['thread_id'], as the old query uses.

Ok, here is a patch that addresses all concerns in #82.

Ok, getting there :)

So in the form submits, you need to change 'body' to 'body[value]' now.

Made changes as per #88.

Hello,

What is the current status of this issue? 2 years in standby.
I recently upgraded from Drupal 6 to 7 and found that privamemsg_forward.module doesn't exist anymore, and then I found this thread.

Anyone is using the patch in a production site ?
Thanks !

I was working on updating the patch for 7.x, but I never finished due to a shift in priorities. You're welcome to take up where I left off.

I tried the patch in #89 and it works for me.

Thanks @ptmkenny !

Thanks very much @ptmkenny for the patch, I confirm that it basically works with PHP 7.2 and MariaDB 10.2.22.

There is a problem though with private threads that are longer than the configured page size. Users that are added later to long conversations only have access to the page of messages that the user that added him has currently open. This is apparently due to:

    + foreach ($thread[‘messages’] as $mid => $message) {
    + foreach ($new_recipients as $new_recipient) {
    + $new_names[privatemsg_recipient_key($new_recipient)] = privatemsg_recipient_format($new_recipient);
    + privatemsg_message_change_recipient($mid, $new_recipient->recipient, $new_recipient->type);
    + }
    + _privatemsg_handle_recipients($mid, $new_recipients);
    + }

It shares all currently loaded messages, as opposed to all in the thread.