Skip to content

Commit 5493f02

Browse files
author
epriestley
committed
Provide a simple "Attach File" explicit workflow for files referenced but not attached
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
1 parent 021e5ab commit 5493f02

File tree

16 files changed

+464
-23
lines changed

16 files changed

+464
-23
lines changed

‎resources/celerity/map.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
'names' => array(
1010
'conpherence.pkg.css' => '0e3cf785',
1111
'conpherence.pkg.js' => '020aebcf',
12-
'core.pkg.css' => '00a2e7f4',
12+
'core.pkg.css' => 'b816811e',
1313
'core.pkg.js' => 'd2de90d9',
1414
'dark-console.pkg.js' => '187792c2',
1515
'differential.pkg.css' => 'ffb69e3d',
@@ -147,7 +147,7 @@
147147
'rsrc/css/phui/phui-comment-form.css' => '68a2d99a',
148148
'rsrc/css/phui/phui-comment-panel.css' => 'ec4e31c0',
149149
'rsrc/css/phui/phui-crumbs-view.css' => '614f43cf',
150-
'rsrc/css/phui/phui-curtain-object-ref-view.css' => '5f752bdb',
150+
'rsrc/css/phui/phui-curtain-object-ref-view.css' => '51d93266',
151151
'rsrc/css/phui/phui-curtain-view.css' => '68c5efb6',
152152
'rsrc/css/phui/phui-document-pro.css' => 'b9613a10',
153153
'rsrc/css/phui/phui-document-summary.css' => 'b068eed1',
@@ -838,7 +838,7 @@
838838
'phui-comment-form-css' => '68a2d99a',
839839
'phui-comment-panel-css' => 'ec4e31c0',
840840
'phui-crumbs-view-css' => '614f43cf',
841-
'phui-curtain-object-ref-view-css' => '5f752bdb',
841+
'phui-curtain-object-ref-view-css' => '51d93266',
842842
'phui-curtain-view-css' => '68c5efb6',
843843
'phui-document-summary-view-css' => 'b068eed1',
844844
'phui-document-view-css' => '52b748a5',

‎src/__phutil_library_map__.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3508,6 +3508,7 @@
35083508
'PhabricatorFileTransformController' => 'applications/files/controller/PhabricatorFileTransformController.php',
35093509
'PhabricatorFileTransformListController' => 'applications/files/controller/PhabricatorFileTransformListController.php',
35103510
'PhabricatorFileTransformTestCase' => 'applications/files/transform/__tests__/PhabricatorFileTransformTestCase.php',
3511+
'PhabricatorFileUICurtainAttachController' => 'applications/files/controller/PhabricatorFileUICurtainAttachController.php',
35113512
'PhabricatorFileUICurtainListController' => 'applications/files/controller/PhabricatorFileUICurtainListController.php',
35123513
'PhabricatorFileUploadController' => 'applications/files/controller/PhabricatorFileUploadController.php',
35133514
'PhabricatorFileUploadDialogController' => 'applications/files/controller/PhabricatorFileUploadDialogController.php',
@@ -9972,6 +9973,7 @@
99729973
'PhabricatorFileTransformController' => 'PhabricatorFileController',
99739974
'PhabricatorFileTransformListController' => 'PhabricatorFileController',
99749975
'PhabricatorFileTransformTestCase' => 'PhabricatorTestCase',
9976+
'PhabricatorFileUICurtainAttachController' => 'PhabricatorFileController',
99759977
'PhabricatorFileUICurtainListController' => 'PhabricatorFileController',
99769978
'PhabricatorFileUploadController' => 'PhabricatorFileController',
99779979
'PhabricatorFileUploadDialogController' => 'PhabricatorFileController',

‎src/applications/base/controller/PhabricatorController.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,10 @@ public function newDialog() {
420420
->setSubmitURI($submit_uri);
421421
}
422422

423+
public function newRedirect() {
424+
return id(new AphrontRedirectResponse());
425+
}
426+
423427
public function newPage() {
424428
$page = id(new PhabricatorStandardPageView())
425429
->setRequest($this->getRequest())

‎src/applications/files/application/PhabricatorFilesApplication.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,14 @@ public function getRoutes() {
9595
),
9696
'document/(?P<engineKey>[^/]+)/(?P<phid>[^/]+)/'
9797
=> 'PhabricatorFileDocumentController',
98-
'ui/curtainlist/(?P<phid>[^/]+)/'
99-
=> 'PhabricatorFileUICurtainListController',
98+
'ui/' => array(
99+
'curtain/' => array(
100+
'list/(?P<phid>[^/]+)/'
101+
=> 'PhabricatorFileUICurtainListController',
102+
'attach/(?P<objectPHID>[^/]+)/(?P<filePHID>[^/]+)/'
103+
=> 'PhabricatorFileUICurtainAttachController',
104+
),
105+
),
100106
) + $this->getResourceSubroutes(),
101107
);
102108
}

