From eebaf583421b67f8cb4a615a4ed4299d9bb9bc65 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 12 Jul 2016 09:05:12 -0700 Subject: [PATCH] Simplify the TYPE_INVITE Calendar Event transaction for EditEngine Summary: Ref T9275. Now that TYPE_ACCEPT and TYPE_DECLINE have been separated out, we can simplify TYPE_INVITE. This now just takes a list of invited PHIDs, uninvites ones that were removed and invites ones that were added. This is simpler, lets more logic live in the Editor, and makes EditEngine/API access easier. Test Plan: Created events, added and removed invitees. Used comment stacked action and "pro" editor to adjust invitees. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9275 Differential Revision: https://secure.phabricator.com/D16280 --- ...PhabricatorCalendarEventEditController.php | 47 +--------- .../editor/PhabricatorCalendarEditEngine.php | 19 ++++ .../editor/PhabricatorCalendarEventEditor.php | 90 +++++++++++++++---- .../storage/PhabricatorCalendarEvent.php | 13 +++ .../PhabricatorCalendarEventTransaction.php | 9 +- 5 files changed, 117 insertions(+), 61 deletions(-) diff --git a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php index 888f52c572..4f92fa5b0c 100644 --- a/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php +++ b/src/applications/calendar/controller/PhabricatorCalendarEventEditController.php @@ -105,15 +105,7 @@ final class PhabricatorCalendarEventEditController $subscribers = PhabricatorSubscribersQuery::loadSubscribersForPHID( $event->getPHID()); - $invitees = array(); - foreach ($event->getInvitees() as $invitee) { - if ($invitee->isUninvited()) { - continue; - } else { - $invitees[] = $invitee->getInviteePHID(); - } - } - + $invitees = $event->getInviteePHIDsForEdit(); $cancel_uri = $event->getURI(); } @@ -172,14 +164,6 @@ final class PhabricatorCalendarEventEditController $icon = $request->getStr('icon'); $invitees = $request->getArr('invitees'); - $new_invitees = $this->getNewInviteeList($invitees, $event); - $status_attending = PhabricatorCalendarEventInvitee::STATUS_ATTENDING; - if ($this->isCreate()) { - $status = idx($new_invitees, $viewer->getPHID()); - if ($status) { - $new_invitees[$viewer->getPHID()] = $status_attending; - } - } $xactions[] = id(new PhabricatorCalendarEventTransaction()) ->setTransactionType( @@ -236,7 +220,7 @@ final class PhabricatorCalendarEventEditController $xactions[] = id(new PhabricatorCalendarEventTransaction()) ->setTransactionType( PhabricatorCalendarEventTransaction::TYPE_INVITE) - ->setNewValue($new_invitees); + ->setNewValue($invitees); $xactions[] = id(new PhabricatorCalendarEventTransaction()) ->setTransactionType( @@ -487,7 +471,6 @@ final class PhabricatorCalendarEventEditController ->setUser($viewer) ->setDatasource(new PhabricatorMetaMTAMailableDatasource()); - $icon = id(new PHUIFormIconSetControl()) ->setLabel(pht('Icon')) ->setName('icon') @@ -574,32 +557,6 @@ final class PhabricatorCalendarEventEditController } - public function getNewInviteeList(array $phids, $event) { - $invitees = $event->getInvitees(); - $invitees = mpull($invitees, null, 'getInviteePHID'); - $invited_status = PhabricatorCalendarEventInvitee::STATUS_INVITED; - $uninvited_status = PhabricatorCalendarEventInvitee::STATUS_UNINVITED; - $phids = array_fuse($phids); - - $new = array(); - foreach ($phids as $phid) { - $old_status = $event->getUserInviteStatus($phid); - if ($old_status != $uninvited_status) { - continue; - } - $new[$phid] = $invited_status; - } - - foreach ($invitees as $invitee) { - $deleted_invitee = !idx($phids, $invitee->getInviteePHID()); - if ($deleted_invitee) { - $new[$invitee->getInviteePHID()] = $uninvited_status; - } - } - - return $new; - } - private function getDefaultTimeValues($viewer) { $start = new DateTime('@'.time()); $start->setTimeZone($viewer->getTimeZone()); diff --git a/src/applications/calendar/editor/PhabricatorCalendarEditEngine.php b/src/applications/calendar/editor/PhabricatorCalendarEditEngine.php index 4aaab4634d..ee36c7e82e 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEditEngine.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEditEngine.php @@ -60,6 +60,14 @@ final class PhabricatorCalendarEditEngine } protected function buildCustomEditFields($object) { + $viewer = $this->getViewer(); + + if ($this->getIsCreate()) { + $invitee_phids = array($viewer->getPHID()); + } else { + $invitee_phids = $object->getInviteePHIDsForEdit(); + } + $fields = array( id(new PhabricatorTextEditField()) ->setKey('name') @@ -90,6 +98,17 @@ final class PhabricatorCalendarEditEngine ->setConduitDescription(pht('Cancel or restore the event.')) ->setConduitTypeDescription(pht('True to cancel the event.')) ->setValue($object->getIsCancelled()), + id(new PhabricatorDatasourceEditField()) + ->setKey('inviteePHIDs') + ->setAliases(array('invite', 'invitee', 'invitees', 'inviteePHID')) + ->setLabel(pht('Invitees')) + ->setDatasource(new PhabricatorMetaMTAMailableDatasource()) + ->setTransactionType(PhabricatorCalendarEventTransaction::TYPE_INVITE) + ->setDescription(pht('Users invited to the event.')) + ->setConduitDescription(pht('Change invited users.')) + ->setConduitTypeDescription(pht('New event invitees.')) + ->setValue($invitee_phids) + ->setCommentActionLabel(pht('Change Invitees')), id(new PhabricatorIconSetEditField()) ->setKey('icon') ->setLabel(pht('Icon')) diff --git a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php index ce886d2ed6..16e00a2873 100644 --- a/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php +++ b/src/applications/calendar/editor/PhabricatorCalendarEventEditor.php @@ -109,20 +109,8 @@ final class PhabricatorCalendarEventEditor $actor_phid = $this->getActingAsPHID(); return $object->getUserInviteStatus($actor_phid); case PhabricatorCalendarEventTransaction::TYPE_INVITE: - $map = $xaction->getNewValue(); - $phids = array_keys($map); - $invitees = mpull($object->getInvitees(), null, 'getInviteePHID'); - - $old = array(); - foreach ($phids as $phid) { - $invitee = idx($invitees, $phid); - if ($invitee) { - $old[$phid] = $invitee->getStatus(); - } else { - $old[$phid] = PhabricatorCalendarEventInvitee::STATUS_UNINVITED; - } - } - return $old; + $invitees = $object->getInvitees(); + return mpull($invitees, 'getStatus', 'getInviteePHID'); } return parent::getCustomTransactionOldValue($object, $xaction); @@ -137,7 +125,6 @@ final class PhabricatorCalendarEventEditor case PhabricatorCalendarEventTransaction::TYPE_NAME: case PhabricatorCalendarEventTransaction::TYPE_DESCRIPTION: case PhabricatorCalendarEventTransaction::TYPE_CANCEL: - case PhabricatorCalendarEventTransaction::TYPE_INVITE: case PhabricatorCalendarEventTransaction::TYPE_ICON: return $xaction->getNewValue(); case PhabricatorCalendarEventTransaction::TYPE_ACCEPT: @@ -150,6 +137,45 @@ final class PhabricatorCalendarEventEditor case PhabricatorCalendarEventTransaction::TYPE_START_DATE: case PhabricatorCalendarEventTransaction::TYPE_END_DATE: return $xaction->getNewValue(); + case PhabricatorCalendarEventTransaction::TYPE_INVITE: + $status_invited = PhabricatorCalendarEventInvitee::STATUS_INVITED; + $status_uninvited = PhabricatorCalendarEventInvitee::STATUS_UNINVITED; + $status_attending = PhabricatorCalendarEventInvitee::STATUS_ATTENDING; + + $invitees = $object->getInvitees(); + foreach ($invitees as $key => $invitee) { + if ($invitee->getStatus() == $status_uninvited) { + unset($invitees[$key]); + } + } + $invitees = mpull($invitees, null, 'getInviteePHID'); + + $new = $xaction->getNewValue(); + $new = array_fuse($new); + + $all = array_keys($invitees + $new); + $map = array(); + foreach ($all as $phid) { + $is_old = isset($invitees[$phid]); + $is_new = isset($new[$phid]); + + if ($is_old && !$is_new) { + $map[$phid] = $status_uninvited; + } else if (!$is_old && $is_new) { + $map[$phid] = $status_invited; + } + } + + // If we're creating this event and the actor is inviting themselves, + // mark them as attending. + if ($this->getIsNewObject()) { + $acting_phid = $this->getActingAsPHID(); + if (isset($map[$acting_phid])) { + $map[$acting_phid] = $status_attending; + } + } + + return $map; } return parent::getCustomTransactionNewValue($object, $xaction); @@ -400,6 +426,40 @@ final class PhabricatorCalendarEventEditor $errors[] = $error; } break; + case PhabricatorCalendarEventTransaction::TYPE_INVITE: + $old = $object->getInvitees(); + $old = mpull($old, null, 'getInviteePHID'); + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + $new = array_fuse($new); + $add = array_diff_key($new, $old); + if (!$add) { + continue; + } + + // In the UI, we only allow you to invite mailable objects, but there + // is no definitive marker for "invitable object" today. Just allow + // any valid object to be invited. + $objects = id(new PhabricatorObjectQuery()) + ->setViewer($this->getActor()) + ->withPHIDs($add) + ->execute(); + $objects = mpull($objects, null, 'getPHID'); + foreach ($add as $phid) { + if (isset($objects[$phid])) { + continue; + } + + $errors[] = new PhabricatorApplicationTransactionValidationError( + $type, + pht('Invalid'), + pht( + 'Invitee "%s" identifies an object that does not exist or '. + 'which you do not have permission to view.', + $phid)); + } + } + break; } return $errors; diff --git a/src/applications/calendar/storage/PhabricatorCalendarEvent.php b/src/applications/calendar/storage/PhabricatorCalendarEvent.php index 991bfbf521..3fba3f2e66 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEvent.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEvent.php @@ -329,6 +329,19 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO return $this; } + public function getInviteePHIDsForEdit() { + $invitees = array(); + + foreach ($this->getInvitees() as $invitee) { + if ($invitee->isUninvited()) { + continue; + } + $invitees[] = $invitee->getInviteePHID(); + } + + return $invitees; + } + public function getUserInviteStatus($phid) { $invitees = $this->getInvitees(); $invitees = mpull($invitees, null, 'getInviteePHID'); diff --git a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php index 584c41e569..0040ff8817 100644 --- a/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php +++ b/src/applications/calendar/storage/PhabricatorCalendarEventTransaction.php @@ -18,7 +18,6 @@ final class PhabricatorCalendarEventTransaction const TYPE_FREQUENCY = 'calendar.frequency'; const TYPE_RECURRENCE_END_DATE = 'calendar.recurrenceenddate'; - const MAILTAG_RESCHEDULE = 'calendar-reschedule'; const MAILTAG_CONTENT = 'calendar-content'; const MAILTAG_OTHER = 'calendar-other'; @@ -177,6 +176,11 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_INVITE: $text = null; + // Fill in any new invitees as "uninvited" in the old data, to make + // some of the rendering logic a little easier. + $status_uninvited = PhabricatorCalendarEventInvitee::STATUS_UNINVITED; + $old = $old + array_fill_keys(array_keys($new), $status_uninvited); + if (count($old) === 1 && count($new) === 1 && isset($old[$author_phid])) { @@ -387,6 +391,9 @@ final class PhabricatorCalendarEventTransaction case self::TYPE_INVITE: $text = null; + $status_uninvited = PhabricatorCalendarEventInvitee::STATUS_UNINVITED; + $old = $old + array_fill_keys(array_keys($new), $status_uninvited); + if (count($old) === 1 && count($new) === 1 && isset($old[$author_phid])) {