revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-19 16:58:48 +02:00
Code Issues Releases Wiki Activity

Provide a simple "Attach File" explicit workflow for files referenced but not attached

Browse source 
Summary: Ref T13682. Allow users to manually attach files which are referenced (but not attached) via the UI.

Test Plan: Reference files via `{F...}`, then attached them via the UI workflow.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21837
This commit is contained in:
epriestley 2022-05-23 17:46:08 -07:00
parent 021e5ab933
commit 5493f028dc
16 changed files with 464 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
resources/celerity/map.php
View file

@ -9,7 +9,7 @@ return array(
'names' => array(
'conpherence.pkg.css' => '0e3cf785',
'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => '00a2e7f4',
'core.pkg.css' => 'b816811e',
'core.pkg.js' => 'd2de90d9',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => 'ffb69e3d',
@ -147,7 +147,7 @@ return array(
'rsrc/css/phui/phui-comment-form.css' => '68a2d99a',
'rsrc/css/phui/phui-comment-panel.css' => 'ec4e31c0',
'rsrc/css/phui/phui-crumbs-view.css' => '614f43cf',
'rsrc/css/phui/phui-curtain-object-ref-view.css' => '5f752bdb',
'rsrc/css/phui/phui-curtain-object-ref-view.css' => '51d93266',
'rsrc/css/phui/phui-curtain-view.css' => '68c5efb6',
'rsrc/css/phui/phui-document-pro.css' => 'b9613a10',
'rsrc/css/phui/phui-document-summary.css' => 'b068eed1',
@ -838,7 +838,7 @@ return array(
'phui-comment-form-css' => '68a2d99a',
'phui-comment-panel-css' => 'ec4e31c0',
'phui-crumbs-view-css' => '614f43cf',
'phui-curtain-object-ref-view-css' => '5f752bdb',
'phui-curtain-object-ref-view-css' => '51d93266',
'phui-curtain-view-css' => '68c5efb6',
'phui-document-summary-view-css' => 'b068eed1',
'phui-document-view-css' => '52b748a5',

2
src/__phutil_library_map__.php
View file

@ -3508,6 +3508,7 @@ phutil_register_library_map(array(
'PhabricatorFileTransformController' => 'applications/files/controller/PhabricatorFileTransformController.php',
'PhabricatorFileTransformListController' => 'applications/files/controller/PhabricatorFileTransformListController.php',
'PhabricatorFileTransformTestCase' => 'applications/files/transform/__tests__/PhabricatorFileTransformTestCase.php',
'PhabricatorFileUICurtainAttachController' => 'applications/files/controller/PhabricatorFileUICurtainAttachController.php',
'PhabricatorFileUICurtainListController' => 'applications/files/controller/PhabricatorFileUICurtainListController.php',
'PhabricatorFileUploadController' => 'applications/files/controller/PhabricatorFileUploadController.php',
'PhabricatorFileUploadDialogController' => 'applications/files/controller/PhabricatorFileUploadDialogController.php',
@ -9972,6 +9973,7 @@ phutil_register_library_map(array(
'PhabricatorFileTransformController' => 'PhabricatorFileController',
'PhabricatorFileTransformListController' => 'PhabricatorFileController',
'PhabricatorFileTransformTestCase' => 'PhabricatorTestCase',
'PhabricatorFileUICurtainAttachController' => 'PhabricatorFileController',
'PhabricatorFileUICurtainListController' => 'PhabricatorFileController',
'PhabricatorFileUploadController' => 'PhabricatorFileController',
'PhabricatorFileUploadDialogController' => 'PhabricatorFileController',

4
src/applications/base/controller/PhabricatorController.php
View file

@ -420,6 +420,10 @@ abstract class PhabricatorController extends AphrontController {
->setSubmitURI($submit_uri);
}
public function newRedirect() {
return id(new AphrontRedirectResponse());
}
public function newPage() {
$page = id(new PhabricatorStandardPageView())
->setRequest($this->getRequest())

8
src/applications/files/application/PhabricatorFilesApplication.php
View file

@ -95,8 +95,14 @@ final class PhabricatorFilesApplication extends PhabricatorApplication {
),
'document/(?P<engineKey>[^/]+)/(?P<phid>[^/]+)/'
=> 'PhabricatorFileDocumentController',
'ui/curtainlist/(?P<phid>[^/]+)/'
'ui/' => array(
'curtain/' => array(
'list/(?P<phid>[^/]+)/'
=> 'PhabricatorFileUICurtainListController',
'attach/(?P<objectPHID>[^/]+)/(?P<filePHID>[^/]+)/'
=> 'PhabricatorFileUICurtainAttachController',
),
),
) + $this->getResourceSubroutes(),
);
}

136
src/applications/files/controller/PhabricatorFileUICurtainAttachController.php Normal file
View file

@ -0,0 +1,136 @@
<?php
final class PhabricatorFileUICurtainAttachController
extends PhabricatorFileController {
public function handleRequest(AphrontRequest $request) {
$viewer = $request->getViewer();
$object_phid = $request->getURIData('objectPHID');
$file_phid = $request->getURIData('filePHID');
$object = id(new PhabricatorObjectQuery())
->setViewer($viewer)
->withPHIDs(array($object_phid))
->executeOne();
if (!$object) {
return new Aphront404Response();
}
$attachment = id(new PhabricatorFileAttachmentQuery())
->setViewer($viewer)
->withObjectPHIDs(array($object->getPHID()))
->withFilePHIDs(array($file_phid))
->needFiles(true)
->withVisibleFiles(true)
->executeOne();
if (!$attachment) {
return new Aphront404Response();
}
$file = $attachment->getFile();
$file_phid = $file->getPHID();
$handles = $viewer->loadHandles(
array(
$object_phid,
$file_phid,
));
$object_handle = $handles[$object_phid];
$file_handle = $handles[$file_phid];
$cancel_uri = $object_handle->getURI();
$dialog = $this->newDialog()
->setViewer($viewer)
->setTitle(pht('Attach File'))
->addCancelButton($object_handle->getURI(), pht('Close'));
$file_link = phutil_tag('strong', array(), $file_handle->renderLink());
$object_link = phutil_tag('strong', array(), $object_handle->renderLink());
if ($attachment->isPolicyAttachment()) {
$body = pht(
'The file %s is already attached to the object %s.',
$file_link,
$object_link);
return $dialog->appendParagraph($body);
}
if (!$request->isDialogFormPost()) {
$dialog->appendRemarkup(
pht(
'(WARNING) This file is referenced by this object, but '.
'not formally attached to it. Users who can see the object may '.
'not be able to see the file.'));
$dialog->appendParagraph(
pht(
'Do you want to attach the file %s to the object %s?',
$file_link,
$object_link));
$dialog->addSubmitButton(pht('Attach File'));
return $dialog;
}
if (!$request->getBool('confirm')) {
$dialog->setTitle(pht('Confirm File Attachment'));
$dialog->addHiddenInput('confirm', 1);
$dialog->appendRemarkup(
pht(
'(IMPORTANT) If you attach this file to this object, any user who '.
'has permission to view the object will be able to view and '.
'download the file!'));
$dialog->appendParagraph(
pht(
'Really attach the file %s to the object %s, allowing any user '.
'who can view the object to view and download the file?',
$file_link,
$object_link));
$dialog->addSubmitButton(pht('Grant Permission'));
return $dialog;
}
if (!($object instanceof PhabricatorApplicationTransactionInterface)) {
$dialog->appendParagraph(
pht(
'This object (of class "%s") does not implement the required '.
'interface ("%s"), so files can not be manually attached to it.',
get_class($object),
'PhabricatorApplicationTransactionInterface'));
return $dialog;
}
$editor = $object->getApplicationTransactionEditor()
->setActor($viewer)
->setContentSourceFromRequest($request)
->setContinueOnNoEffect(true)
->setContinueOnMissingFields(true);
$template = $object->getApplicationTransactionTemplate();
$xactions = array();
$xactions[] = id(clone $template)
->setTransactionType(PhabricatorTransactions::TYPE_FILE)
->setNewValue(
array(
$file_phid => PhabricatorFileAttachment::MODE_ATTACH,
));
$editor->applyTransactions($object, $xactions);
return $this->newRedirect()
->setURI($cancel_uri);
}
}

2
src/applications/files/controller/PhabricatorFileUICurtainListController.php
View file

@ -53,7 +53,7 @@ final class PhabricatorFileUICurtainListController
return $this->newDialog()
->setViewer($viewer)
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle(pht('Attached Files'))
->setTitle(pht('Referenced Files'))
->setObjectList($list)
->addCancelButton($object_handle->getURI(), pht('Close'));
}

50
src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php
View file

@ -34,15 +34,16 @@ final class PhabricatorFilesCurtainExtension
$handles = $viewer->loadHandles($visible_phids);
PhabricatorPolicyFilterSet::loadHandleViewCapabilities(
$viewer,
$handles,
array($object));
$ref_list = id(new PHUICurtainObjectRefListView())
->setViewer($viewer)
->setEmptyMessage(pht('None'));
$view_capability = PhabricatorPolicyCapability::CAN_VIEW;
$object_policies = PhabricatorPolicyQuery::loadPolicies(
$viewer,
$object);
$object_policy = idx($object_policies, $view_capability);
foreach ($visible_attachments as $attachment) {
$file_phid = $attachment->getFilePHID();
$handle = $handles[$file_phid];
@ -50,9 +51,38 @@ final class PhabricatorFilesCurtainExtension
$ref = $ref_list->newObjectRefView()
->setHandle($handle);
if ($handle->hasCapabilities()) {
if (!$handle->hasViewCapability($object)) {
$ref->setExiled(true);
$file = $attachment->getFile();
if (!$file) {
// ...
} else {
if (!$attachment->isPolicyAttachment()) {
$file_policies = PhabricatorPolicyQuery::loadPolicies(
$viewer,
$file);
$file_policy = idx($file_policies, $view_capability);
if ($object_policy->isStrongerThanOrEqualTo($file_policy)) {
// The file is not attached to the object, but the file policy
// allows anyone who can see the object to see the file too, so
// there is no material problem with the file not being attached.
} else {
$attach_uri = urisprintf(
'/file/ui/curtain/attach/%s/%s/',
$object->getPHID(),
$file->getPHID());
$attached_link = javelin_tag(
'a',
array(
'href' => $attach_uri,
'sigil' => 'workflow',
),
pht('File Not Attached'));
$ref->setExiled(
true,
$attached_link);
}
}
}
@ -63,7 +93,7 @@ final class PhabricatorFilesCurtainExtension
$show_all = (count($visible_attachments) < count($attachments));
if ($show_all) {
$view_all_uri = urisprintf(
'/file/ui/curtainlist/%s/',
'/file/ui/curtain/list/%s/',
$object->getPHID());
$loaded_count = count($attachments);
@ -80,7 +110,7 @@ final class PhabricatorFilesCurtainExtension
}
return $this->newPanel()
->setHeaderText(pht('Attached Files'))
->setHeaderText(pht('Referenced Files'))
->setOrder(15000)
->appendChild($ref_list);
}

3
src/applications/files/phid/PhabricatorFileFilePHIDType.php
View file

@ -39,6 +39,9 @@ final class PhabricatorFileFilePHIDType extends PhabricatorPHIDType {
$handle->setName("F{$id}");
$handle->setFullName("F{$id}: {$name}");
$handle->setURI($uri);
$icon = FileTypeIcon::getFileIcon($name);
$handle->setIcon($icon);
}
}

25
src/applications/files/query/PhabricatorFileAttachmentQuery.php
View file

@ -4,13 +4,25 @@ final class PhabricatorFileAttachmentQuery
extends PhabricatorCursorPagedPolicyAwareQuery {
private $objectPHIDs;
private $filePHIDs;
private $needFiles;
private $visibleFiles;
public function withObjectPHIDs(array $object_phids) {
$this->objectPHIDs = $object_phids;
return $this;
}
public function withFilePHIDs(array $file_phids) {
$this->filePHIDs = $file_phids;
return $this;
}
public function withVisibleFiles($visible_files) {
$this->visibleFiles = $visible_files;
return $this;
}
public function needFiles($need) {
$this->needFiles = $need;
return $this;
@ -34,6 +46,13 @@ final class PhabricatorFileAttachmentQuery
$this->objectPHIDs);
}
if ($this->filePHIDs !== null) {
$where[] = qsprintf(
$conn,
'attachments.filePHID IN (%Ls)',
$this->filePHIDs);
}
return $where;
}
@ -92,6 +111,12 @@ final class PhabricatorFileAttachmentQuery
$file_phid = $attachment->getFilePHID();
$file = idx($files, $file_phid);
if ($this->visibleFiles && !$file) {
$this->didRejectResult($attachment);
unset($attachments[$key]);
continue;
}
$attachment->attachFile($file);
}
}

9
src/applications/files/storage/PhabricatorFileAttachment.php
View file

@ -46,6 +46,15 @@ final class PhabricatorFileAttachment
);
}
public function isPolicyAttachment() {
switch ($this->getAttachmentMode()) {
case self::MODE_ATTACH:
return true;
default:
return false;
}
}
public function attachObject($object) {
$this->object = $object;
return $this;

15
src/applications/policy/storage/PhabricatorPolicy.php
View file

@ -417,12 +417,23 @@ final class PhabricatorPolicy
PhabricatorPolicies::POLICY_NOONE => 1,
);
$this_strength = idx($strengths, $this->getPHID(), 0);
$other_strength = idx($strengths, $other->getPHID(), 0);
$this_strength = idx($strengths, $this_policy, 0);
$other_strength = idx($strengths, $other_policy, 0);
return ($this_strength > $other_strength);
}
public function isStrongerThanOrEqualTo(PhabricatorPolicy $other) {
$this_policy = $this->getPHID();
$other_policy = $other->getPHID();
if ($this_policy === $other_policy) {
return true;
}
return $this->isStrongerThan($other);
}
public function isValidPolicyForEdit() {
return $this->getType() !== PhabricatorPolicyType::TYPE_MASKED;
}

