Advertisement:

Author Topic: 2.0.15: Personal Messages: deleted members invalidate message content  (Read 1321 times)

Offline Sh@mbles

  • SMF Hero
  • ******
  • Posts: 4,918
  • Gender: Male
    • i30 Owners Club
System: SMF 2.0.15

Subsystem: Private Messaging service

Installation: Vanilla installation (2.0.15 no modifications), also on home server (2.0.15 modded), my live forum (2.0.15 modded) and here at SM.org

Priority: low

While checking old private messages here at SM.org I noticed that conversations which include a deleted member invalidate portions of  that conversation, such that only the deleted member's entries exist. The PM content then becomes fragmented and unusable.

Replicated this on my own live forum and also on a vanilla installation of 2.0.15.

This obviously is of very low importance and I haven't checked it on 2.1B4 yet.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 69,307
    • StoryBB/StoryBB on GitHub
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #1 on: August 08, 2018, 02:40:02 PM »
Which view method, all at once, one at a time, conversation?
Don’t try to tell me that some power can corrupt a person. You haven’t had enough to know what it’s like.

No good deed goes unpunished / No act of charity goes unresented.

Offline Sh@mbles

  • SMF Hero
  • ******
  • Posts: 4,918
  • Gender: Male
    • i30 Owners Club
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #2 on: August 08, 2018, 04:52:35 PM »
Conversation mode. It's the only one that would show multiple interactions.

Quote
conversations which include a deleted member

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 69,307
    • StoryBB/StoryBB on GitHub
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #3 on: August 08, 2018, 05:21:31 PM »
Figured but I'd figured I'd just check too ;)

Deletion of an account does do quite a number on PMs but IIRC it's not supposed to remove them all.

Let me guess, the ones you're missing are the ones you sent and you don't have 'save a copy of sent items by default' or whatever the option is called set to on.
Don’t try to tell me that some power can corrupt a person. You haven’t had enough to know what it’s like.

No good deed goes unpunished / No act of charity goes unresented.

Offline Sh@mbles

  • SMF Hero
  • ******
  • Posts: 4,918
  • Gender: Male
    • i30 Owners Club
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #4 on: August 08, 2018, 05:32:57 PM »
That option is set, always has been.

All other conversations are intact. A typical conversation includes every PM I sent within the message and all responses, self-contained.

Any convo which includes a deleted member (eg, Kat or K@ in this case) only has his messages, none of mine, disjointing the convo.

As stated, it's so trivial and unimportant that it's probably not worth the effort looking any further, but I figured I'd record it in this section. Posterity sucks, so I find :)

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 69,307
    • StoryBB/StoryBB on GitHub
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #5 on: August 08, 2018, 06:11:36 PM »
Then I'm confused.

Yours should be intact - because it's only meant to delete once both the sender and all recipients have their messages marked deleted (in this case, a person deleting their messages marks any they sent as 'deleted by sender' and any they received as not being received because removing the entry from the recipients table is how it does that)

It really should be including yours too if the option to keep them in sent messages was turned on.
Don’t try to tell me that some power can corrupt a person. You haven’t had enough to know what it’s like.

No good deed goes unpunished / No act of charity goes unresented.

Offline Sh@mbles

  • SMF Hero
  • ******
  • Posts: 4,918
  • Gender: Male
    • i30 Owners Club
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #6 on: August 08, 2018, 06:40:26 PM »
Would be nice if it could be confirmed.

Thing is, as trivial as I'm trying to paint it, it's easily reproducible.

Offline Virginiaz

  • Semi-Newbie
  • *
  • Posts: 46
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #7 on: August 08, 2018, 07:18:16 PM »
Would be nice if it could be confirmed.

Thing is, as trivial as I'm trying to paint it, it's easily reproducible.

I looked at it and tested it, and it does as you described. Looking at the database, it's not marking the message as 'deleted' => 1 in smf_pm_recipients, it's just flat out deleting the entry when the account is deleted.

In Subs-Members.php / deleteMembers():

Code: [Select]
   // They no longer exist, so we don't know who it was sent to.
   $smcFunc['db_query']('', '
      DELETE FROM {db_prefix}pm_recipients
      WHERE id_member IN ({array_int:users})',
      array(
         'users' => $users,
      )
   );


should probably be changed to:

Code: [Select]
   // They no longer exist, so we don't know who it was sent to.
   $smcFunc['db_query']('', '
      UPDATE {db_prefix}pm_recipients
      SET deleted = 1
      WHERE id_member IN ({array_int:users})',
      array(
         'users' => $users,
      )
   );


That changes deleteMembers to do what SMF does when the user deletes their version of the conversation - it marks it deleted instead of removing it entirely.

Thinking about it more, this code may need a little more work, namely to check if the receiving user(s) deleted their copy of the convo, in which case all of it should be deleted because the only user(s) not being deleted do not have the convo anymore either.

Offline Virginiaz

  • Semi-Newbie
  • *
  • Posts: 46
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #8 on: August 08, 2018, 08:43:03 PM »
^ Addendum to above msg (can't seem to modify it?): The proper thing to do I think would be to just take the relevant code from PersonalMessage.php // deleteMessages() function and apply it to the user being deleted, but for all their messages vs specific msgs. It's not enough to just mass-delete their version of all their messages, since it screws up PM convos, like OP pointed out.

It's more work overall for the deleteMembers function but shouldn't be too annoying.

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 69,307
    • StoryBB/StoryBB on GitHub
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #9 on: August 09, 2018, 03:00:40 AM »
Considering that deleteMembers calls deleteMessages...?
Don’t try to tell me that some power can corrupt a person. You haven’t had enough to know what it’s like.

No good deed goes unpunished / No act of charity goes unresented.

Offline Virginiaz

  • Semi-Newbie
  • *
  • Posts: 46
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #10 on: August 09, 2018, 02:33:00 PM »
Oh wow, I completely missed that.
Anyway, maybe then the other query can be removed, because it's definitely causing unexpected results. If deleteMessages() works properly for all the messages, then that should be all that is needed since it's all that is used when a user deletes their copies normally from the PM center.

That's all I have to say since I don't want to thoroughly pick apart that code (apparently my brief skimming + testing won't be enough here)
« Last Edit: August 09, 2018, 02:54:54 PM by Virginiaz »

Offline Arantor

  • Resident Overthinker
  • SMF Friend
  • SMF Legend
  • *
  • Posts: 69,307
    • StoryBB/StoryBB on GitHub
Re: 2.0.15: Personal Messages: deleted members invalidate message content
« Reply #11 on: August 09, 2018, 02:36:14 PM »
No, it's called in a different way to that, based on its owner. I'm not 100% sure it's correct.
Don’t try to tell me that some power can corrupt a person. You haven’t had enough to know what it’s like.

No good deed goes unpunished / No act of charity goes unresented.