From 4caa684724f243d04cb1db12125456a689ee26ea Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 7 Feb 2012 14:59:38 -0800 Subject: [PATCH] Simplify Project status field Summary: This was a sort of speculative feature added by a contributor some time ago and just serves as a label; for now, simplify it into "active" and "archived" and remove "archived" projects from the "active" list. - Fix a bug where we'd publish a "renamed from X to X" transaction that had no effect. - Publish stories about status changes. - Remove the "edit affiliation" controller, which has no links in the UI (effectively replaced by join/leave links). - Add query/conduit support. Test Plan: Edited the status of several projects. Reviewers: btrahan Reviewed By: btrahan CC: aran, epriestley Maniphest Tasks: T681 Differential Revision: https://secure.phabricator.com/D1573 --- resources/sql/patches/104.projectstatuses.sql | 1 + src/__phutil_library_map__.php | 2 - ...AphrontDefaultApplicationConfiguration.php | 2 - .../query/ConduitAPI_project_query_Method.php | 15 +++ .../feed/story/base/PhabricatorFeedStory.php | 4 + .../project/PhabricatorFeedStoryProject.php | 19 +++- .../feed/story/project/__init__.php | 2 +- .../status/PhabricatorProjectStatus.php | 36 ++----- .../PhabricatorProjectTransactionType.php | 1 + .../PhabricatorProjectCreateController.php | 2 - ...icatorProjectAffiliationEditController.php | 95 ------------------- .../controller/editaffiliation/__init__.php | 22 ----- .../list/PhabricatorProjectListController.php | 9 ++ .../project/controller/list/__init__.php | 1 + ...habricatorProjectProfileEditController.php | 7 +- .../project/PhabricatorProjectEditor.php | 24 ++++- .../query/project/PhabricatorProjectQuery.php | 48 ++++++++++ .../project/query/project/__init__.php | 1 + .../storage/project/PhabricatorProject.php | 2 +- 19 files changed, 135 insertions(+), 158 deletions(-) create mode 100644 resources/sql/patches/104.projectstatuses.sql delete mode 100644 src/applications/project/controller/editaffiliation/PhabricatorProjectAffiliationEditController.php delete mode 100644 src/applications/project/controller/editaffiliation/__init__.php diff --git a/resources/sql/patches/104.projectstatuses.sql b/resources/sql/patches/104.projectstatuses.sql new file mode 100644 index 0000000000..ead6c4bacb --- /dev/null +++ b/resources/sql/patches/104.projectstatuses.sql @@ -0,0 +1 @@ +UPDATE phabricator_project.project SET status = IF(status = 5, 100, 0); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5e865c6787..f3038b30f3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -627,7 +627,6 @@ phutil_register_library_map(array( 'PhabricatorProfileHeaderView' => 'view/layout/profileheader', 'PhabricatorProject' => 'applications/project/storage/project', 'PhabricatorProjectAffiliation' => 'applications/project/storage/affiliation', - 'PhabricatorProjectAffiliationEditController' => 'applications/project/controller/editaffiliation', 'PhabricatorProjectConstants' => 'applications/project/constants/base', 'PhabricatorProjectController' => 'applications/project/controller/base', 'PhabricatorProjectCreateController' => 'applications/project/controller/create', @@ -1332,7 +1331,6 @@ phutil_register_library_map(array( 'PhabricatorProfileHeaderView' => 'AphrontView', 'PhabricatorProject' => 'PhabricatorProjectDAO', 'PhabricatorProjectAffiliation' => 'PhabricatorProjectDAO', - 'PhabricatorProjectAffiliationEditController' => 'PhabricatorProjectController', 'PhabricatorProjectController' => 'PhabricatorController', 'PhabricatorProjectCreateController' => 'PhabricatorProjectController', 'PhabricatorProjectDAO' => 'PhabricatorLiskDAO', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index c4154b4192..ca44bd3553 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -206,8 +206,6 @@ class AphrontDefaultApplicationConfiguration 'edit/(?P\d+)/$' => 'PhabricatorProjectProfileEditController', 'view/(?P\d+)/(?:(?P\w+)/)?$' => 'PhabricatorProjectProfileController', - 'affiliation/(?P\d+)/$' - => 'PhabricatorProjectAffiliationEditController', 'create/$' => 'PhabricatorProjectCreateController', 'update/(?P\d+)/(?P[^/]+)/$' => 'PhabricatorProjectUpdateController', diff --git a/src/applications/conduit/method/project/query/ConduitAPI_project_query_Method.php b/src/applications/conduit/method/project/query/ConduitAPI_project_query_Method.php index 7a1cce8ef4..28b312739e 100644 --- a/src/applications/conduit/method/project/query/ConduitAPI_project_query_Method.php +++ b/src/applications/conduit/method/project/query/ConduitAPI_project_query_Method.php @@ -26,9 +26,19 @@ final class ConduitAPI_project_query_Method extends ConduitAPI_project_Method { } public function defineParamTypes() { + + $statuses = array( + PhabricatorProjectQuery::STATUS_ANY, + PhabricatorProjectQuery::STATUS_OPEN, + PhabricatorProjectQuery::STATUS_CLOSED, + PhabricatorProjectQuery::STATUS_ACTIVE, + PhabricatorProjectQuery::STATUS_ARCHIVED, + ); + return array( 'ids' => 'optional list', 'phids' => 'optional list', + 'status' => 'optional enum<'.implode(', ', $statuses).'>', 'members' => 'optional list', @@ -55,6 +65,11 @@ final class ConduitAPI_project_query_Method extends ConduitAPI_project_Method { $query->withIDs($ids); } + $status = $request->getValue('status'); + if ($status) { + $query->withStatus($status); + } + $phids = $request->getValue('phids'); if ($phids) { $query->withPHIDs($phids); diff --git a/src/applications/feed/story/base/PhabricatorFeedStory.php b/src/applications/feed/story/base/PhabricatorFeedStory.php index b5b5d50e5c..5cfcd90b1b 100644 --- a/src/applications/feed/story/base/PhabricatorFeedStory.php +++ b/src/applications/feed/story/base/PhabricatorFeedStory.php @@ -106,6 +106,10 @@ abstract class PhabricatorFeedStory { phutil_escape_html($handle->getLinkName())); } + final protected function renderString($str) { + return ''.phutil_escape_html($str).''; + } + final protected function renderSummary($text, $len = 128) { if ($len) { $text = phutil_utf8_shorten($text, $len); diff --git a/src/applications/feed/story/project/PhabricatorFeedStoryProject.php b/src/applications/feed/story/project/PhabricatorFeedStoryProject.php index 82ee6718e0..89b0c413dc 100644 --- a/src/applications/feed/story/project/PhabricatorFeedStoryProject.php +++ b/src/applications/feed/story/project/PhabricatorFeedStoryProject.php @@ -49,16 +49,29 @@ class PhabricatorFeedStoryProject extends PhabricatorFeedStory { $action = 'renamed project '. $this->linkTo($proj_phid). ' from '. - ''.phutil_escape_html($old).''. + $this->renderString($old). ' to '. - ''.phutil_escape_html($new).'.'; + $this->renderString($new). + '.'; } else { $action = 'created project '. $this->linkTo($proj_phid). ' (as '. - ''.phutil_escape_html($new).').'; + $this->renderString($new). + ').'; } break; + case PhabricatorProjectTransactionType::TYPE_STATUS: + $action = 'changed project '. + $this->linkTo($proj_phid). + ' status from '. + $this->renderString( + PhabricatorProjectStatus::getNameForStatus($old)). + ' to '. + $this->renderString( + PhabricatorProjectStatus::getNameForStatus($new)). + '.'; + break; case PhabricatorProjectTransactionType::TYPE_MEMBERS: $add = array_diff($new, $old); $rem = array_diff($old, $new); diff --git a/src/applications/feed/story/project/__init__.php b/src/applications/feed/story/project/__init__.php index 8aceacee51..d4bdcf1327 100644 --- a/src/applications/feed/story/project/__init__.php +++ b/src/applications/feed/story/project/__init__.php @@ -8,9 +8,9 @@ phutil_require_module('phabricator', 'applications/feed/story/base'); phutil_require_module('phabricator', 'applications/feed/view/story'); +phutil_require_module('phabricator', 'applications/project/constants/status'); phutil_require_module('phabricator', 'applications/project/constants/transaction'); -phutil_require_module('phutil', 'markup'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/project/constants/status/PhabricatorProjectStatus.php b/src/applications/project/constants/status/PhabricatorProjectStatus.php index 1bce8fa0e5..51b3534c58 100644 --- a/src/applications/project/constants/status/PhabricatorProjectStatus.php +++ b/src/applications/project/constants/status/PhabricatorProjectStatus.php @@ -1,7 +1,7 @@ '', - self::NOT_STARTED => 'Not started', - self::IN_PROGRESS => 'In progress', - self::ONGOING => 'Ongoing', - self::REVIEW_PROCESS => 'Review process', - self::RELEASED => 'Released', - self::COMPLETED => 'Completed', - self::DEFERRED => 'Deferred', + self::STATUS_ACTIVE => 'Active', + self::STATUS_ARCHIVED => 'Archived', ); - return idx($map, coalesce($status, '?'), $map[self::UNKNOWN]); + return idx($map, coalesce($status, '?'), 'Unknown'); } public static function getStatusMap() { return array( - self::UNKNOWN => 'Who knows?', - self::NOT_STARTED => 'Not started', - self::IN_PROGRESS => 'In progress', - self::ONGOING => 'Ongoing', - self::REVIEW_PROCESS => 'Review process', - self::RELEASED => 'Released', - self::COMPLETED => 'Completed', - self::DEFERRED => 'Deferred', + self::STATUS_ACTIVE => 'Active', + self::STATUS_ARCHIVED => 'Archived', ); } + } diff --git a/src/applications/project/constants/transaction/PhabricatorProjectTransactionType.php b/src/applications/project/constants/transaction/PhabricatorProjectTransactionType.php index ecff24cd46..49ef0a9d81 100644 --- a/src/applications/project/constants/transaction/PhabricatorProjectTransactionType.php +++ b/src/applications/project/constants/transaction/PhabricatorProjectTransactionType.php @@ -21,5 +21,6 @@ final class PhabricatorProjectTransactionType const TYPE_NAME = 'name'; const TYPE_MEMBERS = 'members'; + const TYPE_STATUS = 'status'; } diff --git a/src/applications/project/controller/create/PhabricatorProjectCreateController.php b/src/applications/project/controller/create/PhabricatorProjectCreateController.php index 552a4197f1..9ff9c1214b 100644 --- a/src/applications/project/controller/create/PhabricatorProjectCreateController.php +++ b/src/applications/project/controller/create/PhabricatorProjectCreateController.php @@ -49,8 +49,6 @@ class PhabricatorProjectCreateController $errors[] = $ex->getMessage(); } - $project->setStatus(PhabricatorProjectStatus::ONGOING); - $profile->setBlurb($request->getStr('blurb')); if (!$errors) { diff --git a/src/applications/project/controller/editaffiliation/PhabricatorProjectAffiliationEditController.php b/src/applications/project/controller/editaffiliation/PhabricatorProjectAffiliationEditController.php deleted file mode 100644 index 59aa068c11..0000000000 --- a/src/applications/project/controller/editaffiliation/PhabricatorProjectAffiliationEditController.php +++ /dev/null @@ -1,95 +0,0 @@ -id = $data['id']; - } - - public function processRequest() { - - $request = $this->getRequest(); - $user = $request->getUser(); - - $project = id(new PhabricatorProject())->load($this->id); - if (!$project) { - return new Aphront404Response(); - } - - $affiliation = id(new PhabricatorProjectAffiliation())->loadOneWhere( - 'projectPHID = %s AND userPHID = %s', - $project->getPHID(), - $user->getPHID()); - - if (!$affiliation) { - $affiliation = new PhabricatorProjectAffiliation(); - $affiliation->setUserPHID($user->getPHID()); - $affiliation->setProjectPHID($project->getPHID()); - } - - if ($request->isFormPost()) { - $affiliation->setRole($request->getStr('role')); - - if (!strlen($affiliation->getRole())) { - if ($affiliation->getID()) { - if ($affiliation->getIsOwner()) { - $affiliation->setRole('Owner'); - $affiliation->save(); - } else { - $affiliation->delete(); - } - } - } else { - $affiliation->save(); - } - - return id(new AphrontRedirectResponse()) - ->setURI('/project/view/'.$project->getID().'/'); - } - - $form = new AphrontFormView(); - $form - ->setUser($user) - ->setAction('/project/affiliation/'.$project->getID().'/') - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel('Role') - ->setName('role') - ->setValue($affiliation->getRole())) - ->appendChild( - id(new AphrontFormSubmitControl()) - ->addCancelButton('/project/view/'.$project->getID().'/') - ->setValue('Save')); - - $panel = new AphrontPanelView(); - $panel->setHeader('Edit Project Affiliation'); - $panel->setWidth(AphrontPanelView::WIDTH_FORM); - $panel->appendChild($form); - - return $this->buildStandardPageResponse( - $panel, - array( - 'title' => 'Edit Project Affiliation', - )); - } - -} diff --git a/src/applications/project/controller/editaffiliation/__init__.php b/src/applications/project/controller/editaffiliation/__init__.php deleted file mode 100644 index a7f2f16f56..0000000000 --- a/src/applications/project/controller/editaffiliation/__init__.php +++ /dev/null @@ -1,22 +0,0 @@ -getUser()->getPHID(); + $status_filter = PhabricatorProjectQuery::STATUS_ANY; + switch ($this->filter) { case 'active': $table_header = 'Active Projects'; $query->setMembers(array($view_phid)); + $query->withStatus(PhabricatorProjectQuery::STATUS_ACTIVE); break; case 'owned': $table_header = 'Owned Projects'; $query->setOwners(array($view_phid)); + $query->withStatus($status_filter); break; case 'all': $table_header = 'All Projects'; + $query->withStatus($status_filter); break; } @@ -128,6 +133,8 @@ class PhabricatorProjectListController 'href' => '/project/view/'.$project->getID().'/', ), phutil_escape_html($project->getName())), + phutil_escape_html( + PhabricatorProjectStatus::getNameForStatus($project->getStatus())), phutil_escape_html($blurb), phutil_escape_html($population), phutil_render_tag( @@ -143,6 +150,7 @@ class PhabricatorProjectListController $table->setHeaders( array( 'Project', + 'Status', 'Description', 'Population', 'Open Tasks', @@ -150,6 +158,7 @@ class PhabricatorProjectListController $table->setColumnClasses( array( 'pri', + '', 'wide', '', '' diff --git a/src/applications/project/controller/list/__init__.php b/src/applications/project/controller/list/__init__.php index d5ad8656c5..25d24e5477 100644 --- a/src/applications/project/controller/list/__init__.php +++ b/src/applications/project/controller/list/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/maniphest/query'); +phutil_require_module('phabricator', 'applications/project/constants/status'); phutil_require_module('phabricator', 'applications/project/controller/base'); phutil_require_module('phabricator', 'applications/project/query/project'); phutil_require_module('phabricator', 'applications/project/storage/affiliation'); diff --git a/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php b/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php index 3f609692cb..42d71d4678 100644 --- a/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php +++ b/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php @@ -65,6 +65,12 @@ class PhabricatorProjectProfileEditController $xaction->setNewValue($request->getStr('name')); $xactions[] = $xaction; + $xaction = new PhabricatorProjectTransaction(); + $xaction->setTransactionType( + PhabricatorProjectTransactionType::TYPE_STATUS); + $xaction->setNewValue($request->getStr('status')); + $xactions[] = $xaction; + $editor = new PhabricatorProjectEditor($project); $editor->setUser($user); $editor->applyTransactions($xactions); @@ -73,7 +79,6 @@ class PhabricatorProjectProfileEditController $errors[] = $ex->getMessage(); } - $project->setStatus($request->getStr('status')); $project->setSubprojectPHIDs($request->getArr('set_subprojects')); $profile->setBlurb($request->getStr('blurb')); diff --git a/src/applications/project/editor/project/PhabricatorProjectEditor.php b/src/applications/project/editor/project/PhabricatorProjectEditor.php index c2452227a0..ab6e92a9d0 100644 --- a/src/applications/project/editor/project/PhabricatorProjectEditor.php +++ b/src/applications/project/editor/project/PhabricatorProjectEditor.php @@ -48,12 +48,21 @@ final class PhabricatorProjectEditor { $project->setAuthorPHID($user->getPHID()); } - foreach ($transactions as $xaction) { + foreach ($transactions as $key => $xaction) { $type = $xaction->getTransactionType(); $this->setTransactionOldValue($project, $xaction); - $this->applyTransactionEffect($project, $xaction); + if (!$this->transactionHasEffect($xaction)) { + unset($transactions[$key]); + continue; + } + + $this->applyTransactionEffect($project, $xaction); + } + + if (!$transactions) { + return $this; } try { @@ -128,6 +137,9 @@ final class PhabricatorProjectEditor { case PhabricatorProjectTransactionType::TYPE_NAME: $xaction->setOldValue($project->getName()); break; + case PhabricatorProjectTransactionType::TYPE_STATUS: + $xaction->setOldValue($project->getStatus()); + break; case PhabricatorProjectTransactionType::TYPE_MEMBERS: $affils = $project->loadAffiliations(); $project->attachAffiliations($affils); @@ -158,6 +170,9 @@ final class PhabricatorProjectEditor { $project->setPhrictionSlug($xaction->getNewValue()); $this->validateName($project); break; + case PhabricatorProjectTransactionType::TYPE_STATUS: + $project->setStatus($xaction->getNewValue()); + break; case PhabricatorProjectTransactionType::TYPE_MEMBERS: $old = array_fill_keys($xaction->getOldValue(), true); $new = array_fill_keys($xaction->getNewValue(), true); @@ -213,4 +228,9 @@ final class PhabricatorProjectEditor { ->publish(); } + private function transactionHasEffect( + PhabricatorProjectTransaction $xaction) { + return ($xaction->getOldValue() !== $xaction->getNewValue()); + } + } diff --git a/src/applications/project/query/project/PhabricatorProjectQuery.php b/src/applications/project/query/project/PhabricatorProjectQuery.php index ba085e7239..58464fdd42 100644 --- a/src/applications/project/query/project/PhabricatorProjectQuery.php +++ b/src/applications/project/query/project/PhabricatorProjectQuery.php @@ -23,6 +23,13 @@ final class PhabricatorProjectQuery { private $owners; private $members; + private $status = 'status-any'; + const STATUS_ANY = 'status-any'; + const STATUS_OPEN = 'status-open'; + const STATUS_CLOSED = 'status-closed'; + const STATUS_ACTIVE = 'status-active'; + const STATUS_ARCHIVED = 'status-archived'; + private $limit; private $offset; @@ -38,6 +45,11 @@ final class PhabricatorProjectQuery { return $this; } + public function withStatus($status) { + $this->status = $status; + return $this; + } + public function setLimit($limit) { $this->limit = $limit; return $this; @@ -113,6 +125,42 @@ final class PhabricatorProjectQuery { private function buildWhereClause($conn_r) { $where = array(); + if ($this->status != self::STATUS_ANY) { + switch ($this->status) { + case self::STATUS_OPEN: + $where[] = qsprintf( + $conn_r, + 'status IN (%Ld)', + array( + PhabricatorProjectStatus::STATUS_ACTIVE, + )); + break; + case self::STATUS_CLOSED: + $where[] = qsprintf( + $conn_r, + 'status IN (%Ld)', + array( + PhabricatorProjectStatus::STATUS_ARCHIVED, + )); + break; + case self::STATUS_ACTIVE: + $where[] = qsprintf( + $conn_r, + 'status = %d', + PhabricatorProjectStatus::STATUS_ACTIVE); + break; + case self::STATUS_ARCHIVED: + $where[] = qsprintf( + $conn_r, + 'status = %d', + PhabricatorProjectStatus::STATUS_ARCHIVED); + break; + default: + throw new Exception( + "Unknown project status '{$this->status}'!"); + } + } + if ($this->ids) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/project/query/project/__init__.php b/src/applications/project/query/project/__init__.php index 0fb198a377..3f6ca60f46 100644 --- a/src/applications/project/query/project/__init__.php +++ b/src/applications/project/query/project/__init__.php @@ -6,6 +6,7 @@ +phutil_require_module('phabricator', 'applications/project/constants/status'); phutil_require_module('phabricator', 'applications/project/storage/affiliation'); phutil_require_module('phabricator', 'applications/project/storage/project'); phutil_require_module('phabricator', 'storage/qsprintf'); diff --git a/src/applications/project/storage/project/PhabricatorProject.php b/src/applications/project/storage/project/PhabricatorProject.php index b3a1e56522..249b546a50 100644 --- a/src/applications/project/storage/project/PhabricatorProject.php +++ b/src/applications/project/storage/project/PhabricatorProject.php @@ -20,7 +20,7 @@ class PhabricatorProject extends PhabricatorProjectDAO { protected $name; protected $phid; - protected $status = PhabricatorProjectStatus::UNKNOWN; + protected $status = PhabricatorProjectStatus::STATUS_ACTIVE; protected $authorPHID; protected $subprojectPHIDs = array(); protected $phrictionSlug;