1
src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
View file

@ -2321,6 +2321,7 @@ abstract class PhabricatorApplicationTransactionEditor
$xaction = $object->getApplicationTransactionTemplate()
->setTransactionType(PhabricatorTransactions::TYPE_FILE)
->setMetadataValue('attach.implicit', true)
->setNewValue($new_map);
return $xaction;

193
src/applications/transactions/storage/PhabricatorApplicationTransaction.php
View file

@ -341,6 +341,9 @@ abstract class PhabricatorApplicationTransaction
$phids[] = $old;
$phids[] = $new;
break;
case PhabricatorTransactions::TYPE_FILE:
$phids[] = array_keys($old + $new);
break;
case PhabricatorTransactions::TYPE_EDGE:
$record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
$phids[] = $record->getChangedPHIDs();
@ -592,8 +595,10 @@ abstract class PhabricatorApplicationTransaction
// Always hide file attach/detach transactions.
if ($xaction_type === PhabricatorTransactions::TYPE_FILE) {
if ($this->getMetadataValue('attach.implicit')) {
return true;
}
}
// Hide creation transactions if the old value is empty. These are
// transactions like "alice set the task title to: ...", which are
@ -1041,6 +1046,124 @@ abstract class PhabricatorApplicationTransaction
'%s updated subscribers...',
$this->renderHandleLink($author_phid));
}
break;
case PhabricatorTransactions::TYPE_FILE:
$add = array_diff_key($new, $old);
$add = array_keys($add);
$rem = array_diff_key($old, $new);
$rem = array_keys($rem);
$mod = array();
foreach ($old + $new as $key => $ignored) {
if (!isset($old[$key])) {
continue;
}
if (!isset($new[$key])) {
continue;
}
if ($old[$key] === $new[$key]) {
continue;
}
$mod[] = $key;
}
// Specialize the specific case of only modifying files and upgrading
// references to attachments. This is accessible via the UI and can
// be shown more clearly than the generic default transaction shows
// it.
$mode_reference = PhabricatorFileAttachment::MODE_REFERENCE;
$mode_attach = PhabricatorFileAttachment::MODE_ATTACH;
$is_refattach = false;
if ($mod && !$add && !$rem) {
$all_refattach = true;
foreach ($mod as $phid) {
if (idx($old, $phid) !== $mode_reference) {
$all_refattach = false;
break;
}
if (idx($new, $phid) !== $mode_attach) {
$all_refattach = false;
break;
}
}
$is_refattach = $all_refattach;
}
if ($is_refattach) {
return pht(
'%s attached %s referenced file(s): %s.',
$this->renderHandleLink($author_phid),
phutil_count($mod),
$this->renderHandleList($mod));
} else if ($add && $rem && $mod) {
return pht(
'%s updated %s attached file(s), added %s: %s; removed %s: %s; '.
'modified %s: %s.',
$this->renderHandleLink($author_phid),
new PhutilNumber(count($add) + count($rem)),
phutil_count($add),
$this->renderHandleList($add),
phutil_count($rem),
$this->renderHandleList($rem),
phutil_count($mod),
$this->renderHandleList($mod));
} else if ($add && $rem) {
return pht(
'%s updated %s attached file(s), added %s: %s; removed %s: %s.',
$this->renderHandleLink($author_phid),
new PhutilNumber(count($add) + count($rem)),
phutil_count($add),
$this->renderHandleList($add),
phutil_count($rem),
$this->renderHandleList($rem));
} else if ($add && $mod) {
return pht(
'%s updated %s attached file(s), added %s: %s; modified %s: %s.',
$this->renderHandleLink($author_phid),
new PhutilNumber(count($add) + count($mod)),
phutil_count($add),
$this->renderHandleList($add),
phutil_count($mod),
$this->renderHandleList($mod));
} else if ($rem && $mod) {
return pht(
'%s updated %s attached file(s), removed %s: %s; modified %s: %s.',
$this->renderHandleLink($author_phid),
new PhutilNumber(count($rem) + count($mod)),
phutil_count($rem),
$this->renderHandleList($rem),
phutil_count($mod),
$this->renderHandleList($mod));
} else if ($add) {
return pht(
'%s attached %s file(s): %s.',
$this->renderHandleLink($author_phid),
phutil_count($add),
$this->renderHandleList($add));
} else if ($rem) {
return pht(
'%s removed %s attached file(s): %s.',
$this->renderHandleLink($author_phid),
phutil_count($rem),
$this->renderHandleList($rem));
} else if ($mod) {
return pht(
'%s modified %s attached file(s): %s.',
$this->renderHandleLink($author_phid),
phutil_count($mod),
$this->renderHandleList($mod));
} else {
return pht(
'%s attached files...',
$this->renderHandleLink($author_phid));
}
break;
case PhabricatorTransactions::TYPE_EDGE:
$record = PhabricatorEdgeChangeRecord::newFromTransaction($this);
@ -1479,6 +1602,8 @@ abstract class PhabricatorApplicationTransaction
public function hasChangeDetails() {
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_FILE:
return true;
case PhabricatorTransactions::TYPE_CUSTOMFIELD:
$field = $this->getTransactionCustomField();
if ($field) {
@ -1494,6 +1619,11 @@ abstract class PhabricatorApplicationTransaction
}
public function renderChangeDetailsForMail(PhabricatorUser $viewer) {
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_FILE:
return false;
}
$view = $this->renderChangeDetails($viewer);
if ($view instanceof PhabricatorApplicationTransactionTextDiffDetailView) {
return $view->renderForMail();
@ -1503,6 +1633,8 @@ abstract class PhabricatorApplicationTransaction
public function renderChangeDetails(PhabricatorUser $viewer) {
switch ($this->getTransactionType()) {
case PhabricatorTransactions::TYPE_FILE:
return $this->newFileTransactionChangeDetails($viewer);
case PhabricatorTransactions::TYPE_CUSTOMFIELD:
$field = $this->getTransactionCustomField();
if ($field) {
@ -1769,6 +1901,66 @@ abstract class PhabricatorApplicationTransaction
->addInt(-$this->getActionStrength());
}
private function newFileTransactionChangeDetails(PhabricatorUser $viewer) {
$old = $this->getOldValue();
$new = $this->getNewValue();
$phids = array_keys($old + $new);
$handles = $viewer->loadHandles($phids);
$names = array(
PhabricatorFileAttachment::MODE_REFERENCE => pht('Referenced'),
PhabricatorFileAttachment::MODE_ATTACH => pht('Attached'),
);
$rows = array();
foreach ($old + $new as $phid => $ignored) {
$handle = $handles[$phid];
$old_mode = idx($old, $phid);
$new_mode = idx($new, $phid);
if ($old_mode === null) {
$old_name = pht('None');
} else if (isset($names[$old_mode])) {
$old_name = $names[$old_mode];
} else {
$old_name = pht('Unknown ("%s")', $old_mode);
}
if ($new_mode === null) {
$new_name = pht('Detached');
} else if (isset($names[$new_mode])) {
$new_name = $names[$new_mode];
} else {
$new_name = pht('Unknown ("%s")', $new_mode);
}
$rows[] = array(
$handle->renderLink(),
$old_name,
$new_name,
);
}
$table = id(new AphrontTableView($rows))
->setHeaders(
array(
pht('File'),
pht('Old Mode'),
pht('New Mode'),
))
->setColumnClasses(
array(
'pri',
));
return id(new PHUIBoxView())
->addMargin(PHUI::MARGIN_SMALL)
->appendChild($table);
}
/* -( PhabricatorPolicyInterface Implementation )-------------------------- */
@ -1836,5 +2028,4 @@ abstract class PhabricatorApplicationTransaction
$this->saveTransaction();
}
}

14
src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
View file

@ -1789,6 +1789,20 @@ final class PhabricatorUSEnglishTranslation
'Executed %s tasks.',
),
'%s modified %s attached file(s): %s.' => array(
array(
'%s modified an attached file: %3$s.',
'%s modified attached files: %3$s.',
),
),
'%s attached %s referenced file(s): %s.' => array(
array(
'%s attached a referenced file: %3$s.',
'%s attached referenced files: %3$s.',
),
),
);
}

