From 2bc9dec85f038c99788eb2a2c23de0285441505d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 14 May 2015 11:15:44 -0700 Subject: [PATCH] Remove "status" field from events Summary: Ref T8183. See that task for discussion. - For now, events always mark users as "Away". - In the future, we may reintroduce "sporradic" or other more complicated availability states, but they would be properties of the invitee, not of the event itself. - This also removes the long-deprecated `user.addstatus` and `user.removestatus` Conduit calls. Test Plan: - Created, edited, viewed events. - Grepped for removed symbols. - Viewed profile calendar. - Viewed Conpherence calendar. - Load Conduit console. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T8183 Differential Revision: https://secure.phabricator.com/D12840 --- .../autopatches/20150514.calendar.status.sql | 2 + src/__phutil_library_map__.php | 4 - ...PhabricatorCalendarEventEditController.php | 14 --- .../editor/PhabricatorCalendarEventEditor.php | 17 +--- .../storage/PhabricatorCalendarEvent.php | 46 ---------- .../PhabricatorCalendarEventTransaction.php | 23 ----- .../ConpherenceWidgetController.php | 14 +-- .../query/ConpherenceThreadQuery.php | 41 ++++++--- .../conduit/UserAddStatusConduitAPIMethod.php | 62 -------------- .../UserRemoveStatusConduitAPIMethod.php | 85 ------------------- 10 files changed, 43 insertions(+), 265 deletions(-) create mode 100644 resources/sql/autopatches/20150514.calendar.status.sql delete mode 100644 src/applications/people/conduit/UserAddStatusConduitAPIMethod.php delete mode 100644 src/applications/people/conduit/UserRemoveStatusConduitAPIMethod.php diff --git a/resources/sql/autopatches/20150514.calendar.status.sql b/resources/sql/autopatches/20150514.calendar.status.sql new file mode 100644 index 0000000000..984205d99e --- /dev/null +++ b/resources/sql/autopatches/20150514.calendar.status.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_calendar.calendar_event + DROP status; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 638674126b..903aaf8a44 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3200,13 +3200,11 @@ phutil_register_library_map(array( 'TokenGiveConduitAPIMethod' => 'applications/tokens/conduit/TokenGiveConduitAPIMethod.php', 'TokenGivenConduitAPIMethod' => 'applications/tokens/conduit/TokenGivenConduitAPIMethod.php', 'TokenQueryConduitAPIMethod' => 'applications/tokens/conduit/TokenQueryConduitAPIMethod.php', - 'UserAddStatusConduitAPIMethod' => 'applications/people/conduit/UserAddStatusConduitAPIMethod.php', 'UserConduitAPIMethod' => 'applications/people/conduit/UserConduitAPIMethod.php', 'UserDisableConduitAPIMethod' => 'applications/people/conduit/UserDisableConduitAPIMethod.php', 'UserEnableConduitAPIMethod' => 'applications/people/conduit/UserEnableConduitAPIMethod.php', 'UserFindConduitAPIMethod' => 'applications/people/conduit/UserFindConduitAPIMethod.php', 'UserQueryConduitAPIMethod' => 'applications/people/conduit/UserQueryConduitAPIMethod.php', - 'UserRemoveStatusConduitAPIMethod' => 'applications/people/conduit/UserRemoveStatusConduitAPIMethod.php', 'UserWhoAmIConduitAPIMethod' => 'applications/people/conduit/UserWhoAmIConduitAPIMethod.php', ), 'function' => array( @@ -6781,13 +6779,11 @@ phutil_register_library_map(array( 'TokenGiveConduitAPIMethod' => 'TokenConduitAPIMethod', 'TokenGivenConduitAPIMethod' => 'TokenConduitAPIMethod', 'TokenQueryConduitAPIMethod' => 'TokenConduitAPIMethod', - 'UserAddStatusConduitAPIMethod' => 'UserConduitAPIMethod', 'UserConduitAPIMethod' => 'ConduitAPIMethod', 'UserDisableConduitAPIMethod' => 'UserConduitAPIMethod', 'UserEnableConduitAPIMethod' => 'UserConduitAPIMethod', 'UserFindConduitAPIMethod' => 'UserConduitAPIMethod', 'UserQueryConduitAPIMethod' => 'UserConduitAPIMethod', - 'UserRemoveStatusConduitAPIMethod' => 'UserConduitAPIMethod', 'UserWhoAmIConduitAPIMethod' => 'UserConduitAPIMethod', ), )); diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php index e31b9bdf0e..900b3ad56b 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php @@ -73,7 +73,6 @@ final class PhabricatorCalendarEventEditController $name = $event->getName(); $description = $event->getDescription(); - $type = $event->getStatus(); $is_all_day = $event->getIsAllDay(); $current_policies = id(new PhabricatorPolicyQuery()) @@ -84,7 +83,6 @@ final class PhabricatorCalendarEventEditController if ($request->isFormPost()) { $xactions = array(); $name = $request->getStr('name'); - $type = $request->getInt('status'); $start_value = AphrontFormDateControlValue::newFromRequest( $request, @@ -128,11 +126,6 @@ final class PhabricatorCalendarEventEditController PhabricatorCalendarEventTransaction::TYPE_END_DATE) ->setNewValue($end_value); - $xactions[] = id(new PhabricatorCalendarEventTransaction()) - ->setTransactionType( - PhabricatorCalendarEventTransaction::TYPE_STATUS) - ->setNewValue($type); - $xactions[] = id(new PhabricatorCalendarEventTransaction()) ->setTransactionType( PhabricatorTransactions::TYPE_SUBSCRIBERS) @@ -195,12 +188,6 @@ final class PhabricatorCalendarEventEditController ->setValue($name) ->setError($error_name); - $status_select = id(new AphrontFormSelectControl()) - ->setLabel(pht('Status')) - ->setName('status') - ->setValue($type) - ->setOptions($event->getStatusOptions()); - $all_day_checkbox = id(new AphrontFormCheckboxControl()) ->addCheckbox( 'isAllDay', @@ -262,7 +249,6 @@ final class PhabricatorCalendarEventEditController $form = id(new AphrontFormView()) ->setUser($user) ->appendChild($name) - ->appendChild($status_select) ->appendChild($all_day_checkbox) ->appendChild($start_control) ->appendChild($end_control) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index 947098ff28..727c33958f 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -17,7 +17,6 @@ final class PhabricatorCalendarEventEditor $types[] = PhabricatorCalendarEventTransaction::TYPE_NAME; $types[] = PhabricatorCalendarEventTransaction::TYPE_START_DATE; $types[] = PhabricatorCalendarEventTransaction::TYPE_END_DATE; - $types[] = PhabricatorCalendarEventTransaction::TYPE_STATUS; $types[] = PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION; $types[] = PhabricatorCalendarEventTransaction::TYPE_CANCEL; $types[] = PhabricatorCalendarEventTransaction::TYPE_INVITE; @@ -40,12 +39,6 @@ final class PhabricatorCalendarEventEditor return $object->getDateFrom(); case PhabricatorCalendarEventTransaction::TYPE_END_DATE: return $object->getDateTo(); - case PhabricatorCalendarEventTransaction::TYPE_STATUS: - $status = $object->getStatus(); - if ($status === null) { - return null; - } - return (int)$status; case PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION: return $object->getDescription(); case PhabricatorCalendarEventTransaction::TYPE_CANCEL: @@ -83,8 +76,6 @@ final class PhabricatorCalendarEventEditor return $xaction->getNewValue(); case PhabricatorCalendarEventTransaction::TYPE_ALL_DAY: return (int)$xaction->getNewValue(); - case PhabricatorCalendarEventTransaction::TYPE_STATUS: - return (int)$xaction->getNewValue(); case PhabricatorCalendarEventTransaction::TYPE_START_DATE: case PhabricatorCalendarEventTransaction::TYPE_END_DATE: return $xaction->getNewValue()->getEpoch(); @@ -107,9 +98,6 @@ final class PhabricatorCalendarEventEditor case PhabricatorCalendarEventTransaction::TYPE_END_DATE: $object->setDateTo($xaction->getNewValue()); return; - case PhabricatorCalendarEventTransaction::TYPE_STATUS: - $object->setStatus($xaction->getNewValue()); - return; case PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION: $object->setDescription($xaction->getNewValue()); return; @@ -139,7 +127,6 @@ final class PhabricatorCalendarEventEditor case PhabricatorCalendarEventTransaction::TYPE_NAME: case PhabricatorCalendarEventTransaction::TYPE_START_DATE: case PhabricatorCalendarEventTransaction::TYPE_END_DATE: - case PhabricatorCalendarEventTransaction::TYPE_STATUS: case PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION: case PhabricatorCalendarEventTransaction::TYPE_CANCEL: case PhabricatorCalendarEventTransaction::TYPE_ALL_DAY: @@ -184,7 +171,9 @@ final class PhabricatorCalendarEventEditor return $xactions; } - protected function applyFinalEffects($object, array $xactions) { + protected function applyFinalEffects( + PhabricatorLiskDAO $object, + array $xactions) { // Clear the availability caches for users whose availability is affected // by this edit. diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index d68c207371..044bc9fc78 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -14,7 +14,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO protected $userPHID; protected $dateFrom; protected $dateTo; - protected $status; protected $description; protected $isCancelled; protected $isAllDay; @@ -26,9 +25,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO private $invitees = self::ATTACHABLE; private $appliedViewer; - const STATUS_AWAY = 1; - const STATUS_SPORADIC = 2; - public static function initializeNewCalendarEvent(PhabricatorUser $actor) { $app = id(new PhabricatorApplicationQuery()) ->setViewer($actor) @@ -160,27 +156,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return ($this->getDateFrom() - phutil_units('15 minutes in seconds')); } - private static $statusTexts = array( - self::STATUS_AWAY => 'away', - self::STATUS_SPORADIC => 'sporadic', - ); - - public function setTextStatus($status) { - $statuses = array_flip(self::$statusTexts); - return $this->setStatus($statuses[$status]); - } - - public function getTextStatus() { - return self::$statusTexts[$this->status]; - } - - public function getStatusOptions() { - return array( - self::STATUS_AWAY => pht('Away'), - self::STATUS_SPORADIC => pht('Sporadic'), - ); - } - protected function getConfiguration() { return array( self::CONFIG_AUX_PHID => true, @@ -188,7 +163,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO 'name' => 'text', 'dateFrom' => 'epoch', 'dateTo' => 'epoch', - 'status' => 'uint32', 'description' => 'text', 'isCancelled' => 'bool', 'isAllDay' => 'bool', @@ -211,26 +185,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return 'E'.$this->getID(); } - public function getTerseSummary(PhabricatorUser $viewer) { - $until = phabricator_date($this->dateTo, $viewer); - if ($this->status == self::STATUS_SPORADIC) { - return pht('Sporadic until %s', $until); - } else { - return pht('Away until %s', $until); - } - } - - public static function getNameForStatus($value) { - switch ($value) { - case self::STATUS_AWAY: - return pht('Away'); - case self::STATUS_SPORADIC: - return pht('Sporadic'); - default: - return pht('Unknown'); - } - } - public function getInvitees() { return $this->assertAttached($this->invitees); } diff --git a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php index d3e7a25efd..91b5d5231f 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php @@ -6,7 +6,6 @@ final class PhabricatorCalendarEventTransaction const TYPE_NAME = 'calendar.name'; const TYPE_START_DATE = 'calendar.startdate'; const TYPE_END_DATE = 'calendar.enddate'; - const TYPE_STATUS = 'calendar.status'; const TYPE_DESCRIPTION = 'calendar.description'; const TYPE_CANCEL = 'calendar.cancel'; const TYPE_ALL_DAY = 'calendar.allday'; @@ -35,7 +34,6 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_NAME: case self::TYPE_START_DATE: case self::TYPE_END_DATE: - case self::TYPE_STATUS: case self::TYPE_DESCRIPTION: case self::TYPE_CANCEL: case self::TYPE_ALL_DAY: @@ -57,7 +55,6 @@ final class PhabricatorCalendarEventTransaction switch ($this->getTransactionType()) { case self::TYPE_START_DATE: case self::TYPE_END_DATE: - case self::TYPE_STATUS: case self::TYPE_DESCRIPTION: case self::TYPE_CANCEL: case self::TYPE_ALL_DAY: @@ -72,7 +69,6 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_NAME: case self::TYPE_START_DATE: case self::TYPE_END_DATE: - case self::TYPE_STATUS: case self::TYPE_DESCRIPTION: case self::TYPE_ALL_DAY: case self::TYPE_CANCEL: @@ -120,14 +116,6 @@ final class PhabricatorCalendarEventTransaction $this->renderHandleLink($author_phid)); } break; - case self::TYPE_STATUS: - $old_name = PhabricatorCalendarEvent::getNameForStatus($old); - $new_name = PhabricatorCalendarEvent::getNameForStatus($new); - return pht( - '%s updated the event status from %s to %s.', - $this->renderHandleLink($author_phid), - $old_name, - $new_name); case self::TYPE_DESCRIPTION: return pht( "%s updated the event's description.", @@ -287,15 +275,6 @@ final class PhabricatorCalendarEventTransaction $new); } break; - case self::TYPE_STATUS: - $old_name = PhabricatorCalendarEvent::getNameForStatus($old); - $new_name = PhabricatorCalendarEvent::getNameForStatus($new); - return pht( - '%s updated the status of %s from %s to %s.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($object_phid), - $old_name, - $new_name); case self::TYPE_DESCRIPTION: return pht( '%s updated the description of %s.', @@ -430,7 +409,6 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_NAME: case self::TYPE_START_DATE: case self::TYPE_END_DATE: - case self::TYPE_STATUS: case self::TYPE_DESCRIPTION: case self::TYPE_CANCEL: case self::TYPE_INVITE: @@ -469,7 +447,6 @@ final class PhabricatorCalendarEventTransaction $tags = array(); switch ($this->getTransactionType()) { case self::TYPE_NAME: - case self::TYPE_STATUS: case self::TYPE_DESCRIPTION: case self::TYPE_INVITE: $tags[] = self::MAILTAG_CONTENT; diff --git a/src/applications/conpherence/controller/ConpherenceWidgetController.php b/src/applications/conpherence/controller/ConpherenceWidgetController.php index 9a06f0dcf9..01a36f6ea1 100644 --- a/src/applications/conpherence/controller/ConpherenceWidgetController.php +++ b/src/applications/conpherence/controller/ConpherenceWidgetController.php @@ -218,7 +218,11 @@ final class ConpherenceWidgetController extends ConpherenceController { $conpherence = $this->getConpherence(); $participants = $conpherence->getParticipants(); $widget_data = $conpherence->getWidgetData(); - $statuses = $widget_data['statuses']; + + // TODO: This panel is built around an outdated notion of events and isn't + // invitee-aware. + + $statuses = $widget_data['events']; $handles = $conpherence->getHandles(); $content = array(); $layout = id(new AphrontMultiColumnView()) @@ -312,7 +316,7 @@ final class ConpherenceWidgetController extends ConpherenceController { $content[] = phutil_tag( 'div', array( - 'class' => 'user-status '.$status->getTextStatus().$top_border, + 'class' => 'user-status '.$top_border, ), array( phutil_tag( @@ -327,7 +331,7 @@ final class ConpherenceWidgetController extends ConpherenceController { 'class' => 'description', ), array( - $status->getTerseSummary($user), + $status->getName(), phutil_tag( 'div', array( @@ -359,9 +363,7 @@ final class ConpherenceWidgetController extends ConpherenceController { if ($status) { $inner_layout[] = phutil_tag( 'div', - array( - 'class' => $status->getTextStatus(), - ), + array(), ''); } else { $inner_layout[] = phutil_tag( diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index b17044ead0..1b311516f8 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -354,13 +354,25 @@ final class ConpherenceThreadQuery $this->getViewer()); $start_epoch = $epochs['start_epoch']; $end_epoch = $epochs['end_epoch']; - $statuses = id(new PhabricatorCalendarEventQuery()) - ->setViewer($this->getViewer()) - ->withInvitedPHIDs($participant_phids) - ->withDateRange($start_epoch, $end_epoch) - ->execute(); - $statuses = mgroup($statuses, 'getUserPHID'); + if ($participant_phids) { + $events = id(new PhabricatorCalendarEventQuery()) + ->setViewer($this->getViewer()) + ->withInvitedPHIDs($participant_phids) + ->withIsCancelled(false) + ->withDateRange($start_epoch, $end_epoch) + ->execute(); + $events = mpull($events, null, 'getPHID'); + } else { + $events = null; + } + + $invitees = array(); + foreach ($events as $event_phid => $event) { + foreach ($event->getInvitees() as $invitee) { + $invitees[$invitee->getInviteePHID()][$event_phid] = true; + } + } // attached files $files = array(); @@ -382,9 +394,16 @@ final class ConpherenceThreadQuery foreach ($conpherences as $phid => $conpherence) { $participant_phids = array_keys($conpherence->getParticipants()); - $statuses = array_select_keys($statuses, $participant_phids); - $statuses = array_mergev($statuses); - $statuses = msort($statuses, 'getDateFrom'); + $widget_data = array(); + + $event_phids = array(); + $participant_invites = array_select_keys($invitees, $participant_phids); + foreach ($participant_invites as $invite_set) { + $event_phids += $invite_set; + } + $thread_events = array_select_keys($events, array_keys($event_phids)); + $thread_events = msort($thread_events, 'getDateFrom'); + $widget_data['events'] = $thread_events; $conpherence_files = array(); $files_authors = array(); @@ -404,11 +423,11 @@ final class ConpherenceThreadQuery } $files_authors[$curr_phid] = $current_author; } - $widget_data = array( - 'statuses' => $statuses, + $widget_data += array( 'files' => $conpherence_files, 'files_authors' => $files_authors, ); + $conpherence->attachWidgetData($widget_data); } diff --git a/src/applications/people/conduit/UserAddStatusConduitAPIMethod.php b/src/applications/people/conduit/UserAddStatusConduitAPIMethod.php deleted file mode 100644 index 39403d9503..0000000000 --- a/src/applications/people/conduit/UserAddStatusConduitAPIMethod.php +++ /dev/null @@ -1,62 +0,0 @@ -formatStringConstants(array('away', 'sporadic')); - - return array( - 'fromEpoch' => 'required int', - 'toEpoch' => 'required int', - 'status' => 'required '.$status_const, - 'description' => 'optional string', - ); - } - - protected function defineReturnType() { - return 'void'; - } - - protected function defineErrorTypes() { - return array( - 'ERR-BAD-EPOCH' => "'toEpoch' must be bigger than 'fromEpoch'.", - 'ERR-OVERLAP' => - 'There must be no status in any part of the specified epoch.', - ); - } - - protected function execute(ConduitAPIRequest $request) { - $user_phid = $request->getUser()->getPHID(); - $from = $request->getValue('fromEpoch'); - $to = $request->getValue('toEpoch'); - $status = $request->getValue('status'); - $description = $request->getValue('description', ''); - - id(new PhabricatorCalendarEvent()) - ->setUserPHID($user_phid) - ->setDateFrom($from) - ->setDateTo($to) - ->setTextStatus($status) - ->setDescription($description) - ->save(); - } - -} diff --git a/src/applications/people/conduit/UserRemoveStatusConduitAPIMethod.php b/src/applications/people/conduit/UserRemoveStatusConduitAPIMethod.php deleted file mode 100644 index 9cb83db1c1..0000000000 --- a/src/applications/people/conduit/UserRemoveStatusConduitAPIMethod.php +++ /dev/null @@ -1,85 +0,0 @@ - 'required int', - 'toEpoch' => 'required int', - ); - } - - protected function defineReturnType() { - return 'int'; - } - - protected function defineErrorTypes() { - return array( - 'ERR-BAD-EPOCH' => "'toEpoch' must be bigger than 'fromEpoch'.", - ); - } - - protected function execute(ConduitAPIRequest $request) { - $user_phid = $request->getUser()->getPHID(); - $from = $request->getValue('fromEpoch'); - $to = $request->getValue('toEpoch'); - - if ($to <= $from) { - throw new ConduitException('ERR-BAD-EPOCH'); - } - - $table = new PhabricatorCalendarEvent(); - $table->openTransaction(); - $table->beginReadLocking(); - - $overlap = $table->loadAllWhere( - 'userPHID = %s AND dateFrom < %d AND dateTo > %d', - $user_phid, - $to, - $from); - foreach ($overlap as $status) { - if ($status->getDateFrom() < $from) { - if ($status->getDateTo() > $to) { - // Split the interval. - id(new PhabricatorCalendarEvent()) - ->setUserPHID($user_phid) - ->setDateFrom($to) - ->setDateTo($status->getDateTo()) - ->setStatus($status->getStatus()) - ->setDescription($status->getDescription()) - ->save(); - } - $status->setDateTo($from); - $status->save(); - } else if ($status->getDateTo() > $to) { - $status->setDateFrom($to); - $status->save(); - } else { - $status->delete(); - } - } - - $table->endReadLocking(); - $table->saveTransaction(); - return count($overlap); - } - -}