[3.0] Keep every recipient of a personal message, not just the last one - #9461
Open
albertlast wants to merge 1 commit into
Open
[3.0] Keep every recipient of a personal message, not just the last one#9461albertlast wants to merge 1 commit into
albertlast wants to merge 1 commit into
Conversation
pm_recipients holds one row per person a PM was sent to, and Received::$loaded is keyed accordingly, first by the PM and then by the member. Received::loadByPm() built its return value with only the PM as the key, so each new row overwrote the one before it and a PM came back with a single recipient - whichever the database handed over last. Two things go wrong from there. PM::canAccess() walks that list looking for the current member, so opening a PM you received alongside somebody else was "You are not allowed to access this section" unless you happened to be last. And PM::format() builds the recipient list from it, so the sent folder named one recipient of a PM that went to several, and the reply-to-all button was hidden because the count said one. Keys the list by recipient as well, which is the shape PM::format() and the popup already read it with, and picks out our own copy in applyActions() rather than labelling everybody's. Signed-off-by: Mathias Albert <mathiaspapealbert@hotmail.com> Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
pm_recipientsholds one row per person a PM was sent to.Received::$loadedis keyed accordingly — first by the PM, then by the member:Received::loadByPm()built its return value with only the PM as the key:so each row overwrote the one before it, and a PM came back with exactly one recipient — whichever the database handed over last. (The cache branch above it does the same thing, looping over the member-keyed list only to flatten it.)
Two things go wrong from there.
You cannot open a PM you received alongside somebody else.
PM::canAccess()walks that list looking for the current member, and if that member was not the one that survived, it isn't there. Sending a PM to two members and opening it from the recipient's inbox:?action=pm;f=inbox;pmid=4And the recipient list is wrong.
PM::format()buildsrecipientsfrom the same list, so the sent folder named one recipient of a PM that went to several:testmemberadmin, testmembernumber_recipientsis that count, soreply_to_allwas also hidden on every multi-recipient PM.The fix keys the list by recipient as well, which is the shape
PM::format()and the popup already read it with (foreach ($this->received as $member => $received_copy)), and picks out our own copy inapplyActions()rather than looping over everyone's — those are our labels, not theirs.Verified afterwards that the eleven pages of the PM area and the quote form all still render with an empty
smf_log_errors.Labelling a PM with more than one recipient needs #9460 as well, which fixes the update behind it; this change alone gets as far as reaching it.
Issues References (Fixes|Related|Closes)
Related: #9460