12
src/view/phui/PHUICurtainObjectRefView.php
View file

@ -7,6 +7,7 @@ final class PHUICurtainObjectRefView
private $epoch;
private $highlighted;
private $exiled;
private $exileNote = false;
public function setHandle(PhabricatorObjectHandle $handle) {
$this->handle = $handle;
@ -23,8 +24,9 @@ final class PHUICurtainObjectRefView
return $this;
}
public function setExiled($is_exiled) {
public function setExiled($is_exiled, $note = false) {
$this->exiled = $is_exiled;
$this->exileNote = $note;
return $this;
}
@ -72,10 +74,16 @@ final class PHUICurtainObjectRefView
}
if ($this->exiled) {
if ($this->exileNote !== false) {
$exile_note = $this->exileNote;
} else {
$exile_note = pht('No View Permission');
}
$exiled_view = array(
id(new PHUIIconView())->setIcon('fa-eye-slash red'),
' ',
pht('No View Permission'),
$exile_note,
);
$exiled_cells = array();

3
webroot/rsrc/css/phui/phui-curtain-object-ref-view.css
View file

@ -92,6 +92,7 @@
opacity: 0.75;
}
.phui-curtain-object-ref-view-exiled-cell {
.phui-curtain-object-ref-view-exiled-cell,
.phui-curtain-object-ref-view-exiled-cell a {
color: {$red};
}