‎src/applications/files/controller/PhabricatorFileUICurtainAttachController.php‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
<?php
2+
3+
final class PhabricatorFileUICurtainAttachController
4+
extends PhabricatorFileController {
5+
6+
public function handleRequest(AphrontRequest $request) {
7+
$viewer = $request->getViewer();
8+
9+
$object_phid = $request->getURIData('objectPHID');
10+
$file_phid = $request->getURIData('filePHID');
11+
12+
$object = id(new PhabricatorObjectQuery())
13+
->setViewer($viewer)
14+
->withPHIDs(array($object_phid))
15+
->executeOne();
16+
if (!$object) {
17+
return new Aphront404Response();
18+
}
19+
20+
$attachment = id(new PhabricatorFileAttachmentQuery())
21+
->setViewer($viewer)
22+
->withObjectPHIDs(array($object->getPHID()))
23+
->withFilePHIDs(array($file_phid))
24+
->needFiles(true)
25+
->withVisibleFiles(true)
26+
->executeOne();
27+
if (!$attachment) {
28+
return new Aphront404Response();
29+
}
30+
31+
$file = $attachment->getFile();
32+
$file_phid = $file->getPHID();
33+
34+
$handles = $viewer->loadHandles(
35+
array(
36+
$object_phid,
37+
$file_phid,
38+
));
39+
40+
$object_handle = $handles[$object_phid];
41+
$file_handle = $handles[$file_phid];
42+
$cancel_uri = $object_handle->getURI();
43+
44+
$dialog = $this->newDialog()
45+
->setViewer($viewer)
46+
->setTitle(pht('Attach File'))
47+
->addCancelButton($object_handle->getURI(), pht('Close'));
48+
49+
$file_link = phutil_tag('strong', array(), $file_handle->renderLink());
50+
$object_link = phutil_tag('strong', array(), $object_handle->renderLink());
51+
52+
if ($attachment->isPolicyAttachment()) {
53+
$body = pht(
54+
'The file %s is already attached to the object %s.',
55+
$file_link,
56+
$object_link);
57+
58+
return $dialog->appendParagraph($body);
59+
}
60+
61+
if (!$request->isDialogFormPost()) {
62+
$dialog->appendRemarkup(
63+
pht(
64+
'(WARNING) This file is referenced by this object, but '.
65+
'not formally attached to it. Users who can see the object may '.
66+
'not be able to see the file.'));
67+
68+
$dialog->appendParagraph(
69+
pht(
70+
'Do you want to attach the file %s to the object %s?',
71+
$file_link,
72+
$object_link));
73+
74+
$dialog->addSubmitButton(pht('Attach File'));
75+
76+
return $dialog;
77+
}
78+
79+
if (!$request->getBool('confirm')) {
80+
$dialog->setTitle(pht('Confirm File Attachment'));
81+
82+
$dialog->addHiddenInput('confirm', 1);
83+
84+
$dialog->appendRemarkup(
85+
pht(
86+
'(IMPORTANT) If you attach this file to this object, any user who '.
87+
'has permission to view the object will be able to view and '.
88+
'download the file!'));
89+
90+
$dialog->appendParagraph(
91+
pht(
92+
'Really attach the file %s to the object %s, allowing any user '.
93+
'who can view the object to view and download the file?',
94+
$file_link,
95+
$object_link));
96+
97+
$dialog->addSubmitButton(pht('Grant Permission'));
98+
99+
return $dialog;
100+
}
101+
102+
if (!($object instanceof PhabricatorApplicationTransactionInterface)) {
103+
$dialog->appendParagraph(
104+
pht(
105+
'This object (of class "%s") does not implement the required '.
106+
'interface ("%s"), so files can not be manually attached to it.',
107+
get_class($object),
108+
'PhabricatorApplicationTransactionInterface'));
109+
110+
return $dialog;
111+
}
112+
113+
$editor = $object->getApplicationTransactionEditor()
114+
->setActor($viewer)
115+
->setContentSourceFromRequest($request)
116+
->setContinueOnNoEffect(true)
117+
->setContinueOnMissingFields(true);
118+
119+
$template = $object->getApplicationTransactionTemplate();
120+
121+
$xactions = array();
122+
123+
$xactions[] = id(clone $template)
124+
->setTransactionType(PhabricatorTransactions::TYPE_FILE)
125+
->setNewValue(
126+
array(
127+
$file_phid => PhabricatorFileAttachment::MODE_ATTACH,
128+
));
129+
130+
$editor->applyTransactions($object, $xactions);
131+
132+
return $this->newRedirect()
133+
->setURI($cancel_uri);
134+
}
135+
136+
}

