Add tracking for replies

Excellent feature addition - really useful.

Agree it needs to be 'sure' so it needs to be late in the process and ideally linked to the successful send. Not sure the best way to do that.

Here is a new patch for this old issue, for the current HEAD. Instead of using a status column, it adds a new field to track whether a message has been replied to or not, and it automatically marks as replied when you do so.

One thing this does not do is surface this in any way, something I am still thinking about because it can get a little weird. For instance, the main listing shows only threads. If I have replied to one message in a thread, and then a later message comes in that I haven't replied to, how do I display this? I'm not sure. Something to be discussed but here is a first pass at this feature anyways.

+++ b/privatemsg.installundefined
@@ -336,3 +343,18 @@ function privatemsg_update_7006() {
+function privatemsg_update_7007() {

Can you rename this to 7200? With 7007, we'd run into conflicts in case we need to add a update function to 7.x-1.x. Already made that error with 6.x-1.x and 6.x-2.x. It's no fun :)

+++ b/privatemsg.moduleundefined
@@ -1667,6 +1675,16 @@ function privatemsg_reply($thread_id, $body, $options = array()) {
   }
+  ¶

Some trailing spaces in different places.

+++ b/privatemsg.moduleundefined
@@ -1667,6 +1675,16 @@ function privatemsg_reply($thread_id, $body, $options = array()) {
+  $thread = privatemsg_thread_load($thread_id);
+  $reply_to_mid = end($thread['messages'])->mid;
+  db_update('pm_index')
+    ->fields(array('is_replied' => PRIVATEMSG_REPLIED))
+    ->condition('mid', $reply_to_mid)
+    ->condition('recipient', $author->uid)

I think it would make more sense to do that in _privatemsg_send() (and consistently pass the reply_to_mid to that, as part of the $message object.

Yes, displaying that information is an issue. The upcoming {pm_thread} table should make it easier.

Powered by Dreditor.

OK so I think this is much cleaner. I added the marking functionality to _privatemsg_send() and was able to remove it in other places. I also realized that since reply_to_mid is added to the reply form, it is automatically added to the message object on form submit, so I was able to remove some code there too.

Also removed white space and renumbered the update function as requested.

+++ b/privatemsg.installundefined
@@ -52,6 +52,13 @@ function privatemsg_schema() {
+      'is_replied'     => array(
+        'description'   => 'Whether the user has replied to this message',
+        'type'          => 'int',
+        'default'       => PRIVATEMSG_NOT_REPLIED,
+        'not null'      => TRUE,
+        'unsigned'      => TRUE,

Hm, is it possible that the constant isn't defined when you install the module or so? As it is defined in .module. Not sure...

+++ b/privatemsg.moduleundefined
@@ -17,6 +17,14 @@ define('PRIVATEMSG_UNREAD', 1);
@@ -1667,6 +1675,16 @@ function privatemsg_reply($thread_id, $body, $options = array()) {

@@ -1667,6 +1675,16 @@ function privatemsg_reply($thread_id, $body, $options = array()) {
       _privatemsg_handle_recipients($validated['message']->mid, $validated['message']->recipients, FALSE);
     }
   }
+
+  // If this is a reply, mark the last message appropriately.
+  if (isset($message->reply_to_mid)) {
+    db_update('pm_index')
+      ->fields(array('is_replied' => PRIVATEMSG_REPLIED))
+      ->condition('mid', $reply_to_mid)
+      ->condition('recipient', $author->uid)
+      ->execute();
+  }

Hm, according to the patch, this is still in privatemsg_reply?

Powered by Dreditor.

Some tests would also be great.

Since we already have a lot of tests both for replying through the UI and with privatemsg_reply(), we'd just need to make sure that the correct entry in pm_index has been marked as replied.

And when that information is actually displayed, that should be tested as well.

For displaying, here are some ideas:

- We could add a "privatemsg-replied" or similar class on the whole row in the listing like we already do for unread. See http://api.worldempire.ch/api/privatemsg/privatemsg.module/function/_pri...
- We could also expose an additional column "Replied" that prints "Yes" or "No" or something like that. I recently changed how to do that in 6.x-2.x and 7.x-2.x, see http://api.worldempire.ch/api/privatemsg/privatemsg.theme.inc/group/them...
- And third, we might want to display it on the thread view page too. Not really sure, though.

Here's a new patch that is actually correct and has tests for the API replies anyways. I need to think through how to test for success through the UI. Somehow I would need to get back the message IDs so I can go into the db and make sure everything got marked properly.

As far as displaying, I think the new class on the row makes sense. I was thinking about how to show it in the thread context, and that gets a little weirder. For instance, lets say I get a message and reply to it, then I get another message back. I have already replied to the thread, but I think most people would want to know if they have replied to the most recent message?

Now with less watchdog

About the querying. Wouldn't it be easier to query if we basically reverse the new replied column? Instead of simply storing 0/1 for the old message that was just replied to, what if we store the mid of the message that the current, new message is reply of?

We could actually do that in {pm_message} instead of {pm_index} too.

Then it should be possible to basically query if there is a newer mid in the thread than the highest replied to reference that is not sent by the current user. Still would be slow, but that's something we can solve with {pm_thread} later on.

Oh! Yes, that is a much better solution. I will look into that.

Subscribing to help with testing...

OK here is a new version that includes a new field - reply_to_mid - in pm_message that contains the mid of the message this one is a reply to (if any.) It is 0 if it is the first message in a thread. Let me know what you think of this approach.

Simple enough, now we need to figure out how to properly query that :)

Also, you're most recent patch is missing the test changes from above, should be possible to re-add them easily and just update that assert.

Ok, here is a new patch..

- Renumbered the update function because I added 7200 and 7201 in another issue :) (You will have to set the schema version back to something lower than 7200 before updating)

- Queried the replied for the thread listing, setting the is_replied flag to TRUE if it is. Not sure if there is a better way than how I did it. Should be pretty fast, though.

- Adds a privatemsg-replied css class to the tr tag for each thread that has no "un-replied" messages.

- I haven't added anything yes to the thread view, not sure about that.

- I also didn't port the tests from above yet.

And now with patch.

Grr. And now the *correct* patch :)

Yeah, I'm tired :)

New patch, with some tests and also fixed the error when there are no messages (Testbot++)

Applied manually to current -dev. Doesn't seem to alter the current tables. Looking closer to see if there's a way to make it work, will report back if I'm so lucky.

@1kentthomas - I just successfully applied the patch via git to the latest dev and the current tables were altered successfully. I also sent some messages between users and confirmed that the reply IDs were being added correctly.

Thanks for testing, committed and pushed to 7.x-2.x.