Convert Calendar Events to use RRULE frequency constants in various other places

Summary:
Ref T10747.

  - Store recurrence as RRULEs internally.
  - Use RRULE constants.
  - Migrate existing rules to RRULEs.

Test Plan: Ran migration, nothing seemed broken?

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10747

Differential Revision: https://secure.phabricator.com/D16670

resources/sql/autopatches/20161005.cal.01.rrules.php

@@ -0,0 +1,44 @@
+<?php
+
+$table = new PhabricatorCalendarEvent();
+$conn = $table->establishConnection('w');
+$table_name = 'calendar_event';
+
+foreach (new LiskRawMigrationIterator($conn, $table_name) as $row) {
+  $parameters = phutil_json_decode($row['parameters']);
+  if (isset($parameters['recurrenceRule'])) {
+    // This event has already been migrated.
+    continue;
+  }
+
+  if (!$row['isRecurring']) {
+    continue;
+  }
+
+  $old_rule = $row['recurrenceFrequency'];
+  if (!$old_rule) {
+    continue;
+  }
+
+  try {
+    $frequency = phutil_json_decode($old_rule);
+    if ($frequency) {
+      $frequency_rule = $frequency['rule'];
+      $frequency_rule = phutil_utf8_strtoupper($frequency_rule);
+
+      $rrule = id(new PhutilCalendarRecurrenceRule())
+        ->setFrequency($frequency_rule);
+    }
+  } catch (Exception $ex) {
+    continue;
+  }
+
+  $parameters['recurrenceRule'] = $rrule->toDictionary();
+
+  queryfx(
+    $conn,
+    'UPDATE %T SET parameters = %s WHERE id = %d',
+    $table_name,
+    phutil_json_encode($parameters),
+    $row['id']);
+}

src/applications/calendar/controller/PhabricatorCalendarEventViewController.php

@@ -348,9 +348,16 @@ final class PhabricatorCalendarEventViewController
         ->render();
     }
 
-    $rule = $event->getFrequencyRule();
+    $rrule = $event->newRecurrenceRule();
-    switch ($rule) {
+
-      case PhabricatorCalendarEvent::FREQUENCY_DAILY:
+    if ($rrule) {
+      $frequency = $rrule->getFrequency();
+    } else {
+      $frequency = null;
+    }
+
+    switch ($frequency) {
+      case PhutilCalendarRecurrenceRule::FREQUENCY_DAILY:
         if ($is_parent) {
           $message = pht('This event repeats every day.');
         } else {
@@ -359,7 +366,7 @@ final class PhabricatorCalendarEventViewController
             $parent_link);
         }
         break;
-      case PhabricatorCalendarEvent::FREQUENCY_WEEKLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY:
         if ($is_parent) {
           $message = pht('This event repeats every week.');
         } else {
@@ -368,7 +375,7 @@ final class PhabricatorCalendarEventViewController
             $parent_link);
         }
         break;
-      case PhabricatorCalendarEvent::FREQUENCY_MONTHLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY:
         if ($is_parent) {
           $message = pht('This event repeats every month.');
         } else {
@@ -377,7 +384,7 @@ final class PhabricatorCalendarEventViewController
             $parent_link);
         }
         break;