‎src/applications/files/controller/PhabricatorFileUICurtainListController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function handleRequest(AphrontRequest $request) {
5353
return $this->newDialog()
5454
->setViewer($viewer)
5555
->setWidth(AphrontDialogView::WIDTH_FORM)
56-
->setTitle(pht('Attached Files'))
56+
->setTitle(pht('Referenced Files'))
5757
->setObjectList($list)
5858
->addCancelButton($object_handle->getURI(), pht('Close'));
5959
}

‎src/applications/files/engineextension/PhabricatorFilesCurtainExtension.php‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,25 +34,55 @@ public function buildCurtainPanel($object) {
3434

3535
$handles = $viewer->loadHandles($visible_phids);
3636

37-
PhabricatorPolicyFilterSet::loadHandleViewCapabilities(
38-
$viewer,
39-
$handles,
40-
array($object));
41-
4237
$ref_list = id(new PHUICurtainObjectRefListView())
4338
->setViewer($viewer)
4439
->setEmptyMessage(pht('None'));
4540

41+
$view_capability = PhabricatorPolicyCapability::CAN_VIEW;
42+
$object_policies = PhabricatorPolicyQuery::loadPolicies(
43+
$viewer,
44+
$object);
45+
$object_policy = idx($object_policies, $view_capability);
46+
4647
foreach ($visible_attachments as $attachment) {
4748
$file_phid = $attachment->getFilePHID();
4849
$handle = $handles[$file_phid];
4950

5051
$ref = $ref_list->newObjectRefView()
5152
->setHandle($handle);
5253

53-
if ($handle->hasCapabilities()) {
54-
if (!$handle->hasViewCapability($object)) {
55-
$ref->setExiled(true);
54+
$file = $attachment->getFile();
55+
if (!$file) {
56+
// ...
57+
} else {
58+
if (!$attachment->isPolicyAttachment()) {
59+
$file_policies = PhabricatorPolicyQuery::loadPolicies(
60+
$viewer,
61+
$file);
62+
$file_policy = idx($file_policies, $view_capability);
63+
64+
if ($object_policy->isStrongerThanOrEqualTo($file_policy)) {
65+
// The file is not attached to the object, but the file policy
66+
// allows anyone who can see the object to see the file too, so
67+
// there is no material problem with the file not being attached.
68+
} else {
69+
$attach_uri = urisprintf(
70+
'/file/ui/curtain/attach/%s/%s/',
71+
$object->getPHID(),
72+
$file->getPHID());
73+
74+
$attached_link = javelin_tag(
75+
'a',
76+
array(
77+
'href' => $attach_uri,
78+
'sigil' => 'workflow',
79+
),
80+
pht('File Not Attached'));
81+
82+
$ref->setExiled(
83+
true,
84+
$attached_link);
85+
}
5686
}
5787
}
5888

@@ -63,7 +93,7 @@ public function buildCurtainPanel($object) {
6393
$show_all = (count($visible_attachments) < count($attachments));
6494
if ($show_all) {
6595
$view_all_uri = urisprintf(
66-
'/file/ui/curtainlist/%s/',
96+
'/file/ui/curtain/list/%s/',
6797
$object->getPHID());
6898

6999
$loaded_count = count($attachments);
@@ -80,7 +110,7 @@ public function buildCurtainPanel($object) {
80110
}
81111

82112
return $this->newPanel()
83-
->setHeaderText(pht('Attached Files'))
113+
->setHeaderText(pht('Referenced Files'))
84114
->setOrder(15000)
85115
->appendChild($ref_list);
86116
}

‎src/applications/files/phid/PhabricatorFileFilePHIDType.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ public function loadHandles(
3939
$handle->setName("F{$id}");
4040
$handle->setFullName("F{$id}: {$name}");
4141
$handle->setURI($uri);
42+
43+
$icon = FileTypeIcon::getFileIcon($name);
44+
$handle->setIcon($icon);
4245
}
4346
}
4447

‎src/applications/files/query/PhabricatorFileAttachmentQuery.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,25 @@ final class PhabricatorFileAttachmentQuery
44
extends PhabricatorCursorPagedPolicyAwareQuery {
55

66
private $objectPHIDs;
7+
private $filePHIDs;
78
private $needFiles;
9+
private $visibleFiles;
810

911
public function withObjectPHIDs(array $object_phids) {
1012
$this->objectPHIDs = $object_phids;
1113
return $this;
1214
}
1315

16+
public function withFilePHIDs(array $file_phids) {
17+
$this->filePHIDs = $file_phids;
18+
return $this;
19+
}
20+
21+
public function withVisibleFiles($visible_files) {
22+
$this->visibleFiles = $visible_files;
23+
return $this;
24+
}
25+
1426
public function needFiles($need) {
1527
$this->needFiles = $need;
1628
return $this;
@@ -34,6 +46,13 @@ protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
3446
$this->objectPHIDs);
3547
}
3648

49+
if ($this->filePHIDs !== null) {
50+
$where[] = qsprintf(
51+
$conn,
52+
'attachments.filePHID IN (%Ls)',
53+
$this->filePHIDs);
54+
}
55+
3756
return $where;
3857
}
3958

@@ -92,6 +111,12 @@ protected function willFilterPage(array $attachments) {
92111
$file_phid = $attachment->getFilePHID();
93112
$file = idx($files, $file_phid);
94113

114+
if ($this->visibleFiles && !$file) {
115+
$this->didRejectResult($attachment);
116+
unset($attachments[$key]);
117+
continue;
118+
}
119+
95120
$attachment->attachFile($file);
96121
}
97122
}

‎src/applications/files/storage/PhabricatorFileAttachment.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ public static function getModeList() {
4646
);
4747
}
4848

49+
public function isPolicyAttachment() {
50+
switch ($this->getAttachmentMode()) {
51+
case self::MODE_ATTACH:
52+
return true;
53+
default:
54+
return false;
55+
}
56+
}
57+
4958
public function attachObject($object) {
5059
$this->object = $object;
5160
return $this;

0 commit comments

Comments
 (0)