Besides the normal private message functionality I am sending predefined private messages to users, i.e. the user can't change the text, but simply has a button that sends a predefined fixed message (invitation, etc). When doing this, the user is not redirected to the chat.
In this case a notification is shown about new private messages (notification block). But the new message is the one send by the user itself.
The same occurs if I'm using a modal to send a private message and afterwards do not redirect the user to the thread.

How can this be avoided and the thread be marked as read? (Of course only if it does not contain messages from the other thread members.)

Maybe it would be even better to exclude messages send by the current user in getUnreadThreadCount() in PrivateMessageService.php. I don't see any case where it makes sense to count the own messages send to others as new ones for oneself.

CommentFileSizeAuthor
#20 interdiff-2977310-19-20.txt1.24 KBeelkeblok
#20 private_message-message_count-2977310-20-d8.patch2.49 KBeelkeblok
#19 interdiff-2977310-14-19.txt899 byteseelkeblok
#19 private_message-message_count-2977310-19-d8.patch2.44 KBeelkeblok
#14 private_message-message_count-2977310-8.patch2.44 KBmike82
#12 private_message-message_count-2977310-8.patch4.97 KBmike82
#7 private_message-message_count-2977310-7.patch2.07 KBohorbatiuk
#6 private_message-message_count-2977310-6.patch1.83 KBohorbatiuk
#5 private_message-message_count-2977310-5.patch1.16 KBlamp5

Comments

mike82 created an issue. See original summary.

mike82’s picture

mike82 commented
Issue summary: View changes
mike82’s picture

mike82 commented 

public function getUnreadThreadCount($uid, $lastCheckTimestamp) {
return $this->database->query(
      'SELECT COUNT(DISTINCT thread.id) FROM {private_messages} AS message JOIN ' .
      '{private_message_thread__private_messages} AS thread_message ' .
      'ON message.id = thread_message.private_messages_target_id JOIN ' .
      '{private_message_threads} AS thread ' .
      'ON thread_message.entity_id = thread.id JOIN ' .
      '{private_message_thread__members} AS member ' .
      'ON member.entity_id = thread.id AND member.members_target_id = :uid ' .
      'WHERE thread.updated > :timestamp AND message.created > :timestamp AND message.owner <> :uid',
      [
        ':uid' => $uid,
        ':timestamp' => $lastCheckTimestamp,
      ]
    )->fetchField();
}
jochemvn’s picture

jochemvn commented

I think another reason for this to happen is the fact that 'unread' is determined wrong.

    $last_access_time = $entity->getLastAccessTimestamp($this->currentUser);
    $newest_message_timestamp = $entity->getNewestMessageCreationTimestamp();
    if ($last_access_time <= $newest_message_timestamp) {
      $classes[] = 'unread-thread';
    }

It compares the last created message in the thread with the last time I've accessed the thread

1. Like you mentioned it's weird that it does that for my own messages
2. If the time of creating that message is the same as the last time I've accessed the thread, which is obviously true... the thread is marked as read.

