Remove Herald rule edit log

Summary:
Fixes T7601. Ref T7803, weakly (this removes a Query subclass with ad-hoc paging). Herald has a very old edit log which predates transactions and is essentially useless and not really policy-aware. I think it's doing more harm than good; remove it.

Herald rules have proper transactions, but rule edits don't currently render something nice into the transaction log. This is definitely the way forward, but we haven't seen requests for this so don't bother building it for now.

I did put a nice end-cap on the transaction log, though.

Test Plan:
  - Viewed Herald UI.
  - Grepped for removed classes and methods.
  - Edited a rule.
  - Viewed rule transaction log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: cburroughs, chad, epriestley

Maniphest Tasks: T7601, T7803

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

resources/sql/autopatches/20150410.nukeruleedit.sql

@@ -0,0 +1 @@
+DROP TABLE {$NAMESPACE}_herald.herald_ruleedit;

src/__phutil_library_map__.php

@@ -902,7 +902,6 @@ phutil_register_library_map(array(
     'HeraldDifferentialDiffAdapter' => 'applications/herald/adapter/HeraldDifferentialDiffAdapter.php',
     'HeraldDifferentialRevisionAdapter' => 'applications/herald/adapter/HeraldDifferentialRevisionAdapter.php',
     'HeraldDisableController' => 'applications/herald/controller/HeraldDisableController.php',
-    'HeraldEditLogQuery' => 'applications/herald/query/HeraldEditLogQuery.php',
     'HeraldEffect' => 'applications/herald/engine/HeraldEffect.php',
     'HeraldEngine' => 'applications/herald/engine/HeraldEngine.php',
     'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php',
@@ -920,9 +919,6 @@ phutil_register_library_map(array(
     'HeraldRepetitionPolicyConfig' => 'applications/herald/config/HeraldRepetitionPolicyConfig.php',
     'HeraldRule' => 'applications/herald/storage/HeraldRule.php',
     'HeraldRuleController' => 'applications/herald/controller/HeraldRuleController.php',
-    'HeraldRuleEdit' => 'applications/herald/storage/HeraldRuleEdit.php',
-    'HeraldRuleEditHistoryController' => 'applications/herald/controller/HeraldRuleEditHistoryController.php',
-    'HeraldRuleEditHistoryView' => 'applications/herald/view/HeraldRuleEditHistoryView.php',
     'HeraldRuleEditor' => 'applications/herald/editor/HeraldRuleEditor.php',
     'HeraldRuleListController' => 'applications/herald/controller/HeraldRuleListController.php',
     'HeraldRulePHIDType' => 'applications/herald/phid/HeraldRulePHIDType.php',
@@ -4146,7 +4142,6 @@ phutil_register_library_map(array(
     'HeraldDifferentialDiffAdapter' => 'HeraldDifferentialAdapter',
     'HeraldDifferentialRevisionAdapter' => 'HeraldDifferentialAdapter',
     'HeraldDisableController' => 'HeraldController',
-    'HeraldEditLogQuery' => 'PhabricatorOffsetPagedQuery',
     'HeraldInvalidActionException' => 'Exception',
     'HeraldInvalidConditionException' => 'Exception',
     'HeraldManageGlobalRulesCapability' => 'PhabricatorPolicyCapability',
@@ -4166,9 +4161,6 @@ phutil_register_library_map(array(
       'PhabricatorDestructibleInterface',
     ),
     'HeraldRuleController' => 'HeraldController',
-    'HeraldRuleEdit' => 'HeraldDAO',
-    'HeraldRuleEditHistoryController' => 'HeraldController',
-    'HeraldRuleEditHistoryView' => 'AphrontView',
     'HeraldRuleEditor' => 'PhabricatorApplicationTransactionEditor',
     'HeraldRuleListController' => 'HeraldController',
     'HeraldRulePHIDType' => 'PhabricatorPHIDType',

src/applications/herald/application/PhabricatorHeraldApplication.php

@@ -54,7 +54,6 @@ final class PhabricatorHeraldApplication extends PhabricatorApplication {
         'edit/(?:(?P<id>[1-9]\d*)/)?' => 'HeraldRuleController',
         'disable/(?P<id>[1-9]\d*)/(?P<action>\w+)/'
           => 'HeraldDisableController',
-        'history/(?:(?P<id>[1-9]\d*)/)?' => 'HeraldRuleEditHistoryController',
         'test/' => 'HeraldTestConsoleController',
         'transcript/' => array(
           '' => 'HeraldTranscriptListController',

src/applications/herald/controller/HeraldController.php

@@ -49,8 +49,7 @@ abstract class HeraldController extends PhabricatorController {
     $nav
       ->addLabel(pht('Utilities'))
       ->addFilter('test', pht('Test Console'))
-      ->addFilter('transcript', pht('Transcripts'))
-      ->addFilter('history',    pht('Edit Log'));
+      ->addFilter('transcript', pht('Transcripts'));
 
     $nav->selectFilter(null);
 

src/applications/herald/controller/HeraldRuleController.php

@@ -328,7 +328,6 @@ final class HeraldRuleController extends HeraldController {
         $rule->save();
         $rule->saveConditions($conditions);
         $rule->saveActions($actions);
-        $rule->logEdit($request->getUser()->getPHID(), $edit_action);
       $rule->saveTransaction();
     }
 

src/applications/herald/controller/HeraldRuleEditHistoryController.php

@@ -1,55 +0,0 @@
-<?php
-
-final class HeraldRuleEditHistoryController extends HeraldController {
-
-  private $id;
-
-  public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
-  }
-
-  public function processRequest() {
-    $request = $this->getRequest();
-
-    $edit_query = new HeraldEditLogQuery();
-    if ($this->id) {
-      $edit_query->withRuleIDs(array($this->id));
-    }
-
-    $pager = new AphrontPagerView();
-    $pager->setURI($request->getRequestURI(), 'offset');
-    $pager->setOffset($request->getStr('offset'));
-
-    $edits = $edit_query->executeWithOffsetPager($pager);
-
-    $need_phids = mpull($edits, 'getEditorPHID');
-    $handles = $this->loadViewerHandles($need_phids);
-
-    $list_view = id(new HeraldRuleEditHistoryView())
-      ->setEdits($edits)
-      ->setHandles($handles)
-      ->setUser($this->getRequest()->getUser());
-
-    $panel = new PHUIObjectBoxView();
-    $panel->setHeaderText(pht('Edit History'));
-    $panel->appendChild($list_view);
-
-    $crumbs = $this
-      ->buildApplicationCrumbs($can_create = false)
-      ->addTextCrumb(
-        pht('Edit History'),
-        $this->getApplicationURI('herald/history'));
-
-    $nav = $this->buildSideNavView();
-    $nav->selectFilter('history');
-    $nav->appendChild($panel);
-    $nav->setCrumbs($crumbs);
-
-    return $this->buildApplicationPage(
-      $nav,
-      array(
-        'title' => pht('Rule Edit History'),
-      ));
-  }
-
-}

src/applications/herald/controller/HeraldRuleViewController.php

@@ -53,6 +53,7 @@ final class HeraldRuleViewController extends HeraldController {
     $timeline = $this->buildTransactionTimeline(
       $rule,
       new HeraldTransactionQuery());
+    $timeline->setShouldTerminate(true);
 
     return $this->buildApplicationPage(
       array(

src/applications/herald/query/HeraldEditLogQuery.php

@@ -1,48 +0,0 @@
-<?php
-
-final class HeraldEditLogQuery extends PhabricatorOffsetPagedQuery {
-
-  private $ruleIDs;
-
-  public function withRuleIDs(array $rule_ids) {
-    $this->ruleIDs = $rule_ids;
-    return $this;
-  }
-
-  public function execute() {
-    $table = new HeraldRuleEdit();
-    $conn_r = $table->establishConnection('r');
-
-    $where = $this->buildWhereClause($conn_r);
-    $order = $this->buildOrderClause($conn_r);
-    $limit = $this->buildLimitClause($conn_r);
-
-    $data = queryfx_all(
-      $conn_r,
-      'SELECT log.* FROM %T log %Q %Q %Q',
-      $table->getTableName(),
-      $where,
-      $order,
-      $limit);
-
-    return $table->loadAllFromArray($data);
-  }
-
-  private function buildWhereClause($conn_r) {
-    $where = array();
-
-    if ($this->ruleIDs) {
-      $where[] = qsprintf(
-        $conn_r,
-        'ruleID IN (%Ld)',
-        $this->ruleIDs);
-    }
-
-    return $this->formatWhereClause($where);
-  }
-
-  private function buildOrderClause($conn_r) {
-    return 'ORDER BY id DESC';
-  }
-
-}

src/applications/herald/storage/HeraldRule.php

@@ -115,26 +115,6 @@ final class HeraldRule extends HeraldDAO
     return $this->actions;
   }
 
-  public function loadEdits() {
-    if (!$this->getID()) {
-      return array();
-    }
-    $edits = id(new HeraldRuleEdit())->loadAllWhere(
-      'ruleID = %d ORDER BY dateCreated DESC',
-      $this->getID());
-
-    return $edits;
-  }
-
-  public function logEdit($editor_phid, $action) {
-    id(new HeraldRuleEdit())
-      ->setRuleID($this->getID())
-      ->setRuleName($this->getName())
-      ->setEditorPHID($editor_phid)
-      ->setAction($action)
-      ->save();
-  }
-
   public function saveConditions(array $conditions) {
     assert_instances_of($conditions, 'HeraldCondition');
     return $this->saveChildren(

src/applications/herald/storage/HeraldRuleEdit.php

@@ -1,24 +0,0 @@
-<?php
-
-final class HeraldRuleEdit extends HeraldDAO {
-
-  protected $editorPHID;
-  protected $ruleID;
-  protected $ruleName;
-  protected $action;
-
-  protected function getConfiguration() {
-    return array(
-      self::CONFIG_COLUMN_SCHEMA => array(
-        'ruleName' => 'text255',
-        'action' => 'text32',
-      ),
-      self::CONFIG_KEY_SCHEMA => array(
-        'ruleID' => array(
-          'columns' => array('ruleID', 'dateCreated'),
-        ),
-      ),
-    ) + parent::getConfiguration();
-  }
-
-}

src/applications/herald/view/HeraldRuleEditHistoryView.php

@@ -1,63 +0,0 @@
-<?php
-
-final class HeraldRuleEditHistoryView extends AphrontView {
-
-  private $edits;
-  private $handles;
-
-  public function setEdits(array $edits) {
-    $this->edits = $edits;
-    return $this;
-  }
-
-  public function getEdits() {
-    return $this->edits;
-  }
-
-  public function setHandles(array $handles) {
-    assert_instances_of($handles, 'PhabricatorObjectHandle');
-    $this->handles = $handles;
-    return $this;
-  }
-
-  public function render() {
-    $list = new PHUIObjectItemListView();
-    $list->setFlush(true);
-
-    foreach ($this->edits as $edit) {
-      $name = nonempty($edit->getRuleName(), 'Unknown Rule');
-      $rule_name = phutil_tag(
-        'strong',
-        array(),
-        $name);
-
-      switch ($edit->getAction()) {
-        case 'create':
-          $details = pht("Created rule '%s'.", $rule_name);
-          break;
-        case 'delete':
-          $details = pht("Deleted rule '%s'.", $rule_name);
-          break;
-        case 'edit':
-        default:
-          $details = pht("Edited rule '%s'.", $rule_name);
-          break;
-      }
-
-      $editor = $this->handles[$edit->getEditorPHID()]->renderLink();
-      $date = phabricator_datetime($edit->getDateCreated(), $this->user);
-
-      $item = id(new PHUIObjectItemView())
-        ->setObjectName(pht('Rule %d', $edit->getRuleID()))
-        ->setSubHead($details)
-        ->addIcon('none', $date)
-        ->addByLine(pht('Editor: %s', $editor));
-
-      $list->addItem($item);
-    }
-
-    $list->setNoDataString(pht('No edits for rule.'));
-
-    return $list;
-  }
-}