From 204d1de68376083373cb5b0880c66ea5cf6b38fa Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 24 Jan 2018 20:21:42 -0800
Subject: [PATCH] Convert storage for Herald repetition policy to "text32"

Summary:
Depends on D18926. Ref T6203. Ref T13048. Herald rule repetition policies are stored as integers but treated as strings in most contexts.

After D18926, the integer stuff is almost totally hidden inside `HeraldRule` and getting rid of it completely isn't too tricky.

Do so now.

Test Plan:
  - Created "only the first time" and "every time" rules. Did a SELECT on their rows in the database.
  - Ran migrations, got a clean bill of health from `storage adjust`.
  - Did another SELECT on the rows, saw a faithful conversion to strings "every" and "first".
  - Edited and reviewed rules, swapping them between "every" and "first".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13048, T6203

Differential Revision: https://secure.phabricator.com/D18927
---
 .../20180124.herald.01.repetition.sql         | 22 ++++++++++++
 .../herald/storage/HeraldRule.php             | 34 +++----------------
 2 files changed, 26 insertions(+), 30 deletions(-)
 create mode 100644 resources/sql/autopatches/20180124.herald.01.repetition.sql

diff --git a/resources/sql/autopatches/20180124.herald.01.repetition.sql b/resources/sql/autopatches/20180124.herald.01.repetition.sql
new file mode 100644
index 0000000000..3be9ec627d
--- /dev/null
+++ b/resources/sql/autopatches/20180124.herald.01.repetition.sql
@@ -0,0 +1,22 @@
+/* This column was previously "uint32?" with these values:
+
+  1: run every time
+  0: run only the first time
+
+*/
+
+ALTER TABLE {$NAMESPACE}_herald.herald_rule
+  CHANGE repetitionPolicy
+    repetitionPolicy VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};
+
+/* If the old value was "0", the new value is "first". */
+
+UPDATE {$NAMESPACE}_herald.herald_rule
+  SET repetitionPolicy = 'first'
+  WHERE repetitionPolicy = '0';
+
+/* If the old value was anything else, the new value is "every". */
+
+UPDATE {$NAMESPACE}_herald.herald_rule
+  SET repetitionPolicy = 'every'
+  WHERE repetitionPolicy NOT IN ('first', '0');
diff --git a/src/applications/herald/storage/HeraldRule.php b/src/applications/herald/storage/HeraldRule.php
index 6d6f04c7c7..8fd1127eb7 100644
--- a/src/applications/herald/storage/HeraldRule.php
+++ b/src/applications/herald/storage/HeraldRule.php
@@ -41,13 +41,10 @@ final class HeraldRule extends HeraldDAO
         'contentType' => 'text255',
         'mustMatchAll' => 'bool',
         'configVersion' => 'uint32',
+        'repetitionPolicy' => 'text32',
         'ruleType' => 'text32',
         'isDisabled' => 'uint32',
         'triggerObjectPHID' => 'phid?',
-
-        // T6203/NULLABILITY
-        // This should not be nullable.
-        'repetitionPolicy' => 'uint32?',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'key_name' => array(
@@ -261,27 +258,11 @@ final class HeraldRule extends HeraldDAO
 
 
   public function getRepetitionPolicyStringConstant() {
-    $map = self::getRepetitionPolicyMap();
-    $map = ipull($map, 'key.string', 'key.int');
-
-    return idx($map, $this->getRepetitionPolicyIntegerConstant());
-  }
-
-  public function getRepetitionPolicyIntegerConstant() {
-    $map = self::getRepetitionPolicyMap();
-    $map = ipull($map, 'key.int', 'key.int');
-    $int = $this->getRepetitionPolicy();
-
-    if (!isset($map[$int])) {
-      return head_key($map);
-    }
-
-    return $int;
+    return $this->getRepetitionPolicy();
   }
 
   public function setRepetitionPolicyStringConstant($value) {
     $map = self::getRepetitionPolicyMap();
-    $map = ipull($map, 'key.int', 'key.string');
 
     if (!isset($map[$value])) {
       throw new Exception(
@@ -290,9 +271,7 @@ final class HeraldRule extends HeraldDAO
           $value));
     }
 
-    $int = $map[$value];
-
-    return $this->setRepetitionPolicy($int);
+    return $this->setRepetitionPolicy($value);
   }
 
   public function isRepeatEvery() {
@@ -305,20 +284,15 @@ final class HeraldRule extends HeraldDAO
 
   public static function getRepetitionPolicySelectOptionMap() {
     $map = self::getRepetitionPolicyMap();
-    $map = ipull($map, 'select', 'key.string');
-    return $map;
+    return ipull($map, 'select');
   }
 
   private static function getRepetitionPolicyMap() {
     return array(
       self::REPEAT_EVERY => array(
-        'key.int' => 1,
-        'key.string' => self::REPEAT_EVERY,
         'select' => pht('every time'),
       ),
       self::REPEAT_FIRST => array(
-        'key.int' => 0,
-        'key.string' => self::REPEAT_FIRST,
         'select' => pht('only the first time'),
       ),
     );