-      case PhabricatorCalendarEvent::FREQUENCY_YEARLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY:
         if ($is_parent) {
           $message = pht('This event repeats every year.');
         } else {

src/applications/calendar/editor/PhabricatorCalendarEventEditEngine.php

@@ -68,10 +68,10 @@ final class PhabricatorCalendarEventEditEngine
     }
 
     $frequency_options = array(
-      PhabricatorCalendarEvent::FREQUENCY_DAILY => pht('Daily'),
+      PhutilCalendarRecurrenceRule::FREQUENCY_DAILY => pht('Daily'),
-      PhabricatorCalendarEvent::FREQUENCY_WEEKLY => pht('Weekly'),
+      PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY => pht('Weekly'),
-      PhabricatorCalendarEvent::FREQUENCY_MONTHLY => pht('Monthly'),
+      PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY => pht('Monthly'),
-      PhabricatorCalendarEvent::FREQUENCY_YEARLY => pht('Yearly'),
+      PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY => pht('Yearly'),
     );
 
     $fields = array(
@@ -142,6 +142,14 @@ final class PhabricatorCalendarEventEditEngine
         ->setConduitTypeDescription(pht('Mark the event as a recurring event.'))
         ->setValue($object->getIsRecurring());
 
+
+      $rrule = $object->newRecurrenceRule();
+      if ($rrule) {
+        $frequency = $rrule->getFrequency();
+      } else {
+        $frequency = null;
+      }
+
       $fields[] = id(new PhabricatorSelectEditField())
         ->setKey('frequency')
         ->setLabel(pht('Frequency'))
@@ -151,7 +159,7 @@ final class PhabricatorCalendarEventEditEngine
         ->setDescription(pht('Recurring event frequency.'))
         ->setConduitDescription(pht('Change the event frequency.'))
         ->setConduitTypeDescription(pht('New event frequency.'))
-        ->setValue($object->getFrequencyRule());
+        ->setValue($frequency);
     }
 
     if ($this->getIsCreate() || $object->getIsRecurring()) {

src/applications/calendar/storage/PhabricatorCalendarEvent.php

@@ -25,7 +25,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
   protected $isStub;
 
   protected $isRecurring = 0;
-  protected $recurrenceFrequency = array();
 
   private $isGhostEvent = false;
   protected $instanceOfEventPHID;
@@ -52,12 +51,7 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
   protected $dateFrom;
   protected $dateTo;
   protected $recurrenceEndDate;
-
+  protected $recurrenceFrequency = array();
-  // Frequency Constants
-  const FREQUENCY_DAILY = 'daily';
-  const FREQUENCY_WEEKLY = 'weekly';
-  const FREQUENCY_MONTHLY = 'monthly';
-  const FREQUENCY_YEARLY = 'yearly';
 
   public static function initializeNewCalendarEvent(PhabricatorUser $actor) {
     $app = id(new PhabricatorApplicationQuery())
@@ -85,10 +79,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
       ->setIsAllDay(0)
       ->setIsStub(0)
       ->setIsRecurring(0)
-      ->setRecurrenceFrequency(
-        array(
-          'rule' => self::FREQUENCY_WEEKLY,
-        ))
       ->setIcon($default_icon)
       ->setViewPolicy($view_policy)
       ->setEditPolicy($edit_policy)
@@ -120,7 +110,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
       ->setInstanceOfEventPHID($this->getPHID())
       ->setSequenceIndex($sequence)
       ->setIsRecurring(true)
-      ->setRecurrenceFrequency($this->getRecurrenceFrequency())
       ->attachParentEvent($this)
       ->setAllDayDateFrom(0)
       ->setAllDayDateTo(0)
@@ -456,27 +445,6 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     return $this;
   }
 
-  public function getFrequencyRule() {
-    return idx($this->recurrenceFrequency, 'rule');
-  }
-
-  public function getFrequencyUnit() {
-    $frequency = $this->getFrequencyRule();
-
-    switch ($frequency) {
-      case 'daily':
-        return 'day';
-      case 'weekly':
-        return 'week';
-      case 'monthly':
-        return 'month';
-      case 'yearly':
-        return 'year';
-      default:
-        return 'day';
-    }
-  }
-
   public function getURI() {
     if ($this->getIsGhostEvent()) {
       $base = $this->getParentEvent()->getURI();
@@ -840,20 +808,27 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
       $datetime->newAbsoluteDateTime()->toDictionary());
   }
 
+  public function setRecurrenceRule(PhutilCalendarRecurrenceRule $rrule) {
+    return $this->setParameter(
+      'recurrenceRule',
+      $rrule->toDictionary());
+  }
 
   public function newRecurrenceRule() {
     if ($this->isChildEvent()) {
       return $this->getParentEvent()->newRecurrenceRule();
     }
 
-    // TODO: This is a little fragile since it relies on the convenient
+    if (!$this->getIsRecurring()) {
-    // definition of FREQUENCY constants here and in RecurrenceRule, but
+      return null;
-    // should be gone soon.
+    }
-    $map = array(
-      'FREQ' => phutil_utf8_strtoupper($this->getFrequencyRule()),
-    );
 
-    $rrule = PhutilCalendarRecurrenceRule::newFromDictionary($map);
+    $dict = $this->getParameter('recurrenceRule');
+    if (!$dict) {
+      return null;
+    }
+
+    $rrule = PhutilCalendarRecurrenceRule::newFromDictionary($dict);
 
     $start = $this->newStartDateTime();
     $rrule->setStartDateTime($start);
@@ -869,6 +844,10 @@ final class PhabricatorCalendarEvent extends PhabricatorCalendarDAO
     $set = new PhutilCalendarRecurrenceSet();
 
     $rrule = $this->newRecurrenceRule();
+    if (!$rrule) {
+      return null;
+    }
+
     $set->addSource($rrule);
 
     return $set;

src/applications/calendar/xaction/PhabricatorCalendarEventFrequencyTransaction.php

@@ -6,32 +6,66 @@ final class PhabricatorCalendarEventFrequencyTransaction
   const TRANSACTIONTYPE = 'calendar.frequency';
 
   public function generateOldValue($object) {
-    return $object->getFrequencyRule();
+    $rrule = $object->newRecurrenceRule();
+
+    if (!$rrule) {
+      return null;
+    }
+
+    return $rrule->getFrequency();
   }
 
   public function applyInternalEffects($object, $value) {
-    $object->setRecurrenceFrequency(
+    $rrule = id(new PhutilCalendarRecurrenceRule())
-      array(
+      ->setFrequency($value);
-        'rule' => $value,
+
-      ));
+    $dict = $rrule->toDictionary();
+    $object->setRecurrenceRule($dict);
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $valid = array(
+      PhutilCalendarRecurrenceRule::FREQUENCY_DAILY,
+      PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY,
+      PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY,
+      PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY,
+    );
+    $valid = array_fuse($valid);
+
+    foreach ($xactions as $xaction) {
+      $value = $xaction->getNewValue();
+
+      if (!isset($valid[$value])) {
+        $errors[] = $this->newInvalidError(
+          pht(
+            'Event frequency "%s" is not valid. Valid frequences are: %s.',
+            $value,
+            implode(', ', $valid)),
+          $xaction);
+      }
+    }
+
+    return $errors;
   }
 
   public function getTitle() {
-    $frequency = $this->getFrequencyRule($this->getNewValue());
+    $frequency = $this->getFrequency($this->getNewValue());
     switch ($frequency) {
-      case PhabricatorCalendarEvent::FREQUENCY_DAILY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_DAILY:
         return pht(
           '%s set this event to repeat daily.',
           $this->renderAuthor());
-      case PhabricatorCalendarEvent::FREQUENCY_WEEKLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY:
         return pht(
           '%s set this event to repeat weekly.',
           $this->renderAuthor());
-      case PhabricatorCalendarEvent::FREQUENCY_MONTHLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY:
         return pht(
           '%s set this event to repeat monthly.',
           $this->renderAuthor());
-      case PhabricatorCalendarEvent::FREQUENCY_YEARLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY:
         return pht(
           '%s set this event to repeat yearly.',
           $this->renderAuthor());
@@ -39,24 +73,24 @@ final class PhabricatorCalendarEventFrequencyTransaction
   }
 
   public function getTitleForFeed() {
-    $frequency = $this->getFrequencyRule($this->getNewValue());
+    $frequency = $this->getFrequency($this->getNewValue());
     switch ($frequency) {
-      case PhabricatorCalendarEvent::FREQUENCY_DAILY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_DAILY:
         return pht(
           '%s set %s to repeat daily.',
           $this->renderAuthor(),
           $this->renderObject());
-      case PhabricatorCalendarEvent::FREQUENCY_WEEKLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_WEEKLY:
         return pht(
           '%s set %s to repeat weekly.',
           $this->renderAuthor(),
           $this->renderObject());
-      case PhabricatorCalendarEvent::FREQUENCY_MONTHLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_MONTHLY:
         return pht(
           '%s set %s to repeat monthly.',
           $this->renderAuthor(),
           $this->renderObject());
-      case PhabricatorCalendarEvent::FREQUENCY_YEARLY:
+      case PhutilCalendarRecurrenceRule::FREQUENCY_YEARLY:
         return pht(
           '%s set %s to repeat yearly.',
           $this->renderAuthor(),
@@ -64,12 +98,18 @@ final class PhabricatorCalendarEventFrequencyTransaction
     }
   }
 
-  private function getFrequencyRule($value) {
+  private function getFrequency($value) {
+    // NOTE: This is normalizing three generations of these transactions
+    // to use RRULE constants. It would be vaguely nice to migrate them
+    // for consistency.
+
     if (is_array($value)) {
       $value = idx($value, 'rule');
     } else {
-      return $value;
+      $value = $value;
     }
+
+    return phutil_utf8_strtoupper($value);
   }
 
 }