From: Vangelis Pantazis <v.pantazis@ixiam.com>
Date: Tue, 3 Nov 2020 17:29:08 +0000 (+0000)
Subject: Adds performance improvement when browsing the logs
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=2f41db9327ef21663cd4bf80448ea00232f73612;p=civicrm-core.git

Adds performance improvement when browsing the logs
---

diff --git a/CRM/Logging/Differ.php b/CRM/Logging/Differ.php
index 863322bb2a..76fc9eb5c6 100644
--- a/CRM/Logging/Differ.php
+++ b/CRM/Logging/Differ.php
@@ -403,11 +403,20 @@ ORDER BY log_date
    *
    * @param array $tables
    *   Array of tables to inspect.
+   * @param int $limit
+   *   Limit result to x
+   * @param int $offset
+   *   Offset result to y
    *
    * @return array
    */
-  public function getAllChangesForConnection($tables) {
-    $params = [1 => [$this->log_conn_id, 'String']];
+  public function getAllChangesForConnection($tables, $limit = 0, $offset = 0) {
+    $params = [
+      1 => [$this->log_conn_id, 'String'],
+      2 => [$limit, 'Integer'],
+      3 => [$offset, 'Integer'],
+    ];
+
     foreach ($tables as $table) {
       if (empty($sql)) {
         $sql = " SELECT '{$table}' as table_name, id FROM {$this->db}.log_{$table} WHERE log_conn_id = %1";
@@ -416,6 +425,12 @@ ORDER BY log_date
         $sql .= " UNION SELECT '{$table}' as table_name, id FROM {$this->db}.log_{$table} WHERE log_conn_id = %1";
       }
     }
+    if ($limit) {
+      $sql .= " LIMIT %2";
+    }
+    if ($offset) {
+      $sql .= " OFFSET %3";
+    }
     $diffs = [];
     $dao = CRM_Core_DAO::executeQuery($sql, $params);
     while ($dao->fetch()) {
@@ -427,6 +442,30 @@ ORDER BY log_date
     return $diffs;
   }
 
+  /**
+   * Get count of all changes made in the connection.
+   *
+   * @param array $tables
+   *   Array of tables to inspect.
+   *
+   * @return array
+   */
+  public function getCountOfAllContactChangesForConnection($tables) {
+    $count = 0;
+    $params = [1 => [$this->log_conn_id, 'String']];
+    foreach ($tables as $table) {
+      if (empty($sql)) {
+        $sql = " SELECT '{$table}' as table_name, id FROM {$this->db}.log_{$table} WHERE log_conn_id = %1";
+      }
+      else {
+        $sql .= " UNION SELECT '{$table}' as table_name, id FROM {$this->db}.log_{$table} WHERE log_conn_id = %1";
+      }
+    }
+    $countSQL = " SELECT count(*) as countOfContacts FROM ({$sql}) count";
+    $count = CRM_Core_DAO::singleValueQuery($countSQL, $params);
+    return $count;
+  }
+
   /**
    * Check that the log record relates to a unique log id.
    *
diff --git a/CRM/Logging/ReportDetail.php b/CRM/Logging/ReportDetail.php
index fdbd92216a..ad16340ec6 100644
--- a/CRM/Logging/ReportDetail.php
+++ b/CRM/Logging/ReportDetail.php
@@ -15,6 +15,8 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 class CRM_Logging_ReportDetail extends CRM_Report_Form {
+
+  const ROW_COUNT_LIMIT = 50;
   protected $cid;
 
   /**
@@ -172,6 +174,7 @@ class CRM_Logging_ReportDetail extends CRM_Report_Form {
 
     // populate $rows with only the differences between $changed and $original (skipping certain columns and NULL ↔ empty changes unless raw requested)
     $skipped = ['id'];
+    $nRows = $rows = [];
     foreach ($this->diffs as $diff) {
       $table = $diff['table'];
       if (empty($metadata[$table])) {
@@ -228,10 +231,31 @@ class CRM_Logging_ReportDetail extends CRM_Report_Form {
           $to = '';
         }
       }
-
-      $rows[] = ['field' => $field . " (id: {$diff['id']})", 'from' => $from, 'to' => $to];
+      // Rework the results to provide grouping based on the ID
+      // We don't need that field displayed so we will output empty
+      if ($field == 'Modified Date') {
+        $nRows[$diff['id']][] = ['field' => '', 'from' => $from, 'to' => $to];
+      }
+      else {
+        $nRows[$diff['id']][] = ['field' => $field . " (id: {$diff['id']})", 'from' => $from, 'to' => $to];
+      }
     }
+    // Transform the output so that we can compact the changes into the proper amount of rows IF trData is holding more than 1 array
+    foreach ($nRows as $trData) {
+      if (count($trData) > 1) {
+        $keys = array_intersect(...array_map('array_keys', $trData));
+        $mergedRes = array_combine($keys, array_map(function ($key) use ($trData) {
+          // If more than 1 entry is found, we are assigning them as subarrays, then the tpls will be responsible for concatenating the results
+          return array_column($trData, $key);
+        }, $keys));
+        $rows[] = $mergedRes;
+      }
+      else {
+        // We always need the first row of that array
+        $rows[] = $trData[0];
+      }
 
+    }
     return $rows;
   }
 
@@ -266,6 +290,9 @@ class CRM_Logging_ReportDetail extends CRM_Report_Form {
    * Calculate all the contact related diffs for the change.
    */
   protected function calculateContactDiffs() {
+    $this->_rowsFound = $this->getCountOfAllContactChangesForConnection();
+    // Apply some limits before asking for all contact changes
+    $this->getLimit();
     $this->diffs = $this->getAllContactChangesForConnection();
   }
 
@@ -280,10 +307,28 @@ class CRM_Logging_ReportDetail extends CRM_Report_Form {
     }
     $this->setDiffer();
     try {
-      return $this->differ->getAllChangesForConnection($this->tables);
+      return $this->differ->getAllChangesForConnection($this->tables, $this->dblimit, $this->dboffset);
+    }
+    catch (CRM_Core_Exception $e) {
+      CRM_Core_Error::statusBounce($e->getMessage());
+    }
+  }
+
+  /**
+   * Get an count of contacts with changes.
+   *
+   * @return mixed
+   */
+  public function getCountOfAllContactChangesForConnection() {
+    if (empty($this->log_conn_id)) {
+      return [];
+    }
+    $this->setDiffer();
+    try {
+      return $this->differ->getCountOfAllContactChangesForConnection($this->tables);
     }
     catch (CRM_Core_Exception $e) {
-      CRM_Core_Error::statusBounce(ts($e->getMessage()));
+      CRM_Core_Error::statusBounce($e->getMessage());
     }
   }
 
@@ -347,6 +392,61 @@ class CRM_Logging_ReportDetail extends CRM_Report_Form {
     $this->altered_name = CRM_Utils_Request::retrieve('alteredName', 'String');
     $this->altered_by = CRM_Utils_Request::retrieve('alteredBy', 'String');
     $this->altered_by_id = CRM_Utils_Request::retrieve('alteredById', 'Integer');
+    $this->layout = CRM_Utils_Request::retrieve('layout', 'String');
+  }
+
+  /**
+   * Override to set limit
+   * @param int $rowCount
+   */
+  public function limit($rowCount = self::ROW_COUNT_LIMIT) {
+    parent::limit($rowCount);
+  }
+
+  /**
+   * Override to set pager with limit
+   * @param int $rowCount
+   */
+  public function setPager($rowCount = self::ROW_COUNT_LIMIT) {
+    // We should not be rendering the pager in overlay mode
+    if (!isset($this->layout)) {
+      $this->_dashBoardRowCount = $rowCount;
+      $this->_limit = TRUE;
+      parent::setPager($rowCount);
+    }
+  }
+
+  /**
+   * This is a function similar to limit, in fact we copied it as-is and removed
+   * some `set` statements
+   *
+   */
+  public function getLimit($rowCount = self::ROW_COUNT_LIMIT) {
+    if ($this->addPaging) {
+
+      $pageId = CRM_Utils_Request::retrieve('crmPID', 'Integer');
+
+      // @todo all http vars should be extracted in the preProcess
+      // - not randomly in the class
+      if (!$pageId && !empty($_POST)) {
+        if (isset($_POST['PagerBottomButton']) && isset($_POST['crmPID_B'])) {
+          $pageId = max((int) $_POST['crmPID_B'], 1);
+        }
+        elseif (isset($_POST['PagerTopButton']) && isset($_POST['crmPID'])) {
+          $pageId = max((int) $_POST['crmPID'], 1);
+        }
+        unset($_POST['crmPID_B'], $_POST['crmPID']);
+      }
+
+      $pageId = $pageId ? $pageId : 1;
+      $offset = ($pageId - 1) * $rowCount;
+
+      $offset = CRM_Utils_Type::escape($offset, 'Int');
+      $rowCount = CRM_Utils_Type::escape($rowCount, 'Int');
+      $this->_limit = " LIMIT $offset, $rowCount";
+      $this->dblimit = $rowCount;
+      $this->dboffset = $offset;
+    }
   }
 
 }
