mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-25 16:22:43 +01:00
Only attach files that are both referenced in Remarkup and attached by explicit metadata
Summary: Ref T13682. When a user uploads a file, then changes their mind and deletes the reference to the file, we don't actually want to attach the file. When choosing which files to attach, only attach files which are both referenced in Remarkup and explicitly attached in remarkup metadata. Test Plan: - Dropped a file into a comment, submitted it, saw it attach normally. - Dropped a file into a comment, deleted the reference, submitted it, saw no attachment. Maniphest Tasks: T13682 Differential Revision: https://secure.phabricator.com/D21832
This commit is contained in:
parent
8cd02e6727
commit
2a0feb3de0
1 changed files with 49 additions and 1 deletions
|
@ -447,14 +447,31 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
$old_map = ipull($rows, 'attachmentMode', 'filePHID');
|
||||
}
|
||||
|
||||
$mode_ref = PhabricatorFileAttachment::MODE_REFERENCE;
|
||||
$mode_detach = PhabricatorFileAttachment::MODE_DETACH;
|
||||
|
||||
$new_map = $old_map;
|
||||
|
||||
foreach ($new as $file_phid => $attachment_mode) {
|
||||
if ($attachment_mode == PhabricatorFileAttachment::MODE_DETACH) {
|
||||
$is_ref = ($attachment_mode === $mode_ref);
|
||||
$is_detach = ($attachment_mode === $mode_detach);
|
||||
|
||||
if ($is_detach) {
|
||||
unset($new_map[$file_phid]);
|
||||
continue;
|
||||
}
|
||||
|
||||
$old_mode = idx($old_map, $file_phid);
|
||||
|
||||
// If we're adding a reference to a file but it is already attached,
|
||||
// don't touch it.
|
||||
|
||||
if ($is_ref) {
|
||||
if ($old_mode !== null) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
$new_map[$file_phid] = $attachment_mode;
|
||||
}
|
||||
|
||||
|
@ -2246,11 +2263,42 @@ abstract class PhabricatorApplicationTransactionEditor
|
|||
|
||||
$new_map = array();
|
||||
|
||||
$viewer = $this->getActor();
|
||||
|
||||
$old_blocks = mpull($remarkup_changes, 'getOldValue');
|
||||
$new_blocks = mpull($remarkup_changes, 'getNewValue');
|
||||
|
||||
$old_refs = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(
|
||||
$viewer,
|
||||
$old_blocks);
|
||||
$old_refs = array_fuse($old_refs);
|
||||
|
||||
$new_refs = PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles(
|
||||
$viewer,
|
||||
$new_blocks);
|
||||
$new_refs = array_fuse($new_refs);
|
||||
|
||||
$add_refs = array_diff_key($new_refs, $old_refs);
|
||||
foreach ($add_refs as $file_phid) {
|
||||
$new_map[$file_phid] = PhabricatorFileAttachment::MODE_REFERENCE;
|
||||
}
|
||||
|
||||
foreach ($remarkup_changes as $remarkup_change) {
|
||||
$metadata = $remarkup_change->getMetadata();
|
||||
|
||||
$attached_phids = idx($metadata, 'attachedFilePHIDs', array());
|
||||
foreach ($attached_phids as $file_phid) {
|
||||
|
||||
// If the blocks don't include a new embedded reference to this file,
|
||||
// do not actually attach it. A common way for this to happen is for
|
||||
// a user to upload a file, then change their mind and remove the
|
||||
// reference. We do not want to attach the file if they decided against
|
||||
// referencing it.
|
||||
|
||||
if (!isset($new_map[$file_phid])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$new_map[$file_phid] = PhabricatorFileAttachment::MODE_ATTACH;
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue