From 34b6c32d2eb5632220096ceae59dceb2b9ea731a Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 5 Feb 2013 11:58:19 -0800 Subject: [PATCH 01/19] fix conpherence menu selection Summary: i think the DOM changed in conpherence with the menu upgrades. just noticed that when you select a new conpherence the old one is not de-selected. this fixes it by updating the javascript to ascend one node higher and then use DOM.scry with the right data sigil to get the nodes Test Plan: read some messages, noted only the one I was reading had the entry highlighted in the left. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2399 Differential Revision: https://secure.phabricator.com/D4819 --- .../conpherence/controller/ConpherenceController.php | 1 + .../rsrc/js/application/conpherence/behavior-menu.js | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index 033e47f690..560f396225 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -206,6 +206,7 @@ abstract class ConpherenceController extends PhabricatorController { 'messages' => 'conpherence-messages', 'widgets_pane' => 'conpherence-widget-pane', 'form_pane' => 'conpherence-form', + 'menu_pane' => 'conpherence-menu', 'fancy_ajax' => (bool) $this->getSelectedConpherencePHID() ) ); diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 4ac216bdbf..64fcbd3130 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -19,14 +19,21 @@ JX.behavior('conpherence-menu', function(config) { var messagesRoot = JX.$(config.messages); var formRoot = JX.$(config.form_pane); var widgetsRoot = JX.$(config.widgets_pane); + var menuRoot = JX.$(config.menu_pane); JX.DOM.setContent(headerRoot, header); JX.DOM.setContent(messagesRoot, messages); messagesRoot.scrollTop = messagesRoot.scrollHeight; JX.DOM.setContent(formRoot, form); JX.DOM.setContent(widgetsRoot, widgets); - for (var i = 0; i < context.parentNode.childNodes.length; i++) { - var current = context.parentNode.childNodes[i]; + var conpherences = JX.DOM.scry( + menuRoot, + 'a', + 'conpherence-menu-click' + ); + + for (var i = 0; i < conpherences.length; i++) { + var current = conpherences[i]; if (current.id == context.id) { JX.DOM.alterClass(current, 'conpherence-selected', true); JX.DOM.alterClass(current, 'hide-unread-count', true); From 1d0058abcf243b00f3a814608f442d267c9ff0af Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 5 Feb 2013 13:46:02 -0800 Subject: [PATCH 02/19] Update PeopleMenu to only show integration with applications if they are installed Summary: do so via event engine. note different order now... Test Plan: toggled "show beta applications" to off and noted that Conpherence disappeared. Otherwise noted that links showed. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2424 Differential Revision: https://secure.phabricator.com/D4708 --- src/__phutil_library_map__.php | 10 ++ .../PhabricatorApplicationAudit.php | 6 ++ .../events/AuditPeopleMenuEventListener.php | 38 ++++++++ .../PhabricatorApplicationConpherence.php | 6 ++ .../ConpherencePeopleMenuEventListener.php | 38 ++++++++ .../PhabricatorApplicationDifferential.php | 6 ++ .../DifferentialPeopleMenuEventListener.php | 38 ++++++++ .../PhabricatorApplicationDiffusion.php | 6 ++ .../DiffusionPeopleMenuEventListener.php | 37 ++++++++ .../PhabricatorApplicationManiphest.php | 6 ++ .../ManiphestPeopleMenuEventListener.php | 36 +++++++ .../PhabricatorPeopleProfileController.php | 93 +++++++++++-------- .../events/constant/PhabricatorEventType.php | 1 + src/view/layout/PhabricatorMenuView.php | 10 +- 14 files changed, 287 insertions(+), 44 deletions(-) create mode 100644 src/applications/audit/events/AuditPeopleMenuEventListener.php create mode 100644 src/applications/conpherence/events/ConpherencePeopleMenuEventListener.php create mode 100644 src/applications/differential/events/DifferentialPeopleMenuEventListener.php create mode 100644 src/applications/diffusion/events/DiffusionPeopleMenuEventListener.php create mode 100644 src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index b40b32e2a1..9e5aa560b7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -90,6 +90,7 @@ phutil_register_library_map(array( 'AphrontUsageException' => 'aphront/exception/AphrontUsageException.php', 'AphrontView' => 'view/AphrontView.php', 'AphrontWebpageResponse' => 'aphront/response/AphrontWebpageResponse.php', + 'AuditPeopleMenuEventListener' => 'applications/audit/events/AuditPeopleMenuEventListener.php', 'CelerityAPI' => 'infrastructure/celerity/CelerityAPI.php', 'CelerityPhabricatorResourceController' => 'infrastructure/celerity/CelerityPhabricatorResourceController.php', 'CelerityResourceController' => 'infrastructure/celerity/CelerityResourceController.php', @@ -206,6 +207,7 @@ phutil_register_library_map(array( 'ConpherenceParticipant' => 'applications/conpherence/storage/ConpherenceParticipant.php', 'ConpherenceParticipantQuery' => 'applications/conpherence/query/ConpherenceParticipantQuery.php', 'ConpherenceParticipationStatus' => 'applications/conpherence/constants/ConpherenceParticipationStatus.php', + 'ConpherencePeopleMenuEventListener' => 'applications/conpherence/events/ConpherencePeopleMenuEventListener.php', 'ConpherenceReplyHandler' => 'applications/conpherence/mail/ConpherenceReplyHandler.php', 'ConpherenceThread' => 'applications/conpherence/storage/ConpherenceThread.php', 'ConpherenceThreadQuery' => 'applications/conpherence/query/ConpherenceThreadQuery.php', @@ -309,6 +311,7 @@ phutil_register_library_map(array( 'DifferentialNewDiffMail' => 'applications/differential/mail/DifferentialNewDiffMail.php', 'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php', 'DifferentialPathFieldSpecification' => 'applications/differential/field/specification/DifferentialPathFieldSpecification.php', + 'DifferentialPeopleMenuEventListener' => 'applications/differential/events/DifferentialPeopleMenuEventListener.php', 'DifferentialPrimaryPaneView' => 'applications/differential/view/DifferentialPrimaryPaneView.php', 'DifferentialReplyHandler' => 'applications/differential/DifferentialReplyHandler.php', 'DifferentialResultsTableView' => 'applications/differential/view/DifferentialResultsTableView.php', @@ -422,6 +425,7 @@ phutil_register_library_map(array( 'DiffusionPathQuery' => 'applications/diffusion/query/DiffusionPathQuery.php', 'DiffusionPathQueryTestCase' => 'applications/diffusion/query/pathid/__tests__/DiffusionPathQueryTestCase.php', 'DiffusionPathValidateController' => 'applications/diffusion/controller/DiffusionPathValidateController.php', + 'DiffusionPeopleMenuEventListener' => 'applications/diffusion/events/DiffusionPeopleMenuEventListener.php', 'DiffusionQuery' => 'applications/diffusion/query/DiffusionQuery.php', 'DiffusionRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionRawDiffQuery.php', 'DiffusionRenameHistoryQuery' => 'applications/diffusion/query/DiffusionRenameHistoryQuery.php', @@ -565,6 +569,7 @@ phutil_register_library_map(array( 'ManiphestDefaultTaskExtensions' => 'applications/maniphest/extensions/ManiphestDefaultTaskExtensions.php', 'ManiphestEdgeEventListener' => 'applications/maniphest/event/ManiphestEdgeEventListener.php', 'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php', + 'ManiphestPeopleMenuEventListener' => 'applications/maniphest/event/ManiphestPeopleMenuEventListener.php', 'ManiphestReplyHandler' => 'applications/maniphest/ManiphestReplyHandler.php', 'ManiphestReportController' => 'applications/maniphest/controller/ManiphestReportController.php', 'ManiphestSavedQuery' => 'applications/maniphest/storage/ManiphestSavedQuery.php', @@ -1576,6 +1581,7 @@ phutil_register_library_map(array( 'AphrontUsageException' => 'AphrontException', 'AphrontView' => 'Phobject', 'AphrontWebpageResponse' => 'AphrontHTMLResponse', + 'AuditPeopleMenuEventListener' => 'PhutilEventListener', 'CelerityPhabricatorResourceController' => 'CelerityResourceController', 'CelerityResourceController' => 'PhabricatorController', 'CelerityResourceGraph' => 'AbstractDirectedGraph', @@ -1682,6 +1688,7 @@ phutil_register_library_map(array( 'ConpherenceParticipant' => 'ConpherenceDAO', 'ConpherenceParticipantQuery' => 'PhabricatorOffsetPagedQuery', 'ConpherenceParticipationStatus' => 'ConpherenceConstants', + 'ConpherencePeopleMenuEventListener' => 'PhutilEventListener', 'ConpherenceReplyHandler' => 'PhabricatorMailReplyHandler', 'ConpherenceThread' => array( @@ -1781,6 +1788,7 @@ phutil_register_library_map(array( 'DifferentialNewDiffMail' => 'DifferentialReviewRequestMail', 'DifferentialParseRenderTestCase' => 'PhabricatorTestCase', 'DifferentialPathFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialPeopleMenuEventListener' => 'PhutilEventListener', 'DifferentialPrimaryPaneView' => 'AphrontView', 'DifferentialReplyHandler' => 'PhabricatorMailReplyHandler', 'DifferentialResultsTableView' => 'AphrontView', @@ -1876,6 +1884,7 @@ phutil_register_library_map(array( 'DiffusionPathCompleteController' => 'DiffusionController', 'DiffusionPathQueryTestCase' => 'PhabricatorTestCase', 'DiffusionPathValidateController' => 'DiffusionController', + 'DiffusionPeopleMenuEventListener' => 'PhutilEventListener', 'DiffusionRawDiffQuery' => 'DiffusionQuery', 'DiffusionRepositoryController' => 'DiffusionController', 'DiffusionSetupException' => 'AphrontUsageException', @@ -1988,6 +1997,7 @@ phutil_register_library_map(array( 'ManiphestDefaultTaskExtensions' => 'ManiphestTaskExtensions', 'ManiphestEdgeEventListener' => 'PhutilEventListener', 'ManiphestExportController' => 'ManiphestController', + 'ManiphestPeopleMenuEventListener' => 'PhutilEventListener', 'ManiphestReplyHandler' => 'PhabricatorMailReplyHandler', 'ManiphestReportController' => 'ManiphestController', 'ManiphestSavedQuery' => 'ManiphestDAO', diff --git a/src/applications/audit/application/PhabricatorApplicationAudit.php b/src/applications/audit/application/PhabricatorApplicationAudit.php index d24e3a71a0..73ad9a2be3 100644 --- a/src/applications/audit/application/PhabricatorApplicationAudit.php +++ b/src/applications/audit/application/PhabricatorApplicationAudit.php @@ -18,6 +18,12 @@ final class PhabricatorApplicationAudit extends PhabricatorApplication { return PhabricatorEnv::getDoclink('article/Audit_User_Guide.html'); } + public function getEventListeners() { + return array( + new AuditPeopleMenuEventListener() + ); + } + public function getRoutes() { return array( '/audit/' => array( diff --git a/src/applications/audit/events/AuditPeopleMenuEventListener.php b/src/applications/audit/events/AuditPeopleMenuEventListener.php new file mode 100644 index 0000000000..3ab4b7739f --- /dev/null +++ b/src/applications/audit/events/AuditPeopleMenuEventListener.php @@ -0,0 +1,38 @@ +listen(PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU: + $this->handleMenuEvent($event); + break; + } + } + + private function handleMenuEvent($event) { + $viewer = $event->getUser(); + $menu = $event->getValue('menu'); + $person = $event->getValue('person'); + $username = phutil_escape_uri($person->getUsername()); + + $href = '/audit/view/author/'.$username.'/'; + $name = pht('Commits'); + + $menu->addMenuItemToLabel('activity', + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setName($name) + ->setHref($href) + ->setKey($name) + ); + + $event->setValue('menu', $menu); + } + +} + diff --git a/src/applications/conpherence/application/PhabricatorApplicationConpherence.php b/src/applications/conpherence/application/PhabricatorApplicationConpherence.php index 71e7fb1b9f..9f78872453 100644 --- a/src/applications/conpherence/application/PhabricatorApplicationConpherence.php +++ b/src/applications/conpherence/application/PhabricatorApplicationConpherence.php @@ -33,6 +33,12 @@ final class PhabricatorApplicationConpherence extends PhabricatorApplication { return self::GROUP_COMMUNICATION; } + public function getEventListeners() { + return array( + new ConpherencePeopleMenuEventListener(), + ); + } + public function getRoutes() { return array( '/conpherence/' => array( diff --git a/src/applications/conpherence/events/ConpherencePeopleMenuEventListener.php b/src/applications/conpherence/events/ConpherencePeopleMenuEventListener.php new file mode 100644 index 0000000000..813763f9ae --- /dev/null +++ b/src/applications/conpherence/events/ConpherencePeopleMenuEventListener.php @@ -0,0 +1,38 @@ +listen(PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU: + $this->handleMenuEvent($event); + break; + } + } + + private function handleMenuEvent($event) { + $viewer = $event->getUser(); + $menu = $event->getValue('menu'); + $person = $event->getValue('person'); + + $conpherence_uri = + new PhutilURI('/conpherence/new/?participant='.$person->getPHID()); + $name = pht('Conpherence'); + + $menu->addMenuItemBefore('activity', + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setName($name) + ->setHref($conpherence_uri) + ->setKey($name) + ); + + $event->setValue('menu', $menu); + } + +} + diff --git a/src/applications/differential/application/PhabricatorApplicationDifferential.php b/src/applications/differential/application/PhabricatorApplicationDifferential.php index baa795589a..a198b98d59 100644 --- a/src/applications/differential/application/PhabricatorApplicationDifferential.php +++ b/src/applications/differential/application/PhabricatorApplicationDifferential.php @@ -28,6 +28,12 @@ final class PhabricatorApplicationDifferential extends PhabricatorApplication { return "\xE2\x9A\x99"; } + public function getEventListeners() { + return array( + new DifferentialPeopleMenuEventListener() + ); + } + public function getRoutes() { return array( '/D(?P[1-9]\d*)' => 'DifferentialRevisionViewController', diff --git a/src/applications/differential/events/DifferentialPeopleMenuEventListener.php b/src/applications/differential/events/DifferentialPeopleMenuEventListener.php new file mode 100644 index 0000000000..59163ecdf3 --- /dev/null +++ b/src/applications/differential/events/DifferentialPeopleMenuEventListener.php @@ -0,0 +1,38 @@ +listen(PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU: + $this->handleMenuEvent($event); + break; + } + } + + private function handleMenuEvent($event) { + $viewer = $event->getUser(); + $menu = $event->getValue('menu'); + $person = $event->getValue('person'); + $username = phutil_escape_uri($person->getUserName()); + + $href = '/differential/filter/revisions/'.$username.'/'; + $name = pht('Revisions'); + + $menu->addMenuItemToLabel('activity', + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setHref($href) + ->setName($name) + ->setKey($name) + ); + + $event->setValue('menu', $menu); + } + +} + diff --git a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php index 7eb4f27ccd..588c179d15 100644 --- a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php +++ b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php @@ -24,6 +24,12 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication { ); } + public function getEventListeners() { + return array( + new DiffusionPeopleMenuEventListener() + ); + } + public function getRoutes() { return array( '/r(?P[A-Z]+)(?P[a-z0-9]+)' diff --git a/src/applications/diffusion/events/DiffusionPeopleMenuEventListener.php b/src/applications/diffusion/events/DiffusionPeopleMenuEventListener.php new file mode 100644 index 0000000000..fc7e5a670d --- /dev/null +++ b/src/applications/diffusion/events/DiffusionPeopleMenuEventListener.php @@ -0,0 +1,37 @@ +listen(PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU: + $this->handleMenuEvent($event); + break; + } + } + + private function handleMenuEvent($event) { + $viewer = $event->getUser(); + $menu = $event->getValue('menu'); + $person_phid = $event->getValue('person')->getPHID(); + + $href = '/diffusion/lint/?owner[0]='.$person_phid; + $name = pht('Lint Messages'); + + $menu->addMenuItemToLabel('activity', + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setHref($href) + ->setName($name) + ->setKey($name) + ); + + $event->setValue('menu', $menu); + } + +} + diff --git a/src/applications/maniphest/application/PhabricatorApplicationManiphest.php b/src/applications/maniphest/application/PhabricatorApplicationManiphest.php index 787c5eb6f7..654d99f341 100644 --- a/src/applications/maniphest/application/PhabricatorApplicationManiphest.php +++ b/src/applications/maniphest/application/PhabricatorApplicationManiphest.php @@ -36,6 +36,12 @@ final class PhabricatorApplicationManiphest extends PhabricatorApplication { return $this->getBaseURI().'task/create/'; } + public function getEventListeners() { + return array( + new ManiphestPeopleMenuEventListener() + ); + } + public function getRoutes() { return array( '/T(?P[1-9]\d*)' => 'ManiphestTaskDetailController', diff --git a/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php b/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php new file mode 100644 index 0000000000..99f0f8283b --- /dev/null +++ b/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php @@ -0,0 +1,36 @@ +listen(PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU); + } + + public function handleEvent(PhutilEvent $event) { + switch ($event->getType()) { + case PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU: + $this->handleMenuEvent($event); + break; + } + } + + private function handleMenuEvent($event) { + $viewer = $event->getUser(); + $menu = $event->getValue('menu'); + $person_phid = $event->getValue('person')->getPHID(); + + $href = '/maniphest/view/action/?persons='.$person_phid; + $name = pht('Tasks'); + + $menu->addMenuItemToLabel('activity', + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setHref($href) + ->setName($name) + ->setKey($name) + ); + + $event->setValue('menu', $menu); + } + +} diff --git a/src/applications/people/controller/PhabricatorPeopleProfileController.php b/src/applications/people/controller/PhabricatorPeopleProfileController.php index 88a62a3dab..04cd8ec338 100644 --- a/src/applications/people/controller/PhabricatorPeopleProfileController.php +++ b/src/applications/people/controller/PhabricatorPeopleProfileController.php @@ -16,6 +16,21 @@ final class PhabricatorPeopleProfileController return $this->profileUser; } + private function getMainFilters($username) { + return array( + array( + 'key' => 'feed', + 'name' => pht('Feed'), + 'href' => '/p/'.$username.'/feed/' + ), + array( + 'key' => 'about', + 'name' => pht('About'), + 'href' => '/p/'.$username.'/about/' + ) + ); + } + public function processRequest() { $viewer = $this->getRequest()->getUser(); @@ -39,40 +54,14 @@ final class PhabricatorPeopleProfileController } $username = phutil_escape_uri($user->getUserName()); - $external_arrow = "\xE2\x86\x97"; + $menu = new PhabricatorMenuView(); + foreach ($this->getMainFilters($username) as $filter) { + $menu->newLink($filter['name'], $filter['href'], $filter['key']); + } - $conpherence_uri = - new PhutilURI('/conpherence/new/?participant='.$user->getPHID()); - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI('/p/'.$username.'/')); - $nav->addFilter('feed', 'Feed'); - $nav->addMenuItem( - id(new PhabricatorMenuItemView()) - ->setName(pht('Conpherence').' '.$external_arrow) - ->setHref($conpherence_uri) - ); - $nav->addFilter('about', 'About'); - $nav->addLabel('Activity'); - - $nav->addFilter( - null, - "Revisions {$external_arrow}", - '/differential/filter/revisions/'.$username.'/'); - - $nav->addFilter( - null, - "Tasks {$external_arrow}", - '/maniphest/view/action/?users='.$user->getPHID()); - - $nav->addFilter( - null, - "Commits {$external_arrow}", - '/audit/view/author/'.$username.'/'); - - $nav->addFilter( - null, - "Lint Messages {$external_arrow}", - '/diffusion/lint/?owner[0]='.$user->getPHID()); + $menu->newLabel(pht('Activity'), 'activity'); + // NOTE: applications install the various links through PhabricatorEvent + // listeners $oauths = id(new PhabricatorUserOAuthInfo())->loadAllWhere( 'userID = %d', @@ -92,18 +81,34 @@ final class PhabricatorPeopleProfileController continue; } - $name = $provider->getProviderName().' Profile'; + $name = pht('%s Profile', $provider->getProviderName()); $href = $oauths[$provider_key]->getAccountURI(); if ($href) { if (!$added_label) { - $nav->addLabel('Linked Accounts'); + $menu->newLabel(pht('Linked Accounts'), 'linked_accounts'); $added_label = true; } - $nav->addFilter(null, $name.' '.$external_arrow, $href); + $menu->addMenuItem( + id(new PhabricatorMenuItemView()) + ->setIsExternal(true) + ->setName($name) + ->setHref($href) + ->setType(PhabricatorMenuItemView::TYPE_LINK) + ); } } + $event = new PhabricatorEvent( + PhabricatorEventType::TYPE_PEOPLE_DIDRENDERMENU, + array( + 'menu' => $menu, + 'person' => $user, + )); + $event->setUser($viewer); + PhutilEventEngine::dispatchEvent($event); + $nav = AphrontSideNavFilterView::newFromMenu($event->getValue('menu')); + $this->page = $nav->selectFilter($this->page, 'feed'); switch ($this->page) { @@ -141,14 +146,19 @@ final class PhabricatorPeopleProfileController $header->appendChild($content); if ($user->getPHID() == $viewer->getPHID()) { - $nav->addFilter(null, 'Edit Profile...', '/settings/panel/profile/'); + $nav->addFilter( + null, + pht('Edit Profile...'), + '/settings/panel/profile/' + ); } if ($viewer->getIsAdmin()) { $nav->addFilter( null, - 'Administrate User...', - '/people/edit/'.$user->getID().'/'); + pht('Administrate User...'), + '/people/edit/'.$user->getID().'/' + ); } return $this->buildApplicationPage( @@ -162,7 +172,10 @@ final class PhabricatorPeopleProfileController $blurb = nonempty( $profile->getBlurb(), - '//Nothing is known about this rare specimen.//'); + '//'. + pht('Nothing is known about this rare specimen.') + .'//' + ); $engine = PhabricatorMarkupEngine::newProfileMarkupEngine(); $blurb = $engine->markupText($blurb); diff --git a/src/infrastructure/events/constant/PhabricatorEventType.php b/src/infrastructure/events/constant/PhabricatorEventType.php index 39772ea4e4..f6f91e9e82 100644 --- a/src/infrastructure/events/constant/PhabricatorEventType.php +++ b/src/infrastructure/events/constant/PhabricatorEventType.php @@ -30,4 +30,5 @@ final class PhabricatorEventType extends PhutilEventType { const TYPE_UI_DDIDRENDEROBJECT = 'ui.didRenderObject'; const TYPE_UI_DIDRENDEROBJECTS = 'ui.didRenderObjects'; + const TYPE_PEOPLE_DIDRENDERMENU = 'people.didRenderMenu'; } diff --git a/src/view/layout/PhabricatorMenuView.php b/src/view/layout/PhabricatorMenuView.php index bf1d601eb2..c430fe76fd 100644 --- a/src/view/layout/PhabricatorMenuView.php +++ b/src/view/layout/PhabricatorMenuView.php @@ -8,21 +8,23 @@ final class PhabricatorMenuView extends AphrontTagView { return false; } - public function newLabel($name) { + public function newLabel($name, $key = null) { $item = id(new PhabricatorMenuItemView()) ->setType(PhabricatorMenuItemView::TYPE_LABEL) - ->setName($name); + ->setName($name) + ->setKey($key); $this->addMenuItem($item); return $item; } - public function newLink($name, $href) { + public function newLink($name, $href, $key = null) { $item = id(new PhabricatorMenuItemView()) ->setType(PhabricatorMenuItemView::TYPE_LINK) ->setName($name) - ->setHref($href); + ->setHref($href) + ->setKey($key); $this->addMenuItem($item); From cb38ab27ce8a873e41954bd3bef6f57f44e4158c Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Tue, 5 Feb 2013 13:48:31 -0800 Subject: [PATCH 03/19] fix find / replace error from addressing feedback in D4708. now links to tasks work again. --- .../maniphest/event/ManiphestPeopleMenuEventListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php b/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php index 99f0f8283b..c7008e6ed9 100644 --- a/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php +++ b/src/applications/maniphest/event/ManiphestPeopleMenuEventListener.php @@ -19,7 +19,7 @@ final class ManiphestPeopleMenuEventListener extends PhutilEventListener { $menu = $event->getValue('menu'); $person_phid = $event->getValue('person')->getPHID(); - $href = '/maniphest/view/action/?persons='.$person_phid; + $href = '/maniphest/view/action/?users='.$person_phid; $name = pht('Tasks'); $menu->addMenuItemToLabel('activity', From 574bc3ba31cca2767bafe7844d7f854d90d6be1c Mon Sep 17 00:00:00 2001 From: indiefan Date: Tue, 5 Feb 2013 18:45:20 -0800 Subject: [PATCH 04/19] First pass at decoupling Phabricator bot behavior from the protocol it's running on, this pulls the connection, reading, and writing functionalities out of the bot itself and into the adapter. Summary: Ugh, just wrote out a huge message, only to lose it with a fat-fingered ctrl-c. Le sigh. First pass at decoupling the bot from the protocol. Noticeably absent is the command/message coupling. After this design pass I'll give that a go. Could use some advice, thinking that handlers should only create messages (which can be public or private) and not open ended, undefined 'commands'. The problem being that there needs to be some consistant api if we want handlers to be protocol agnostic. Perhaps that's a pipedream, what are your thoughts? Secondly, a few notes, design review requests on the changes i did make: # Config. For now i'm passing config through to the adapter. This was mainly to remain backwards compatible on the config. I was thinking it should probably be namespaced into it's own subobject though to distinguish the adapter config from the bot config. # Adapter selection. This flavor is the one-bot-daemon, config specified protocol version. The upside is that in the future they won't have to run different daemons for this stuff, just have different config, and the door is open for multiple protocol adapters down the road if need be. The downside is that I had to rename the daemon (non-backwards compatible change) and there will need to be some sort of runtime evaluation for instatiation of the adapter. For now I just have a crude switch, but I was thinking of just taking the string they supply as the class name (ala `try { new $clasName(); } catch...`) so as to allow for homegrown adapters, but I wasn't sure how such runtime magic would go over. Also, an alternative would be to make the PhabricatorBot class a non-abstract non-final base class and have the adapters be accompanied by a bot class that just defines their adapter as a property. The upside of which is backwards compatibility (welcome back PhabricatorIRCBot) and perhaps a little bit clearer plugin path for homegrowners. # Logging. You'll notice I commented out two very important logging lines in the irc adapter. This isn't intended to remain commented out, but I'm not sure what the best way is to get logging at this layer. I'm wary of just composing the daemon back down into the adapter (bi-directional object composition makes my skin crawl), but something needs to happen, obviously. Advice? That's it. After the feedback on the above, you can either merge down, or wait until i finish the command/message refactor if you don't think the diff will grow too large. Up to you, this all functions as is. Test Plan: Ran an irc bot, connected, read input, and wrote output including handler integration. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2462 Differential Revision: https://secure.phabricator.com/D4757 --- resources/ircbot/example_config.json | 12 +- src/__phutil_library_map__.php | 45 ++-- .../daemon/bot/PhabricatorBot.php | 132 +++++++++ .../PhabricatorBotMessage.php} | 2 +- .../daemon/bot/PhabricatorIRCBot.php | 11 + .../PhabricatorBaseProtocolAdapter.php | 37 +++ .../adapter/PhabricatorIRCProtocolAdapter.php | 156 +++++++++++ .../handler/PhabricatorBotDebugLogHandler.php | 17 ++ ...torBotDifferentialNotificationHandler.php} | 6 +- ...PhabricatorBotFeedNotificationHandler.php} | 6 +- .../handler/PhabricatorBotHandler.php} | 8 +- .../handler/PhabricatorBotLogHandler.php} | 4 +- .../handler/PhabricatorBotMacroHandler.php} | 4 +- .../PhabricatorBotObjectNameHandler.php} | 4 +- .../handler/PhabricatorBotSymbolHandler.php} | 4 +- .../PhabricatorBotWhatsNewHandler.php} | 4 +- .../handler/PhabricatorIRCProtocolHandler.php | 4 +- .../daemon/irc/PhabricatorIRCBot.php | 250 ------------------ 18 files changed, 408 insertions(+), 298 deletions(-) create mode 100644 src/infrastructure/daemon/bot/PhabricatorBot.php rename src/infrastructure/daemon/{irc/PhabricatorIRCMessage.php => bot/PhabricatorBotMessage.php} (97%) create mode 100644 src/infrastructure/daemon/bot/PhabricatorIRCBot.php create mode 100644 src/infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php create mode 100644 src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php create mode 100644 src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCDifferentialNotificationHandler.php => bot/handler/PhabricatorBotDifferentialNotificationHandler.php} (89%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCFeedNotificationHandler.php => bot/handler/PhabricatorBotFeedNotificationHandler.php} (96%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCHandler.php => bot/handler/PhabricatorBotHandler.php} (78%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCLogHandler.php => bot/handler/PhabricatorBotLogHandler.php} (92%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCMacroHandler.php => bot/handler/PhabricatorBotMacroHandler.php} (96%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCObjectNameHandler.php => bot/handler/PhabricatorBotObjectNameHandler.php} (97%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCSymbolHandler.php => bot/handler/PhabricatorBotSymbolHandler.php} (90%) rename src/infrastructure/daemon/{irc/handler/PhabricatorIRCWhatsNewHandler.php => bot/handler/PhabricatorBotWhatsNewHandler.php} (96%) rename src/infrastructure/daemon/{irc => bot}/handler/PhabricatorIRCProtocolHandler.php (85%) delete mode 100644 src/infrastructure/daemon/irc/PhabricatorIRCBot.php diff --git a/resources/ircbot/example_config.json b/resources/ircbot/example_config.json index 0e246ce536..6dadec2952 100644 --- a/resources/ircbot/example_config.json +++ b/resources/ircbot/example_config.json @@ -7,12 +7,12 @@ ], "handlers" : [ "PhabricatorIRCProtocolHandler", - "PhabricatorIRCObjectNameHandler", - "PhabricatorIRCSymbolHandler", - "PhabricatorIRCLogHandler", - "PhabricatorIRCWhatsNewHandler", - "PhabricatorIRCDifferentialNotificationHandler", - "PhabricatorIRCMacroHandler" + "PhabricatorBotObjectNameHandler", + "PhabricatorBotSymbolHandler", + "PhabricatorBotLogHandler", + "PhabricatorBotWhatsNewHandler", + "PhabricatorBotDifferentialNotificationHandler", + "PhabricatorBotMacroHandler" ], "conduit.uri" : null, diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9e5aa560b7..4f11a0fc87 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -702,6 +702,18 @@ phutil_register_library_map(array( 'PhabricatorBarePageExample' => 'applications/uiexample/examples/PhabricatorBarePageExample.php', 'PhabricatorBarePageView' => 'view/page/PhabricatorBarePageView.php', 'PhabricatorBaseEnglishTranslation' => 'infrastructure/internationalization/PhabricatorBaseEnglishTranslation.php', + 'PhabricatorBaseProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php', + 'PhabricatorBot' => 'infrastructure/daemon/bot/PhabricatorBot.php', + 'PhabricatorBotDebugLogHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php', + 'PhabricatorBotDifferentialNotificationHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php', + 'PhabricatorBotFeedNotificationHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php', + 'PhabricatorBotHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotHandler.php', + 'PhabricatorBotLogHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php', + 'PhabricatorBotMacroHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php', + 'PhabricatorBotMessage' => 'infrastructure/daemon/bot/PhabricatorBotMessage.php', + 'PhabricatorBotObjectNameHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php', + 'PhabricatorBotSymbolHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php', + 'PhabricatorBotWhatsNewHandler' => 'infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php', 'PhabricatorBuiltinPatchList' => 'infrastructure/storage/patch/PhabricatorBuiltinPatchList.php', 'PhabricatorButtonsExample' => 'applications/uiexample/examples/PhabricatorButtonsExample.php', 'PhabricatorCacheDAO' => 'applications/cache/storage/PhabricatorCacheDAO.php', @@ -917,17 +929,9 @@ phutil_register_library_map(array( 'PhabricatorHeaderView' => 'view/layout/PhabricatorHeaderView.php', 'PhabricatorHelpController' => 'applications/help/controller/PhabricatorHelpController.php', 'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/PhabricatorHelpKeyboardShortcutController.php', - 'PhabricatorIRCBot' => 'infrastructure/daemon/irc/PhabricatorIRCBot.php', - 'PhabricatorIRCDifferentialNotificationHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCDifferentialNotificationHandler.php', - 'PhabricatorIRCFeedNotificationHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCFeedNotificationHandler.php', - 'PhabricatorIRCHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCHandler.php', - 'PhabricatorIRCLogHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCLogHandler.php', - 'PhabricatorIRCMacroHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php', - 'PhabricatorIRCMessage' => 'infrastructure/daemon/irc/PhabricatorIRCMessage.php', - 'PhabricatorIRCObjectNameHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCObjectNameHandler.php', - 'PhabricatorIRCProtocolHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCProtocolHandler.php', - 'PhabricatorIRCSymbolHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCSymbolHandler.php', - 'PhabricatorIRCWhatsNewHandler' => 'infrastructure/daemon/irc/handler/PhabricatorIRCWhatsNewHandler.php', + 'PhabricatorIRCBot' => 'infrastructure/daemon/bot/PhabricatorIRCBot.php', + 'PhabricatorIRCProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php', + 'PhabricatorIRCProtocolHandler' => 'infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php', 'PhabricatorImageTransformer' => 'applications/files/PhabricatorImageTransformer.php', 'PhabricatorInfrastructureTestCase' => 'infrastructure/__tests__/PhabricatorInfrastructureTestCase.php', 'PhabricatorInlineCommentController' => 'infrastructure/diff/PhabricatorInlineCommentController.php', @@ -2144,6 +2148,15 @@ phutil_register_library_map(array( 'PhabricatorBarePageExample' => 'PhabricatorUIExample', 'PhabricatorBarePageView' => 'AphrontPageView', 'PhabricatorBaseEnglishTranslation' => 'PhabricatorTranslation', + 'PhabricatorBot' => 'PhabricatorDaemon', + 'PhabricatorBotDebugLogHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotDifferentialNotificationHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotFeedNotificationHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotLogHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotMacroHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotObjectNameHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotSymbolHandler' => 'PhabricatorBotHandler', + 'PhabricatorBotWhatsNewHandler' => 'PhabricatorBotHandler', 'PhabricatorBuiltinPatchList' => 'PhabricatorSQLPatchList', 'PhabricatorButtonsExample' => 'PhabricatorUIExample', 'PhabricatorCacheDAO' => 'PhabricatorLiskDAO', @@ -2353,14 +2366,8 @@ phutil_register_library_map(array( 'PhabricatorHelpController' => 'PhabricatorController', 'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController', 'PhabricatorIRCBot' => 'PhabricatorDaemon', - 'PhabricatorIRCDifferentialNotificationHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCFeedNotificationHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCLogHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCMacroHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCObjectNameHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCProtocolHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCSymbolHandler' => 'PhabricatorIRCHandler', - 'PhabricatorIRCWhatsNewHandler' => 'PhabricatorIRCHandler', + 'PhabricatorIRCProtocolAdapter' => 'PhabricatorBaseProtocolAdapter', + 'PhabricatorIRCProtocolHandler' => 'PhabricatorBotHandler', 'PhabricatorInfrastructureTestCase' => 'PhabricatorTestCase', 'PhabricatorInlineCommentController' => 'PhabricatorController', 'PhabricatorInlineCommentInterface' => 'PhabricatorMarkupInterface', diff --git a/src/infrastructure/daemon/bot/PhabricatorBot.php b/src/infrastructure/daemon/bot/PhabricatorBot.php new file mode 100644 index 0000000000..7f83307c64 --- /dev/null +++ b/src/infrastructure/daemon/bot/PhabricatorBot.php @@ -0,0 +1,132 @@ +getArgv(); + if (count($argv) !== 1) { + throw new Exception("usage: PhabricatorBot "); + } + + $json_raw = Filesystem::readFile($argv[0]); + $config = json_decode($json_raw, true); + if (!is_array($config)) { + throw new Exception("File '{$argv[0]}' is not valid JSON!"); + } + + $nick = idx($config, 'nick', 'phabot'); + $handlers = idx($config, 'handlers', array()); + $protocol_adapter_class = idx( + $config, + 'protocol-adapter', + 'PhabricatorIRCProtocolAdapter'); + $this->pollFrequency = idx($config, 'poll-frequency', 1); + + $this->config = $config; + + foreach ($handlers as $handler) { + $obj = newv($handler, array($this)); + $this->handlers[] = $obj; + } + + $conduit_uri = idx($config, 'conduit.uri'); + if ($conduit_uri) { + $conduit_user = idx($config, 'conduit.user'); + $conduit_cert = idx($config, 'conduit.cert'); + + // Normalize the path component of the URI so users can enter the + // domain without the "/api/" part. + $conduit_uri = new PhutilURI($conduit_uri); + $conduit_uri->setPath('/api/'); + $conduit_uri = (string)$conduit_uri; + + $conduit = new ConduitClient($conduit_uri); + $response = $conduit->callMethodSynchronous( + 'conduit.connect', + array( + 'client' => 'PhabricatorBot', + 'clientVersion' => '1.0', + 'clientDescription' => php_uname('n').':'.$nick, + 'user' => $conduit_user, + 'certificate' => $conduit_cert, + )); + + $this->conduit = $conduit; + } + + // Instantiate Protocol Adapter, for now follow same technique as + // handler instantiation + $this->protocolAdapter = newv($protocol_adapter_class, array()); + $this->protocolAdapter + ->setConfig($this->config) + ->connect(); + + $this->runLoop(); + } + + public function getConfig($key, $default = null) { + return idx($this->config, $key, $default); + } + + private function runLoop() { + do { + $this->stillWorking(); + + $messages = $this->protocolAdapter->getNextMessages($this->pollFrequency); + if (count($messages) > 0) { + foreach ($messages as $message) { + $this->routeMessage($message); + } + } + + foreach ($this->handlers as $handler) { + $handler->runBackgroundTasks(); + } + } while (true); + } + + public function writeCommand($command, $message) { + return $this->protocolAdapter->writeCommand($command, $message); + } + + private function routeMessage(PhabricatorBotMessage $message) { + $ignore = $this->getConfig('ignore'); + if ($ignore && in_array($message->getSenderNickName(), $ignore)) { + return; + } + + foreach ($this->handlers as $handler) { + try { + $handler->receiveMessage($message); + } catch (Exception $ex) { + phlog($ex); + } + } + } + + public function getConduit() { + if (empty($this->conduit)) { + throw new Exception( + "This bot is not configured with a Conduit uplink. Set 'conduit.uri', ". + "'conduit.user' and 'conduit.cert' in the configuration to connect."); + } + return $this->conduit; + } + +} diff --git a/src/infrastructure/daemon/irc/PhabricatorIRCMessage.php b/src/infrastructure/daemon/bot/PhabricatorBotMessage.php similarity index 97% rename from src/infrastructure/daemon/irc/PhabricatorIRCMessage.php rename to src/infrastructure/daemon/bot/PhabricatorBotMessage.php index 8d3bb14153..95e7f42bc0 100644 --- a/src/infrastructure/daemon/irc/PhabricatorIRCMessage.php +++ b/src/infrastructure/daemon/bot/PhabricatorBotMessage.php @@ -1,6 +1,6 @@ config = $config; + return $this; + } + + /** + * Performs any connection logic necessary for the protocol + */ + abstract public function connect(); + + /** + * This is the spout for messages coming in from the protocol. + * This will be called in the main event loop of the bot daemon + * So if if doesn't implement some sort of blocking timeout + * (e.g. select-based socket polling), it should at least sleep + * for some period of time in order to not overwhelm the processor. + * + * @param Int $poll_frequency The number of seconds between polls + */ + abstract public function getNextMessages($poll_frequency); + + /** + * This is the output mechanism for the protocol. + * + * @param String $command The command for the message + * @param String $message The contents of the message itself + */ + abstract public function writeCommand($command, $message); +} diff --git a/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php b/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php new file mode 100644 index 0000000000..ccd96fc0ea --- /dev/null +++ b/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php @@ -0,0 +1,156 @@ +config, 'nick', 'phabot'); + $server = idx($this->config, 'server'); + $port = idx($this->config, 'port', 6667); + $pass = idx($this->config, 'pass'); + $ssl = idx($this->config, 'ssl', false); + $user = idx($this->config, 'user', $nick); + + if (!preg_match('/^[A-Za-z0-9_`[{}^|\]\\-]+$/', $nick)) { + throw new Exception( + "Nickname '{$nick}' is invalid!"); + } + + $errno = null; + $error = null; + if (!$ssl) { + $socket = fsockopen($server, $port, $errno, $error); + } else { + $socket = fsockopen('ssl://'.$server, $port, $errno, $error); + } + if (!$socket) { + throw new Exception("Failed to connect, #{$errno}: {$error}"); + } + $ok = stream_set_blocking($socket, false); + if (!$ok) { + throw new Exception("Failed to set stream nonblocking."); + } + + $this->socket = $socket; + $this->writeCommand('USER', "{$user} 0 * :{$user}"); + if ($pass) { + $this->writeCommand('PASS', "{$pass}"); + } + + $this->writeCommand('NICK', "{$nick}"); + } + + public function getNextMessages($poll_frequency) { + $messages = array(); + + $read = array($this->socket); + if (strlen($this->writeBuffer)) { + $write = array($this->socket); + } else { + $write = array(); + } + $except = array(); + + $ok = @stream_select($read, $write, $except, $timeout_sec = 1); + if ($ok === false) { + throw new Exception( + "socket_select() failed: ".socket_strerror(socket_last_error())); + } + + if ($read) { + // Test for connection termination; in PHP, fread() off a nonblocking, + // closed socket is empty string. + if (feof($this->socket)) { + // This indicates the connection was terminated on the other side, + // just exit via exception and let the overseer restart us after a + // delay so we can reconnect. + throw new Exception("Remote host closed connection."); + } + do { + $data = fread($this->socket, 4096); + if ($data === false) { + throw new Exception("fread() failed!"); + } else { + $messages[] = new PhabricatorBotMessage( + null, + "LOG", + "<<< ".$data + ); + + $this->readBuffer .= $data; + } + } while (strlen($data)); + } + + if ($write) { + do { + $len = fwrite($this->socket, $this->writeBuffer); + if ($len === false) { + throw new Exception("fwrite() failed!"); + } else { + $messages[] = new PhabricatorBotMessage( + null, + "LOG", + ">>> ".substr($this->writeBuffer, 0, $len)); + $this->writeBuffer = substr($this->writeBuffer, $len); + } + } while (strlen($this->writeBuffer)); + } + + while ($m = $this->processReadBuffer()) { + $messages[] = $m; + } + + return $messages; + } + + private function write($message) { + $this->writeBuffer .= $message; + return $this; + } + + public function writeCommand($command, $message) { + return $this->write($command.' '.$message."\r\n"); + } + + private function processReadBuffer() { + $until = strpos($this->readBuffer, "\r\n"); + if ($until === false) { + return false; + } + + $message = substr($this->readBuffer, 0, $until); + $this->readBuffer = substr($this->readBuffer, $until + 2); + + $pattern = + '/^'. + '(?:(?P:(\S+)) )?'. // This may not be present. + '(?P[A-Z0-9]+) '. + '(?P.*)'. + '$/'; + + $matches = null; + if (!preg_match($pattern, $message, $matches)) { + throw new Exception("Unexpected message from server: {$message}"); + } + + $irc_message = new PhabricatorBotMessage( + idx($matches, 'sender'), + $matches['command'], + $matches['data']); + + return $irc_message; + } + + public function __destruct() { + $this->write("QUIT Goodbye.\r\n"); + fclose($this->socket); + } + +} diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php new file mode 100644 index 0000000000..19ff95c562 --- /dev/null +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php @@ -0,0 +1,17 @@ +getCommand()) { + case 'LOG': + echo addcslashes( + $message->getRawData(), + "\0..\37\177..\377"); + echo "\n"; + break; + } + } +} diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCDifferentialNotificationHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php similarity index 89% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCDifferentialNotificationHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php index c0a1f9fc88..1f801d472e 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCDifferentialNotificationHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php @@ -3,12 +3,12 @@ /** * @group irc */ -final class PhabricatorIRCDifferentialNotificationHandler - extends PhabricatorIRCHandler { +final class PhabricatorBotDifferentialNotificationHandler + extends PhabricatorBotHandler { private $skippedOldEvents; - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { return; } diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCFeedNotificationHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php similarity index 96% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCFeedNotificationHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php index 6f45ed936f..bd8b7e5375 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCFeedNotificationHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php @@ -5,8 +5,8 @@ * * @group irc */ -final class PhabricatorIRCFeedNotificationHandler - extends PhabricatorIRCHandler { +final class PhabricatorBotFeedNotificationHandler + extends PhabricatorBotHandler { private $startupDelay = 30; private $lastSeenChronoKey = 0; @@ -82,7 +82,7 @@ final class PhabricatorIRCFeedNotificationHandler return false; } - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { return; } diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php similarity index 78% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php index 8f059bed67..ec1dbc117b 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php @@ -2,15 +2,15 @@ /** * Responds to IRC messages. You plug a bunch of these into a - * @{class:PhabricatorIRCBot} to give it special behavior. + * @{class:PhabricatorBot} to give it special behavior. * * @group irc */ -abstract class PhabricatorIRCHandler { +abstract class PhabricatorBotHandler { private $bot; - final public function __construct(PhabricatorIRCBot $irc_bot) { + final public function __construct(PhabricatorBot $irc_bot) { $this->bot = $irc_bot; } @@ -37,7 +37,7 @@ abstract class PhabricatorIRCHandler { return (strncmp($name, '#', 1) === 0); } - abstract public function receiveMessage(PhabricatorIRCMessage $message); + abstract public function receiveMessage(PhabricatorBotMessage $message); public function runBackgroundTasks() { return; diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCLogHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php similarity index 92% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCLogHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php index 5576aa8bb3..3e1bd620a8 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCLogHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php @@ -5,11 +5,11 @@ * * @group irc */ -final class PhabricatorIRCLogHandler extends PhabricatorIRCHandler { +final class PhabricatorBotLogHandler extends PhabricatorBotHandler { private $futures = array(); - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { case 'PRIVMSG': diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php similarity index 96% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php index 078d77ac31..59e698a04d 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCMacroHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php @@ -3,7 +3,7 @@ /** * @group irc */ -final class PhabricatorIRCMacroHandler extends PhabricatorIRCHandler { +final class PhabricatorBotMacroHandler extends PhabricatorBotHandler { private $macros; private $regexp; @@ -40,7 +40,7 @@ final class PhabricatorIRCMacroHandler extends PhabricatorIRCHandler { return true; } - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { if (!$this->init()) { return; } diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCObjectNameHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php similarity index 97% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCObjectNameHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php index ba551d1d33..7832880bb7 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCObjectNameHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php @@ -5,7 +5,7 @@ * * @group irc */ -final class PhabricatorIRCObjectNameHandler extends PhabricatorIRCHandler { +final class PhabricatorBotObjectNameHandler extends PhabricatorBotHandler { /** * Map of PHIDs to the last mention of them (as an epoch timestamp); prevents @@ -13,7 +13,7 @@ final class PhabricatorIRCObjectNameHandler extends PhabricatorIRCHandler { */ private $recentlyMentioned = array(); - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { case 'PRIVMSG': diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCSymbolHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php similarity index 90% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCSymbolHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php index 8b4343d3ed..2e57de3418 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCSymbolHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php @@ -5,9 +5,9 @@ * * @group irc */ -final class PhabricatorIRCSymbolHandler extends PhabricatorIRCHandler { +final class PhabricatorBotSymbolHandler extends PhabricatorBotHandler { - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { case 'PRIVMSG': diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCWhatsNewHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php similarity index 96% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCWhatsNewHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php index d921dca89c..1bd53ef194 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCWhatsNewHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php @@ -5,11 +5,11 @@ * * @group irc */ -final class PhabricatorIRCWhatsNewHandler extends PhabricatorIRCHandler { +final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler { private $floodblock = 0; - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { case 'PRIVMSG': diff --git a/src/infrastructure/daemon/irc/handler/PhabricatorIRCProtocolHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php similarity index 85% rename from src/infrastructure/daemon/irc/handler/PhabricatorIRCProtocolHandler.php rename to src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php index 1347d2bbdc..80b76077b1 100644 --- a/src/infrastructure/daemon/irc/handler/PhabricatorIRCProtocolHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php @@ -5,9 +5,9 @@ * * @group irc */ -final class PhabricatorIRCProtocolHandler extends PhabricatorIRCHandler { +final class PhabricatorIRCProtocolHandler extends PhabricatorBotHandler { - public function receiveMessage(PhabricatorIRCMessage $message) { + public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { case '422': // Error - no MOTD case '376': // End of MOTD diff --git a/src/infrastructure/daemon/irc/PhabricatorIRCBot.php b/src/infrastructure/daemon/irc/PhabricatorIRCBot.php deleted file mode 100644 index a75352d589..0000000000 --- a/src/infrastructure/daemon/irc/PhabricatorIRCBot.php +++ /dev/null @@ -1,250 +0,0 @@ -getArgv(); - if (count($argv) !== 1) { - throw new Exception("usage: PhabricatorIRCBot "); - } - - $json_raw = Filesystem::readFile($argv[0]); - $config = json_decode($json_raw, true); - if (!is_array($config)) { - throw new Exception("File '{$argv[0]}' is not valid JSON!"); - } - - $server = idx($config, 'server'); - $port = idx($config, 'port', 6667); - $handlers = idx($config, 'handlers', array()); - $pass = idx($config, 'pass'); - $nick = idx($config, 'nick', 'phabot'); - $user = idx($config, 'user', $nick); - $ssl = idx($config, 'ssl', false); - $nickpass = idx($config, 'nickpass'); - - $this->config = $config; - - if (!preg_match('/^[A-Za-z0-9_`[{}^|\]\\-]+$/', $nick)) { - throw new Exception( - "Nickname '{$nick}' is invalid!"); - } - - foreach ($handlers as $handler) { - $obj = newv($handler, array($this)); - $this->handlers[] = $obj; - } - - $conduit_uri = idx($config, 'conduit.uri'); - if ($conduit_uri) { - $conduit_user = idx($config, 'conduit.user'); - $conduit_cert = idx($config, 'conduit.cert'); - - // Normalize the path component of the URI so users can enter the - // domain without the "/api/" part. - $conduit_uri = new PhutilURI($conduit_uri); - $conduit_uri->setPath('/api/'); - $conduit_uri = (string)$conduit_uri; - - $conduit = new ConduitClient($conduit_uri); - $response = $conduit->callMethodSynchronous( - 'conduit.connect', - array( - 'client' => 'PhabricatorIRCBot', - 'clientVersion' => '1.0', - 'clientDescription' => php_uname('n').':'.$nick, - 'user' => $conduit_user, - 'certificate' => $conduit_cert, - )); - - $this->conduit = $conduit; - } - - $errno = null; - $error = null; - if (!$ssl) { - $socket = fsockopen($server, $port, $errno, $error); - } else { - $socket = fsockopen('ssl://'.$server, $port, $errno, $error); - } - if (!$socket) { - throw new Exception("Failed to connect, #{$errno}: {$error}"); - } - $ok = stream_set_blocking($socket, false); - if (!$ok) { - throw new Exception("Failed to set stream nonblocking."); - } - - $this->socket = $socket; - $this->writeCommand('USER', "{$user} 0 * :{$user}"); - if ($pass) { - $this->writeCommand('PASS', "{$pass}"); - } - - $this->writeCommand('NICK', "{$nick}"); - $this->runSelectLoop(); - } - - public function getConfig($key, $default = null) { - return idx($this->config, $key, $default); - } - - private function runSelectLoop() { - do { - $this->stillWorking(); - - $read = array($this->socket); - if (strlen($this->writeBuffer)) { - $write = array($this->socket); - } else { - $write = array(); - } - $except = array(); - - $ok = @stream_select($read, $write, $except, $timeout_sec = 1); - if ($ok === false) { - throw new Exception( - "socket_select() failed: ".socket_strerror(socket_last_error())); - } - - if ($read) { - // Test for connection termination; in PHP, fread() off a nonblocking, - // closed socket is empty string. - if (feof($this->socket)) { - // This indicates the connection was terminated on the other side, - // just exit via exception and let the overseer restart us after a - // delay so we can reconnect. - throw new Exception("Remote host closed connection."); - } - do { - $data = fread($this->socket, 4096); - if ($data === false) { - throw new Exception("fread() failed!"); - } else { - $this->debugLog(true, $data); - $this->readBuffer .= $data; - } - } while (strlen($data)); - } - - if ($write) { - do { - $len = fwrite($this->socket, $this->writeBuffer); - if ($len === false) { - throw new Exception("fwrite() failed!"); - } else { - $this->debugLog(false, substr($this->writeBuffer, 0, $len)); - $this->writeBuffer = substr($this->writeBuffer, $len); - } - } while (strlen($this->writeBuffer)); - } - - do { - $routed_message = $this->processReadBuffer(); - } while ($routed_message); - - foreach ($this->handlers as $handler) { - $handler->runBackgroundTasks(); - } - - } while (true); - } - - private function write($message) { - $this->writeBuffer .= $message; - return $this; - } - - public function writeCommand($command, $message) { - return $this->write($command.' '.$message."\r\n"); - } - - private function processReadBuffer() { - $until = strpos($this->readBuffer, "\r\n"); - if ($until === false) { - return false; - } - - $message = substr($this->readBuffer, 0, $until); - $this->readBuffer = substr($this->readBuffer, $until + 2); - - $pattern = - '/^'. - '(?:(?P:(\S+)) )?'. // This may not be present. - '(?P[A-Z0-9]+) '. - '(?P.*)'. - '$/'; - - $matches = null; - if (!preg_match($pattern, $message, $matches)) { - throw new Exception("Unexpected message from server: {$message}"); - } - - $irc_message = new PhabricatorIRCMessage( - idx($matches, 'sender'), - $matches['command'], - $matches['data']); - - $this->routeMessage($irc_message); - - return true; - } - - private function routeMessage(PhabricatorIRCMessage $message) { - $ignore = $this->getConfig('ignore'); - if ($ignore && in_array($message->getSenderNickName(), $ignore)) { - return; - } - - foreach ($this->handlers as $handler) { - try { - $handler->receiveMessage($message); - } catch (Exception $ex) { - phlog($ex); - } - } - } - - public function __destruct() { - $this->write("QUIT Goodbye.\r\n"); - fclose($this->socket); - } - - private function debugLog($is_read, $message) { - if ($this->getTraceMode()) { - echo $is_read ? '<<< ' : '>>> '; - echo addcslashes($message, "\0..\37\177..\377"); - echo "\n"; - } - } - - public function getConduit() { - if (empty($this->conduit)) { - throw new Exception( - "This bot is not configured with a Conduit uplink. Set 'conduit.uri', ". - "'conduit.user' and 'conduit.cert' in the configuration to connect."); - } - return $this->conduit; - } - -} From 24ced7e7bd7902584e1da4786e10cada98e66ade Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 5 Feb 2013 20:10:57 -0800 Subject: [PATCH 05/19] Expose commit information via conduit instead of user information Summary: After D4825, this information is often available to us in a safe way. Provide it explictly. This removes or reduces functionality in some cases, but I think we can plug those holes with Conpherence addresses and/or explicit user acknowledgement/config. Test Plan: Patched a commit with `arc patch` and got the original address out. Reviewers: btrahan, edward, vrana Reviewed By: btrahan CC: aran Differential Revision: https://secure.phabricator.com/D4828 --- conf/default.conf.php | 11 ---- ...ConduitAPI_differential_getdiff_Method.php | 1 - .../PhabricatorDifferentialConfigOptions.php | 22 -------- .../differential/storage/DifferentialDiff.php | 54 +++---------------- 4 files changed, 8 insertions(+), 80 deletions(-) diff --git a/conf/default.conf.php b/conf/default.conf.php index 26f76dd3ce..b22e053827 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -1015,17 +1015,6 @@ return array( // interact with the revisions. 'differential.anonymous-access' => false, - // If you set this to true, revision author email address information will - // be exposed in Conduit. This is useful for Arcanist. - // - // For example, consider the "arc patch DX" workflow which needs to ask - // Differential for the revision DX. This data often should contain - // the author's email address, eg "George Washington - // " when DX is a git or mercurial revision. If this - // option is false, Differential defaults to the best it can, something like - // "George Washington" or "gwashington". - 'differential.expose-emails-prudently' => false, - // List of file regexps that should be treated as if they are generated by // an automatic process, and thus get hidden by default in differential. 'differential.generated-paths' => array( diff --git a/src/applications/differential/conduit/ConduitAPI_differential_getdiff_Method.php b/src/applications/differential/conduit/ConduitAPI_differential_getdiff_Method.php index 5f2719691d..55872bec32 100644 --- a/src/applications/differential/conduit/ConduitAPI_differential_getdiff_Method.php +++ b/src/applications/differential/conduit/ConduitAPI_differential_getdiff_Method.php @@ -72,7 +72,6 @@ final class ConduitAPI_differential_getdiff_Method extends ConduitAPIMethod { $project_name = null; } $basic_dict['projectName'] = $project_name; - $basic_dict['author'] = $diff->loadAuthorInformation(); return $basic_dict; } diff --git a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php index 9f077e1c85..f16bfbfc98 100644 --- a/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php +++ b/src/applications/differential/config/PhabricatorDifferentialConfigOptions.php @@ -127,28 +127,6 @@ final class PhabricatorDifferentialConfigOptions "If you set this to true, users won't need to login to view ". "Differential revisions. Anonymous users will have read-only ". "access and won't be able to interact with the revisions.")), - $this->newOption('differential.expose-emails-prudently', 'bool', false) - ->setBoolOptions( - array( - pht("Expose revision author email address via Conduit"), - pht("Don't expose revision author email address via Conduit"), - )) - ->setSummary( - pht( - "Determines whether or not the author's email address should be ". - "exposed via Conduit.")) - ->setDescription( - pht( - "If you set this to true, revision author email address ". - "information will be exposed in Conduit. This is useful for ". - "Arcanist.\n\n". - "For example, consider the 'arc patch DX' workflow which needs ". - "to ask Differential for the revision DX. This data often should ". - "contain the author's email address, eg 'George Washington ". - "' when DX is a git or mercurial ". - "revision. If this option is false, Differential defaults to the ". - "best it can, something like 'George Washington' or ". - "'gwashington'.")), $this->newOption('differential.generated-paths', 'list', array()) ->setSummary(pht("File regexps to treat as automatically generated.")) ->setDescription( diff --git a/src/applications/differential/storage/DifferentialDiff.php b/src/applications/differential/storage/DifferentialDiff.php index 11e4a754b2..1acf8f5508 100644 --- a/src/applications/differential/storage/DifferentialDiff.php +++ b/src/applications/differential/storage/DifferentialDiff.php @@ -240,55 +240,17 @@ final class DifferentialDiff extends DifferentialDAO { $this->getID()); foreach ($properties as $property) { $dict['properties'][$property->getName()] = $property->getData(); + + if ($property->getName() == 'local:commits') { + foreach ($property->getData() as $commit) { + $dict['authorName'] = $commit['author']; + $dict['authorEmail'] = $commit['authorEmail']; + break; + } + } } return $dict; } - /** - * Figures out the right author information for a given diff based on the - * repository and Phabricator configuration settings. - * - * Git is particularly finicky as it requires author information to be in - * the format "George Washington " to - * consistently work. If the Phabricator instance isn't configured to - * expose emails prudently, then we are unable to get any author information - * for git. - */ - public function loadAuthorInformation() { - $author = id(new PhabricatorUser()) - ->loadOneWhere('phid = %s', $this->getAuthorPHID()); - - $use_emails = - PhabricatorEnv::getEnvConfig('differential.expose-emails-prudently'); - - switch ($this->getSourceControlSystem()) { - case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - if (!$use_emails) { - $author_info = ''; - } else { - $author_info = $this->getFullAuthorInfo($author); - } - break; - case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - if (!$use_emails) { - $author_info = $author->getUsername(); - } else { - $author_info = $this->getFullAuthorInfo($author); - } - break; - case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - default: - $author_info = $author->getUsername(); - break; - } - - return $author_info; - } - - private function getFullAuthorInfo(PhabricatorUser $author) { - return sprintf('%s <%s>', - $author->getRealName(), - $author->loadPrimaryEmailAddress()); - } } From f912fa1ab799444d10f92b9574061af19307653d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2013 08:32:36 -0800 Subject: [PATCH 06/19] Minor, fix celerity map. --- src/__celerity_resource_map__.php | 136 ++++++++++++++++-------------- 1 file changed, 74 insertions(+), 62 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 7d4df11a0b..9bdd9fc488 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -784,7 +784,7 @@ celerity_register_resource_map(array( ), 'conpherence-widget-pane-css' => array( - 'uri' => '/res/b3e6a558/rsrc/css/application/conpherence/widget-pane.css', + 'uri' => '/res/6e5755bb/rsrc/css/application/conpherence/widget-pane.css', 'type' => 'css', 'requires' => array( @@ -1120,7 +1120,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-conpherence-menu' => array( - 'uri' => '/res/9d21fb86/rsrc/js/application/conpherence/behavior-menu.js', + 'uri' => '/res/986774a0/rsrc/js/application/conpherence/behavior-menu.js', 'type' => 'js', 'requires' => array( @@ -1135,7 +1135,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-conpherence-widget-pane' => array( - 'uri' => '/res/f3e0dbba/rsrc/js/application/conpherence/behavior-widget-pane.js', + 'uri' => '/res/4fb51b46/rsrc/js/application/conpherence/behavior-widget-pane.js', 'type' => 'js', 'requires' => array( @@ -1292,7 +1292,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-differential-keyboard-navigation' => array( - 'uri' => '/res/a7798465/rsrc/js/application/differential/behavior-keyboard-nav.js', + 'uri' => '/res/89e93cc9/rsrc/js/application/differential/behavior-keyboard-nav.js', 'type' => 'js', 'requires' => array( @@ -1664,6 +1664,18 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/behavior-autofocus.js', ), + 'javelin-behavior-phabricator-file-tree' => + array( + 'uri' => '/res/e9c96597/rsrc/js/application/core/behavior-file-tree.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'phabricator-keyboard-shortcut', + 2 => 'javelin-stratcom', + ), + 'disk' => '/rsrc/js/application/core/behavior-file-tree.js', + ), 'javelin-behavior-phabricator-home-reveal-tiles' => array( 'uri' => '/res/b3c5cea9/rsrc/js/application/core/behavior-home-reveal-tiles.js', @@ -1704,7 +1716,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-phabricator-nav' => array( - 'uri' => '/res/cec31f3f/rsrc/js/application/core/behavior-phabricator-nav.js', + 'uri' => '/res/222b9329/rsrc/js/application/core/behavior-phabricator-nav.js', 'type' => 'js', 'requires' => array( @@ -3320,7 +3332,7 @@ celerity_register_resource_map(array( ), 'sprite-conpher-css' => array( - 'uri' => '/res/f640f0c5/rsrc/css/sprite-conph.css', + 'uri' => '/res/89821322/rsrc/css/sprite-conph.css', 'type' => 'css', 'requires' => array( @@ -3431,7 +3443,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/7d347135/core.pkg.css', 'type' => 'css', ), - 'd2e59a7e' => + '58743fec' => array( 'name' => 'core.pkg.js', 'symbols' => @@ -3470,7 +3482,7 @@ celerity_register_resource_map(array( 31 => 'javelin-behavior-global-drag-and-drop', 32 => 'javelin-behavior-phabricator-home-reveal-tiles', ), - 'uri' => '/res/pkg/d2e59a7e/core.pkg.js', + 'uri' => '/res/pkg/58743fec/core.pkg.js', 'type' => 'js', ), '8edbada5' => @@ -3508,7 +3520,7 @@ celerity_register_resource_map(array( 'uri' => '/res/pkg/ec01d039/differential.pkg.css', 'type' => 'css', ), - '1b2c991b' => + '95d0d865' => array( 'name' => 'differential.pkg.js', 'symbols' => @@ -3533,7 +3545,7 @@ celerity_register_resource_map(array( 17 => 'javelin-behavior-differential-toggle-files', 18 => 'javelin-behavior-differential-user-select', ), - 'uri' => '/res/pkg/1b2c991b/differential.pkg.js', + 'uri' => '/res/pkg/95d0d865/differential.pkg.js', 'type' => 'js', ), 'c8ce2d88' => @@ -3633,7 +3645,7 @@ celerity_register_resource_map(array( 'aphront-typeahead-control-css' => '7d347135', 'differential-changeset-view-css' => 'ec01d039', 'differential-core-view-css' => 'ec01d039', - 'differential-inline-comment-editor' => '1b2c991b', + 'differential-inline-comment-editor' => '95d0d865', 'differential-local-commits-view-css' => 'ec01d039', 'differential-results-table-css' => 'ec01d039', 'differential-revision-add-comment-css' => 'ec01d039', @@ -3646,56 +3658,56 @@ celerity_register_resource_map(array( 'diffusion-icons-css' => 'c8ce2d88', 'global-drag-and-drop-css' => '7d347135', 'inline-comment-summary-css' => 'ec01d039', - 'javelin-aphlict' => 'd2e59a7e', + 'javelin-aphlict' => '58743fec', 'javelin-behavior' => '88225b70', - 'javelin-behavior-aphlict-dropdown' => 'd2e59a7e', - 'javelin-behavior-aphlict-listen' => 'd2e59a7e', - 'javelin-behavior-aphront-basic-tokenizer' => 'd2e59a7e', - 'javelin-behavior-aphront-drag-and-drop' => '1b2c991b', - 'javelin-behavior-aphront-drag-and-drop-textarea' => '1b2c991b', - 'javelin-behavior-aphront-form-disable-on-submit' => 'd2e59a7e', + 'javelin-behavior-aphlict-dropdown' => '58743fec', + 'javelin-behavior-aphlict-listen' => '58743fec', + 'javelin-behavior-aphront-basic-tokenizer' => '58743fec', + 'javelin-behavior-aphront-drag-and-drop' => '95d0d865', + 'javelin-behavior-aphront-drag-and-drop-textarea' => '95d0d865', + 'javelin-behavior-aphront-form-disable-on-submit' => '58743fec', 'javelin-behavior-audit-preview' => 'f96657b8', 'javelin-behavior-dark-console' => '8edbada5', 'javelin-behavior-dark-console-ajax' => '8edbada5', - 'javelin-behavior-device' => 'd2e59a7e', - 'javelin-behavior-differential-accept-with-errors' => '1b2c991b', - 'javelin-behavior-differential-add-reviewers-and-ccs' => '1b2c991b', - 'javelin-behavior-differential-comment-jump' => '1b2c991b', - 'javelin-behavior-differential-diff-radios' => '1b2c991b', - 'javelin-behavior-differential-dropdown-menus' => '1b2c991b', - 'javelin-behavior-differential-edit-inline-comments' => '1b2c991b', - 'javelin-behavior-differential-feedback-preview' => '1b2c991b', - 'javelin-behavior-differential-keyboard-navigation' => '1b2c991b', - 'javelin-behavior-differential-populate' => '1b2c991b', - 'javelin-behavior-differential-show-more' => '1b2c991b', - 'javelin-behavior-differential-toggle-files' => '1b2c991b', - 'javelin-behavior-differential-user-select' => '1b2c991b', + 'javelin-behavior-device' => '58743fec', + 'javelin-behavior-differential-accept-with-errors' => '95d0d865', + 'javelin-behavior-differential-add-reviewers-and-ccs' => '95d0d865', + 'javelin-behavior-differential-comment-jump' => '95d0d865', + 'javelin-behavior-differential-diff-radios' => '95d0d865', + 'javelin-behavior-differential-dropdown-menus' => '95d0d865', + 'javelin-behavior-differential-edit-inline-comments' => '95d0d865', + 'javelin-behavior-differential-feedback-preview' => '95d0d865', + 'javelin-behavior-differential-keyboard-navigation' => '95d0d865', + 'javelin-behavior-differential-populate' => '95d0d865', + 'javelin-behavior-differential-show-more' => '95d0d865', + 'javelin-behavior-differential-toggle-files' => '95d0d865', + 'javelin-behavior-differential-user-select' => '95d0d865', 'javelin-behavior-diffusion-commit-graph' => 'f96657b8', 'javelin-behavior-diffusion-pull-lastmodified' => 'f96657b8', 'javelin-behavior-error-log' => '8edbada5', - 'javelin-behavior-global-drag-and-drop' => 'd2e59a7e', - 'javelin-behavior-konami' => 'd2e59a7e', - 'javelin-behavior-lightbox-attachments' => 'd2e59a7e', + 'javelin-behavior-global-drag-and-drop' => '58743fec', + 'javelin-behavior-konami' => '58743fec', + 'javelin-behavior-lightbox-attachments' => '58743fec', 'javelin-behavior-maniphest-batch-selector' => '7707de41', 'javelin-behavior-maniphest-subpriority-editor' => '7707de41', 'javelin-behavior-maniphest-transaction-controls' => '7707de41', 'javelin-behavior-maniphest-transaction-expand' => '7707de41', 'javelin-behavior-maniphest-transaction-preview' => '7707de41', - 'javelin-behavior-phabricator-active-nav' => 'd2e59a7e', - 'javelin-behavior-phabricator-autofocus' => 'd2e59a7e', - 'javelin-behavior-phabricator-home-reveal-tiles' => 'd2e59a7e', - 'javelin-behavior-phabricator-keyboard-shortcuts' => 'd2e59a7e', - 'javelin-behavior-phabricator-nav' => 'd2e59a7e', - 'javelin-behavior-phabricator-object-selector' => '1b2c991b', - 'javelin-behavior-phabricator-oncopy' => 'd2e59a7e', - 'javelin-behavior-phabricator-remarkup-assist' => 'd2e59a7e', - 'javelin-behavior-phabricator-search-typeahead' => 'd2e59a7e', - 'javelin-behavior-phabricator-tooltips' => 'd2e59a7e', - 'javelin-behavior-phabricator-watch-anchor' => 'd2e59a7e', - 'javelin-behavior-refresh-csrf' => 'd2e59a7e', - 'javelin-behavior-repository-crossreference' => '1b2c991b', - 'javelin-behavior-toggle-class' => 'd2e59a7e', - 'javelin-behavior-workflow' => 'd2e59a7e', + 'javelin-behavior-phabricator-active-nav' => '58743fec', + 'javelin-behavior-phabricator-autofocus' => '58743fec', + 'javelin-behavior-phabricator-home-reveal-tiles' => '58743fec', + 'javelin-behavior-phabricator-keyboard-shortcuts' => '58743fec', + 'javelin-behavior-phabricator-nav' => '58743fec', + 'javelin-behavior-phabricator-object-selector' => '95d0d865', + 'javelin-behavior-phabricator-oncopy' => '58743fec', + 'javelin-behavior-phabricator-remarkup-assist' => '58743fec', + 'javelin-behavior-phabricator-search-typeahead' => '58743fec', + 'javelin-behavior-phabricator-tooltips' => '58743fec', + 'javelin-behavior-phabricator-watch-anchor' => '58743fec', + 'javelin-behavior-refresh-csrf' => '58743fec', + 'javelin-behavior-repository-crossreference' => '95d0d865', + 'javelin-behavior-toggle-class' => '58743fec', + 'javelin-behavior-workflow' => '58743fec', 'javelin-dom' => '88225b70', 'javelin-event' => '88225b70', 'javelin-install' => '88225b70', @@ -3717,39 +3729,39 @@ celerity_register_resource_map(array( 'lightbox-attachment-css' => '7d347135', 'maniphest-task-summary-css' => 'e30a3fa8', 'maniphest-transaction-detail-css' => 'e30a3fa8', - 'phabricator-busy' => 'd2e59a7e', + 'phabricator-busy' => '58743fec', 'phabricator-content-source-view-css' => 'ec01d039', 'phabricator-core-buttons-css' => '7d347135', 'phabricator-core-css' => '7d347135', 'phabricator-crumbs-view-css' => '7d347135', 'phabricator-directory-css' => '7d347135', - 'phabricator-drag-and-drop-file-upload' => '1b2c991b', - 'phabricator-dropdown-menu' => 'd2e59a7e', - 'phabricator-file-upload' => 'd2e59a7e', + 'phabricator-drag-and-drop-file-upload' => '95d0d865', + 'phabricator-dropdown-menu' => '58743fec', + 'phabricator-file-upload' => '58743fec', 'phabricator-filetree-view-css' => '7d347135', 'phabricator-flag-css' => '7d347135', 'phabricator-form-view-css' => '7d347135', 'phabricator-header-view-css' => '7d347135', 'phabricator-jump-nav' => '7d347135', - 'phabricator-keyboard-shortcut' => 'd2e59a7e', - 'phabricator-keyboard-shortcut-manager' => 'd2e59a7e', + 'phabricator-keyboard-shortcut' => '58743fec', + 'phabricator-keyboard-shortcut-manager' => '58743fec', 'phabricator-main-menu-view' => '7d347135', - 'phabricator-menu-item' => 'd2e59a7e', + 'phabricator-menu-item' => '58743fec', 'phabricator-nav-view-css' => '7d347135', - 'phabricator-notification' => 'd2e59a7e', + 'phabricator-notification' => '58743fec', 'phabricator-notification-css' => '7d347135', 'phabricator-notification-menu-css' => '7d347135', 'phabricator-object-item-list-view-css' => '7d347135', 'phabricator-object-selector-css' => 'ec01d039', - 'phabricator-paste-file-upload' => 'd2e59a7e', - 'phabricator-prefab' => 'd2e59a7e', + 'phabricator-paste-file-upload' => '58743fec', + 'phabricator-prefab' => '58743fec', 'phabricator-project-tag-css' => 'e30a3fa8', 'phabricator-remarkup-css' => '7d347135', - 'phabricator-shaped-request' => '1b2c991b', + 'phabricator-shaped-request' => '95d0d865', 'phabricator-side-menu-view-css' => '7d347135', 'phabricator-standard-page-view' => '7d347135', - 'phabricator-textareautils' => 'd2e59a7e', - 'phabricator-tooltip' => 'd2e59a7e', + 'phabricator-textareautils' => '58743fec', + 'phabricator-tooltip' => '58743fec', 'phabricator-transaction-view-css' => '7d347135', 'phabricator-zindex-css' => '7d347135', 'sprite-apps-large-css' => '7d347135', From 1188876ea92fa1fe19e21b0381f9deb991041795 Mon Sep 17 00:00:00 2001 From: Toby Hughes Date: Wed, 6 Feb 2013 08:59:53 -0800 Subject: [PATCH 07/19] Fix exception in OAuthServerAuthController Summary: We've been building a Jenkins plugin that allows you to use your Phabricator login details in Jenkins using the inbuilt OAuthServer. I noticed that when making a request to /oauthserver/auth/?client_id=&response_type=code I get an error back from the server. I've traced this down to two bugs in PhabricatorOAuthServerAuthController, the first causes a null value error on $access_token_uri, and the second fails on userHasAuthorizedClient without a $scope array. Test Plan: Go to /oauthserver/auth/?client_id=&response_type=code and get a valid authorization code back Reviewers: epriestley, btrahan Reviewed By: btrahan CC: aran, Korvin Differential Revision: https://secure.phabricator.com/D4808 --- .../controller/PhabricatorOAuthServerAuthController.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php index da9f6438be..a0b4e1d592 100644 --- a/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php +++ b/src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php @@ -15,7 +15,7 @@ extends PhabricatorAuthController { $current_user = $request->getUser(); $server = new PhabricatorOAuthServer(); $client_phid = $request->getStr('client_id'); - $scope = $request->getStr('scope'); + $scope = $request->getStr('scope', array()); $redirect_uri = $request->getStr('redirect_uri'); $state = $request->getStr('state'); $response_type = $request->getStr('response_type'); @@ -63,10 +63,8 @@ extends PhabricatorAuthController { return $response; } $uri = $redirect_uri; - $access_token_uri = $uri; } else { $uri = new PhutilURI($client->getRedirectURI()); - $access_token_uri = null; } // we've now validated this request enough overall such that we // can safely redirect to the client with the response @@ -121,7 +119,7 @@ extends PhabricatorAuthController { if ($return_auth_code) { // step 1 -- generate authorization code $auth_code = - $server->generateAuthorizationCode($access_token_uri); + $server->generateAuthorizationCode($uri); // step 2 return it $content = array( From 57c001f522441cadcd4d65518e33fead1e9e7a7e Mon Sep 17 00:00:00 2001 From: Lauri-Henrik Jalonen Date: Wed, 6 Feb 2013 11:28:03 -0800 Subject: [PATCH 08/19] Select portions from mock Summary: Comment draft is now saved Test Plan: Verified that draft is saved Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin, chad Maniphest Tasks: T2446 Differential Revision: https://secure.phabricator.com/D4831 --- src/__phutil_library_map__.php | 2 + .../PhabricatorApplicationPholio.php | 1 + .../controller/PholioInlineSaveController.php | 54 +++++++++++++++++++ .../pholio/view/PholioMockImagesView.php | 4 +- .../pholio/behavior-pholio-mock-view.js | 23 ++++++-- 5 files changed, 78 insertions(+), 6 deletions(-) create mode 100644 src/applications/pholio/controller/PholioInlineSaveController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 4f11a0fc87..eab3d46eb7 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1404,6 +1404,7 @@ phutil_register_library_map(array( 'PholioController' => 'applications/pholio/controller/PholioController.php', 'PholioDAO' => 'applications/pholio/storage/PholioDAO.php', 'PholioImage' => 'applications/pholio/storage/PholioImage.php', + 'PholioInlineSaveController' => 'applications/pholio/controller/PholioInlineSaveController.php', 'PholioMock' => 'applications/pholio/storage/PholioMock.php', 'PholioMockCommentController' => 'applications/pholio/controller/PholioMockCommentController.php', 'PholioMockEditController' => 'applications/pholio/controller/PholioMockEditController.php', @@ -2816,6 +2817,7 @@ phutil_register_library_map(array( 0 => 'PholioDAO', 1 => 'PhabricatorMarkupInterface', ), + 'PholioInlineSaveController' => 'PholioController', 'PholioMock' => array( 0 => 'PholioDAO', diff --git a/src/applications/pholio/application/PhabricatorApplicationPholio.php b/src/applications/pholio/application/PhabricatorApplicationPholio.php index 5948f43c3d..10e4cafb07 100644 --- a/src/applications/pholio/application/PhabricatorApplicationPholio.php +++ b/src/applications/pholio/application/PhabricatorApplicationPholio.php @@ -43,6 +43,7 @@ final class PhabricatorApplicationPholio extends PhabricatorApplication { 'new/' => 'PholioMockEditController', 'edit/(?P\d+)/' => 'PholioMockEditController', 'comment/(?P\d+)/' => 'PholioMockCommentController', + 'inline/(?P\d+)/' => 'PholioInlineSaveController', ), ); } diff --git a/src/applications/pholio/controller/PholioInlineSaveController.php b/src/applications/pholio/controller/PholioInlineSaveController.php new file mode 100644 index 0000000000..5db75f6931 --- /dev/null +++ b/src/applications/pholio/controller/PholioInlineSaveController.php @@ -0,0 +1,54 @@ +getRequest(); + $user = $request->getUser(); + + $mock = id(new PholioMockQuery()) + ->setViewer($user) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withIDs(array($request->getInt('mockID'))) + ->executeOne(); + + if (!$mock) { + return new Aphront404Response(); + } + + $draft = id(new PholioTransactionComment()); + $draft->setImageID($request->getInt('imageID')); + $draft->setX($request->getInt('startX')); + $draft->setY($request->getInt('startY')); + + $draft->setCommentVersion(1); + $draft->setAuthorPHID($user->getPHID()); + $draft->setEditPolicy($user->getPHID()); + $draft->setViewPolicy(PhabricatorPolicies::POLICY_PUBLIC); + + $content_source = PhabricatorContentSource::newForSource( + PhabricatorContentSource::SOURCE_WEB, + array( + 'ip' => $request->getRemoteAddr(), + )); + + $draft->setContentSource($content_source); + + $draft->setWidth($request->getInt('endX') - $request->getInt('startX')); + $draft->setHeight($request->getInt('endY') - $request->getInt('startY')); + + $draft->setContent($request->getStr('comment')); + + $draft->save(); + + return id(new AphrontAjaxResponse())->setContent(array()); + } + +} diff --git a/src/applications/pholio/view/PholioMockImagesView.php b/src/applications/pholio/view/PholioMockImagesView.php index c6f8d1325d..1f29b39db6 100644 --- a/src/applications/pholio/view/PholioMockImagesView.php +++ b/src/applications/pholio/view/PholioMockImagesView.php @@ -15,7 +15,9 @@ final class PholioMockImagesView extends AphrontView { $main_image_id = celerity_generate_unique_node_id(); require_celerity_resource('javelin-behavior-pholio-mock-view'); - $config = array('mainID' => $main_image_id); + $config = array( + 'mainID' => $main_image_id, + 'mockID' => $this->mock->getID()); Javelin::initBehavior('pholio-mock-view', $config); $mockview = ""; diff --git a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js index d9c438eb80..afeabe579a 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js @@ -113,11 +113,24 @@ JX.behavior('pholio-mock-view', function(config) { selection.title = comment; - console.log("ImageID: " + imageData['imageID'] + - ", coords: (" + Math.min(startPos.x, endPos.x) + "," + - Math.min(startPos.y, endPos.y) + ") -> (" + - Math.max(startPos.x,endPos.x) + "," + Math.max(startPos.y,endPos.y) + - "), comment: " + comment); + var saveURL = "/pholio/inline/" + imageData['imageID'] + "/"; + + var inlineComment = new JX.Request(saveURL, function(r) { + + }); + + var commentToAdd = { + mockID: config.mockID, + imageID: imageData['imageID'], + startX: Math.min(startPos.x, endPos.x), + startY: Math.min(startPos.y, endPos.y), + endX: Math.max(startPos.x,endPos.x), + endY: Math.max(startPos.y,endPos.y), + comment: comment}; + + + inlineComment.addData(commentToAdd); + inlineComment.send(); }); From 84efcb86698a00f863dbdb4cbd4c149c96e498dc Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2013 11:41:57 -0800 Subject: [PATCH 09/19] Minor, fix an issue with PhabricatorMenuView and default null keys. This is kind of gross, but breaking some menus right now which end up with double-`''` keys. The current meaning of setKey(null) is different from not calling it (it means `setKey('')`). This should be fixed more reasonably but there's a lot of legacy cruft in PhabricatorSideNavFilterView. Auditors: btrahan --- src/view/layout/PhabricatorMenuView.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/view/layout/PhabricatorMenuView.php b/src/view/layout/PhabricatorMenuView.php index c430fe76fd..ab2a53784c 100644 --- a/src/view/layout/PhabricatorMenuView.php +++ b/src/view/layout/PhabricatorMenuView.php @@ -11,8 +11,11 @@ final class PhabricatorMenuView extends AphrontTagView { public function newLabel($name, $key = null) { $item = id(new PhabricatorMenuItemView()) ->setType(PhabricatorMenuItemView::TYPE_LABEL) - ->setName($name) - ->setKey($key); + ->setName($name); + + if ($key !== null) { + $item->setKey($key); + } $this->addMenuItem($item); @@ -23,8 +26,11 @@ final class PhabricatorMenuView extends AphrontTagView { $item = id(new PhabricatorMenuItemView()) ->setType(PhabricatorMenuItemView::TYPE_LINK) ->setName($name) - ->setHref($href) - ->setKey($key); + ->setHref($href); + + if ($key !== null) { + $key->setKey($key); + } $this->addMenuItem($item); From e518135dfb545377e8907c070559f1a2775fdc6a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2013 13:37:31 -0800 Subject: [PATCH 10/19] Improve STRICT_ALL_TABLES warning Summary: - Make the warning describe rationale and point at the MySQL manual explicitly. - Add a reference to the developer mode config, in case the user wants to resolve the probelm by disabling developer mode. - Now that the message is huge, provide a summary. - Move from "Database" to "MySQL" setup checks -- this is kind of arbitrary, but the former is used for fatals (pre-install) and the latter for warnings (post-install) right now. This has no practical impact on anything and is purely stylistic. Test Plan: {F31798} {F31799} Reviewers: edward, blc Reviewed By: edward CC: aran Differential Revision: https://secure.phabricator.com/D4835 --- .../check/PhabricatorSetupCheckDatabase.php | 14 ----------- .../check/PhabricatorSetupCheckMySQL.php | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/applications/config/check/PhabricatorSetupCheckDatabase.php b/src/applications/config/check/PhabricatorSetupCheckDatabase.php index 67ce6bc94c..adef6a7cc1 100644 --- a/src/applications/config/check/PhabricatorSetupCheckDatabase.php +++ b/src/applications/config/check/PhabricatorSetupCheckDatabase.php @@ -65,20 +65,6 @@ final class PhabricatorSetupCheckDatabase extends PhabricatorSetupCheck { return; } - if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { - $mode_string = queryfx_one($conn_raw, "SELECT @@sql_mode"); - $modes = explode(',', $mode_string['@@sql_mode']); - if (!in_array('STRICT_ALL_TABLES', $modes)) { - $message = pht( - "The global sql_mode is not set to 'STRICT_ALL_TABLES'. It is ". - "recommended that you set this mode while developing Phabricator."); - - $this->newIssue('mysql.mode') - ->setName(pht('MySQL STRICT_ALL_TABLES mode not set.')) - ->setMessage($message); - } - } - $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace'); $databases = queryfx_all($conn_raw, 'SHOW DATABASES'); diff --git a/src/applications/config/check/PhabricatorSetupCheckMySQL.php b/src/applications/config/check/PhabricatorSetupCheckMySQL.php index e8de03bb51..cabb822e81 100644 --- a/src/applications/config/check/PhabricatorSetupCheckMySQL.php +++ b/src/applications/config/check/PhabricatorSetupCheckMySQL.php @@ -24,6 +24,31 @@ final class PhabricatorSetupCheckMySQL extends PhabricatorSetupCheck { ->setName(pht('Small MySQL "max_allowed_packet"')) ->setMessage($message); } + + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $mode_string = queryfx_one($conn_raw, "SELECT @@sql_mode"); + $modes = explode(',', $mode_string['@@sql_mode']); + if (!in_array('STRICT_ALL_TABLES', $modes)) { + $summary = pht( + "MySQL is not in strict mode, but should be for Phabricator ". + "development."); + + $message = pht( + "This install is in developer mode, but the global sql_mode is not ". + "set to 'STRICT_ALL_TABLES'. It is recommended that you set this ". + "mode while developing Phabricator. Strict mode will promote some ". + "query warnings to errors, and ensure you don't miss them during ". + "development. You can find more information about this mode (and ". + "how to configure it) in the MySQL manual."); + + $this->newIssue('mysql.mode') + ->setName(pht('MySQL STRICT_ALL_TABLES Mode Not Set')) + ->addPhabricatorConfig('phabricator.developer-mode') + ->setSummary($summary) + ->setMessage($message); + } + } + } } From 361ce491f201a0f2dcc440c063df7085fd9283eb Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2013 13:37:42 -0800 Subject: [PATCH 11/19] Make file storage more testable Summary: Fixes T2474. Adds a storage dummy storage engine for unit tests, and adds a couple of simple tests for basic file storage. Test Plan: Ran `arc unit` to execute unit tests. Reviewers: kwadwon Reviewed By: kwadwon CC: aran Maniphest Tasks: T2474 Differential Revision: https://secure.phabricator.com/D4777 --- src/__phutil_library_map__.php | 4 ++ .../engine/PhabricatorTestStorageEngine.php | 36 +++++++++++ .../files/storage/PhabricatorFile.php | 11 +++- .../__tests__/PhabricatorFileTestCase.php | 61 +++++++++++++++++++ 4 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 src/applications/files/engine/PhabricatorTestStorageEngine.php create mode 100644 src/applications/files/storage/__tests__/PhabricatorFileTestCase.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index eab3d46eb7..a1bc47e859 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -896,6 +896,7 @@ phutil_register_library_map(array( 'PhabricatorFileStorageConfigurationException' => 'applications/files/exception/PhabricatorFileStorageConfigurationException.php', 'PhabricatorFileStorageEngine' => 'applications/files/engine/PhabricatorFileStorageEngine.php', 'PhabricatorFileStorageEngineSelector' => 'applications/files/engineselector/PhabricatorFileStorageEngineSelector.php', + 'PhabricatorFileTestCase' => 'applications/files/storage/__tests__/PhabricatorFileTestCase.php', 'PhabricatorFileTransformController' => 'applications/files/controller/PhabricatorFileTransformController.php', 'PhabricatorFileUploadController' => 'applications/files/controller/PhabricatorFileUploadController.php', 'PhabricatorFileUploadException' => 'applications/files/exception/PhabricatorFileUploadException.php', @@ -1301,6 +1302,7 @@ phutil_register_library_map(array( 'PhabricatorTagView' => 'view/layout/PhabricatorTagView.php', 'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php', 'PhabricatorTestCase' => 'infrastructure/testing/PhabricatorTestCase.php', + 'PhabricatorTestStorageEngine' => 'applications/files/engine/PhabricatorTestStorageEngine.php', 'PhabricatorTestWorker' => 'infrastructure/daemon/workers/__tests__/PhabricatorTestWorker.php', 'PhabricatorTimelineCursor' => 'infrastructure/daemon/timeline/storage/PhabricatorTimelineCursor.php', 'PhabricatorTimelineDAO' => 'infrastructure/daemon/timeline/storage/PhabricatorTimelineDAO.php', @@ -2338,6 +2340,7 @@ phutil_register_library_map(array( 'PhabricatorFileShortcutController' => 'PhabricatorFileController', 'PhabricatorFileStorageBlob' => 'PhabricatorFileDAO', 'PhabricatorFileStorageConfigurationException' => 'Exception', + 'PhabricatorFileTestCase' => 'PhabricatorTestCase', 'PhabricatorFileTransformController' => 'PhabricatorFileController', 'PhabricatorFileUploadController' => 'PhabricatorFileController', 'PhabricatorFileUploadException' => 'Exception', @@ -2703,6 +2706,7 @@ phutil_register_library_map(array( 'PhabricatorTagView' => 'AphrontView', 'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon', 'PhabricatorTestCase' => 'ArcanistPhutilTestCase', + 'PhabricatorTestStorageEngine' => 'PhabricatorFileStorageEngine', 'PhabricatorTestWorker' => 'PhabricatorWorker', 'PhabricatorTimelineCursor' => 'PhabricatorTimelineDAO', 'PhabricatorTimelineDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/files/engine/PhabricatorTestStorageEngine.php b/src/applications/files/engine/PhabricatorTestStorageEngine.php new file mode 100644 index 0000000000..af45c6574a --- /dev/null +++ b/src/applications/files/engine/PhabricatorTestStorageEngine.php @@ -0,0 +1,36 @@ +selectStorageEngines($data, $params); + if (isset($params['storageEngines'])) { + $engines = $params['storageEngines']; + } else { + $selector = PhabricatorEnv::newObjectFromConfig( + 'storage.engine-selector'); + $engines = $selector->selectStorageEngines($data, $params); + } + + assert_instances_of($engines, 'PhabricatorFileStorageEngine'); if (!$engines) { throw new Exception("No valid storage engines are available!"); } diff --git a/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php new file mode 100644 index 0000000000..334d3a92f1 --- /dev/null +++ b/src/applications/files/storage/__tests__/PhabricatorFileTestCase.php @@ -0,0 +1,61 @@ + true, + ); + } + + public function testFileStorageReadWrite() { + $engine = new PhabricatorTestStorageEngine(); + + $data = Filesystem::readRandomCharacters(64); + + $params = array( + 'name' => 'test.dat', + 'storageEngines' => array( + $engine, + ), + ); + + $file = PhabricatorFile::newFromFileData($data, $params); + + // Test that the storage engine worked, and was the target of the write. We + // don't actually care what the data is (future changes may compress or + // encrypt it), just that it exists in the test storage engine. + $engine->readFile($file->getStorageHandle()); + + // Now test that we get the same data back out. + $this->assertEqual($data, $file->loadFileData()); + } + + + public function testFileStorageDelete() { + $engine = new PhabricatorTestStorageEngine(); + + $data = Filesystem::readRandomCharacters(64); + + $params = array( + 'name' => 'test.dat', + 'storageEngines' => array( + $engine, + ), + ); + + $file = PhabricatorFile::newFromFileData($data, $params); + $handle = $file->getStorageHandle(); + $file->delete(); + + $caught = null; + try { + $engine->readFile($handle); + } catch (Exception $ex) { + $caught = $ex; + } + + $this->assertEqual(true, $caught instanceof Exception); + } + +} From fb7d5d17a2bec5ba1815cb69c908bac33f831cc2 Mon Sep 17 00:00:00 2001 From: Edward Speyer Date: Wed, 6 Feb 2013 21:43:14 +0000 Subject: [PATCH 12/19] Don't do image stuff with unviewable images Summary: If a file isn't a viewable image, don't try to figure out metadata (size, etc.) when rendering a `{F...}` tag in Remarkup. Test Plan: Uploaded a .rtf, added it as `{F1}` in a new Maniphest task, saw no errors in the dark console. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2479 Differential Revision: https://secure.phabricator.com/D4837 --- .../rule/PhabricatorRemarkupRuleEmbedFile.php | 52 ++++++++++--------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php index 31ec888b21..45df8b9c8b 100644 --- a/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php +++ b/src/infrastructure/markup/rule/PhabricatorRemarkupRuleEmbedFile.php @@ -50,37 +50,41 @@ final class PhabricatorRemarkupRuleEmbedFile $file_name = coalesce($options['name'], $file->getName()); $options['name'] = $file_name; + $is_viewable_image = $file->isViewableImage(); + $attrs = array(); - switch ((string)$options['size']) { - case 'full': - $attrs['src'] = $file->getBestURI(); - $options['image_class'] = null; - $file_data = $file->getMetadata(); - $height = idx($file_data, PhabricatorFile::METADATA_IMAGE_HEIGHT); - if ($height) { - $attrs['height'] = $height; - } - $width = idx($file_data, PhabricatorFile::METADATA_IMAGE_WIDTH); - if ($width) { - $attrs['width'] = $width; - } - break; - case 'thumb': - default: - $attrs['src'] = $file->getPreview220URI(); - $dimensions = - PhabricatorImageTransformer::getPreviewDimensions($file, 220); - $attrs['width'] = $dimensions['sdx']; - $attrs['height'] = $dimensions['sdy']; - $options['image_class'] = 'phabricator-remarkup-embed-image'; - break; + if ($is_viewable_image) { + switch ((string)$options['size']) { + case 'full': + $attrs['src'] = $file->getBestURI(); + $options['image_class'] = null; + $file_data = $file->getMetadata(); + $height = idx($file_data, PhabricatorFile::METADATA_IMAGE_HEIGHT); + if ($height) { + $attrs['height'] = $height; + } + $width = idx($file_data, PhabricatorFile::METADATA_IMAGE_WIDTH); + if ($width) { + $attrs['width'] = $width; + } + break; + case 'thumb': + default: + $attrs['src'] = $file->getPreview220URI(); + $dimensions = + PhabricatorImageTransformer::getPreviewDimensions($file, 220); + $attrs['width'] = $dimensions['sdx']; + $attrs['height'] = $dimensions['sdy']; + $options['image_class'] = 'phabricator-remarkup-embed-image'; + break; + } } $bundle['attrs'] = $attrs; $bundle['options'] = $options; $bundle['meta'] = array( 'phid' => $file->getPHID(), - 'viewable' => $file->isViewableImage(), + 'viewable' => $is_viewable_image, 'uri' => $file->getBestURI(), 'dUri' => $file->getDownloadURI(), 'name' => $options['name'], From 1cde41b9946c5157681c9945a47eae76795b84d9 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 6 Feb 2013 14:03:52 -0800 Subject: [PATCH 13/19] Conpherence - add crop Summary: mainly, this adds the image cropper - yay! - also removes the file image from the handle stuff I added in V1. now we do all this crazy photo stuff. Test Plan: - uploaded a photo by dragging to header and noted 120 x 80 showed up on reload - uploaded a photo by dragging to edit dialogue spot and noted 120 x 80 showed up on reload - cropped a photo - noted it cropped right - cropped a photo again and again and again - seems like it crops okay Reviewers: epriestley, chad Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2418, T2399 Differential Revision: https://secure.phabricator.com/D4790 --- .../sql/patches/20130131.conpherencepics.sql | 6 + src/__celerity_resource_map__.php | 114 ++++++++------ src/__phutil_library_map__.php | 6 + .../constants/ConpherenceImageData.php | 11 ++ .../constants/ConpherenceTransactionType.php | 1 + .../controller/ConpherenceController.php | 13 +- .../ConpherenceUpdateController.php | 116 ++++++++++---- .../controller/ConpherenceViewController.php | 22 ++- .../conpherence/editor/ConpherenceEditor.php | 17 ++- .../query/ConpherenceThreadQuery.php | 53 +++++++ .../conpherence/storage/ConpherenceThread.php | 144 +++++++++++++----- .../storage/ConpherenceTransaction.php | 5 +- ...onpherenceFormDragAndDropUploadControl.php | 42 +++++ .../view/ConpherenceTransactionView.php | 13 +- .../files/PhabricatorImageTransformer.php | 84 +++++++++- .../handle/PhabricatorObjectHandleData.php | 3 - .../form/control/AphrontFormCropControl.php | 99 ++++++++++++ webroot/rsrc/css/aphront/form-view.css | 12 ++ .../application/conpherence/header-pane.css | 48 ++++++ .../css/application/conpherence/update.css | 10 ++ .../behavior-drag-and-drop-photo.js | 39 +++++ .../rsrc/js/application/core/behavior-crop.js | 94 ++++++++++++ 22 files changed, 803 insertions(+), 149 deletions(-) create mode 100644 resources/sql/patches/20130131.conpherencepics.sql create mode 100644 src/applications/conpherence/constants/ConpherenceImageData.php create mode 100644 src/applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php create mode 100644 src/view/form/control/AphrontFormCropControl.php create mode 100644 webroot/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js create mode 100644 webroot/rsrc/js/application/core/behavior-crop.js diff --git a/resources/sql/patches/20130131.conpherencepics.sql b/resources/sql/patches/20130131.conpherencepics.sql new file mode 100644 index 0000000000..421291225f --- /dev/null +++ b/resources/sql/patches/20130131.conpherencepics.sql @@ -0,0 +1,6 @@ +ALTER TABLE {$NAMESPACE}_conpherence.conpherence_thread + DROP imagePHID, + ADD imagePHIDs LONGTEXT COLLATE utf8_bin NOT NULL AFTER title; + +UPDATE {$NAMESPACE}_conpherence.conpherence_thread + SET imagePHIDs = '{}' WHERE imagePHIDs = ''; diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 9bdd9fc488..cad7602664 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -639,7 +639,7 @@ celerity_register_resource_map(array( ), 'aphront-form-view-css' => array( - 'uri' => '/res/428fbcd8/rsrc/css/aphront/form-view.css', + 'uri' => '/res/ec323d34/rsrc/css/aphront/form-view.css', 'type' => 'css', 'requires' => array( @@ -748,7 +748,7 @@ celerity_register_resource_map(array( ), 'conpherence-header-pane-css' => array( - 'uri' => '/res/4b8aebd2/rsrc/css/application/conpherence/header-pane.css', + 'uri' => '/res/11c32adc/rsrc/css/application/conpherence/header-pane.css', 'type' => 'css', 'requires' => array( @@ -775,7 +775,7 @@ celerity_register_resource_map(array( ), 'conpherence-update-css' => array( - 'uri' => '/res/8e4757b5/rsrc/css/application/conpherence/update.css', + 'uri' => '/res/92094ed7/rsrc/css/application/conpherence/update.css', 'type' => 'css', 'requires' => array( @@ -1042,6 +1042,19 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/behavior-tokenizer.js', ), + 'javelin-behavior-aphront-crop' => + array( + 'uri' => '/res/cda1eace/rsrc/js/application/core/behavior-crop.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-vector', + 3 => 'javelin-magical-init', + ), + 'disk' => '/rsrc/js/application/core/behavior-crop.js', + ), 'javelin-behavior-aphront-drag-and-drop' => array( 'uri' => '/res/3d809b40/rsrc/js/application/core/behavior-drag-and-drop.js', @@ -1106,6 +1119,19 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/diffusion/behavior-audit-preview.js', ), + 'javelin-behavior-conpherence-drag-and-drop-photo' => + array( + 'uri' => '/res/9e3eb1cd/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-workflow', + 3 => 'phabricator-drag-and-drop-file-upload', + ), + 'disk' => '/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js', + ), 'javelin-behavior-conpherence-init' => array( 'uri' => '/res/bd911b43/rsrc/js/application/conpherence/behavior-init.js', @@ -1863,7 +1889,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-pholio-mock-view' => array( - 'uri' => '/res/1364872e/rsrc/js/application/pholio/behavior-pholio-mock-view.js', + 'uri' => '/res/5dd61f73/rsrc/js/application/pholio/behavior-pholio-mock-view.js', 'type' => 'js', 'requires' => array( @@ -3396,7 +3422,7 @@ celerity_register_resource_map(array( ), array( 'packages' => array( - '7d347135' => + '23503cb9' => array( 'name' => 'core.pkg.css', 'symbols' => @@ -3440,7 +3466,7 @@ celerity_register_resource_map(array( 36 => 'phabricator-object-item-list-view-css', 37 => 'global-drag-and-drop-css', ), - 'uri' => '/res/pkg/7d347135/core.pkg.css', + 'uri' => '/res/pkg/23503cb9/core.pkg.css', 'type' => 'css', ), '58743fec' => @@ -3630,19 +3656,19 @@ celerity_register_resource_map(array( 'reverse' => array( 'aphront-attached-file-view-css' => 'e30a3fa8', - 'aphront-crumbs-view-css' => '7d347135', - 'aphront-dialog-view-css' => '7d347135', - 'aphront-error-view-css' => '7d347135', - 'aphront-form-view-css' => '7d347135', + 'aphront-crumbs-view-css' => '23503cb9', + 'aphront-dialog-view-css' => '23503cb9', + 'aphront-error-view-css' => '23503cb9', + 'aphront-form-view-css' => '23503cb9', 'aphront-headsup-action-list-view-css' => 'ec01d039', - 'aphront-headsup-view-css' => '7d347135', - 'aphront-list-filter-view-css' => '7d347135', - 'aphront-pager-view-css' => '7d347135', - 'aphront-panel-view-css' => '7d347135', - 'aphront-table-view-css' => '7d347135', - 'aphront-tokenizer-control-css' => '7d347135', - 'aphront-tooltip-css' => '7d347135', - 'aphront-typeahead-control-css' => '7d347135', + 'aphront-headsup-view-css' => '23503cb9', + 'aphront-list-filter-view-css' => '23503cb9', + 'aphront-pager-view-css' => '23503cb9', + 'aphront-panel-view-css' => '23503cb9', + 'aphront-table-view-css' => '23503cb9', + 'aphront-tokenizer-control-css' => '23503cb9', + 'aphront-tooltip-css' => '23503cb9', + 'aphront-typeahead-control-css' => '23503cb9', 'differential-changeset-view-css' => 'ec01d039', 'differential-core-view-css' => 'ec01d039', 'differential-inline-comment-editor' => '95d0d865', @@ -3656,7 +3682,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => 'ec01d039', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'global-drag-and-drop-css' => '7d347135', + 'global-drag-and-drop-css' => '23503cb9', 'inline-comment-summary-css' => 'ec01d039', 'javelin-aphlict' => '58743fec', 'javelin-behavior' => '88225b70', @@ -3726,48 +3752,48 @@ celerity_register_resource_map(array( 'javelin-util' => '88225b70', 'javelin-vector' => '88225b70', 'javelin-workflow' => '88225b70', - 'lightbox-attachment-css' => '7d347135', + 'lightbox-attachment-css' => '23503cb9', 'maniphest-task-summary-css' => 'e30a3fa8', 'maniphest-transaction-detail-css' => 'e30a3fa8', 'phabricator-busy' => '58743fec', 'phabricator-content-source-view-css' => 'ec01d039', - 'phabricator-core-buttons-css' => '7d347135', - 'phabricator-core-css' => '7d347135', - 'phabricator-crumbs-view-css' => '7d347135', - 'phabricator-directory-css' => '7d347135', + 'phabricator-core-buttons-css' => '23503cb9', + 'phabricator-core-css' => '23503cb9', + 'phabricator-crumbs-view-css' => '23503cb9', + 'phabricator-directory-css' => '23503cb9', 'phabricator-drag-and-drop-file-upload' => '95d0d865', 'phabricator-dropdown-menu' => '58743fec', 'phabricator-file-upload' => '58743fec', - 'phabricator-filetree-view-css' => '7d347135', - 'phabricator-flag-css' => '7d347135', - 'phabricator-form-view-css' => '7d347135', - 'phabricator-header-view-css' => '7d347135', - 'phabricator-jump-nav' => '7d347135', + 'phabricator-filetree-view-css' => '23503cb9', + 'phabricator-flag-css' => '23503cb9', + 'phabricator-form-view-css' => '23503cb9', + 'phabricator-header-view-css' => '23503cb9', + 'phabricator-jump-nav' => '23503cb9', 'phabricator-keyboard-shortcut' => '58743fec', 'phabricator-keyboard-shortcut-manager' => '58743fec', - 'phabricator-main-menu-view' => '7d347135', + 'phabricator-main-menu-view' => '23503cb9', 'phabricator-menu-item' => '58743fec', - 'phabricator-nav-view-css' => '7d347135', + 'phabricator-nav-view-css' => '23503cb9', 'phabricator-notification' => '58743fec', - 'phabricator-notification-css' => '7d347135', - 'phabricator-notification-menu-css' => '7d347135', - 'phabricator-object-item-list-view-css' => '7d347135', + 'phabricator-notification-css' => '23503cb9', + 'phabricator-notification-menu-css' => '23503cb9', + 'phabricator-object-item-list-view-css' => '23503cb9', 'phabricator-object-selector-css' => 'ec01d039', 'phabricator-paste-file-upload' => '58743fec', 'phabricator-prefab' => '58743fec', 'phabricator-project-tag-css' => 'e30a3fa8', - 'phabricator-remarkup-css' => '7d347135', + 'phabricator-remarkup-css' => '23503cb9', 'phabricator-shaped-request' => '95d0d865', - 'phabricator-side-menu-view-css' => '7d347135', - 'phabricator-standard-page-view' => '7d347135', + 'phabricator-side-menu-view-css' => '23503cb9', + 'phabricator-standard-page-view' => '23503cb9', 'phabricator-textareautils' => '58743fec', 'phabricator-tooltip' => '58743fec', - 'phabricator-transaction-view-css' => '7d347135', - 'phabricator-zindex-css' => '7d347135', - 'sprite-apps-large-css' => '7d347135', - 'sprite-gradient-css' => '7d347135', - 'sprite-icon-css' => '7d347135', - 'sprite-menu-css' => '7d347135', - 'syntax-highlighting-css' => '7d347135', + 'phabricator-transaction-view-css' => '23503cb9', + 'phabricator-zindex-css' => '23503cb9', + 'sprite-apps-large-css' => '23503cb9', + 'sprite-gradient-css' => '23503cb9', + 'sprite-icon-css' => '23503cb9', + 'sprite-menu-css' => '23503cb9', + 'syntax-highlighting-css' => '23503cb9', ), )); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a1bc47e859..a738ea4e8a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -32,6 +32,7 @@ phutil_register_library_map(array( 'AphrontFileResponse' => 'aphront/response/AphrontFileResponse.php', 'AphrontFormCheckboxControl' => 'view/form/control/AphrontFormCheckboxControl.php', 'AphrontFormControl' => 'view/form/control/AphrontFormControl.php', + 'AphrontFormCropControl' => 'view/form/control/AphrontFormCropControl.php', 'AphrontFormDateControl' => 'view/form/control/AphrontFormDateControl.php', 'AphrontFormDividerControl' => 'view/form/control/AphrontFormDividerControl.php', 'AphrontFormDragAndDropUploadControl' => 'view/form/control/AphrontFormDragAndDropUploadControl.php', @@ -201,6 +202,8 @@ phutil_register_library_map(array( 'ConpherenceController' => 'applications/conpherence/controller/ConpherenceController.php', 'ConpherenceDAO' => 'applications/conpherence/storage/ConpherenceDAO.php', 'ConpherenceEditor' => 'applications/conpherence/editor/ConpherenceEditor.php', + 'ConpherenceFormDragAndDropUploadControl' => 'applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php', + 'ConpherenceImageData' => 'applications/conpherence/constants/ConpherenceImageData.php', 'ConpherenceListController' => 'applications/conpherence/controller/ConpherenceListController.php', 'ConpherenceMenuItemView' => 'applications/conpherence/view/ConpherenceMenuItemView.php', 'ConpherenceNewController' => 'applications/conpherence/controller/ConpherenceNewController.php', @@ -1534,6 +1537,7 @@ phutil_register_library_map(array( 'AphrontFileResponse' => 'AphrontResponse', 'AphrontFormCheckboxControl' => 'AphrontFormControl', 'AphrontFormControl' => 'AphrontView', + 'AphrontFormCropControl' => 'AphrontFormControl', 'AphrontFormDateControl' => 'AphrontFormControl', 'AphrontFormDividerControl' => 'AphrontFormControl', 'AphrontFormDragAndDropUploadControl' => 'AphrontFormControl', @@ -1689,6 +1693,8 @@ phutil_register_library_map(array( 'ConpherenceController' => 'PhabricatorController', 'ConpherenceDAO' => 'PhabricatorLiskDAO', 'ConpherenceEditor' => 'PhabricatorApplicationTransactionEditor', + 'ConpherenceFormDragAndDropUploadControl' => 'AphrontFormControl', + 'ConpherenceImageData' => 'ConpherenceConstants', 'ConpherenceListController' => 'ConpherenceController', 'ConpherenceMenuItemView' => 'AphrontTagView', 'ConpherenceNewController' => 'ConpherenceController', diff --git a/src/applications/conpherence/constants/ConpherenceImageData.php b/src/applications/conpherence/constants/ConpherenceImageData.php new file mode 100644 index 0000000000..618acb4b35 --- /dev/null +++ b/src/applications/conpherence/constants/ConpherenceImageData.php @@ -0,0 +1,11 @@ +getRequest()->getUser(); foreach ($conpherences as $conpherence) { $uri = $this->getApplicationURI('view/'.$conpherence->getID().'/'); - $data = $conpherence->getDisplayData($user); + $data = $conpherence->getDisplayData( + $user, + null + ); $title = $data['title']; $subtitle = $data['subtitle']; $unread_count = $data['unread_count']; @@ -220,6 +223,14 @@ abstract class ConpherenceController extends PhabricatorController { 'form_pane' => 'conpherence-form' ) ); + Javelin::initBehavior('conpherence-drag-and-drop-photo', + array( + 'target' => 'conpherence-header-pane', + 'form_pane' => 'conpherence-form', + 'upload_uri' => '/file/dropupload/', + 'activated_class' => 'conpherence-header-upload-photo', + ) + ); } } diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 2d8deee715..ad3bf86fa1 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -30,6 +30,8 @@ final class ConpherenceUpdateController extends $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) ->withIDs(array($conpherence_id)) + ->needOrigPics(true) + ->needHeaderPics(true) ->executeOne(); $supported_formats = PhabricatorFile::getTransformableImageFormats(); @@ -59,31 +61,63 @@ final class ConpherenceUpdateController extends break; case 'metadata': $xactions = array(); - $images = $request->getArr('image'); - if ($images) { - // just take the first one - $file_phid = reset($images); - $file = id(new PhabricatorFileQuery()) + $top = $request->getInt('image_y'); + $left = $request->getInt('image_x'); + $file_id = $request->getInt('file_id'); + if ($file_id) { + $orig_file = id(new PhabricatorFileQuery()) ->setViewer($user) - ->withPHIDs(array($file_phid)) + ->withIDs(array($file_id)) ->executeOne(); - $okay = $file->isTransformableImage(); + $okay = $orig_file->isTransformableImage(); if ($okay) { - $xformer = new PhabricatorImageTransformer(); - $xformed = $xformer->executeThumbTransform( - $file, - $x = 50, - $y = 50); - $image_phid = $xformed->getPHID(); $xactions[] = id(new ConpherenceTransaction()) ->setTransactionType(ConpherenceTransactionType::TYPE_PICTURE) - ->setNewValue($image_phid); + ->setNewValue($orig_file->getPHID()); + // do 2 transformations "crudely" + $xformer = new PhabricatorImageTransformer(); + $header_file = $xformer->executeConpherenceTransform( + $orig_file, + 0, + 0, + ConpherenceImageData::HEAD_WIDTH, + ConpherenceImageData::HEAD_HEIGHT + ); + // this is handled outside the editor for now. no particularly + // good reason to move it inside + $conpherence->setImagePHIDs( + array( + ConpherenceImageData::SIZE_HEAD => $header_file->getPHID(), + ) + ); + $conpherence->setImages( + array( + ConpherenceImageData::SIZE_HEAD => $header_file, + ) + ); } else { - $e_file[] = $file; + $e_file[] = $orig_file; $errors[] = pht('This server only supports these image formats: %s.', implode(', ', $supported_formats)); } + } else if ($top !== null || $left !== null) { + $file = $conpherence->getImage(ConpherenceImageData::SIZE_ORIG); + $xformer = new PhabricatorImageTransformer(); + $xformed = $xformer->executeConpherenceTransform( + $file, + $top, + $left, + ConpherenceImageData::HEAD_WIDTH, + ConpherenceImageData::HEAD_HEIGHT + ); + $image_phid = $xformed->getPHID(); + + $xactions[] = id(new ConpherenceTransaction()) + ->setTransactionType( + ConpherenceTransactionType::TYPE_PICTURE_CROP + ) + ->setNewValue($image_phid); } $title = $request->getStr('title'); if ($title != $conpherence->getTitle()) { @@ -131,24 +165,42 @@ final class ConpherenceUpdateController extends ->setLabel(pht('Title')) ->setName('title') ->setValue($conpherence->getTitle()) - ) - ->appendChild( - id(new AphrontFormMarkupControl()) - ->setLabel(pht('Image')) - ->setValue(phutil_tag( - 'img', - array( - 'src' => $conpherence->loadImageURI(), - )) - ) - ) - ->appendChild( - id(new AphrontFormDragAndDropUploadControl()) - ->setLabel(pht('Change Image')) - ->setName('image') - ->setValue($e_file) - ->setCaption('Supported formats: '.implode(', ', $supported_formats)) + ); + + $image = $conpherence->getImage(ConpherenceImageData::SIZE_ORIG); + if ($image) { + $form + ->appendChild( + id(new AphrontFormMarkupControl()) + ->setLabel(pht('Image')) + ->setValue(phutil_tag( + 'img', + array( + 'src' => + $conpherence->loadImageURI(ConpherenceImageData::SIZE_HEAD), + )) + ) + ) + ->appendChild( + id(new AphrontFormCropControl()) + ->setLabel(pht('Crop Image')) + ->setValue($image) + ->setWidth(ConpherenceImageData::HEAD_WIDTH) + ->setHeight(ConpherenceImageData::HEAD_HEIGHT) + ) + ->appendChild( + id(new ConpherenceFormDragAndDropUploadControl()) + ->setLabel(pht('Change Image')) + ); + + } else { + + $form + ->appendChild( + id(new ConpherenceFormDragAndDropUploadControl()) + ->setLabel(pht('Image')) ); + } require_celerity_resource('conpherence-update-css'); return id(new AphrontDialogResponse()) diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index f082ccf78c..b57ba021a2 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -45,6 +45,7 @@ final class ConpherenceViewController extends ->setViewer($user) ->withIDs(array($conpherence_id)) ->needWidgetData(true) + ->needHeaderPics(true) ->executeOne(); $this->setConpherence($conpherence); @@ -67,23 +68,34 @@ final class ConpherenceViewController extends require_celerity_resource('conpherence-header-pane-css'); $user = $this->getRequest()->getUser(); $conpherence = $this->getConpherence(); - $display_data = $conpherence->getDisplayData($user); + $display_data = $conpherence->getDisplayData( + $user, + ConpherenceImageData::SIZE_HEAD + ); $edit_href = $this->getApplicationURI('update/'.$conpherence->getID().'/'); + $class_mod = $display_data['image_class']; $header = + phutil_tag( + 'div', + array( + 'class' => 'upload-photo' + ), + pht('Drop photo here to change this Conpherence photo.') + ). javelin_tag( 'a', array( 'class' => 'edit', 'href' => $edit_href, - 'sigil' => 'workflow', + 'sigil' => 'workflow edit-action', ), '' ). phutil_tag( 'div', array( - 'class' => 'header-image', + 'class' => $class_mod.'header-image', 'style' => 'background-image: url('.$display_data['image'].');' ), '' @@ -91,14 +103,14 @@ final class ConpherenceViewController extends phutil_tag( 'div', array( - 'class' => 'title', + 'class' => $class_mod.'title', ), $display_data['title'] ). phutil_tag( 'div', array( - 'class' => 'subtitle', + 'class' => $class_mod.'subtitle', ), $display_data['subtitle'] ); diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index a5b42b93ce..feabfd810b 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -48,6 +48,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $types[] = ConpherenceTransactionType::TYPE_TITLE; $types[] = ConpherenceTransactionType::TYPE_PICTURE; + $types[] = ConpherenceTransactionType::TYPE_PICTURE_CROP; $types[] = ConpherenceTransactionType::TYPE_PARTICIPANTS; $types[] = ConpherenceTransactionType::TYPE_FILES; @@ -62,7 +63,9 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { case ConpherenceTransactionType::TYPE_TITLE: return $object->getTitle(); case ConpherenceTransactionType::TYPE_PICTURE: - return $object->getImagePHID(); + return $object->getImagePHID(ConpherenceImageData::SIZE_ORIG); + case ConpherenceTransactionType::TYPE_PICTURE_CROP: + return $object->getImagePHID(ConpherenceImageData::SIZE_HEAD); case ConpherenceTransactionType::TYPE_PARTICIPANTS: return $object->getParticipantPHIDs(); case ConpherenceTransactionType::TYPE_FILES: @@ -77,6 +80,7 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { switch ($xaction->getTransactionType()) { case ConpherenceTransactionType::TYPE_TITLE: case ConpherenceTransactionType::TYPE_PICTURE: + case ConpherenceTransactionType::TYPE_PICTURE_CROP: return $xaction->getNewValue(); case ConpherenceTransactionType::TYPE_PARTICIPANTS: case ConpherenceTransactionType::TYPE_FILES: @@ -93,7 +97,16 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $object->setTitle($xaction->getNewValue()); break; case ConpherenceTransactionType::TYPE_PICTURE: - $object->setImagePHID($xaction->getNewValue()); + $object->setImagePHID( + $xaction->getNewValue(), + ConpherenceImageData::SIZE_ORIG + ); + break; + case ConpherenceTransactionType::TYPE_PICTURE_CROP: + $object->setImagePHID( + $xaction->getNewValue(), + ConpherenceImageData::SIZE_HEAD + ); break; } } diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index d22fd03820..e21951cf09 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -9,6 +9,18 @@ final class ConpherenceThreadQuery private $phids; private $ids; private $needWidgetData; + private $needHeaderPics; + private $needOrigPics; + + public function needOrigPics($need_orig_pics) { + $this->needOrigPics = $need_orig_pics; + return $this; + } + + public function needHeaderPics($need_header_pics) { + $this->needHeaderPics = $need_header_pics; + return $this; + } public function needWidgetData($need_widget_data) { $this->needWidgetData = $need_widget_data; @@ -47,6 +59,12 @@ final class ConpherenceThreadQuery if ($this->needWidgetData) { $this->loadWidgetData($conpherences); } + if ($this->needOrigPics) { + $this->loadOrigPics($conpherences); + } + if ($this->needHeaderPics) { + $this->loadHeaderPics($conpherences); + } } return $conpherences; @@ -187,4 +205,39 @@ final class ConpherenceThreadQuery return $this; } + private function loadOrigPics(array $conpherences) { + return $this->loadPics( + $conpherences, + ConpherenceImageData::SIZE_ORIG + ); + } + + private function loadHeaderPics(array $conpherences) { + return $this->loadPics( + $conpherences, + ConpherenceImageData::SIZE_HEAD + ); + } + + private function loadPics(array $conpherences, $size) { + $conpherence_pic_phids = array(); + foreach ($conpherences as $conpherence) { + $phid = $conpherence->getImagePHID($size); + if ($phid) { + $conpherence_pic_phids[$conpherence->getPHID()] = $phid; + } + } + $files = id(new PhabricatorFileQuery()) + ->setViewer($this->getViewer()) + ->withPHIDs($conpherence_pic_phids) + ->execute(); + $files = mpull($files, null, 'getPHID'); + + foreach ($conpherence_pic_phids as $conpherence_phid => $pic_phid) { + $conpherences[$conpherence_phid]->setImage($files[$pic_phid], $size); + } + + return $this; + } + } diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index 1b80aec1a5..e824ad6207 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -9,7 +9,7 @@ final class ConpherenceThread extends ConpherenceDAO protected $id; protected $phid; protected $title; - protected $imagePHID; + protected $imagePHIDs = array(); protected $mailKey; private $participants; @@ -17,10 +17,14 @@ final class ConpherenceThread extends ConpherenceDAO private $handles; private $filePHIDs; private $widgetData; + private $images = array(); public function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, + self::CONFIG_SERIALIZATION => array( + 'imagePHIDs' => self::SERIALIZATION_JSON, + ), ) + parent::getConfiguration(); } @@ -36,6 +40,33 @@ final class ConpherenceThread extends ConpherenceDAO return parent::save(); } + public function getImagePHID($size) { + $image_phids = $this->getImagePHIDs(); + return idx($image_phids, $size); + } + public function setImagePHID($phid, $size) { + $image_phids = $this->getImagePHIDs(); + $image_phids[$size] = $phid; + return $this->setImagePHIDs($image_phids); + } + + public function getImage($size) { + $images = $this->getImages(); + return idx($images, $size); + } + public function setImage(PhabricatorFile $file, $size) { + $files = $this->getImages(); + $files[$size] = $file; + return $this->setImages($files); + } + public function setImages(array $files) { + $this->images = $files; + return $this; + } + private function getImages() { + return $this->images; + } + public function attachParticipants(array $participants) { assert_instances_of($participants, 'ConpherenceParticipant'); $this->participants = $participants; @@ -112,59 +143,36 @@ final class ConpherenceThread extends ConpherenceDAO return $this->widgetData; } - public function loadImageURI() { - $src_phid = $this->getImagePHID(); + public function loadImageURI($size) { + $file = $this->getImage($size); - if ($src_phid) { - $file = id(new PhabricatorFile())->loadOneWhere('phid = %s', $src_phid); - if ($file) { - return $file->getBestURI(); - } + if ($file) { + return $file->getBestURI(); } return PhabricatorUser::getDefaultProfileImageURI(); } - public function getDisplayData(PhabricatorUser $user) { + public function getDisplayData(PhabricatorUser $user, $size) { $transactions = $this->getTransactions(); - $latest_transaction = end($transactions); - $latest_participant = $latest_transaction->getAuthorPHID(); - $handles = $this->getHandles(); - $latest_handle = $handles[$latest_participant]; - if ($this->getImagePHID()) { - $img_src = $this->loadImageURI(); - } else { - $img_src = $latest_handle->getImageURI(); - } - $title = $this->getTitle(); - if (!$title) { - $title = $latest_handle->getName(); - unset($handles[$latest_participant]); - } - unset($handles[$user->getPHID()]); - $subtitle = ''; - $count = 0; - $final = false; - foreach ($handles as $handle) { - if ($handle->getType() != PhabricatorPHIDConstants::PHID_TYPE_USER) { - continue; - } - if ($subtitle) { - if ($final) { - $subtitle .= '...'; - break; - } else { - $subtitle .= ', '; - } - } - $subtitle .= $handle->getName(); - $count++; - $final = $count == 3; + $handles = $this->getHandles(); + // we don't want to show the user unless they are babbling to themselves + if (count($handles) > 1) { + unset($handles[$user->getPHID()]); } $participants = $this->getParticipants(); $user_participation = $participants[$user->getPHID()]; + $latest_transaction = null; + $title = $this->getTitle(); + $subtitle = ''; + $img_src = null; + $img_class = null; + if ($this->getImagePHID($size)) { + $img_src = $this->getImage($size)->getBestURI(); + $img_class = 'custom-'; + } $unread_count = 0; $max_count = 10; $snippet = null; @@ -186,9 +194,28 @@ final class ConpherenceThread extends ConpherenceDAO $transaction->getComment()->getContent(), 48 ); + if ($transaction->getAuthorPHID() == $user->getPHID()) { + $snippet = "\xE2\x86\xB0 " . $snippet; + } } // fallthrough intentionally here case ConpherenceTransactionType::TYPE_FILES: + if (!$latest_transaction) { + $latest_transaction = $transaction; + } + $latest_participant_phid = $transaction->getAuthorPHID(); + if ((!$title || !$img_src) && + $latest_participant_phid != $user->getPHID()) { + $latest_handle = $handles[$latest_participant_phid]; + if (!$img_src) { + $img_src = $latest_handle->getImageURI(); + } + if (!$title) { + $title = $latest_handle->getName(); + // (maybs) used the pic, definitely used the name -- discard + unset($handles[$latest_participant_phid]); + } + } if ($behind_transaction_phid) { $unread_count++; if ($transaction->getPHID() == $behind_transaction_phid) { @@ -210,12 +237,45 @@ final class ConpherenceThread extends ConpherenceDAO $unread_count = $max_count.'+'; } + // This happens if the user has been babbling, maybs just to themselves, + // but enough un-responded to transactions for our SQL limit would + // hit this too... Also happens on new threads since only the first + // author has participated. + // ...so just pick a different handle in these cases. + $some_handle = reset($handles); + if (!$img_src) { + $img_src = $some_handle->getImageURI(); + } + if (!$title) { + $title = $some_handle->getName(); + } + + $count = 0; + $final = false; + foreach ($handles as $handle) { + if ($handle->getType() != PhabricatorPHIDConstants::PHID_TYPE_USER) { + continue; + } + if ($subtitle) { + if ($final) { + $subtitle .= '...'; + break; + } else { + $subtitle .= ', '; + } + } + $subtitle .= $handle->getName(); + $count++; + $final = $count == 3; + } + return array( 'title' => $title, 'subtitle' => $subtitle, 'unread_count' => $unread_count, 'epoch' => $latest_transaction->getDateCreated(), 'image' => $img_src, + 'image_class' => $img_class, 'snippet' => $snippet, ); } diff --git a/src/applications/conpherence/storage/ConpherenceTransaction.php b/src/applications/conpherence/storage/ConpherenceTransaction.php index 9b242275c6..22a4aa666e 100644 --- a/src/applications/conpherence/storage/ConpherenceTransaction.php +++ b/src/applications/conpherence/storage/ConpherenceTransaction.php @@ -31,6 +31,7 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { case ConpherenceTransactionType::TYPE_PICTURE: return false; case ConpherenceTransactionType::TYPE_FILES: + case ConpherenceTransactionType::TYPE_PICTURE_CROP: return true; } @@ -107,12 +108,10 @@ final class ConpherenceTransaction extends PhabricatorApplicationTransaction { $phids[] = $this->getAuthorPHID(); switch ($this->getTransactionType()) { case ConpherenceTransactionType::TYPE_PICTURE: - $phids[] = $new; - break; case ConpherenceTransactionType::TYPE_TITLE: + case ConpherenceTransactionType::TYPE_FILES: break; case ConpherenceTransactionType::TYPE_PARTICIPANTS: - case ConpherenceTransactionType::TYPE_FILES: $phids = array_merge($phids, $this->getOldValue()); $phids = array_merge($phids, $this->getNewValue()); break; diff --git a/src/applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php b/src/applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php new file mode 100644 index 0000000000..032d8a9aa7 --- /dev/null +++ b/src/applications/conpherence/view/ConpherenceFormDragAndDropUploadControl.php @@ -0,0 +1,42 @@ +dropID = $drop_id; + return $this; + } + public function getDropID() { + return $this->dropID; + } + + protected function getCustomControlClass() { + return null; + } + + protected function renderInput() { + + $drop_id = celerity_generate_unique_node_id(); + Javelin::initBehavior('conpherence-drag-and-drop-photo', + array( + 'target' => $drop_id, + 'form_pane' => 'conpherence-form', + 'upload_uri' => '/file/dropupload/', + 'activated_class' => 'conpherence-dialogue-upload-photo', + ) + ); + require_celerity_resource('conpherence-update-css'); + + return phutil_tag( + 'div', + array( + 'id' => $drop_id, + 'class' => 'conpherence-dialogue-drag-photo', + ), + pht('Drag and drop an image here to upload it.') + ); + } + +} diff --git a/src/applications/conpherence/view/ConpherenceTransactionView.php b/src/applications/conpherence/view/ConpherenceTransactionView.php index d8163b3ecd..72f4b6656b 100644 --- a/src/applications/conpherence/view/ConpherenceTransactionView.php +++ b/src/applications/conpherence/view/ConpherenceTransactionView.php @@ -38,23 +38,14 @@ final class ConpherenceTransactionView extends AphrontView { $content_class = null; switch ($transaction->getTransactionType()) { case ConpherenceTransactionType::TYPE_TITLE: + case ConpherenceTransactionType::TYPE_PICTURE: + case ConpherenceTransactionType::TYPE_PICTURE_CROP: $content = $transaction->getTitle(); $transaction_view->addClass('conpherence-edited'); break; case ConpherenceTransactionType::TYPE_FILES: $content = $transaction->getTitle(); break; - case ConpherenceTransactionType::TYPE_PICTURE: - $img = $transaction->getHandle($transaction->getNewValue()); - $content = $transaction->getTitle() . - phutil_tag( - 'img', - array( - 'src' => $img->getImageURI() - ) - ); - $transaction_view->addClass('conpherence-edited'); - break; case ConpherenceTransactionType::TYPE_PARTICIPANTS: $content = $transaction->getTitle(); $transaction_view->addClass('conpherence-edited'); diff --git a/src/applications/files/PhabricatorImageTransformer.php b/src/applications/files/PhabricatorImageTransformer.php index a073d14030..8e651eba69 100644 --- a/src/applications/files/PhabricatorImageTransformer.php +++ b/src/applications/files/PhabricatorImageTransformer.php @@ -56,6 +56,29 @@ final class PhabricatorImageTransformer { )); } + public function executeConpherenceTransform( + PhabricatorFile $file, + $top, + $left, + $width, + $height + ) { + + $image = $this->crasslyCropTo( + $file, + $top, + $left, + $width, + $height + ); + + return PhabricatorFile::newFromFileData( + $image, + array( + 'name' => 'conpherence-'.$file->getName(), + ) + ); + } private function crudelyCropTo(PhabricatorFile $file, $x, $min_y, $max_y) { $data = $file->loadFileData(); @@ -80,6 +103,30 @@ final class PhabricatorImageTransformer { return $this->saveImageDataInAnyFormat($img, $file->getMimeType()); } + private function crasslyCropTo(PhabricatorFile $file, $top, $left, $w, $h) { + $data = $file->loadFileData(); + $src = imagecreatefromstring($data); + $dst = $this->getBlankDestinationFile($w, $h); + + $scale = self::getScaleForCrop($file, $w, $h); + $orig_x = $left / $scale; + $orig_y = $top / $scale; + $orig_w = $w / $scale; + $orig_h = $h / $scale; + + imagecopyresampled( + $dst, + $src, + 0, 0, + $orig_x, $orig_y, + $w, $h, + $orig_w, $orig_h + ); + + return $this->saveImageDataInAnyFormat($dst, $file->getMimeType()); + } + + /** * Very crudely scale an image up or down to an exact size. */ @@ -92,15 +139,21 @@ final class PhabricatorImageTransformer { return $this->saveImageDataInAnyFormat($dst, $file->getMimeType()); } + private function getBlankDestinationFile($dx, $dy) { + $dst = imagecreatetruecolor($dx, $dy); + imagesavealpha($dst, true); + imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127)); + + return $dst; + } + private function applyScaleTo($src, $dx, $dy) { $x = imagesx($src); $y = imagesy($src); $scale = min(($dx / $x), ($dy / $y), 1); - $dst = imagecreatetruecolor($dx, $dy); - imagesavealpha($dst, true); - imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127)); + $dst = $this->getBlankDestinationFile($dx, $dy); $sdx = $scale * $x; $sdy = $scale * $y; @@ -141,6 +194,27 @@ final class PhabricatorImageTransformer { ); } + public static function getScaleForCrop( + PhabricatorFile $file, + $des_width, + $des_height) { + + $metadata = $file->getMetadata(); + $width = $metadata[PhabricatorFile::METADATA_IMAGE_WIDTH]; + $height = $metadata[PhabricatorFile::METADATA_IMAGE_HEIGHT]; + + if ($height < $des_height) { + $scale = $height / $des_height; + } else if ($width < $des_width) { + $scale = $width / $des_width; + } else { + $scale_x = $des_width / $width; + $scale_y = $des_height / $height; + $scale = max($scale_x, $scale_y); + } + + return $scale; + } private function generatePreview(PhabricatorFile $file, $size) { $data = $file->loadFileData(); $src = imagecreatefromstring($data); @@ -153,9 +227,7 @@ final class PhabricatorImageTransformer { $sdx = $dimensions['sdx']; $sdy = $dimensions['sdy']; - $dst = imagecreatetruecolor($dx, $dy); - imagesavealpha($dst, true); - imagefill($dst, 0, 0, imagecolorallocatealpha($dst, 255, 255, 255, 127)); + $dst = $this->getBlankDestinationFile($dx, $dy); imagecopyresampled( $dst, diff --git a/src/applications/phid/handle/PhabricatorObjectHandleData.php b/src/applications/phid/handle/PhabricatorObjectHandleData.php index 52c1bb7d0c..67eaaf8e63 100644 --- a/src/applications/phid/handle/PhabricatorObjectHandleData.php +++ b/src/applications/phid/handle/PhabricatorObjectHandleData.php @@ -413,9 +413,6 @@ final class PhabricatorObjectHandleData { $handle->setName($file->getName()); $handle->setURI($file->getBestURI()); $handle->setComplete(true); - if ($file->isViewableImage()) { - $handle->setImageURI($file->getBestURI()); - } } $handles[$phid] = $handle; } diff --git a/src/view/form/control/AphrontFormCropControl.php b/src/view/form/control/AphrontFormCropControl.php new file mode 100644 index 0000000000..26cb70c0e4 --- /dev/null +++ b/src/view/form/control/AphrontFormCropControl.php @@ -0,0 +1,99 @@ +height = $height; + return $this; + } + public function getHeight() { + return $this->height; + } + + public function setWidth($width) { + $this->width = $width; + return $this; + } + public function getWidth() { + return $this->width; + } + + protected function getCustomControlClass() { + return 'aphront-form-crop'; + } + + protected function renderInput() { + $file = $this->getValue(); + + if ($file === null) { + return phutil_render_tag( + 'img', + array( + 'src' => PhabricatorUser::getDefaultProfileImageURI() + ), + '' + ); + } + + $c_id = celerity_generate_unique_node_id(); + $metadata = $file->getMetadata(); + $scale = PhabricatorImageTransformer::getScaleForCrop( + $file, + $this->getWidth(), + $this->getHeight() + ); + + Javelin::initBehavior( + 'aphront-crop', + array( + 'cropBoxID' => $c_id, + 'width' => $this->getWidth(), + 'height' => $this->getHeight(), + 'scale' => $scale, + 'imageH' => $metadata[PhabricatorFile::METADATA_IMAGE_HEIGHT], + 'imageW' => $metadata[PhabricatorFile::METADATA_IMAGE_WIDTH], + ) + ); + + return javelin_render_tag( + 'div', + array( + 'id' => $c_id, + 'sigil' => 'crop-box', + 'mustcapture' => true, + 'class' => 'crop-box' + ), + javelin_render_tag( + 'img', + array( + 'src' => $file->getBestURI(), + 'class' => 'crop-image', + 'sigil' => 'crop-image' + ), + '' + ). + javelin_render_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => 'image_x', + 'sigil' => 'crop-x', + ), + '' + ). + javelin_render_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => 'image_y', + 'sigil' => 'crop-y', + ), + '' + ) + ); + } + +} diff --git a/webroot/rsrc/css/aphront/form-view.css b/webroot/rsrc/css/aphront/form-view.css index 52dc73e307..abca6a6b2d 100644 --- a/webroot/rsrc/css/aphront/form-view.css +++ b/webroot/rsrc/css/aphront/form-view.css @@ -231,6 +231,18 @@ table.aphront-form-control-checkbox-layout th { border-color: #669966; } +.aphront-form-crop .crop-box { + cursor: move; + overflow: hidden; +} + +.aphront-form-crop .crop-box .crop-image { + position: relative; + top: 0px; + left: 0px; +} + + .calendar-button { display: inline; background: url(/rsrc/image/icon/fatcow/calendar_edit.png) diff --git a/webroot/rsrc/css/application/conpherence/header-pane.css b/webroot/rsrc/css/application/conpherence/header-pane.css index d007091de3..27198a2879 100644 --- a/webroot/rsrc/css/application/conpherence/header-pane.css +++ b/webroot/rsrc/css/application/conpherence/header-pane.css @@ -29,6 +29,14 @@ width: 50px; } +.conpherence-header-pane .custom-header-image { + position: absolute; + top: 0px; + left: 0px; + height: 80px; + width: 120px; +} + .conpherence-header-pane .title { position: relative; font-size: 16px; @@ -39,6 +47,16 @@ overflow-x: auto; } +.conpherence-header-pane .custom-title { + position: relative; + font-size: 16px; + font-weight: bold; + left: 132px; + top: 21px; + max-width: 80%; + overflow-x: auto; +} + .conpherence-header-pane .subtitle { position: relative; left: 77px; @@ -47,4 +65,34 @@ max-width: 80%; } +.conpherence-header-pane .custom-subtitle { + position: relative; + left: 132px; + top: 21px; + color: #bfbfbf; + max-width: 80%; +} + +.conpherence-header-pane .upload-photo { + display: none; +} + +.conpherence-header-upload-photo .upload-photo { + display: block; + width: 100%; + padding: 32px; + font-size: 16px; + background: #99ff99; + border-color: #669966; +} + +.conpherence-header-upload-photo .edit, +.conpherence-header-upload-photo .header-image, +.conpherence-header-upload-photo .custom-header-image, +.conpherence-header-upload-photo .title, +.conpherence-header-upload-photo .custom-title, +.conpherence-header-upload-photo .subtitle, +.conpherence-header-upload-photo .custom-subtitle { + display: none; +} diff --git a/webroot/rsrc/css/application/conpherence/update.css b/webroot/rsrc/css/application/conpherence/update.css index fcf26c9674..bcd26fb4b4 100644 --- a/webroot/rsrc/css/application/conpherence/update.css +++ b/webroot/rsrc/css/application/conpherence/update.css @@ -5,3 +5,13 @@ .phabricator-standard-page-body .aphront-dialog-view { margin: 20px auto 0px auto; } + +.aphront-dialog-view .conpherence-dialogue-drag-photo { + border: 1px dashed #bfbfbf; + padding: 10px 0px 10px 10px; +} + +.aphront-dialog-view .conpherence-dialogue-upload-photo { + background: #99ff99; + border-color: #669966; +} diff --git a/webroot/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js b/webroot/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js new file mode 100644 index 0000000000..a19bba029f --- /dev/null +++ b/webroot/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js @@ -0,0 +1,39 @@ +/** + * @provides javelin-behavior-conpherence-drag-and-drop-photo + * @requires javelin-behavior + * javelin-dom + * javelin-workflow + * phabricator-drag-and-drop-file-upload + */ + +JX.behavior('conpherence-drag-and-drop-photo', function(config) { + + var target = JX.$(config.target); + var form_pane = JX.$(config.form_pane); + + function onupload(f) { + var data = { + 'file_id' : f.getID(), + 'action' : 'metadata' + }; + + var form = JX.DOM.find(form_pane, 'form'); + var workflow = JX.Workflow.newFromForm(form, data); + workflow.start(); + } + + if (JX.PhabricatorDragAndDropFileUpload.isSupported()) { + var drop = new JX.PhabricatorDragAndDropFileUpload(target) + .setURI(config.upload_uri); + drop.listen('didBeginDrag', function(e) { + JX.DOM.alterClass(target, config.activated_class, true); + }); + drop.listen('didEndDrag', function(e) { + JX.DOM.alterClass(target, config.activated_class, false); + }); + drop.listen('didUpload', onupload); + drop.start(); + } + +}); + diff --git a/webroot/rsrc/js/application/core/behavior-crop.js b/webroot/rsrc/js/application/core/behavior-crop.js new file mode 100644 index 0000000000..965e3336f2 --- /dev/null +++ b/webroot/rsrc/js/application/core/behavior-crop.js @@ -0,0 +1,94 @@ +/** + * @provides javelin-behavior-aphront-crop + * @requires javelin-behavior + * javelin-dom + * javelin-vector + * javelin-magical-init + */ + + JX.behavior('aphront-crop', function(config) { + + var dragging = false; + var startX, startY; + var finalX, finalY; + var dScale = config.scale; + + var cropBox = JX.$(config.cropBoxID); + var basePos = JX.$V(cropBox); + cropBox.style.height = config.height + 'px'; + cropBox.style.width = config.width + 'px'; + var baseD = JX.$V(config.width, config.height); + + var image = JX.DOM.find(cropBox, 'img', 'crop-image'); + image.style.height = (config.imageH * config.scale) + 'px'; + image.style.width = (config.imageW * config.scale) + 'px'; + var imageD = JX.$V( + config.imageW * config.scale, + config.imageH * config.scale + ); + var minLeft = baseD.x - imageD.x; + var minTop = baseD.y - imageD.y; + + var minScale = Math.min( + config.width / config.imageW, + config.height / config.imageH, + 1 + ); + var maxScale = Math.max( + config.imageW / config.width, + config.imageH / config.height, + 2 + ); + + var ondrag = function(e) { + e.kill(); + dragging = true; + var p = JX.$V(e); + startX = p.x; + startY = p.y; + }; + + var onmove = function(e) { + if (!dragging) { + return; + } + e.kill(); + + var p = JX.$V(e); + var dx = startX - p.x; + var dy = startY - p.y; + var imagePos = JX.$V(image); + var moveLeft = imagePos.x - basePos.x - dx; + var moveTop = imagePos.y - basePos.y - dy; + + image.style.left = Math.min(Math.max(minLeft, moveLeft), 0) + 'px'; + image.style.top = Math.min(Math.max(minTop, moveTop), 0) + 'px'; + + // reset these; a new beginning! + startX = p.x; + startY = p.y; + + // save off where we are right now + imagePos = JX.$V(image); + finalX = Math.abs(imagePos.x - basePos.x); + finalY = Math.abs(imagePos.y - basePos.y); + JX.DOM.find(cropBox, 'input', 'crop-x').value = finalX; + JX.DOM.find(cropBox, 'input', 'crop-y').value = finalY; + }; + + var ondrop = function(e) { + if (!dragging) { + return; + } + dragging = false; + }; + + // NOTE: Javelin does not dispatch mousemove by default. + JX.enableDispatch(cropBox, 'mousemove'); + + JX.DOM.listen(cropBox, 'mousedown', [], ondrag); + JX.DOM.listen(cropBox, 'mousemove', [], onmove); + JX.DOM.listen(cropBox, 'mouseup', [], ondrop); + JX.DOM.listen(cropBox, 'mouseout', [], ondrop); + +}); From 6a23cc1677f5c0c15e409605c6cda4854b739a19 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 6 Feb 2013 14:05:11 -0800 Subject: [PATCH 14/19] fix derp from rP84efcb86698a00f863dbdb4cbd4c149c96e498dc i missed --- src/view/layout/PhabricatorMenuView.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/layout/PhabricatorMenuView.php b/src/view/layout/PhabricatorMenuView.php index ab2a53784c..c350d00351 100644 --- a/src/view/layout/PhabricatorMenuView.php +++ b/src/view/layout/PhabricatorMenuView.php @@ -29,7 +29,7 @@ final class PhabricatorMenuView extends AphrontTagView { ->setHref($href); if ($key !== null) { - $key->setKey($key); + $item->setKey($key); } $this->addMenuItem($item); From 68814d4ecab077124f7456ffcfd622ba7b355bee Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 6 Feb 2013 15:28:24 -0800 Subject: [PATCH 15/19] add conpherence schema patch to the list so it runs Summary: ...i tend to forget to do this for some reason. my bad. Test Plan: NA Reviewers: epriestley, chad Reviewed By: chad CC: aran, Korvin Maniphest Tasks: T2503 Differential Revision: https://secure.phabricator.com/D4840 --- .../storage/patch/PhabricatorBuiltinPatchList.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index 71dbe66291..029f30fe0a 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1105,6 +1105,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130201.revisionunsubscribed.sql'), ), + '20130131.conpherencepics.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130131.conpherencepics.sql'), + ), ); } From 5b39bbe71b20c3ac2cc2024da81bc601adecccc4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 6 Feb 2013 17:53:51 -0800 Subject: [PATCH 16/19] Batch Conpherence access to markup cache Summary: Bulk process markup instead of doing them one at a time. Fixes T2504. Test Plan: Viewed service profile, saw a single call for all the cache entries. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2504 Differential Revision: https://secure.phabricator.com/D4844 --- .../controller/ConpherenceViewController.php | 13 ++++++++++++ .../view/ConpherenceTransactionView.php | 20 +++++++++---------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index b57ba021a2..89994c8f7f 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -126,6 +126,18 @@ final class ConpherenceViewController extends $rendered_transactions = array(); $transactions = $conpherence->getTransactions(); + + $engine = id(new PhabricatorMarkupEngine()) + ->setViewer($user); + foreach ($transactions as $transaction) { + if ($transaction->getComment()) { + $engine->addObject( + $transaction->getComment(), + PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); + } + } + $engine->process(); + foreach ($transactions as $transaction) { if ($transaction->shouldHide()) { continue; @@ -134,6 +146,7 @@ final class ConpherenceViewController extends ->setUser($user) ->setConpherenceTransaction($transaction) ->setHandles($handles) + ->setMarkupEngine($engine) ->render(); } $transactions = implode(' ', $rendered_transactions); diff --git a/src/applications/conpherence/view/ConpherenceTransactionView.php b/src/applications/conpherence/view/ConpherenceTransactionView.php index 72f4b6656b..92a7984c8c 100644 --- a/src/applications/conpherence/view/ConpherenceTransactionView.php +++ b/src/applications/conpherence/view/ConpherenceTransactionView.php @@ -7,6 +7,12 @@ final class ConpherenceTransactionView extends AphrontView { private $conpherenceTransaction; private $handles; + private $markupEngine; + + public function setMarkupEngine(PhabricatorMarkupEngine $markup_engine) { + $this->markupEngine = $markup_engine; + return $this; + } public function setHandles(array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); @@ -36,6 +42,7 @@ final class ConpherenceTransactionView extends AphrontView { ->setContentSource($transaction->getContentSource()); $content_class = null; + $content = null; switch ($transaction->getTransactionType()) { case ConpherenceTransactionType::TYPE_TITLE: case ConpherenceTransactionType::TYPE_PICTURE: @@ -56,18 +63,9 @@ final class ConpherenceTransactionView extends AphrontView { PhabricatorMarkupEngine::extractFilePHIDsFromEmbeddedFiles( array($comment->getContent()) ); - $markup_field = ConpherenceTransactionComment::MARKUP_FIELD_COMMENT; - $engine = id(new PhabricatorMarkupEngine()) - ->setViewer($this->getUser()); - $engine->addObject( + $content = $this->markupEngine->getOutput( $comment, - $markup_field - ); - $engine->process(); - $content = $engine->getOutput( - $comment, - $markup_field - ); + PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT); $content_class = 'conpherence-message phabricator-remarkup'; $transaction_view ->setImageURI($author->getImageURI()) From 4004621d76bea2e1cd0b4ef4bebeea77e29b5d11 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Feb 2013 06:26:36 -0800 Subject: [PATCH 17/19] Make Pholio selection reticle prettier Summary: The selection reticle in Pholio is functional but not very pretty right now. Make it look a little nicer. - Using `box-sizing: border-box;` allows us to get rid of the `x - 1` and `y - 2` stuff. - I draw the reticle with two elements: one mostly-transparent which creates a fill, and one fully opaque to create a strong dashed border. Test Plan: {F31803} Reviewers: ljalonen, chad Reviewed By: chad CC: aran, kchr Differential Revision: https://secure.phabricator.com/D4836 --- src/__celerity_resource_map__.php | 119 +++++++----------- .../rsrc/css/application/pholio/pholio.css | 15 ++- .../pholio/behavior-pholio-mock-view.js | 62 +++++---- 3 files changed, 95 insertions(+), 101 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index cad7602664..b245c6583f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -639,7 +639,7 @@ celerity_register_resource_map(array( ), 'aphront-form-view-css' => array( - 'uri' => '/res/ec323d34/rsrc/css/aphront/form-view.css', + 'uri' => '/res/428fbcd8/rsrc/css/aphront/form-view.css', 'type' => 'css', 'requires' => array( @@ -748,7 +748,7 @@ celerity_register_resource_map(array( ), 'conpherence-header-pane-css' => array( - 'uri' => '/res/11c32adc/rsrc/css/application/conpherence/header-pane.css', + 'uri' => '/res/4b8aebd2/rsrc/css/application/conpherence/header-pane.css', 'type' => 'css', 'requires' => array( @@ -775,7 +775,7 @@ celerity_register_resource_map(array( ), 'conpherence-update-css' => array( - 'uri' => '/res/92094ed7/rsrc/css/application/conpherence/update.css', + 'uri' => '/res/8e4757b5/rsrc/css/application/conpherence/update.css', 'type' => 'css', 'requires' => array( @@ -1042,19 +1042,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/behavior-tokenizer.js', ), - 'javelin-behavior-aphront-crop' => - array( - 'uri' => '/res/cda1eace/rsrc/js/application/core/behavior-crop.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-vector', - 3 => 'javelin-magical-init', - ), - 'disk' => '/rsrc/js/application/core/behavior-crop.js', - ), 'javelin-behavior-aphront-drag-and-drop' => array( 'uri' => '/res/3d809b40/rsrc/js/application/core/behavior-drag-and-drop.js', @@ -1119,19 +1106,6 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/diffusion/behavior-audit-preview.js', ), - 'javelin-behavior-conpherence-drag-and-drop-photo' => - array( - 'uri' => '/res/9e3eb1cd/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js', - 'type' => 'js', - 'requires' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-workflow', - 3 => 'phabricator-drag-and-drop-file-upload', - ), - 'disk' => '/rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js', - ), 'javelin-behavior-conpherence-init' => array( 'uri' => '/res/bd911b43/rsrc/js/application/conpherence/behavior-init.js', @@ -1889,7 +1863,7 @@ celerity_register_resource_map(array( ), 'javelin-behavior-pholio-mock-view' => array( - 'uri' => '/res/5dd61f73/rsrc/js/application/pholio/behavior-pholio-mock-view.js', + 'uri' => '/res/518a169e/rsrc/js/application/pholio/behavior-pholio-mock-view.js', 'type' => 'js', 'requires' => array( @@ -1897,7 +1871,8 @@ celerity_register_resource_map(array( 1 => 'javelin-stratcom', 2 => 'javelin-dom', 3 => 'javelin-vector', - 4 => 'javelin-event', + 4 => 'javelin-magical-init', + 5 => 'javelin-request', ), 'disk' => '/rsrc/js/application/pholio/behavior-pholio-mock-view.js', ), @@ -3232,7 +3207,7 @@ celerity_register_resource_map(array( ), 'pholio-css' => array( - 'uri' => '/res/595f2721/rsrc/css/application/pholio/pholio.css', + 'uri' => '/res/9de6e0b2/rsrc/css/application/pholio/pholio.css', 'type' => 'css', 'requires' => array( @@ -3422,7 +3397,7 @@ celerity_register_resource_map(array( ), array( 'packages' => array( - '23503cb9' => + '7d347135' => array( 'name' => 'core.pkg.css', 'symbols' => @@ -3466,7 +3441,7 @@ celerity_register_resource_map(array( 36 => 'phabricator-object-item-list-view-css', 37 => 'global-drag-and-drop-css', ), - 'uri' => '/res/pkg/23503cb9/core.pkg.css', + 'uri' => '/res/pkg/7d347135/core.pkg.css', 'type' => 'css', ), '58743fec' => @@ -3656,19 +3631,19 @@ celerity_register_resource_map(array( 'reverse' => array( 'aphront-attached-file-view-css' => 'e30a3fa8', - 'aphront-crumbs-view-css' => '23503cb9', - 'aphront-dialog-view-css' => '23503cb9', - 'aphront-error-view-css' => '23503cb9', - 'aphront-form-view-css' => '23503cb9', + 'aphront-crumbs-view-css' => '7d347135', + 'aphront-dialog-view-css' => '7d347135', + 'aphront-error-view-css' => '7d347135', + 'aphront-form-view-css' => '7d347135', 'aphront-headsup-action-list-view-css' => 'ec01d039', - 'aphront-headsup-view-css' => '23503cb9', - 'aphront-list-filter-view-css' => '23503cb9', - 'aphront-pager-view-css' => '23503cb9', - 'aphront-panel-view-css' => '23503cb9', - 'aphront-table-view-css' => '23503cb9', - 'aphront-tokenizer-control-css' => '23503cb9', - 'aphront-tooltip-css' => '23503cb9', - 'aphront-typeahead-control-css' => '23503cb9', + 'aphront-headsup-view-css' => '7d347135', + 'aphront-list-filter-view-css' => '7d347135', + 'aphront-pager-view-css' => '7d347135', + 'aphront-panel-view-css' => '7d347135', + 'aphront-table-view-css' => '7d347135', + 'aphront-tokenizer-control-css' => '7d347135', + 'aphront-tooltip-css' => '7d347135', + 'aphront-typeahead-control-css' => '7d347135', 'differential-changeset-view-css' => 'ec01d039', 'differential-core-view-css' => 'ec01d039', 'differential-inline-comment-editor' => '95d0d865', @@ -3682,7 +3657,7 @@ celerity_register_resource_map(array( 'differential-table-of-contents-css' => 'ec01d039', 'diffusion-commit-view-css' => 'c8ce2d88', 'diffusion-icons-css' => 'c8ce2d88', - 'global-drag-and-drop-css' => '23503cb9', + 'global-drag-and-drop-css' => '7d347135', 'inline-comment-summary-css' => 'ec01d039', 'javelin-aphlict' => '58743fec', 'javelin-behavior' => '88225b70', @@ -3752,48 +3727,48 @@ celerity_register_resource_map(array( 'javelin-util' => '88225b70', 'javelin-vector' => '88225b70', 'javelin-workflow' => '88225b70', - 'lightbox-attachment-css' => '23503cb9', + 'lightbox-attachment-css' => '7d347135', 'maniphest-task-summary-css' => 'e30a3fa8', 'maniphest-transaction-detail-css' => 'e30a3fa8', 'phabricator-busy' => '58743fec', 'phabricator-content-source-view-css' => 'ec01d039', - 'phabricator-core-buttons-css' => '23503cb9', - 'phabricator-core-css' => '23503cb9', - 'phabricator-crumbs-view-css' => '23503cb9', - 'phabricator-directory-css' => '23503cb9', + 'phabricator-core-buttons-css' => '7d347135', + 'phabricator-core-css' => '7d347135', + 'phabricator-crumbs-view-css' => '7d347135', + 'phabricator-directory-css' => '7d347135', 'phabricator-drag-and-drop-file-upload' => '95d0d865', 'phabricator-dropdown-menu' => '58743fec', 'phabricator-file-upload' => '58743fec', - 'phabricator-filetree-view-css' => '23503cb9', - 'phabricator-flag-css' => '23503cb9', - 'phabricator-form-view-css' => '23503cb9', - 'phabricator-header-view-css' => '23503cb9', - 'phabricator-jump-nav' => '23503cb9', + 'phabricator-filetree-view-css' => '7d347135', + 'phabricator-flag-css' => '7d347135', + 'phabricator-form-view-css' => '7d347135', + 'phabricator-header-view-css' => '7d347135', + 'phabricator-jump-nav' => '7d347135', 'phabricator-keyboard-shortcut' => '58743fec', 'phabricator-keyboard-shortcut-manager' => '58743fec', - 'phabricator-main-menu-view' => '23503cb9', + 'phabricator-main-menu-view' => '7d347135', 'phabricator-menu-item' => '58743fec', - 'phabricator-nav-view-css' => '23503cb9', + 'phabricator-nav-view-css' => '7d347135', 'phabricator-notification' => '58743fec', - 'phabricator-notification-css' => '23503cb9', - 'phabricator-notification-menu-css' => '23503cb9', - 'phabricator-object-item-list-view-css' => '23503cb9', + 'phabricator-notification-css' => '7d347135', + 'phabricator-notification-menu-css' => '7d347135', + 'phabricator-object-item-list-view-css' => '7d347135', 'phabricator-object-selector-css' => 'ec01d039', 'phabricator-paste-file-upload' => '58743fec', 'phabricator-prefab' => '58743fec', 'phabricator-project-tag-css' => 'e30a3fa8', - 'phabricator-remarkup-css' => '23503cb9', + 'phabricator-remarkup-css' => '7d347135', 'phabricator-shaped-request' => '95d0d865', - 'phabricator-side-menu-view-css' => '23503cb9', - 'phabricator-standard-page-view' => '23503cb9', + 'phabricator-side-menu-view-css' => '7d347135', + 'phabricator-standard-page-view' => '7d347135', 'phabricator-textareautils' => '58743fec', 'phabricator-tooltip' => '58743fec', - 'phabricator-transaction-view-css' => '23503cb9', - 'phabricator-zindex-css' => '23503cb9', - 'sprite-apps-large-css' => '23503cb9', - 'sprite-gradient-css' => '23503cb9', - 'sprite-icon-css' => '23503cb9', - 'sprite-menu-css' => '23503cb9', - 'syntax-highlighting-css' => '23503cb9', + 'phabricator-transaction-view-css' => '7d347135', + 'phabricator-zindex-css' => '7d347135', + 'sprite-apps-large-css' => '7d347135', + 'sprite-gradient-css' => '7d347135', + 'sprite-icon-css' => '7d347135', + 'sprite-menu-css' => '7d347135', + 'syntax-highlighting-css' => '7d347135', ), )); diff --git a/webroot/rsrc/css/application/pholio/pholio.css b/webroot/rsrc/css/application/pholio/pholio.css index 86031e9d5e..9a44602fb2 100644 --- a/webroot/rsrc/css/application/pholio/pholio.css +++ b/webroot/rsrc/css/application/pholio/pholio.css @@ -21,9 +21,18 @@ display: inline-block; } -.pholio-mock-select { - border: 1px solid #FF0000; - position: absolute; +.pholio-mock-select-border { + position: absolute; + background: #ffffff; + opacity: 0.25; + box-sizing: border-box; + border: 1px solid #000000; +} + +.pholio-mock-select-fill { + position: absolute; + border: 1px dashed #ffffff; + box-sizing: border-box; } .pholio-mock-wrapper { diff --git a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js index afeabe579a..7b0e0fb1d1 100644 --- a/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js +++ b/webroot/rsrc/js/application/pholio/behavior-pholio-mock-view.js @@ -4,7 +4,8 @@ * javelin-stratcom * javelin-dom * javelin-vector - * javelin-event + * javelin-magical-init + * javelin-request */ JX.behavior('pholio-mock-view', function(config) { var is_dragging = false; @@ -13,7 +14,9 @@ JX.behavior('pholio-mock-view', function(config) { var imageData; var startPos; var endPos; - var selection; + + var selection_border; + var selection_fill; JX.Stratcom.listen( 'click', // Listen for clicks... @@ -35,26 +38,28 @@ JX.behavior('pholio-mock-view', function(config) { }); - function draw_rectangle(node, current, init) { - JX.$V( - Math.abs(current.x-init.x), - Math.abs(current.y-init.y)) - .setDim(node); + function draw_rectangle(nodes, current, init) { + for (var ii = 0; ii < nodes.length; ii++) { + var node = nodes[ii]; - JX.$V( - (current.x-init.x < 0) ? current.x:init.x, - (current.y-init.y < 0) ? current.y:init.y) - .setPos(node); + JX.$V( + Math.abs(current.x-init.x), + Math.abs(current.y-init.y)) + .setDim(node); + + JX.$V( + (current.x-init.x < 0) ? current.x:init.x, + (current.y-init.y < 0) ? current.y:init.y) + .setPos(node); + } } function getRealXY(parent, point) { var pos = {x: (point.x - parent.x), y: (point.y - parent.y)}; + var dim = JX.Vector.getDim(image); - if (pos.x < 0) pos.x = 0; - else if (pos.x > image.clientWidth) pos.x = image.clientWidth - 1; - - if (pos.y < 0) pos.y = 0; - else if (pos.y > image.clientHeight) pos.y = image.clientHeight - 2; + pos.x = Math.max(0, Math.min(pos.x, dim.x)); + pos.y = Math.max(0, Math.min(pos.y, dim.y)); return pos; } @@ -72,17 +77,18 @@ JX.behavior('pholio-mock-view', function(config) { startPos = getRealXY(JX.$V(wrapper),JX.$V(e)); - selection = JX.$N( + selection_border = JX.$N( 'div', - {className: 'pholio-mock-select'} - ); + {className: 'pholio-mock-select-border'}); + selection_fill = JX.$N( + 'div', + {className: 'pholio-mock-select-fill'}); - JX.$V(startPos.x,startPos.y).setPos(selection); - - JX.DOM.appendContent(wrapper, selection); - + JX.$V(startPos.x, startPos.y).setPos(selection_border); + JX.$V(startPos.x, startPos.y).setPos(selection_fill); + JX.DOM.appendContent(wrapper, [selection_border, selection_fill]); }); JX.enableDispatch(document.body, 'mousemove'); @@ -91,7 +97,10 @@ JX.behavior('pholio-mock-view', function(config) { return; } - draw_rectangle(selection, getRealXY(JX.$V(wrapper), JX.$V(e)), startPos); + draw_rectangle( + [selection_border, selection_fill], + getRealXY(JX.$V(wrapper), + JX.$V(e)), startPos); }); JX.Stratcom.listen( @@ -107,11 +116,12 @@ JX.behavior('pholio-mock-view', function(config) { comment = window.prompt("Add your comment"); if (comment == null || comment == "") { - selection.remove(); + JX.DOM.remove(selection_border); + JX.DOM.remove(selection_fill); return; } - selection.title = comment; + selection_fill.title = comment; var saveURL = "/pholio/inline/" + imageData['imageID'] + "/"; From 431e2bee6e7c827bf843d728712798b8d81852cf Mon Sep 17 00:00:00 2001 From: indiefan Date: Thu, 7 Feb 2013 06:31:29 -0800 Subject: [PATCH 18/19] First (rough) pass at campfire protocol adapter for bot. Summary: Decided the best approach for refactoring the message/command stuff would be to actually start implementing the campfire adapter to get a better idea of what the abstractions should look like. It feels awkward and unwieldy trying to maintain the irc command interface (notice the message instantiation in the `processReadBuffer()` method. However, i'm still not clear what the best approach is without requiring a re-write of nearly all the existing handlers and defining essentially a custom dsl on top of irc's. I suppose given that alternative, implementing to irc's dsl doesn't sound all that bad. Just feels like poor coupling. Also, I know that there is some http stuff in libphutil's futures library, but the https future is shit and I need to do some custom curlopt stuff I wasn't sure how to do with that. But if you think this should be refactored, let me know. I tested this with the ObjectHandler (messages with DXXX initiate the bot to respond with the title/link just as with irc), but beyond that, I haven't tried any of the other handlers, so if there are complications you think i'm going to run into, just let me know (this is one of the reasons for requesting review early on). Also, this diff is against my last one, even though that hasn't been merged down yet. It was starting to get large and I'd prefer to keep to two conversations separate. Fixing some lint issues. Test Plan: Ran the bot with the Object Handler in campfire and observed it behaving properly. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2462 Differential Revision: https://secure.phabricator.com/D4830 --- src/__phutil_library_map__.php | 2 + .../daemon/bot/PhabricatorBot.php | 4 +- .../daemon/bot/PhabricatorBotMessage.php | 84 +++-- .../PhabricatorBaseProtocolAdapter.php | 5 +- .../PhabricatorCampfireProtocolAdapter.php | 201 +++++++++++ .../adapter/PhabricatorIRCProtocolAdapter.php | 113 ++++-- .../handler/PhabricatorBotDebugLogHandler.php | 2 +- ...atorBotDifferentialNotificationHandler.php | 11 +- .../PhabricatorBotFeedNotificationHandler.php | 6 +- .../bot/handler/PhabricatorBotHandler.php | 32 +- .../bot/handler/PhabricatorBotLogHandler.php | 13 +- .../handler/PhabricatorBotMacroHandler.php | 68 ++-- .../PhabricatorBotObjectNameHandler.php | 326 +++++++++--------- .../handler/PhabricatorBotSymbolHandler.php | 59 ++-- .../handler/PhabricatorBotWhatsNewHandler.php | 18 +- .../handler/PhabricatorIRCProtocolHandler.php | 16 +- 16 files changed, 632 insertions(+), 328 deletions(-) create mode 100644 src/infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a738ea4e8a..add675ee98 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -729,6 +729,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarHoliday' => 'applications/calendar/storage/PhabricatorCalendarHoliday.php', 'PhabricatorCalendarHolidayTestCase' => 'applications/calendar/storage/__tests__/PhabricatorCalendarHolidayTestCase.php', 'PhabricatorCalendarViewStatusController' => 'applications/calendar/controller/PhabricatorCalendarViewStatusController.php', + 'PhabricatorCampfireProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php', 'PhabricatorChangesetResponse' => 'infrastructure/diff/PhabricatorChangesetResponse.php', 'PhabricatorChatLogChannelListController' => 'applications/chatlog/controller/PhabricatorChatLogChannelListController.php', 'PhabricatorChatLogChannelLogController' => 'applications/chatlog/controller/PhabricatorChatLogChannelLogController.php', @@ -2177,6 +2178,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarHoliday' => 'PhabricatorCalendarDAO', 'PhabricatorCalendarHolidayTestCase' => 'PhabricatorTestCase', 'PhabricatorCalendarViewStatusController' => 'PhabricatorCalendarController', + 'PhabricatorCampfireProtocolAdapter' => 'PhabricatorBaseProtocolAdapter', 'PhabricatorChangesetResponse' => 'AphrontProxyResponse', 'PhabricatorChatLogChannelListController' => 'PhabricatorChatLogController', 'PhabricatorChatLogChannelLogController' => 'PhabricatorChatLogController', diff --git a/src/infrastructure/daemon/bot/PhabricatorBot.php b/src/infrastructure/daemon/bot/PhabricatorBot.php index 7f83307c64..1159726acb 100644 --- a/src/infrastructure/daemon/bot/PhabricatorBot.php +++ b/src/infrastructure/daemon/bot/PhabricatorBot.php @@ -101,8 +101,8 @@ final class PhabricatorBot extends PhabricatorDaemon { } while (true); } - public function writeCommand($command, $message) { - return $this->protocolAdapter->writeCommand($command, $message); + public function writeMessage(PhabricatorBotMessage $message) { + return $this->protocolAdapter->writeMessage($message); } private function routeMessage(PhabricatorBotMessage $message) { diff --git a/src/infrastructure/daemon/bot/PhabricatorBotMessage.php b/src/infrastructure/daemon/bot/PhabricatorBotMessage.php index 95e7f42bc0..c3c03748f1 100644 --- a/src/infrastructure/daemon/bot/PhabricatorBotMessage.php +++ b/src/infrastructure/daemon/bot/PhabricatorBotMessage.php @@ -4,69 +4,65 @@ final class PhabricatorBotMessage { private $sender; private $command; - private $data; + private $body; + private $target; + private $public; - public function __construct($sender, $command, $data) { - $this->sender = $sender; - $this->command = $command; - $this->data = $data; + public function __construct() { + // By default messages are public + $this->public = true; } - public function getRawSender() { + public function setSender($sender) { + $this->sender = $sender; + return $this; + } + + public function getSender() { return $this->sender; } - public function getRawData() { - return $this->data; + public function setCommand($command) { + $this->command = $command; + return $this; } public function getCommand() { return $this->command; } - public function getReplyTo() { - switch ($this->getCommand()) { - case 'PRIVMSG': - $target = $this->getTarget(); - if ($target[0] == '#') { - return $target; - } - break; - } - return null; + public function setBody($body) { + $this->body = $body; + return $this; } - public function getSenderNickname() { - $nick = $this->getRawSender(); - $nick = ltrim($nick, ':'); - $nick = head(explode('!', $nick)); - return $nick; + public function getBody() { + return $this->body; + } + + public function setTarget($target) { + $this->target = $target; + return $this; } public function getTarget() { - switch ($this->getCommand()) { - case 'PRIVMSG': - $matches = null; - $raw = $this->getRawData(); - if (preg_match('/^(\S+)\s/', $raw, $matches)) { - return $matches[1]; - } - break; - } - return null; + return $this->target; } - public function getMessageText() { - switch ($this->getCommand()) { - case 'PRIVMSG': - $matches = null; - $raw = $this->getRawData(); - if (preg_match('/^\S+\s+:?(.*)$/', $raw, $matches)) { - return rtrim($matches[1], "\r\n"); - } - break; - } - return null; + public function isPublic() { + return $this->public; } + public function setPublic($is_public) { + $this->public = $is_public; + return $this; + } + + public function getReplyTo() { + if ($this->public) { + return $this->target; + } else { + return $this->sender; + } + } } diff --git a/src/infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php b/src/infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php index db3cc76b15..55cb90ebd1 100644 --- a/src/infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php +++ b/src/infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php @@ -30,8 +30,7 @@ abstract class PhabricatorBaseProtocolAdapter { /** * This is the output mechanism for the protocol. * - * @param String $command The command for the message - * @param String $message The contents of the message itself + * @param PhabricatorBotMessage $message The message to write */ - abstract public function writeCommand($command, $message); + abstract public function writeMessage(PhabricatorBotMessage $message); } diff --git a/src/infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php b/src/infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php new file mode 100644 index 0000000000..59323baf6f --- /dev/null +++ b/src/infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php @@ -0,0 +1,201 @@ +server = idx($this->config, 'server'); + $this->authtoken = idx($this->config, 'authtoken'); + $ssl = idx($this->config, 'ssl', false); + $this->rooms = idx($this->config, 'join'); + + // First, join the room + if (!$this->rooms) { + throw new Exception("Not configured to join any rooms!"); + } + + $this->readBuffers = array(); + + // Set up our long poll in a curl multi request so we can + // continue running while it executes in the background + $this->multiHandle = curl_multi_init(); + $this->readHandles = array(); + + foreach ($this->rooms as $room_id) { + $this->joinRoom($room_id); + + // Set up the curl stream for reading + $url = ($ssl) ? "https://" : "http://"; + $url .= "streaming.campfirenow.com/room/{$room_id}/live.json"; + $this->readHandle[$url] = curl_init(); + curl_setopt($this->readHandle[$url], CURLOPT_URL, $url); + curl_setopt($this->readHandle[$url], CURLOPT_RETURNTRANSFER, true); + curl_setopt($this->readHandle[$url], CURLOPT_FOLLOWLOCATION, 1); + curl_setopt( + $this->readHandle[$url], + CURLOPT_USERPWD, + $this->authtoken.':x'); + curl_setopt( + $this->readHandle[$url], + CURLOPT_HTTPHEADER, + array("Content-type: application/json")); + curl_setopt( + $this->readHandle[$url], + CURLOPT_WRITEFUNCTION, + array($this, 'read')); + curl_setopt($this->readHandle[$url], CURLOPT_BUFFERSIZE, 128); + curl_setopt($this->readHandle[$url], CURLOPT_TIMEOUT, 0); + + curl_multi_add_handle($this->multiHandle, $this->readHandle[$url]); + + // Initialize read buffer + $this->readBuffers[$url] = ''; + } + + $this->active = null; + $this->blockingMultiExec(); + } + + // This is our callback for the background curl multi-request. + // Puts the data read in on the readBuffer for processing. + private function read($ch, $data) { + $info = curl_getinfo($ch); + $length = strlen($data); + $this->readBuffers[$info['url']] .= $data; + return $length; + } + + private function blockingMultiExec() { + do { + $status = curl_multi_exec($this->multiHandle, $this->active); + } while ($status == CURLM_CALL_MULTI_PERFORM); + + // Check for errors + if ($status != CURLM_OK) { + throw new Exception( + "Phabricator Bot had a problem reading from campfire."); + } + } + + public function getNextMessages($poll_frequency) { + $messages = array(); + + if (!$this->active) { + throw new Exception("Phabricator Bot stopped reading from campfire."); + } + + // Prod our http request + curl_multi_select($this->multiHandle, $poll_frequency); + $this->blockingMultiExec(); + + // Process anything waiting on the read buffer + while ($m = $this->processReadBuffer()) { + $messages[] = $m; + } + + return $messages; + } + + private function processReadBuffer() { + foreach ($this->readBuffers as $url => &$buffer) { + $until = strpos($buffer, "}\r"); + if ($until == false) { + continue; + } + + $message = substr($buffer, 0, $until + 1); + $buffer = substr($buffer, $until + 2); + + $m_obj = json_decode($message, true); + + return id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget($m_obj['room_id']) + ->setBody($m_obj['body']); + } + + // If we're here, there's nothing to process + return false; + } + + public function writeMessage(PhabricatorBotMessage $message) { + switch ($message->getCommand()) { + case 'MESSAGE': + $this->speak( + $message->getBody(), + $message->getTarget()); + break; + } + } + + private function joinRoom($room_id) { + $this->performPost("/room/{$room_id}/join.json"); + } + + private function leaveRoom($room_id) { + $this->performPost("/room/{$room_id}/leave.json"); + } + + private function speak($message, $room_id) { + $this->performPost( + "/room/{$room_id}/speak.json", + array( + 'message' => array( + 'type' => 'TextMessage', + 'body' => $message))); + } + + private function performPost($endpoint, $data = Null) { + $url = $this->server.$endpoint; + + $payload = json_encode($data); + + // cURL init & config + $ch = curl_init(); + curl_setopt($ch, CURLOPT_URL, $url); + curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_POST, 1); + curl_setopt($ch, CURLOPT_FOLLOWLOCATION, 1); + curl_setopt($ch, CURLOPT_USERPWD, $this->authtoken . ':x'); + curl_setopt( + $ch, + CURLOPT_HTTPHEADER, + array("Content-type: application/json")); + + curl_setopt($ch, CURLOPT_POSTFIELDS, $payload); + $output = curl_exec($ch); + + curl_close($ch); + + $output = trim($output); + + if (strlen($output)) { + return json_decode($output); + } + + return true; + } + + public function __destruct() { + if ($this->rooms) { + foreach ($this->rooms as $room_id) { + $this->leaveRoom($room_id); + } + } + if ($this->readHandles) { + foreach ($this->readHandles as $read_handle) { + curl_multi_remove_handle($this->multiHandle, $read_handle); + curl_close($read_handle); + } + } + curl_multi_close($this->multiHandle); + } +} diff --git a/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php b/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php index ccd96fc0ea..f2d34db246 100644 --- a/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php +++ b/src/infrastructure/daemon/bot/adapter/PhabricatorIRCProtocolAdapter.php @@ -1,14 +1,17 @@ 'MESSAGE'); + public function connect() { $nick = idx($this->config, 'nick', 'phabot'); $server = idx($this->config, 'server'); @@ -38,12 +41,21 @@ final class PhabricatorIRCProtocolAdapter } $this->socket = $socket; - $this->writeCommand('USER', "{$user} 0 * :{$user}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('USER') + ->setBody("{$user} 0 * :{$user}")); if ($pass) { - $this->writeCommand('PASS', "{$pass}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('PASS') + ->setBody("{$pass}")); } - $this->writeCommand('NICK', "{$nick}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('NICK') + ->setBody("{$nick}")); } public function getNextMessages($poll_frequency) { @@ -77,12 +89,9 @@ final class PhabricatorIRCProtocolAdapter if ($data === false) { throw new Exception("fread() failed!"); } else { - $messages[] = new PhabricatorBotMessage( - null, - "LOG", - "<<< ".$data - ); - + $messages[] = id(new PhabricatorBotMessage()) + ->setCommand("LOG") + ->setBody(">>> ".$data); $this->readBuffer .= $data; } } while (strlen($data)); @@ -94,10 +103,9 @@ final class PhabricatorIRCProtocolAdapter if ($len === false) { throw new Exception("fwrite() failed!"); } else { - $messages[] = new PhabricatorBotMessage( - null, - "LOG", - ">>> ".substr($this->writeBuffer, 0, $len)); + $messages[] = id(new PhabricatorBotMessage()) + ->setCommand("LOG") + ->setBody(">>> ".substr($this->writeBuffer, 0, $len)); $this->writeBuffer = substr($this->writeBuffer, $len); } } while (strlen($this->writeBuffer)); @@ -115,8 +123,21 @@ final class PhabricatorIRCProtocolAdapter return $this; } - public function writeCommand($command, $message) { - return $this->write($command.' '.$message."\r\n"); + public function writeMessage(PhabricatorBotMessage $message) { + $irc_command = $this->getIRCCommand($message->getCommand()); + switch ($message->getCommand()) { + case 'MESSAGE': + $data = $irc_command.' '. + $message->getTarget().' :'. + $message->getBody()."\r\n"; + break; + default: + $data = $irc_command.' '. + $message->getBody()."\r\n"; + break; + } + + return $this->write($data); } private function processReadBuffer() { @@ -130,7 +151,7 @@ final class PhabricatorIRCProtocolAdapter $pattern = '/^'. - '(?:(?P:(\S+)) )?'. // This may not be present. + '(?::(?P(\S+?))(?:!\S*)? )?'. // This may not be present. '(?P[A-Z0-9]+) '. '(?P.*)'. '$/'; @@ -140,17 +161,61 @@ final class PhabricatorIRCProtocolAdapter throw new Exception("Unexpected message from server: {$message}"); } - $irc_message = new PhabricatorBotMessage( - idx($matches, 'sender'), - $matches['command'], - $matches['data']); + $command = $this->getBotCommand($matches['command']); + list($target, $body) = $this->parseMessageData($command, $matches['data']); - return $irc_message; + $bot_message = id(new PhabricatorBotMessage()) + ->setSender(idx($matches, 'sender')) + ->setCommand($command) + ->setTarget($target) + ->setBody($body); + + if (!empty($target) && strncmp($target, '#', 1) !== 0) { + $bot_message->setPublic(false); + } + + return $bot_message; + } + + private function getBotCommand($irc_command) { + if (isset(self::$commandTranslations[$irc_command])) { + return self::$commandTranslations[$irc_command]; + } + + // We have no translation for this command, use as-is + return $irc_command; + } + + private function getIRCCommand($original_bot_command) { + foreach (self::$commandTranslations as $irc_command=>$bot_command) { + if ($bot_command === $original_bot_command) { + return $irc_command; + } + } + + return $original_bot_command; + } + + private function parseMessageData($command, $data) { + switch ($command) { + case 'MESSAGE': + $matches = null; + if (preg_match('/^(\S+)\s+:?(.*)$/', $data, $matches)) { + return array( + $matches[1], + rtrim($matches[2], "\r\n")); + } + break; + } + + // By default we assume there is no target, only a body + return array( + null, + $data); } public function __destruct() { $this->write("QUIT Goodbye.\r\n"); fclose($this->socket); } - } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php index 19ff95c562..7c2d0b57cb 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotDebugLogHandler.php @@ -8,7 +8,7 @@ final class PhabricatorBotDebugLogHandler extends PhabricatorBotHandler { switch ($message->getCommand()) { case 'LOG': echo addcslashes( - $message->getRawData(), + $message->getBody(), "\0..\37\177..\377"); echo "\n"; break; diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php index 1f801d472e..3a252cfc5e 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotDifferentialNotificationHandler.php @@ -4,7 +4,7 @@ * @group irc */ final class PhabricatorBotDifferentialNotificationHandler - extends PhabricatorBotHandler { +extends PhabricatorBotHandler { private $skippedOldEvents; @@ -39,11 +39,16 @@ final class PhabricatorBotDifferentialNotificationHandler $verb = DifferentialAction::getActionPastTenseVerb($data['action']); $actor_name = $handles[$actor_phid]->getName(); - $message = "{$actor_name} {$verb} revision D".$data['revision_id']."."; + $message_body = + "{$actor_name} {$verb} revision D".$data['revision_id']."."; $channels = $this->getConfig('notification.channels', array()); foreach ($channels as $channel) { - $this->write('PRIVMSG', "{$channel} :{$message}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget($channel) + ->setBody($message_body)); } } } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php index bd8b7e5375..abec6a5664 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotFeedNotificationHandler.php @@ -150,7 +150,11 @@ final class PhabricatorBotFeedNotificationHandler $channels = $this->getConfig('join'); foreach ($channels as $channel) { - $this->write('PRIVMSG', "{$channel} :{$story['text']}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget($channel) + ->setBody($story['text'])); } } } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php index ec1dbc117b..bf32d01cb6 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotHandler.php @@ -14,8 +14,8 @@ abstract class PhabricatorBotHandler { $this->bot = $irc_bot; } - final protected function write($command, $message) { - $this->bot->writeCommand($command, $message); + final protected function writeMessage(PhabricatorBotMessage $message) { + $this->bot->writeMessage($message); return $this; } @@ -33,14 +33,34 @@ abstract class PhabricatorBotHandler { return (string)$base_uri; } - final protected function isChannelName($name) { - return (strncmp($name, '#', 1) === 0); - } - abstract public function receiveMessage(PhabricatorBotMessage $message); public function runBackgroundTasks() { return; } + public function replyTo($original_message, $body) { + if ($original_message->getCommand() != 'MESSAGE') { + throw new Exception( + "Handler is trying to reply to something which is not a message!"); + } + + $reply = id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE'); + + if ($original_message->isPublic()) { + // This is a public target, like a chatroom. Send the response to the + // chatroom. + $reply->setTarget($original_message->getTarget()); + } else { + // This is a private target, like a private message. Send the response + // back to the sender (presumably, we are the target). + $reply->setTarget($original_message->getSender()) + ->setPublic(false); + } + + $reply->setBody($body); + + return $this->writeMessage($reply); + } } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php index 3e1bd620a8..bc2962365e 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotLogHandler.php @@ -12,12 +12,12 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler { public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { - case 'PRIVMSG': + case 'MESSAGE': $reply_to = $message->getReplyTo(); if (!$reply_to) { break; } - if (!$this->isChannelName($reply_to)) { + if (!$message->isPublic()) { // Don't log private messages, although maybe we should for debugging? break; } @@ -27,8 +27,8 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler { 'channel' => $reply_to, 'type' => 'mesg', 'epoch' => time(), - 'author' => $message->getSenderNickname(), - 'message' => $message->getMessageText(), + 'author' => $message->getSender(), + 'message' => $message->getBody(), ), ); @@ -46,7 +46,7 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler { $tell = false; foreach ($prompts as $prompt) { - if (preg_match($prompt, $message->getMessageText())) { + if (preg_match($prompt, $message->getBody())) { $tell = true; break; } @@ -55,7 +55,8 @@ final class PhabricatorBotLogHandler extends PhabricatorBotHandler { if ($tell) { $response = $this->getURI( '/chatlog/channel/'.phutil_escape_uri($reply_to).'/'); - $this->write('PRIVMSG', "{$reply_to} :{$response}"); + + $this->replyTo($message, $response); } break; diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php index 59e698a04d..daa92424a0 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotMacroHandler.php @@ -46,38 +46,38 @@ final class PhabricatorBotMacroHandler extends PhabricatorBotHandler { } switch ($message->getCommand()) { - case 'PRIVMSG': - $reply_to = $message->getReplyTo(); - if (!$reply_to) { - break; - } - - $message = $message->getMessageText(); - - $matches = null; - if (!preg_match($this->regexp, $message, $matches)) { - return; - } - - $macro = $matches[1]; - - $ascii = idx($this->macros[$macro], 'ascii'); - if ($ascii === false) { - return; - } - - if (!$ascii) { - $this->macros[$macro]['ascii'] = $this->rasterize( - $this->macros[$macro], - $this->getConfig('macro.size', 48), - $this->getConfig('macro.aspect', 0.66)); - $ascii = $this->macros[$macro]['ascii']; - } - - foreach ($ascii as $line) { - $this->buffer[$reply_to][] = $line; - } + case 'MESSAGE': + $reply_to = $message->getReplyTo(); + if (!$reply_to) { break; + } + + $message_body = $message->getBody(); + + $matches = null; + if (!preg_match($this->regexp, $message_body, $matches)) { + return; + } + + $macro = $matches[1]; + + $ascii = idx($this->macros[$macro], 'ascii'); + if ($ascii === false) { + return; + } + + if (!$ascii) { + $this->macros[$macro]['ascii'] = $this->rasterize( + $this->macros[$macro], + $this->getConfig('macro.size', 48), + $this->getConfig('macro.aspect', 0.66)); + $ascii = $this->macros[$macro]['ascii']; + } + + foreach ($ascii as $line) { + $this->buffer[$reply_to][] = $line; + } + break; } } @@ -92,7 +92,11 @@ final class PhabricatorBotMacroHandler extends PhabricatorBotHandler { continue; } foreach ($lines as $key => $line) { - $this->write('PRIVMSG', "{$channel} :{$line}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget($channel) + ->setBody($line)); unset($this->buffer[$channel][$key]); break 2; } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php index 7832880bb7..5a6ff48ad9 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php @@ -13,186 +13,182 @@ final class PhabricatorBotObjectNameHandler extends PhabricatorBotHandler { */ private $recentlyMentioned = array(); - public function receiveMessage(PhabricatorBotMessage $message) { + public function receiveMessage(PhabricatorBotMessage $original_message) { - switch ($message->getCommand()) { - case 'PRIVMSG': - $reply_to = $message->getReplyTo(); - if (!$reply_to) { - break; - } + switch ($original_message->getCommand()) { + case 'MESSAGE': + $message = $original_message->getBody(); + $matches = null; - $message = $message->getMessageText(); - $matches = null; + $pattern = + '@'. + '(?getConduit()->callMethodSynchronous( - 'differential.query', - array( - 'ids' => $revision_ids, - )); - $revisions = array_select_keys( - ipull($revisions, null, 'id'), - $revision_ids - ); - foreach ($revisions as $revision) { - $output[$revision['phid']] = - 'D'.$revision['id'].' '.$revision['title'].' - '. - $revision['uri']; - } + if ($revision_ids) { + $revisions = $this->getConduit()->callMethodSynchronous( + 'differential.query', + array( + 'ids' => $revision_ids, + )); + $revisions = array_select_keys( + ipull($revisions, null, 'id'), + $revision_ids + ); + foreach ($revisions as $revision) { + $output[$revision['phid']] = + 'D'.$revision['id'].' '.$revision['title'].' - '. + $revision['uri']; } + } - if ($task_ids) { - foreach ($task_ids as $task_id) { - if ($task_id == 1000) { - $output[1000] = 'T1000: A nanomorph mimetic poly-alloy' - .'(liquid metal) assassin controlled by Skynet: ' - .'http://en.wikipedia.org/wiki/T-1000'; - continue; - } - $task = $this->getConduit()->callMethodSynchronous( - 'maniphest.info', - array( - 'task_id' => $task_id, - )); - $output[$task['phid']] = 'T'.$task['id'].': '.$task['title']. - ' (Priority: '.$task['priority'].') - '.$task['uri']; - } - } - - if ($vote_ids) { - foreach ($vote_ids as $vote_id) { - $vote = $this->getConduit()->callMethodSynchronous( - 'slowvote.info', - array( - 'poll_id' => $vote_id, - )); - $output[$vote['phid']] = 'V'.$vote['id'].': '.$vote['question']. - ' Come Vote '.$vote['uri']; - } - } - - if ($file_ids) { - foreach ($file_ids as $file_id) { - $file = $this->getConduit()->callMethodSynchronous( - 'file.info', - array( - 'id' => $file_id, - )); - $output[$file['phid']] = $file['objectName'].": ".$file['uri']." - ". - $file['name']; - } - } - - if ($paste_ids) { - foreach ($paste_ids as $paste_id) { - $paste = $this->getConduit()->callMethodSynchronous( - 'paste.info', - array( - 'paste_id' => $paste_id, - )); - // Eventually I'd like to show the username of the paster as well, - // however that will need something like a user.username_from_phid - // since we (ideally) want to keep the bot to Conduit calls...and - // not call to Phabricator-specific stuff (like actually loading - // the User object and fetching his/her username.) - $output[$paste['phid']] = 'P'.$paste['id'].': '.$paste['uri'].' - '. - $paste['title']; - - if ($paste['language']) { - $output[$paste['phid']] .= ' ('.$paste['language'].')'; - } - } - } - - if ($commit_names) { - $commits = $this->getConduit()->callMethodSynchronous( - 'diffusion.getcommits', - array( - 'commits' => $commit_names, - )); - foreach ($commits as $commit) { - if (isset($commit['error'])) { - continue; - } - $output[$commit['commitPHID']] = $commit['uri']; - } - } - - foreach ($output as $phid => $description) { - - // Don't mention the same object more than once every 10 minutes - // in public channels, so we avoid spamming the chat over and over - // again for discsussions of a specific revision, for example. - - if (empty($this->recentlyMentioned[$reply_to])) { - $this->recentlyMentioned[$reply_to] = array(); - } - - $quiet_until = idx( - $this->recentlyMentioned[$reply_to], - $phid, - 0) + (60 * 10); - - if (time() < $quiet_until) { - // Remain quiet on this channel. + if ($task_ids) { + foreach ($task_ids as $task_id) { + if ($task_id == 1000) { + $output[1000] = 'T1000: A nanomorph mimetic poly-alloy' + .'(liquid metal) assassin controlled by Skynet: ' + .'http://en.wikipedia.org/wiki/T-1000'; continue; } - - $this->recentlyMentioned[$reply_to][$phid] = time(); - $this->write('PRIVMSG', "{$reply_to} :{$description}"); + $task = $this->getConduit()->callMethodSynchronous( + 'maniphest.info', + array( + 'task_id' => $task_id, + )); + $output[$task['phid']] = 'T'.$task['id'].': '.$task['title']. + ' (Priority: '.$task['priority'].') - '.$task['uri']; } - break; + } + + if ($vote_ids) { + foreach ($vote_ids as $vote_id) { + $vote = $this->getConduit()->callMethodSynchronous( + 'slowvote.info', + array( + 'poll_id' => $vote_id, + )); + $output[$vote['phid']] = 'V'.$vote['id'].': '.$vote['question']. + ' Come Vote '.$vote['uri']; + } + } + + if ($file_ids) { + foreach ($file_ids as $file_id) { + $file = $this->getConduit()->callMethodSynchronous( + 'file.info', + array( + 'id' => $file_id, + )); + $output[$file['phid']] = $file['objectName'].": ".$file['uri']." - ". + $file['name']; + } + } + + if ($paste_ids) { + foreach ($paste_ids as $paste_id) { + $paste = $this->getConduit()->callMethodSynchronous( + 'paste.info', + array( + 'paste_id' => $paste_id, + )); + // Eventually I'd like to show the username of the paster as well, + // however that will need something like a user.username_from_phid + // since we (ideally) want to keep the bot to Conduit calls...and + // not call to Phabricator-specific stuff (like actually loading + // the User object and fetching his/her username.) + $output[$paste['phid']] = 'P'.$paste['id'].': '.$paste['uri'].' - '. + $paste['title']; + + if ($paste['language']) { + $output[$paste['phid']] .= ' ('.$paste['language'].')'; + } + } + } + + if ($commit_names) { + $commits = $this->getConduit()->callMethodSynchronous( + 'diffusion.getcommits', + array( + 'commits' => $commit_names, + )); + foreach ($commits as $commit) { + if (isset($commit['error'])) { + continue; + } + $output[$commit['commitPHID']] = $commit['uri']; + } + } + + foreach ($output as $phid => $description) { + + // Don't mention the same object more than once every 10 minutes + // in public channels, so we avoid spamming the chat over and over + // again for discsussions of a specific revision, for example. + + $reply_to = $original_message->getReplyTo(); + if (empty($this->recentlyMentioned[$reply_to])) { + $this->recentlyMentioned[$reply_to] = array(); + } + + $quiet_until = idx( + $this->recentlyMentioned[$reply_to], + $phid, + 0) + (60 * 10); + + if (time() < $quiet_until) { + // Remain quiet on this channel. + continue; + } + + $this->recentlyMentioned[$reply_to][$phid] = time(); + $this->replyTo($original_message, $description); + } + break; } } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php index 2e57de3418..536f8713ad 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotSymbolHandler.php @@ -10,45 +10,40 @@ final class PhabricatorBotSymbolHandler extends PhabricatorBotHandler { public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { - case 'PRIVMSG': - $reply_to = $message->getReplyTo(); - if (!$reply_to) { + case 'MESSAGE': + $text = $message->getBody(); + + $matches = null; + if (!preg_match('/where(?: in the world)? is (\S+?)\?/i', + $text, $matches)) { break; } - $text = $message->getMessageText(); + $symbol = $matches[1]; + $results = $this->getConduit()->callMethodSynchronous( + 'diffusion.findsymbols', + array( + 'name' => $symbol, + )); - $matches = null; - if (!preg_match('/where(?: in the world)? is (\S+?)\?/i', - $text, $matches)) { - break; - } + $default_uri = $this->getURI('/diffusion/symbol/'.$symbol.'/'); - $symbol = $matches[1]; - $results = $this->getConduit()->callMethodSynchronous( - 'diffusion.findsymbols', - array( - 'name' => $symbol, - )); + if (count($results) > 1) { + $response = "Multiple symbols named '{$symbol}': {$default_uri}"; + } else if (count($results) == 1) { + $result = head($results); + $response = + $result['type'].' '. + $result['name'].' '. + '('.$result['language'].'): '. + nonempty($result['uri'], $default_uri); + } else { + $response = "No symbol '{$symbol}' found anywhere."; + } - $default_uri = $this->getURI('/diffusion/symbol/'.$symbol.'/'); + $this->replyTo($message, $response); - if (count($results) > 1) { - $response = "Multiple symbols named '{$symbol}': {$default_uri}"; - } else if (count($results) == 1) { - $result = head($results); - $response = - $result['type'].' '. - $result['name'].' '. - '('.$result['language'].'): '. - nonempty($result['uri'], $default_uri); - } else { - $response = "No symbol '{$symbol}' found anywhere."; - } - - $this->write('PRIVMSG', "{$reply_to} :{$response}"); - - break; + break; } } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php index 1bd53ef194..15fc2ecb85 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotWhatsNewHandler.php @@ -12,16 +12,16 @@ final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler { public function receiveMessage(PhabricatorBotMessage $message) { switch ($message->getCommand()) { - case 'PRIVMSG': + case 'MESSAGE': $reply_to = $message->getReplyTo(); if (!$reply_to) { break; } - $message = $message->getMessageText(); + $message_body = $message->getBody(); $prompt = '~what( i|\')?s new\?~i'; - if (preg_match($prompt, $message)) { + if (preg_match($prompt, $message_body)) { if (time() < $this->floodblock) { return; } @@ -108,9 +108,15 @@ final class PhabricatorBotWhatsNewHandler extends PhabricatorBotHandler { $gray = $color.'15'; $bold = chr(2); $reset = chr(15); - $content = "{$bold}{$user}{$reset} {$gray}{$action} {$blue}{$bold}". - "{$title}{$reset} - {$gray}{$uri}{$reset}"; - $this->write('PRIVMSG',"{$reply_to} :{$content}"); + // Disabling irc-specific styling, at least for now + // $content = "{$bold}{$user}{$reset} {$gray}{$action} {$blue}{$bold}". + // "{$title}{$reset} - {$gray}{$uri}{$reset}"; + $content = "{$user} {$action} {$title} - {$uri}"; + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget($reply_to) + ->setBody($content)); } return; } diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php index 80b76077b1..023ffc065f 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorIRCProtocolHandler.php @@ -13,18 +13,28 @@ final class PhabricatorIRCProtocolHandler extends PhabricatorBotHandler { case '376': // End of MOTD $nickpass = $this->getConfig('nickpass'); if ($nickpass) { - $this->write('PRIVMSG', "nickserv :IDENTIFY {$nickpass}"); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('MESSAGE') + ->setTarget('nickserv') + ->setBody("IDENTIFY {$nickpass}")); } $join = $this->getConfig('join'); if (!$join) { throw new Exception("Not configured to join any channels!"); } foreach ($join as $channel) { - $this->write('JOIN', $channel); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('JOIN') + ->setBody($channel)); } break; case 'PING': - $this->write('PONG', $message->getRawData()); + $this->writeMessage( + id(new PhabricatorBotMessage()) + ->setCommand('PONG') + ->setBody($message->getBody())); break; } } From 3ce3f5d368031e9597bd0c5119e0a829dfff714f Mon Sep 17 00:00:00 2001 From: Lauri-Henrik Jalonen Date: Thu, 7 Feb 2013 08:02:52 -0800 Subject: [PATCH 19/19] Drafts are saved as inline comments for images when user comments mock Summary: Drafts are saved as inline comments for images when user comments mock. Test Plan: Verified that drafts receive transactionphid when user comments mock. Reviewers: epriestley Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2446 Differential Revision: https://secure.phabricator.com/D4850 --- .../pholio/constants/PholioTransactionType.php | 1 + .../controller/PholioMockCommentController.php | 13 +++++++++++++ src/applications/pholio/editor/PholioMockEditor.php | 13 +++++++++++++ ...abricatorApplicationTransactionCommentEditor.php | 7 +++++-- 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/applications/pholio/constants/PholioTransactionType.php b/src/applications/pholio/constants/PholioTransactionType.php index f2227a0401..561e813b66 100644 --- a/src/applications/pholio/constants/PholioTransactionType.php +++ b/src/applications/pholio/constants/PholioTransactionType.php @@ -4,5 +4,6 @@ final class PholioTransactionType extends PholioConstants { const TYPE_NAME = 'name'; const TYPE_DESCRIPTION = 'description'; + const TYPE_INLINE = 'inline'; } diff --git a/src/applications/pholio/controller/PholioMockCommentController.php b/src/applications/pholio/controller/PholioMockCommentController.php index 51ade41e94..2830adeec8 100644 --- a/src/applications/pholio/controller/PholioMockCommentController.php +++ b/src/applications/pholio/controller/PholioMockCommentController.php @@ -22,6 +22,7 @@ final class PholioMockCommentController extends PholioController { $mock = id(new PholioMockQuery()) ->setViewer($user) ->withIDs(array($this->id)) + ->needImages(true) ->executeOne(); if (!$mock) { @@ -49,6 +50,18 @@ final class PholioMockCommentController extends PholioController { id(new PholioTransactionComment()) ->setContent($comment)); + $inlineComments = id(new PholioTransactionComment())->loadAllWhere( + 'authorphid = %s AND transactionphid IS NULL AND imageid IN (%Ld)', + $user->getPHID(), + mpull($mock->getImages(), 'getID') + ); + + foreach ($inlineComments as $inlineComment) { + $xactions[] = id(new PholioTransaction()) + ->setTransactionType(PholioTransactionType::TYPE_INLINE) + ->attachComment($inlineComment); + } + $editor = id(new PholioMockEditor()) ->setActor($user) ->setContentSource($content_source) diff --git a/src/applications/pholio/editor/PholioMockEditor.php b/src/applications/pholio/editor/PholioMockEditor.php index 36ab5a847a..51502bb593 100644 --- a/src/applications/pholio/editor/PholioMockEditor.php +++ b/src/applications/pholio/editor/PholioMockEditor.php @@ -14,6 +14,7 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { $types[] = PholioTransactionType::TYPE_NAME; $types[] = PholioTransactionType::TYPE_DESCRIPTION; + $types[] = PholioTransactionType::TYPE_INLINE; return $types; } @@ -40,6 +41,18 @@ final class PholioMockEditor extends PhabricatorApplicationTransactionEditor { } } + protected function transactionHasEffect( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PholioTransactionType::TYPE_INLINE: + return true; + } + + return parent::transactionHasEffect($object, $xaction); + } + protected function applyCustomInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php index fe0680a121..fc35653e6b 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php @@ -75,9 +75,12 @@ final class PhabricatorApplicationTransactionCommentEditor "Transaction must have a PHID before calling applyEdit()!"); } - if ($comment->getPHID()) { - throw new Exception( + $type_comment = PhabricatorTransactions::TYPE_COMMENT; + if ($xaction->getTransactionType() == $type_comment) { + if ($comment->getPHID()) { + throw new Exception( "Transaction comment must not yet have a PHID!"); + } } if (!$this->getContentSource()) {