diff --git a/templates/CRM/Report/Form/Layout/Overlay.tpl b/templates/CRM/Report/Form/Layout/Overlay.tpl
index cb582857e3..a75e03b04f 100644
--- a/templates/CRM/Report/Form/Layout/Overlay.tpl
+++ b/templates/CRM/Report/Form/Layout/Overlay.tpl
@@ -86,7 +86,11 @@
                             <a title="{$row.$fieldHover|escape}" href="{$row.$fieldLink}" {$row.$fieldClass}>
                         {/if}
 
-                        {if $row.$field eq 'Subtotal'}
+                        {if is_array($row.$field)}
+                            {foreach from=$row.$field item=fieldrow key=fieldid}
+                                <div class="crm-report-{$field}-row-{$fieldid}">{$fieldrow}</div>
+                            {/foreach}
+                        {elseif $row.$field eq 'Subtotal'}
                             {$row.$field}
                         {elseif $header.type & 4 OR $header.type & 256}
                             {if $header.group_by eq 'MONTH' or $header.group_by eq 'QUARTER'}
diff --git a/templates/CRM/Report/Form/Layout/Table.tpl b/templates/CRM/Report/Form/Layout/Table.tpl
index 4e0c9c2401..3e28f420ac 100644
--- a/templates/CRM/Report/Form/Layout/Table.tpl
+++ b/templates/CRM/Report/Form/Layout/Table.tpl
@@ -100,7 +100,11 @@
                             <a title="{$row.$fieldHover|escape}" href="{$row.$fieldLink}"  {if $row.$fieldClass} class="{$row.$fieldClass}"{/if}>
                         {/if}
 
-                        {if $row.$field eq 'Subtotal'}
+                        {if is_array($row.$field)}
+                            {foreach from=$row.$field item=fieldrow key=fieldid}
+                                <div class="crm-report-{$field}-row-{$fieldid}">{$fieldrow}</div>
+                            {/foreach}
+                        {elseif $row.$field eq 'Subtotal'}
                             {$row.$field}
                         {elseif $header.type & 4 OR $header.type & 256}
                             {if $header.group_by eq 'MONTH' or $header.group_by eq 'QUARTER'}