From 76cd181bf379ef1650613b446f2a2c95f4746063 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 31 Jul 2019 12:40:53 -0700 Subject: [PATCH 01/15] Don't try to emit project board update events if there are no projects to update Summary: Ref T4900. We may execute a bad query here if the task has no projects at all. Test Plan: Edited a task with no new or old projects. Instead of an exception, things worked. Maniphest Tasks: T4900 Differential Revision: https://secure.phabricator.com/D20689 --- .../editor/ManiphestTransactionEditor.php | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/applications/maniphest/editor/ManiphestTransactionEditor.php b/src/applications/maniphest/editor/ManiphestTransactionEditor.php index 247f5ce1fe..ed98ad8ad8 100644 --- a/src/applications/maniphest/editor/ManiphestTransactionEditor.php +++ b/src/applications/maniphest/editor/ManiphestTransactionEditor.php @@ -879,34 +879,36 @@ final class ManiphestTransactionEditor $project_phids = array_fuse($old_phids) + array_fuse($new_phids); $project_phids = array_keys($project_phids); - $projects = id(new PhabricatorProjectQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withPHIDs($project_phids) - ->execute(); + if ($project_phids) { + $projects = id(new PhabricatorProjectQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs($project_phids) + ->execute(); - $notify_projects = array(); - foreach ($projects as $project) { - $notify_projects[$project->getPHID()] = $project; - foreach ($project->getAncestorProjects() as $ancestor) { - $notify_projects[$ancestor->getPHID()] = $ancestor; + $notify_projects = array(); + foreach ($projects as $project) { + $notify_projects[$project->getPHID()] = $project; + foreach ($project->getAncestorProjects() as $ancestor) { + $notify_projects[$ancestor->getPHID()] = $ancestor; + } } - } - foreach ($notify_projects as $key => $project) { - if (!$project->getHasWorkboard()) { - unset($notify_projects[$key]); + foreach ($notify_projects as $key => $project) { + if (!$project->getHasWorkboard()) { + unset($notify_projects[$key]); + } } - } - $notify_phids = array_keys($notify_projects); + $notify_phids = array_keys($notify_projects); - if ($notify_phids) { - $data = array( - 'type' => 'workboards', - 'subscribers' => $notify_phids, - ); + if ($notify_phids) { + $data = array( + 'type' => 'workboards', + 'subscribers' => $notify_phids, + ); - PhabricatorNotificationClient::tryToPostMessage($data); + PhabricatorNotificationClient::tryToPostMessage($data); + } } } From 8e263a2f6482203322de9ee5c0ec5664afa260c5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 31 Jul 2019 12:40:56 -0700 Subject: [PATCH 02/15] Support "date" custom fields in "*.edit" endpoints Summary: Fixes T13355. This didn't appear to be a ton of extra work, we just didn't get it for free in the original implementation in D14635. Test Plan: - Saw "date" custom fields appear in Conduit API documentation for "maniphest.edit". - Set custom "date" field to null and non-null values via the API. {F6666582} Maniphest Tasks: T13355 Differential Revision: https://secure.phabricator.com/D20690 --- .../parametertype/ConduitEpochParameterType.php | 16 ++++++++++++++++ .../editfield/PhabricatorEpochEditField.php | 3 ++- .../PhabricatorStandardCustomFieldDate.php | 10 ++-------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/applications/conduit/parametertype/ConduitEpochParameterType.php b/src/applications/conduit/parametertype/ConduitEpochParameterType.php index e8fe095c50..8f2ca2c98a 100644 --- a/src/applications/conduit/parametertype/ConduitEpochParameterType.php +++ b/src/applications/conduit/parametertype/ConduitEpochParameterType.php @@ -3,8 +3,24 @@ final class ConduitEpochParameterType extends ConduitParameterType { + private $allowNull; + + public function setAllowNull($allow_null) { + $this->allowNull = $allow_null; + return $this; + } + + public function getAllowNull() { + return $this->allowNull; + } + protected function getParameterValue(array $request, $key, $strict) { $value = parent::getParameterValue($request, $key, $strict); + + if ($this->allowNull && ($value === null)) { + return $value; + } + $value = $this->parseIntValue($request, $key, $value, $strict); if ($value <= 0) { diff --git a/src/applications/transactions/editfield/PhabricatorEpochEditField.php b/src/applications/transactions/editfield/PhabricatorEpochEditField.php index 9ac9726593..b50f013177 100644 --- a/src/applications/transactions/editfield/PhabricatorEpochEditField.php +++ b/src/applications/transactions/editfield/PhabricatorEpochEditField.php @@ -37,7 +37,8 @@ final class PhabricatorEpochEditField } protected function newConduitParameterType() { - return new ConduitEpochParameterType(); + return id(new ConduitEpochParameterType()) + ->setAllowNull($this->getAllowNull()); } } diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php index 994bb99403..4aba7543e7 100644 --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldDate.php @@ -226,20 +226,14 @@ final class PhabricatorStandardCustomFieldDate } } - - public function shouldAppearInConduitTransactions() { - // TODO: Dates are complicated and we don't yet support handling them from - // Conduit. - return false; - } - protected function newConduitSearchParameterType() { // TODO: Build a new "pair" type or similar. return null; } protected function newConduitEditParameterType() { - return new ConduitEpochParameterType(); + return id(new ConduitEpochParameterType()) + ->setAllowNull(!$this->getRequired()); } protected function newExportFieldType() { From b81c8380fb9753287bcf7c2b79eb064e55ea0bbf Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 31 Jul 2019 13:00:57 -0700 Subject: [PATCH 03/15] Document support for "limit" in tokenizer-based Custom Fields Summary: Fixes T13356. This option is supported and works fine, it just isn't documented. Add documentation and fix the config option to actually link to it to make life a little easier. Test Plan: Read documentation. Maniphest Tasks: T13356 Differential Revision: https://secure.phabricator.com/D20691 --- .../config/PhabricatorManiphestConfigOptions.php | 15 ++++++++++----- src/docs/user/configuration/custom_fields.diviner | 4 ++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index f1916cffec..077fc511ce 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -451,15 +451,20 @@ You can choose the default priority for newly created tasks with EOTEXT )); + $fields_description = $this->deformat(pht(<<newOption('maniphest.custom-field-definitions', 'wild', array()) ->setSummary(pht('Custom Maniphest fields.')) - ->setDescription( - pht( - 'Array of custom fields for Maniphest tasks. For details on '. - 'adding custom fields to Maniphest, see "Configuring Custom '. - 'Fields" in the documentation.')) + ->setDescription($fields_description) ->addExample($fields_json, pht('Valid setting')), $this->newOption('maniphest.fields', $custom_field_type, $default_fields) ->setCustomData(id(new ManiphestTask())->getCustomFieldBaseClass()) diff --git a/src/docs/user/configuration/custom_fields.diviner b/src/docs/user/configuration/custom_fields.diviner index ecb7382648..75d83fc8ba 100644 --- a/src/docs/user/configuration/custom_fields.diviner +++ b/src/docs/user/configuration/custom_fields.diviner @@ -121,6 +121,10 @@ When defining custom fields using a configuration option like supported in text, int and remarkup fields (optional). - **copy**: If true, this field's value will be copied when an object is created using another object as a template. + - **limit**: For control types which use a tokenizer control to let the user + select a list of values, this limits how many values can be selected. For + example, a "users" field with a limit of "1" will behave like the "Owner" + field in Maniphest and only allow selection of a single user. The `strings` value supports different strings per control type. They are: From f5c380bfc94cd195a04e77d3864968a0fd2ae656 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 1 Aug 2019 10:21:43 -0700 Subject: [PATCH 04/15] Add very basic support for generating PDF documents Summary: Ref T13358. This is very minimal, but technically works. The eventual goal is to generate PDF invoices to make my life easier when I have to interact with Enterprise Vendor Procurement. Test Plan: {F6672439} Maniphest Tasks: T13358 Differential Revision: https://secure.phabricator.com/D20692 --- src/__phutil_library_map__.php | 31 ++++++ .../pdf/PhabricatorPDFCatalogObject.php | 26 +++++ .../pdf/PhabricatorPDFContentsObject.php | 25 +++++ .../phortune/pdf/PhabricatorPDFFontObject.php | 14 +++ .../phortune/pdf/PhabricatorPDFFragment.php | 38 +++++++ .../pdf/PhabricatorPDFFragmentOffset.php | 27 +++++ .../phortune/pdf/PhabricatorPDFGenerator.php | 59 ++++++++++ .../pdf/PhabricatorPDFHeadFragment.php | 10 ++ .../phortune/pdf/PhabricatorPDFInfoObject.php | 11 ++ .../phortune/pdf/PhabricatorPDFIterator.php | 103 ++++++++++++++++++ .../phortune/pdf/PhabricatorPDFObject.php | 95 ++++++++++++++++ .../phortune/pdf/PhabricatorPDFPageObject.php | 48 ++++++++ .../pdf/PhabricatorPDFPagesObject.php | 38 +++++++ .../pdf/PhabricatorPDFResourcesObject.php | 28 +++++ .../pdf/PhabricatorPDFTailFragment.php | 72 ++++++++++++ 15 files changed, 625 insertions(+) create mode 100644 src/applications/phortune/pdf/PhabricatorPDFCatalogObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFContentsObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFFontObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFFragment.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFFragmentOffset.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFGenerator.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFHeadFragment.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFInfoObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFIterator.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFPageObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFPagesObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFResourcesObject.php create mode 100644 src/applications/phortune/pdf/PhabricatorPDFTailFragment.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5abfd1bc4d..03e0163ec8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3878,7 +3878,21 @@ phutil_register_library_map(array( 'PhabricatorOwnersPathsSearchEngineAttachment' => 'applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php', 'PhabricatorOwnersSchemaSpec' => 'applications/owners/storage/PhabricatorOwnersSchemaSpec.php', 'PhabricatorOwnersSearchField' => 'applications/owners/searchfield/PhabricatorOwnersSearchField.php', + 'PhabricatorPDFCatalogObject' => 'applications/phortune/pdf/PhabricatorPDFCatalogObject.php', + 'PhabricatorPDFContentsObject' => 'applications/phortune/pdf/PhabricatorPDFContentsObject.php', 'PhabricatorPDFDocumentEngine' => 'applications/files/document/PhabricatorPDFDocumentEngine.php', + 'PhabricatorPDFFontObject' => 'applications/phortune/pdf/PhabricatorPDFFontObject.php', + 'PhabricatorPDFFragment' => 'applications/phortune/pdf/PhabricatorPDFFragment.php', + 'PhabricatorPDFFragmentOffset' => 'applications/phortune/pdf/PhabricatorPDFFragmentOffset.php', + 'PhabricatorPDFGenerator' => 'applications/phortune/pdf/PhabricatorPDFGenerator.php', + 'PhabricatorPDFHeadFragment' => 'applications/phortune/pdf/PhabricatorPDFHeadFragment.php', + 'PhabricatorPDFInfoObject' => 'applications/phortune/pdf/PhabricatorPDFInfoObject.php', + 'PhabricatorPDFIterator' => 'applications/phortune/pdf/PhabricatorPDFIterator.php', + 'PhabricatorPDFObject' => 'applications/phortune/pdf/PhabricatorPDFObject.php', + 'PhabricatorPDFPageObject' => 'applications/phortune/pdf/PhabricatorPDFPageObject.php', + 'PhabricatorPDFPagesObject' => 'applications/phortune/pdf/PhabricatorPDFPagesObject.php', + 'PhabricatorPDFResourcesObject' => 'applications/phortune/pdf/PhabricatorPDFResourcesObject.php', + 'PhabricatorPDFTailFragment' => 'applications/phortune/pdf/PhabricatorPDFTailFragment.php', 'PhabricatorPHDConfigOptions' => 'applications/config/option/PhabricatorPHDConfigOptions.php', 'PhabricatorPHID' => 'applications/phid/storage/PhabricatorPHID.php', 'PhabricatorPHIDConstants' => 'applications/phid/PhabricatorPHIDConstants.php', @@ -10101,7 +10115,24 @@ phutil_register_library_map(array( 'PhabricatorOwnersPathsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment', 'PhabricatorOwnersSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorOwnersSearchField' => 'PhabricatorSearchTokenizerField', + 'PhabricatorPDFCatalogObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFContentsObject' => 'PhabricatorPDFObject', 'PhabricatorPDFDocumentEngine' => 'PhabricatorDocumentEngine', + 'PhabricatorPDFFontObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFFragment' => 'Phobject', + 'PhabricatorPDFFragmentOffset' => 'Phobject', + 'PhabricatorPDFGenerator' => 'Phobject', + 'PhabricatorPDFHeadFragment' => 'PhabricatorPDFFragment', + 'PhabricatorPDFInfoObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFIterator' => array( + 'Phobject', + 'Iterator', + ), + 'PhabricatorPDFObject' => 'PhabricatorPDFFragment', + 'PhabricatorPDFPageObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFPagesObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFResourcesObject' => 'PhabricatorPDFObject', + 'PhabricatorPDFTailFragment' => 'PhabricatorPDFFragment', 'PhabricatorPHDConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPHID' => 'Phobject', 'PhabricatorPHIDConstants' => 'Phobject', diff --git a/src/applications/phortune/pdf/PhabricatorPDFCatalogObject.php b/src/applications/phortune/pdf/PhabricatorPDFCatalogObject.php new file mode 100644 index 0000000000..9cf4d2324e --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFCatalogObject.php @@ -0,0 +1,26 @@ +pagesObject = $this->newChildObject($pages_object); + return $this; + } + + public function getPagesObject() { + return $this->pagesObject; + } + + protected function writeObject() { + $this->writeLine('/Type /Catalog'); + + $pages_object = $this->getPagesObject(); + if ($pages_object) { + $this->writeLine('/Pages %d 0 R', $pages_object->getObjectIndex()); + } + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFContentsObject.php b/src/applications/phortune/pdf/PhabricatorPDFContentsObject.php new file mode 100644 index 0000000000..f49a32df33 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFContentsObject.php @@ -0,0 +1,25 @@ +rawContent = $raw_content; + return $this; + } + + public function getRawContent() { + return $this->rawContent; + } + + protected function writeObject() { + $data = $this->getRawContent(); + + $stream_length = $this->newStream($data); + + $this->writeLine('/Filter /FlateDecode /Length %d', $stream_length); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFFontObject.php b/src/applications/phortune/pdf/PhabricatorPDFFontObject.php new file mode 100644 index 0000000000..71f128d3a5 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFFontObject.php @@ -0,0 +1,14 @@ +writeLine('/Type /Font'); + + $this->writeLine('/BaseFont /Helvetica-Bold'); + $this->writeLine('/Subtype /Type1'); + $this->writeLine('/Encoding /WinAnsiEncoding'); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFFragment.php b/src/applications/phortune/pdf/PhabricatorPDFFragment.php new file mode 100644 index 0000000000..eb113b6140 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFFragment.php @@ -0,0 +1,38 @@ +rope = new PhutilRope(); + + $this->writeFragment(); + + $rope = $this->rope; + $this->rope = null; + + return $rope->getAsString(); + } + + public function hasRefTableEntry() { + return false; + } + + abstract protected function writeFragment(); + + final protected function writeLine($pattern) { + $pattern = $pattern."\n"; + + $argv = func_get_args(); + $argv[0] = $pattern; + + $line = call_user_func_array('sprintf', $argv); + + $this->rope->append($line); + + return $this; + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFFragmentOffset.php b/src/applications/phortune/pdf/PhabricatorPDFFragmentOffset.php new file mode 100644 index 0000000000..c8b2769d7a --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFFragmentOffset.php @@ -0,0 +1,27 @@ +fragment = $fragment; + return $this; + } + + public function getFragment() { + return $this->fragment; + } + + public function setOffset($offset) { + $this->offset = $offset; + return $this; + } + + public function getOffset() { + return $this->offset; + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFGenerator.php b/src/applications/phortune/pdf/PhabricatorPDFGenerator.php new file mode 100644 index 0000000000..f2c5c92359 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFGenerator.php @@ -0,0 +1,59 @@ +hasIterator) { + throw new Exception( + pht( + 'This generator has already emitted an iterator. You can not '. + 'modify the PDF document after you begin writing it.')); + } + + $this->objects[] = $object; + $index = count($this->objects); + + $object->setGenerator($this, $index); + + return $this; + } + + public function getObjects() { + return $this->objects; + } + + public function newIterator() { + $this->hasIterator = true; + return id(new PhabricatorPDFIterator()) + ->setGenerator($this); + } + + public function setInfoObject(PhabricatorPDFInfoObject $info_object) { + $this->addObject($info_object); + $this->infoObject = $info_object; + return $this; + } + + public function getInfoObject() { + return $this->infoObject; + } + + public function setCatalogObject( + PhabricatorPDFCatalogObject $catalog_object) { + $this->addObject($catalog_object); + $this->catalogObject = $catalog_object; + return $this; + } + + public function getCatalogObject() { + return $this->catalogObject; + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFHeadFragment.php b/src/applications/phortune/pdf/PhabricatorPDFHeadFragment.php new file mode 100644 index 0000000000..ef12bacce9 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFHeadFragment.php @@ -0,0 +1,10 @@ +writeLine('%s', '%PDF-1.3'); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFInfoObject.php b/src/applications/phortune/pdf/PhabricatorPDFInfoObject.php new file mode 100644 index 0000000000..2aba63c407 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFInfoObject.php @@ -0,0 +1,11 @@ +writeLine('/Producer (Phabricator 20190801)'); + $this->writeLine('/CreationDate (D:%s)', date('YmdHis')); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFIterator.php b/src/applications/phortune/pdf/PhabricatorPDFIterator.php new file mode 100644 index 0000000000..d39168369d --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFIterator.php @@ -0,0 +1,103 @@ +generator) { + throw new Exception( + pht( + 'This iterator already has a generator. You can not modify the '. + 'generator for a given iterator.')); + } + + $this->generator = $generator; + + return $this; + } + + public function getGenerator() { + if (!$this->generator) { + throw new Exception( + pht( + 'This PDF iterator has no associated PDF generator.')); + } + + return $this->generator; + } + + public function getFragmentOffsets() { + return $this->fragmentOffsets; + } + + public function current() { + return $this->fragmentBytes; + } + + public function key() { + return $this->framgentKey; + } + + public function next() { + $this->fragmentKey++; + + if (!$this->valid()) { + return; + } + + $fragment = $this->fragments[$this->fragmentKey]; + + $this->fragmentOffsets[] = id(new PhabricatorPDFFragmentOffset()) + ->setFragment($fragment) + ->setOffset($this->byteLength); + + $bytes = $fragment->getAsBytes(); + + $this->fragmentBytes = $bytes; + $this->byteLength += strlen($bytes); + } + + public function rewind() { + if ($this->hasRewound) { + throw new Exception( + pht( + 'PDF iterators may not be rewound. Create a new iterator to emit '. + 'another PDF.')); + } + + $generator = $this->getGenerator(); + $objects = $generator->getObjects(); + + $this->fragments = array(); + $this->fragments[] = new PhabricatorPDFHeadFragment(); + + foreach ($objects as $object) { + $this->fragments[] = $object; + } + + $this->fragments[] = id(new PhabricatorPDFTailFragment()) + ->setIterator($this); + + $this->hasRewound = true; + + $this->fragmentKey = -1; + $this->byteLength = 0; + + $this->next(); + } + + public function valid() { + return isset($this->fragments[$this->fragmentKey]); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFObject.php b/src/applications/phortune/pdf/PhabricatorPDFObject.php new file mode 100644 index 0000000000..49c14d2f00 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFObject.php @@ -0,0 +1,95 @@ +writeLine('%d 0 obj', $this->getObjectIndex()); + $this->writeLine('<<'); + $this->writeObject(); + $this->writeLine('>>'); + + $streams = $this->streams; + $this->streams = array(); + foreach ($streams as $stream) { + $this->writeLine('stream'); + $this->writeLine('%s', $stream); + $this->writeLine('endstream'); + } + + $this->writeLine('endobj'); + } + + final public function setGenerator( + PhabricatorPDFGenerator $generator, + $index) { + + if ($this->getGenerator()) { + throw new Exception( + pht( + 'This PDF object is already registered with a PDF generator. You '. + 'can not register an object with more than one generator.')); + } + + $this->generator = $generator; + $this->objectIndex = $index; + + foreach ($this->getChildren() as $child) { + $generator->addObject($child); + } + + return $this; + } + + final public function getGenerator() { + return $this->generator; + } + + final public function getObjectIndex() { + if (!$this->objectIndex) { + throw new Exception( + pht( + 'Trying to get index for object ("%s") which has not been '. + 'registered with a generator.', + get_class($this))); + } + + return $this->objectIndex; + } + + final protected function newChildObject(PhabricatorPDFObject $object) { + if ($this->generator) { + throw new Exception( + pht( + 'Trying to add a new PDF Object child after already registering '. + 'the object with a generator.')); + } + + $this->children[] = $object; + return $object; + } + + private function getChildren() { + return $this->children; + } + + abstract protected function writeObject(); + + final protected function newStream($raw_data) { + $stream_data = gzcompress($raw_data); + + $this->streams[] = $stream_data; + + return strlen($stream_data); + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFPageObject.php b/src/applications/phortune/pdf/PhabricatorPDFPageObject.php new file mode 100644 index 0000000000..3137d45d12 --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFPageObject.php @@ -0,0 +1,48 @@ +pagesObject = $pages; + return $this; + } + + public function setContentsObject(PhabricatorPDFContentsObject $contents) { + $this->contentsObject = $this->newChildObject($contents); + return $this; + } + + public function setResourcesObject(PhabricatorPDFResourcesObject $resources) { + $this->resourcesObject = $this->newChildObject($resources); + return $this; + } + + protected function writeObject() { + $this->writeLine('/Type /Page'); + + $pages_object = $this->pagesObject; + $contents_object = $this->contentsObject; + $resources_object = $this->resourcesObject; + + if ($pages_object) { + $pages_index = $pages_object->getObjectIndex(); + $this->writeLine('/Parent %d 0 R', $pages_index); + } + + if ($contents_object) { + $contents_index = $contents_object->getObjectIndex(); + $this->writeLine('/Contents %d 0 R', $contents_index); + } + + if ($resources_object) { + $resources_index = $resources_object->getObjectIndex(); + $this->writeLine('/Resources %d 0 R', $resources_index); + } + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFPagesObject.php b/src/applications/phortune/pdf/PhabricatorPDFPagesObject.php new file mode 100644 index 0000000000..4f0b89886e --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFPagesObject.php @@ -0,0 +1,38 @@ +setPagesObject($this); + $this->pageObjects[] = $this->newChildObject($page); + return $this; + } + + public function getPageObjects() { + return $this->pageObjects; + } + + protected function writeObject() { + $this->writeLine('/Type /Pages'); + + $page_objects = $this->getPageObjects(); + + $this->writeLine('/Count %d', count($page_objects)); + $this->writeLine('/MediaBox [%d %d %0.2f %0.2f]', 0, 0, 595.28, 841.89); + + if ($page_objects) { + $kids = array(); + foreach ($page_objects as $page_object) { + $kids[] = sprintf( + '%d 0 R', + $page_object->getObjectIndex()); + } + + $this->writeLine('/Kids [%s]', implode(' ', $kids)); + } + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFResourcesObject.php b/src/applications/phortune/pdf/PhabricatorPDFResourcesObject.php new file mode 100644 index 0000000000..9414708f6d --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFResourcesObject.php @@ -0,0 +1,28 @@ +fontObjects[] = $this->newChildObject($font); + return $this; + } + + public function getFontObjects() { + return $this->fontObjects; + } + + protected function writeObject() { + $this->writeLine('/ProcSet [/PDF /Text /ImageB /ImageC /ImageI]'); + + $fonts = $this->getFontObjects(); + foreach ($fonts as $font) { + $this->writeLine('/Font <<'); + $this->writeLine('/F%d %d 0 R', 1, $font->getObjectIndex()); + $this->writeLine('>>'); + } + } + +} diff --git a/src/applications/phortune/pdf/PhabricatorPDFTailFragment.php b/src/applications/phortune/pdf/PhabricatorPDFTailFragment.php new file mode 100644 index 0000000000..2f606a1c8c --- /dev/null +++ b/src/applications/phortune/pdf/PhabricatorPDFTailFragment.php @@ -0,0 +1,72 @@ +iterator = $iterator; + return $this; + } + + public function getIterator() { + return $this->iterator; + } + + protected function writeFragment() { + $iterator = $this->getIterator(); + $generator = $iterator->getGenerator(); + $objects = $generator->getObjects(); + + $xref_offset = null; + + $this->writeLine('xref'); + $this->writeLine('0 %d', count($objects) + 1); + $this->writeLine('%010d %05d f ', 0, 0xFFFF); + + $offset_map = array(); + + $fragment_offsets = $iterator->getFragmentOffsets(); + foreach ($fragment_offsets as $fragment_offset) { + $fragment = $fragment_offset->getFragment(); + $offset = $fragment_offset->getOffset(); + + if ($fragment === $this) { + $xref_offset = $offset; + } + + if (!$fragment->hasRefTableEntry()) { + continue; + } + + $offset_map[$fragment->getObjectIndex()] = $offset; + } + + ksort($offset_map); + + foreach ($offset_map as $offset) { + $this->writeLine('%010d %05d n ', $offset, 0); + } + + $this->writeLine('trailer'); + $this->writeLine('<<'); + $this->writeLine('/Size %d', count($objects) + 1); + + $info_object = $generator->getInfoObject(); + if ($info_object) { + $this->writeLine('/Info %d 0 R', $info_object->getObjectIndex()); + } + + $catalog_object = $generator->getCatalogObject(); + if ($catalog_object) { + $this->writeLine('/Root %d 0 R', $catalog_object->getObjectIndex()); + } + + $this->writeLine('>>'); + $this->writeLine('startxref'); + $this->writeLine('%d', $xref_offset); + $this->writeLine('%s', '%%EOF'); + } + +} From 3069ef41662d916ea57d254b10491d6e852d8d46 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 1 Aug 2019 12:02:01 -0700 Subject: [PATCH 05/15] Prevent object titles in the "Object Attacher" dialog from triggering Quicksand "Close Dialog on Navigation" behavior Summary: Fixes T13363. Currently, these are genuine links which we intercept events for. Make them pseudolinks instead. Possible alternative approaches are: - Keep them as genuine links, but mark them as non-navigation links for Quicksand. (But: yuck, weird special case.) - Keep them as genuine links, and have the dialog handler `JX.Stratcom.pass()` to see if anything handles the event. (But: the "pass()" pattern generally feels bad.) "Tableaus" or whatever comes out of T10469 some day will probably break everything anyway? Test Plan: - Opened the "Edit Related Tasks... > Edit Subtasks" dialog. - Clicked task title links (not the "open in new window" icon, and not the "Select" button). - Before: Dialog (sometimes) closed abruptly. - After: Task is consistently selected as part of the attachment set. Maniphest Tasks: T13363 Differential Revision: https://secure.phabricator.com/D20693 --- resources/celerity/map.php | 18 +++++++++--------- .../rsrc/js/core/behavior-object-selector.js | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index ca09a081e5..9dc1d68fd9 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -12,7 +12,7 @@ return array( 'core.pkg.css' => 'af983028', 'core.pkg.js' => '73a06a9f', 'differential.pkg.css' => '8d8360fb', - 'differential.pkg.js' => '67e02996', + 'differential.pkg.js' => '0b037a4f', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -484,7 +484,7 @@ return array( 'rsrc/js/core/behavior-line-linker.js' => 'e15c8b1f', 'rsrc/js/core/behavior-linked-container.js' => '74446546', 'rsrc/js/core/behavior-more.js' => '506aa3f4', - 'rsrc/js/core/behavior-object-selector.js' => 'a4af0b4a', + 'rsrc/js/core/behavior-object-selector.js' => '98ef467f', 'rsrc/js/core/behavior-oncopy.js' => 'ff7b3f22', 'rsrc/js/core/behavior-phabricator-nav.js' => 'f166c949', 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '2f80333f', @@ -645,7 +645,7 @@ return array( 'javelin-behavior-phabricator-line-linker' => 'e15c8b1f', 'javelin-behavior-phabricator-nav' => 'f166c949', 'javelin-behavior-phabricator-notification-example' => '29819b75', - 'javelin-behavior-phabricator-object-selector' => 'a4af0b4a', + 'javelin-behavior-phabricator-object-selector' => '98ef467f', 'javelin-behavior-phabricator-oncopy' => 'ff7b3f22', 'javelin-behavior-phabricator-remarkup-assist' => '2f80333f', 'javelin-behavior-phabricator-reveal-content' => 'b105a3a6', @@ -1730,6 +1730,12 @@ return array( 'javelin-dom', 'javelin-router', ), + '98ef467f' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-request', + 'javelin-util', + ), '9aae2b66' => array( 'javelin-install', 'javelin-util', @@ -1790,12 +1796,6 @@ return array( 'phui-button-css', 'phui-button-simple-css', ), - 'a4af0b4a' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-request', - 'javelin-util', - ), 'a5257c4e' => array( 'javelin-install', 'javelin-dom', diff --git a/webroot/rsrc/js/core/behavior-object-selector.js b/webroot/rsrc/js/core/behavior-object-selector.js index 722cfdd562..b28dac0926 100644 --- a/webroot/rsrc/js/core/behavior-object-selector.js +++ b/webroot/rsrc/js/core/behavior-object-selector.js @@ -132,7 +132,7 @@ JX.behavior('phabricator-object-selector', function(config) { var select_object_link = JX.$N( 'a', - {href: h.uri, sigil: 'object-attacher'}, + {href: '#', sigil: 'object-attacher'}, h.name); var select_object_button = JX.$N( From 1fe631116771b834a8e25e801d08bc3b0e9945e5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Aug 2019 09:15:06 -0700 Subject: [PATCH 06/15] Modernize user and repository "delete" workflows and improve documentation Summary: Fixes T8830. Fixes T13364. - The inability to destroy objects from the web UI is intentional. Make this clear in the messaging, which is somewhat out of date and partly reflects an earlier era when things could be destroyed. - `bin/remove destroy` can't rewind time. Document expectations around the "put the cat back in the bag" use case. Test Plan: Read documentation, clicked through both workflows. Maniphest Tasks: T13364, T8830 Differential Revision: https://secure.phabricator.com/D20694 --- resources/celerity/map.php | 6 +- ...iffusionRepositoryEditDeleteController.php | 47 +++++----- ...ffusionRepositoryBasicsManagementPanel.php | 2 - .../PhabricatorPeopleDeleteController.php | 75 ++++++--------- .../field/permanently_destroying_data.diviner | 92 +++++++++++++++++++ .../user/userguide/diffusion_managing.diviner | 9 +- src/view/AphrontDialogView.php | 35 +++++-- webroot/rsrc/css/aphront/dialog-view.css | 5 + 8 files changed, 181 insertions(+), 90 deletions(-) create mode 100644 src/docs/user/field/permanently_destroying_data.diviner diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 9dc1d68fd9..4eeb433e96 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'af983028', + 'core.pkg.css' => '5a4a5010', 'core.pkg.js' => '73a06a9f', 'differential.pkg.css' => '8d8360fb', 'differential.pkg.js' => '0b037a4f', @@ -24,7 +24,7 @@ return array( 'rsrc/audio/basic/ting.mp3' => 'a6b6540e', 'rsrc/css/aphront/aphront-bars.css' => '4a327b4a', 'rsrc/css/aphront/dark-console.css' => '7f06cda2', - 'rsrc/css/aphront/dialog-view.css' => 'b70c70df', + 'rsrc/css/aphront/dialog-view.css' => '874f5c06', 'rsrc/css/aphront/list-filter-view.css' => 'feb64255', 'rsrc/css/aphront/multi-column.css' => 'fbc00ba3', 'rsrc/css/aphront/notification.css' => '30240bd2', @@ -530,7 +530,7 @@ return array( 'almanac-css' => '2e050f4f', 'aphront-bars' => '4a327b4a', 'aphront-dark-console-css' => '7f06cda2', - 'aphront-dialog-view-css' => 'b70c70df', + 'aphront-dialog-view-css' => '874f5c06', 'aphront-list-filter-view-css' => 'feb64255', 'aphront-multi-column-view-css' => 'fbc00ba3', 'aphront-panel-view-css' => '46923d46', diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php index 0380b99f0f..93fe70c0fe 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php @@ -17,32 +17,31 @@ final class DiffusionRepositoryEditDeleteController ->setRepository($repository) ->getPanelURI(); - $dialog = new AphrontDialogView(); - $text_1 = pht( - 'If you really want to delete the repository, run this command from '. - 'the command line:'); - $command = csprintf( - 'phabricator/ $ ./bin/remove destroy %R', - $repository->getMonogram()); - $text_2 = pht( - 'Repositories touch many objects and as such deletes are '. - 'prohibitively expensive to run from the web UI.'); - $body = phutil_tag( - 'div', - array( - 'class' => 'phabricator-remarkup', - ), - array( - phutil_tag('p', array(), $text_1), - phutil_tag('p', array(), - phutil_tag('tt', array(), $command)), - phutil_tag('p', array(), $text_2), - )); + $doc_uri = PhabricatorEnv::getDoclink( + 'Permanently Destroying Data'); return $this->newDialog() - ->setTitle(pht('Really want to delete the repository?')) - ->appendChild($body) - ->addCancelButton($panel_uri, pht('Okay')); + ->setTitle(pht('Delete Repository')) + ->appendParagraph( + pht( + 'To permanently destroy this repository, run this command from '. + 'the command line:')) + ->appendCommand( + csprintf( + 'phabricator/ $ ./bin/remove destroy %R', + $repository->getMonogram())) + ->appendParagraph( + pht( + 'Repositories can not be permanently destroyed from the web '. + 'interface. See %s in the documentation for more information.', + phutil_tag( + 'a', + array( + 'href' => $doc_uri, + 'target' => '_blank', + ), + pht('Permanently Destroying Data')))) + ->addCancelButton($panel_uri, pht('Close')); } } diff --git a/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php index 2d006136e3..02df82c7c0 100644 --- a/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryBasicsManagementPanel.php @@ -155,8 +155,6 @@ final class DiffusionRepositoryBasicsManagementPanel ->setName(pht('Delete Repository')) ->setHref($delete_uri) ->setIcon('fa-times') - ->setColor(PhabricatorActionView::RED) - ->setDisabled(true) ->setWorkflow(true)); return $this->newCurtainView() diff --git a/src/applications/people/controller/PhabricatorPeopleDeleteController.php b/src/applications/people/controller/PhabricatorPeopleDeleteController.php index 29ae7edd93..8e6ac91da7 100644 --- a/src/applications/people/controller/PhabricatorPeopleDeleteController.php +++ b/src/applications/people/controller/PhabricatorPeopleDeleteController.php @@ -17,58 +17,35 @@ final class PhabricatorPeopleDeleteController $manage_uri = $this->getApplicationURI("manage/{$id}/"); - if ($user->getPHID() == $viewer->getPHID()) { - return $this->buildDeleteSelfResponse($manage_uri); - } - - $str1 = pht( - 'Be careful when deleting users! This will permanently and '. - 'irreversibly destroy this user account.'); - - $str2 = pht( - 'If this user interacted with anything, it is generally better to '. - 'disable them, not delete them. If you delete them, it will no longer '. - 'be possible to (for example) search for objects they created, and you '. - 'will lose other information about their history. Disabling them '. - 'instead will prevent them from logging in, but will not destroy any of '. - 'their data.'); - - $str3 = pht( - 'It is generally safe to delete newly created users (and test users and '. - 'so on), but less safe to delete established users. If possible, '. - 'disable them instead.'); - - $str4 = pht('To permanently destroy this user, run this command:'); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->appendRemarkupInstructions( - csprintf( - " phabricator/ $ ./bin/remove destroy %R\n", - '@'.$user->getUsername())); + $doc_uri = PhabricatorEnv::getDoclink( + 'Permanently Destroying Data'); return $this->newDialog() - ->setWidth(AphrontDialogView::WIDTH_FORM) - ->setTitle(pht('Permanently Delete User')) - ->setShortTitle(pht('Delete User')) - ->appendParagraph($str1) - ->appendParagraph($str2) - ->appendParagraph($str3) - ->appendParagraph($str4) - ->appendChild($form->buildLayoutView()) + ->setTitle(pht('Delete User')) + ->appendParagraph( + pht( + 'To permanently destroy this user, run this command from the '. + 'command line:')) + ->appendCommand( + csprintf( + 'phabricator/ $ ./bin/remove destroy %R', + $user->getMonogram())) + ->appendParagraph( + pht( + 'Unless you have a very good reason to delete this user, consider '. + 'disabling them instead.')) + ->appendParagraph( + pht( + 'Users can not be permanently destroyed from the web interface. '. + 'See %s in the documentation for more information.', + phutil_tag( + 'a', + array( + 'href' => $doc_uri, + 'target' => '_blank', + ), + pht('Permanently Destroying Data')))) ->addCancelButton($manage_uri, pht('Close')); } - private function buildDeleteSelfResponse($cancel_uri) { - return $this->newDialog() - ->setTitle(pht('You Shall Journey No Farther')) - ->appendParagraph( - pht( - 'As you stare into the gaping maw of the abyss, something '. - 'holds you back.')) - ->appendParagraph(pht('You can not delete your own account.')) - ->addCancelButton($cancel_uri, pht('Turn Back')); - } - - } diff --git a/src/docs/user/field/permanently_destroying_data.diviner b/src/docs/user/field/permanently_destroying_data.diviner new file mode 100644 index 0000000000..04907fc0be --- /dev/null +++ b/src/docs/user/field/permanently_destroying_data.diviner @@ -0,0 +1,92 @@ +@title Permanently Destroying Data +@group fieldmanual + +How to permanently destroy data and manage leaked secrets. + +Overview +======== + +Phabricator intentionally makes it difficult to permanently destroy data, but +provides a command-line tool for destroying objects if you're certain that +you want to destroy something. + +**Disable vs Destroy**: Most kinds of objects can be disabled, deactivated, +closed, or archived. These operations place them in inactive states and +preserve their transaction history. + +(NOTE) Disabling (rather than destroying) objects is strongly recommended +unless you have a very good reason to want to permanently destroy an object. + + +Destroying Data +=============== + +To permanently destroy an object, run this command from the command line: + +``` +phabricator/ $ ./bin/remove destroy +``` + +The `` may be an object monogram or PHID. For instance, you can use +`@alice` to destroy a particular user, or `T123` to destroy a particular task. + +(IMPORTANT) This operation is permanent and can not be undone. + + +CLI Access Required +=================== + +In almost all cases, Phabricator requires operational access from the CLI to +permanently destroy data. One major reason for this requirement is that it +limits the reach of an attacker who compromises a privileged account. + +The web UI is generally append-only and actions generally leave an audit +trail, usually in the transaction log. Thus, an attacker who compromises an +account but only gains access to the web UI usually can not do much permanent +damage and usually can not hide their actions or cover their tracks. + +Another reason that destroying data is hard is simply that it's permanent and +can not be undone, so there's no way to recover from mistakes. + + +Leaked Secrets +============== + +Sometimes you may want to destroy an object because it has leaked a secret, +like an API key or another credential. For example, an engineer might +accidentally send a change for review which includes a sensitive private key. + +No Phabricator command can rewind time, and once data is written to Phabricator +the cat is often out of the bag: it has often been transmitted to external +systems which Phabricator can not interact with via email, webhooks, API calls, +repository mirroring, CDN caching, and so on. You can try to clean up the mess, +but you're generally already too late. + +The `bin/remove destroy` command will make a reasonable attempt to completely +destroy objects, but this is just an attempt. It can not unsend email or uncall +the API, and no command can rewind time and undo a leak. + +**Revoking Credentials**: If Phabricator credentials were accidentally +disclosed, you can revoke them so they no longer function. See +@{article:Revoking Credentials} for more information. + + +Preventing Leaks +================ + +Because time can not be rewound, it is best to prevent sensitive data from +leaking in the first place. Phabricator supports some technical measures that +can make it more difficult to accidentally disclose secrets: + +**Differential Diff Herald Rules**: You can write "Differential Diff" rules +in Herald that reject diffs before they are written to disk by using the +"Block diff with message" action. + +These rules can reject diffs based on affected file names or file content. +This is a coarse tool, but rejecting diffs which contain strings like +`BEGIN RSA PRIVATE KEY` may make it more difficult to accidentally disclose +certain secrets. + +**Commit Content Herald Rules**: For hosted repositories, you can write +"Commit Hook: Commit Content" rules in Herald which reject pushes that contain +commit which match certain rules (like file name or file content rules). diff --git a/src/docs/user/userguide/diffusion_managing.diviner b/src/docs/user/userguide/diffusion_managing.diviner index aa52c1c475..138bc918bc 100644 --- a/src/docs/user/userguide/diffusion_managing.diviner +++ b/src/docs/user/userguide/diffusion_managing.diviner @@ -169,8 +169,8 @@ start working normally. Basics: Delete Repository ========================= -Repositories can not be deleted from the web UI, so this option is always -disabled. Clicking it gives you information about how to delete a repository. +Repositories can not be deleted from the web UI, so this option only gives you +information about how to delete a repository. Repositories can only be deleted from the command line, with `bin/remove`: @@ -178,9 +178,8 @@ Repositories can only be deleted from the command line, with `bin/remove`: $ ./bin/remove destroy ``` -WARNING: This command will issue you a dire warning about the severity of the -action you are taking. Heed this warning. You are **strongly discouraged** from -destroying repositories. Instead, deactivate them. +This command will permanently destroy the repository. For more information +about destroying things, see @{article:Permanently Destroying Data}. Policies diff --git a/src/view/AphrontDialogView.php b/src/view/AphrontDialogView.php index 52681845d2..09fc8e7a16 100644 --- a/src/view/AphrontDialogView.php +++ b/src/view/AphrontDialogView.php @@ -161,15 +161,36 @@ final class AphrontDialogView } public function appendParagraph($paragraph) { - return $this->appendChild( - phutil_tag( - 'p', - array( - 'class' => 'aphront-dialog-view-paragraph', - ), - $paragraph)); + return $this->appendParagraphTag($paragraph); } + public function appendCommand($command) { + $command_tag = phutil_tag('tt', array(), $command); + return $this->appendParagraphTag( + $command_tag, + 'aphront-dialog-view-command'); + } + + private function appendParagraphTag($content, $classes = null) { + if ($classes) { + $classes = (array)$classes; + } else { + $classes = array(); + } + + array_unshift($classes, 'aphront-dialog-view-paragraph'); + + $paragraph_tag = phutil_tag( + 'p', + array( + 'class' => implode(' ', $classes), + ), + $content); + + return $this->appendChild($paragraph_tag); + } + + public function appendList(array $items) { $listitems = array(); foreach ($items as $item) { diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css index 153685548e..b47ca14850 100644 --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -158,6 +158,11 @@ margin-top: 16px; } +.aphront-dialog-view-command { + padding: 8px 16px; + background: {$greybackground}; +} + .device-desktop .aphront-dialog-flush .phui-oi-list-view { margin: 0; padding: 0; From 6c41508906778464cf60be5207821abe80ff57a3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Aug 2019 09:38:13 -0700 Subject: [PATCH 07/15] Fix an issue where lines with more than one pattern match highlighted improperly in Diffusion Summary: Ref T13339. If a search pattern matches more than once on a line, we currently render the line incorreclty, duplicating some of the text. `substr()` is being called as though the third parameter was `end_offset`, but it's actually `length`. Correct the parameter. Test Plan: Before: {F6676625} After: {F6676623} Maniphest Tasks: T13339 Differential Revision: https://secure.phabricator.com/D20695 --- src/applications/diffusion/view/DiffusionPatternSearchView.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/diffusion/view/DiffusionPatternSearchView.php b/src/applications/diffusion/view/DiffusionPatternSearchView.php index 93ef42d46d..39a1df4e89 100644 --- a/src/applications/diffusion/view/DiffusionPatternSearchView.php +++ b/src/applications/diffusion/view/DiffusionPatternSearchView.php @@ -47,7 +47,7 @@ final class DiffusionPatternSearchView extends DiffusionView { $offset = $match[1]; if ($cursor != $offset) { $output[] = array( - 'text' => substr($string, $cursor, $offset), + 'text' => substr($string, $cursor, ($offset - $cursor)), 'highlight' => false, ); } From 87f878ec8a774e3de0f4f596304bebf8f44c0d30 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 2 Aug 2019 10:44:24 -0700 Subject: [PATCH 08/15] Stop trying to CC merchants on invoices/receipts Summary: Fixes T13341. Currently, cart emails (invoices/receipts) are sent to members of the associated merchant account. This was just a simple way to keep an eye on things when this was first written. The system works fine, and recent changes (almost certainly D20525) stopped these emails from working (presumably because of the slightly weird merchant permissions model). This could be sorted out in more detail, but it looks like the path forward is to introduce a side channel for email anyway (via T8389), and that's a better way to implement this behavior since it means the normal recipients won't see a bunch of random staff/merchant email addresses on their receipts. Test Plan: Grepped for `merchant` in this editor. Maniphest Tasks: T13341 Differential Revision: https://secure.phabricator.com/D20696 --- src/applications/phortune/editor/PhortuneCartEditor.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/applications/phortune/editor/PhortuneCartEditor.php b/src/applications/phortune/editor/PhortuneCartEditor.php index dcf1f2d0e0..854c4ab9c7 100644 --- a/src/applications/phortune/editor/PhortuneCartEditor.php +++ b/src/applications/phortune/editor/PhortuneCartEditor.php @@ -188,8 +188,8 @@ final class PhortuneCartEditor protected function getMailTo(PhabricatorLiskDAO $object) { $phids = array(); - // Reload the cart to pull merchant and account information, in case we - // just created the object. + // Reload the cart to pull account information, in case we just created the + // object. $cart = id(new PhortuneCartQuery()) ->setViewer($this->requireActor()) ->withPHIDs(array($object->getPHID())) @@ -199,10 +199,6 @@ final class PhortuneCartEditor $phids[] = $account_member; } - foreach ($cart->getMerchant()->getMemberPHIDs() as $merchant_member) { - $phids[] = $merchant_member; - } - return $phids; } From 31254c5124d5d91a020fd003c94e8d95d6afb266 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Aug 2019 08:57:19 -0700 Subject: [PATCH 09/15] Correct column options presented in "Move tasks to project..." on workboards Summary: Ref T13368. The column options presented to the user are currently incorrect because the wrong set of columns are drawn from. Test Plan: On a workboard, used "Move tasks to project..." to target another board, saw that board's columns. Maniphest Tasks: T13368 Differential Revision: https://secure.phabricator.com/D20698 --- .../controller/PhabricatorProjectColumnBulkMoveController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php index 2b4c536c8d..7dc4c77e1c 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php @@ -107,7 +107,7 @@ final class PhabricatorProjectColumnBulkMoveController ->executeLayout(); $dst_columns = $layout_engine->getColumns($dst_project->getPHID()); - $dst_columns = mpull($columns, null, 'getPHID'); + $dst_columns = mpull($dst_columns, null, 'getPHID'); $has_column = false; $dst_column = null; From 6deac356599ffff00572ba52c6706a81337d76d3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Aug 2019 08:59:34 -0700 Subject: [PATCH 10/15] Don't show proxy (subproject/milestone) columns as options in "Move tasks..." workflows from workboards Summary: Ref T13368. Proxy columns should not be selectable from this workflow. If you want to move tasks to milestone/subproject X, do "Move tasks to project..." and pick X as the project. (This could be made to work some day.) Test Plan: Went through a "Move tasks to project..." workflow targeting a project with subprojects. No longer saw subproject columns presented as dropdown options. Maniphest Tasks: T13368 Differential Revision: https://secure.phabricator.com/D20699 --- .../PhabricatorProjectColumnBulkMoveController.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php index 7dc4c77e1c..5792664110 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php @@ -109,6 +109,15 @@ final class PhabricatorProjectColumnBulkMoveController $dst_columns = $layout_engine->getColumns($dst_project->getPHID()); $dst_columns = mpull($dst_columns, null, 'getPHID'); + // Prevent moves to milestones or subprojects by selecting their + // columns, since the implications aren't obvious and this doesn't + // work the same way as normal column moves. + foreach ($dst_columns as $key => $dst_column) { + if ($dst_column->getProxyPHID()) { + unset($dst_columns[$key]); + } + } + $has_column = false; $dst_column = null; From 0561043a1f575567b9619637837e86723d977207 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Aug 2019 09:04:36 -0700 Subject: [PATCH 11/15] In "Move task to..." workflow, separate visible and hidden columns in the dropdown Summary: Ref T13368. Currently, both visible and hidden columns are shown in the "Move tasks to..." dropdown on workflows from workboards. When the dropdown contains hidden columns, move them to a separate section to make it clear that they're not likely targets. Test Plan: - Used "Move tasks to project..." targeting a board with no hidden columns. Saw a single ungrouped dropdown. - Used "Move tasks to project..." targeting a board with hidden columns. Saw a dropdown grouped into "Visible" and "Hidden" columns. Maniphest Tasks: T13368 Differential Revision: https://secure.phabricator.com/D20700 --- ...ricatorProjectColumnBulkMoveController.php | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php index 5792664110..0931e4f7be 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnBulkMoveController.php @@ -219,12 +219,40 @@ final class PhabricatorProjectColumnBulkMoveController ->setValue($dst_project->getDisplayName())); } + $column_options = array( + 'visible' => array(), + 'hidden' => array(), + ); + + $any_hidden = false; + foreach ($dst_columns as $column) { + if (!$column->isHidden()) { + $group = 'visible'; + } else { + $group = 'hidden'; + } + + $phid = $column->getPHID(); + $display_name = $column->getDisplayName(); + + $column_options[$group][$phid] = $display_name; + } + + if ($column_options['hidden']) { + $column_options = array( + pht('Visible Columns') => $column_options['visible'], + pht('Hidden Columns') => $column_options['hidden'], + ); + } else { + $column_options = $column_options['visible']; + } + $form->appendControl( - id(new AphrontFormSelectControl()) - ->setName('dstColumnPHID') - ->setLabel(pht('Move to Column')) - ->setValue($dst_column_phid) - ->setOptions(mpull($dst_columns, 'getDisplayName', 'getPHID'))); + id(new AphrontFormSelectControl()) + ->setName('dstColumnPHID') + ->setLabel(pht('Move to Column')) + ->setValue($dst_column_phid) + ->setOptions($column_options)); $submit = pht('Move Tasks'); From 937edcdc580cb720c9690c7071edc73acc9cdc4d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 7 Aug 2019 09:04:39 -0700 Subject: [PATCH 12/15] Fix a warning in BoardLayoutEngine when no objects are being updated Summary: Fixes T13368. Some workflows (like "Move tasks to...") execute board layout without objects to update. In these cases, we can hit a warning because `objectPHIDs` is not initialized to `array()`. Test Plan: Went through the "Move tasks to..." workflow on a workboard, no longer saw a warning when trying to iterate over an empty `objectPHIDs` list. Maniphest Tasks: T13368 Differential Revision: https://secure.phabricator.com/D20701 --- .../project/engine/PhabricatorBoardLayoutEngine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/project/engine/PhabricatorBoardLayoutEngine.php b/src/applications/project/engine/PhabricatorBoardLayoutEngine.php index 83fb943b76..22aaed2d4b 100644 --- a/src/applications/project/engine/PhabricatorBoardLayoutEngine.php +++ b/src/applications/project/engine/PhabricatorBoardLayoutEngine.php @@ -4,7 +4,7 @@ final class PhabricatorBoardLayoutEngine extends Phobject { private $viewer; private $boardPHIDs; - private $objectPHIDs; + private $objectPHIDs = array(); private $boards; private $columnMap = array(); private $objectColumnMap = array(); From 46d9065bf148496777b73c8f97dee989c1774ab0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Aug 2019 10:13:41 -0700 Subject: [PATCH 13/15] Drop test for awardable badges on "Badges" tab of user profiles to avoid overheating Summary: Fixes T13370. We currently show an "Award Badge" button conditionally, based on whether the viewer can award any badges or not. The query to test this may overheat and this pattern isn't consistent with other UI anyway. Stop doing this test. Test Plan: - Created 12 badges. - As a user who could not edit any of the badges, viewed the "Badges" section of a user profile. Maniphest Tasks: T13370 Differential Revision: https://secure.phabricator.com/D20702 --- ...abricatorPeopleProfileBadgesController.php | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php index f98970ef73..e4861e488a 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileBadgesController.php @@ -34,20 +34,6 @@ final class PhabricatorPeopleProfileBadgesController $user, PhabricatorPeopleProfileMenuEngine::ITEM_BADGES); - // Best option? - $badges = id(new PhabricatorBadgesQuery()) - ->setViewer($viewer) - ->withStatuses(array( - PhabricatorBadgesBadge::STATUS_ACTIVE, - )) - ->requireCapabilities( - array( - PhabricatorPolicyCapability::CAN_VIEW, - PhabricatorPolicyCapability::CAN_EDIT, - )) - ->setLimit(1) - ->execute(); - $button = id(new PHUIButtonView()) ->setTag('a') ->setIcon('fa-plus') @@ -55,17 +41,16 @@ final class PhabricatorPeopleProfileBadgesController ->setWorkflow(true) ->setHref('/badges/award/'.$user->getID().'/'); - if ($badges) { - $header->addActionLink($button); - } + $header->addActionLink($button); $view = id(new PHUITwoColumnView()) ->setHeader($header) ->addClass('project-view-home') ->addClass('project-view-people-home') - ->setFooter(array( - $this->buildBadgesView($user) - )); + ->setFooter( + array( + $badges, + )); return $this->newPage() ->setTitle($title) From 9bd74dfa6c076958ae0d23675812ff93b4685cc9 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Aug 2019 10:26:46 -0700 Subject: [PATCH 14/15] Autofocus the "App Code" input on the TOTP prompt during MFA gates after login Summary: See downstream . The "autofocus" attribute mostly just works, so add it to this input. Test Plan: As a user with TOTP enabled, established a new session. Saw browser automatically focus the "App Code" input on the TOTP prompt screen. Differential Revision: https://secure.phabricator.com/D20703 --- src/applications/auth/factor/PhabricatorTOTPAuthFactor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php index 7e77dfc11a..ebdf1b7218 100644 --- a/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorTOTPAuthFactor.php @@ -194,6 +194,7 @@ final class PhabricatorTOTPAuthFactor extends PhabricatorAuthFactor { $control = id(new PHUIFormNumberControl()) ->setName($name) ->setDisableAutocomplete(true) + ->setAutofocus(true) ->setValue($value) ->setError($error); } From 0a3c26998fc985a0cc660fe6a8e92184c6b08f69 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 8 Aug 2019 10:35:04 -0700 Subject: [PATCH 15/15] When the feed query on project profile pages overheats, contain the damage Summary: Ref T13349. This is almost the same change as D20678, but for project profiles instead of user profiles. The general reproduction case is "view a project where you can't see more than 50 of the 500 most recent feed stories". Test Plan: - Forced all queries to overheat. - Viewed a project profile page. - Before: overheating fatal near top level. - After: damage contained to feed panel. Maniphest Tasks: T13349 Differential Revision: https://secure.phabricator.com/D20704 --- .../PhabricatorProjectProfileController.php | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/applications/project/controller/PhabricatorProjectProfileController.php b/src/applications/project/controller/PhabricatorProjectProfileController.php index 67b94f7fa9..386a649238 100644 --- a/src/applications/project/controller/PhabricatorProjectProfileController.php +++ b/src/applications/project/controller/PhabricatorProjectProfileController.php @@ -78,14 +78,29 @@ final class PhabricatorProjectProfileController $project, PhabricatorProject::ITEM_PROFILE); - $stories = id(new PhabricatorFeedQuery()) + $query = id(new PhabricatorFeedQuery()) ->setViewer($viewer) - ->withFilterPHIDs( - array( - $project->getPHID(), - )) + ->withFilterPHIDs(array($project->getPHID())) ->setLimit(50) - ->execute(); + ->setReturnPartialResultsOnOverheat(true); + + $stories = $query->execute(); + + $overheated_view = null; + $is_overheated = $query->getIsOverheated(); + if ($is_overheated) { + $overheated_message = + PhabricatorApplicationSearchController::newOverheatedError( + (bool)$stories); + + $overheated_view = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setTitle(pht('Query Overheated')) + ->setErrors( + array( + $overheated_message, + )); + } $view_all = id(new PHUIButtonView()) ->setTag('a') @@ -103,7 +118,11 @@ final class PhabricatorProjectProfileController $feed = id(new PHUIObjectBoxView()) ->setHeader($feed_header) ->addClass('project-view-feed') - ->appendChild($feed); + ->appendChild( + array( + $overheated_view, + $feed, + )); require_celerity_resource('project-view-css');