I think
if ($last_access_time <= $newest_message_timestamp) {
should be changed to
if ($last_access_time < $newest_message_timestamp) {

lamp5’s picture

lamp5
Primary language Polish
Location Rzeszow
commented
Status: Active » Needs review
StatusFileSize
new private_message-message_count-2977310-5.patch1.16 KB

I can confirm that @mike82 code from #3 fixed this issue on branch 8.1 and 8.2. I prepared a patch.

ohorbatiuk’s picture

ohorbatiuk
he/him/his
Primary language Ukrainian
Location Ukraine 🇺🇦
commented
StatusFileSize
new private_message-message_count-2977310-6.patch1.83 KB

Contain solutions from #3 and #4.

ohorbatiuk’s picture

ohorbatiuk
he/him/his
Primary language Ukrainian
Location Ukraine 🇺🇦
commented
StatusFileSize
new private_message-message_count-2977310-7.patch2.07 KB

Show thread as read when it contains few messages and the last message is own.

zenimagine’s picture

zenimagine commented

Is this patch related to this problem that there is no solution:

https://www.drupal.org/project/private_message/issues/2984799

zenimagine’s picture

zenimagine commented
Related issues: +#2995327: Unread message count is showing wrongly.
mike82’s picture

mike82 commented
Related issues: +#2984799: Unread Thread Count not updated correctly

Looks good.

It can be still corrected so that is also fixes Unread Thread Count not updated correctly.

  public function getUnreadThreadCount($uid, $lastCheckTimestamp) {
    return $this->database->query(
      'SELECT COUNT(DISTINCT thread.id) FROM {private_messages} AS message ' .
      'JOIN {private_message_thread__private_messages} AS thread_message ' .
      'ON message.id = thread_message.private_messages_target_id ' .
      'JOIN {private_message_threads} AS thread ' .
      'ON thread_message.entity_id = thread.id ' .
      'JOIN {private_message_thread__members} AS member ' .
      'ON member.entity_id = thread.id AND member.members_target_id = :uid ' .
      'JOIN {private_message_thread__last_access_time} AS last_access ' .
      'ON last_access.entity_id = thread.id ' .
      'JOIN {pm_thread_access_time} as access_time ' .
      'ON access_time.id = last_access.last_access_time_target_id AND access_time.owner = :uid AND access_time.access_time < thread.updated ' .	  
      'WHERE thread.updated > :timestamp AND message.created > :timestamp AND message.owner <> :uid',
      [
        ':uid' => $uid,
        ':timestamp' => $lastCheckTimestamp,
      ]
    )->fetchField();
  }
zenimagine’s picture

zenimagine commented

@mike82 Hello, would it be possible to have a patch and close the other problem if the patch solves

mike82’s picture

mike82 commented
StatusFileSize
new private_message-message_count-2977310-8.patch4.97 KB

here you go. Please test it.

zenimagine’s picture

zenimagine commented

@mike82 Thank you, but I have not managed to apply the patch :

ubuntu@www-domaine-com /var/www/www-domaine-com $ composer update --with-dependencies
Gathering patches for root package.
> DrupalProject\composer\ScriptHandler::checkComposerVersion
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Installing drupal/private_message (dev-2.x 9ded9bc): Cloning 9ded9bc50d from cache
  - Applying patches for drupal/private_message
    https://www.drupal.org/files/issues/2018-07-31/private_message-access_denied_for_administrators_on_edit_view_delete-2989580-2.patch (Access denied for administrators on edit/view/delete)
    https://www.drupal.org/files/issues/2018-07-09/private-message-2984541-0.patch (PrivateMessageService createRenderablePrivateMessageThreadLink throws an error)
    https://www.drupal.org/files/issues/2019-02-05/private_message-message_count-2977310-8.patch (Own send messages are shown as new messages)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2019-02-05/private_message-message_count-2977310-8.patch
mike82’s picture

mike82 commented
StatusFileSize
new private_message-message_count-2977310-8.patch2.44 KB
zenimagine’s picture

zenimagine commented

@mike82 Thank you, I managed to apply your last patch and it seems to work. I think you can close the other problem and review this one

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
commented

When I was sending a message and leaving the page, I had a notification even when it was me that sent the message. With the patch it looks to be fixed, thanks.

mike82’s picture

mike82 commented
Status: Needs review » Reviewed & tested by the community
eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented
Status: Reviewed & tested by the community » Needs work

We are seeing some issues with the unread count for newly created threads. We need to do some more investigating (most likely tomorrow), but chances are this is in scope for this patch. I hope to be able to provide some input and possibly an update to the patch (or a "false alarm") tomorrow by the end of the day (CET).

eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented
Status: Needs work » Needs review
StatusFileSize
new private_message-message_count-2977310-19-d8.patch2.44 KB
new interdiff-2977310-14-19.txt899 bytes

OK, turned out to be an easy one. The query checks whether the last access time for the current user is smaller than the time it was updated. Fortunately, a foreseeing soul already made sure the user has an access time for the newly created thread at all, so we don't need to mess with LEFT JOINs, but the access time registered for the target user is set to the creation time, which obviously is not smaller than itself... So without further ado, here's the updated patch.

eelkeblok’s picture

eelkeblok
he/him
Primary language Dutch
Location Netherlands 🇳🇱
commented
StatusFileSize
new private_message-message_count-2977310-20-d8.patch2.49 KB
new interdiff-2977310-19-20.txt1.24 KB

Here's an updated patch. It occured to me that the way the unread status of a thread was determined is a bit convoluted. The getNewestMessageCreationTimestamp() loops over all messages in the thread, after which - in this patch - the view method in the ThreadViewBuilder also loops over them. I personally also think this is a bit more intuitive, but that might just be personal preference.

anmolgoyal74’s picture

anmolgoyal74 commented

The patch works for me.
Let's wait for others to review the patch. Then I will move forward with the patch.

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

It seems to be working for me too.

anmolgoyal74’s picture

anmolgoyal74 commented
Status: Reviewed & tested by the community » Fixed

Thanks for the patch.
Fixed.

Status: Fixed » Closed (fixed)

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