Skip to content

Commit

Permalink
Enable tests on the logging summary report.
Browse files Browse the repository at this point in the history 
Despite seeming like a lot of change this is mostly just extraction to run via the tests. I can break out into a couple of refactor commits for reviewablility.

I have tested this on mysql 5.6 with full group by mode enabled.
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed May 1, 2018
1 parent d4fc822 commit 1b25f59
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 100 deletions.
4 changes: 2 additions & 2 deletions CRM/Core/DAO.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public static function getConnection() {
*/
public static function disableFullGroupByMode() {
$currentModes = CRM_Utils_SQL::getSqlModes();
if (CRM_Utils_SQL::supportsFullGroupBy() && in_array('ONLY_FULL_GROUP_BY', $currentModes) && CRM_Utils_SQL::isGroupByModeInDefault()) {
if (in_array('ONLY_FULL_GROUP_BY', $currentModes) && CRM_Utils_SQL::isGroupByModeInDefault()) {
$key = array_search('ONLY_FULL_GROUP_BY', $currentModes);
unset($currentModes[$key]);
CRM_Core_DAO::executeQuery("SET SESSION sql_mode = %1", array(1 => array(implode(',', $currentModes), 'String')));
Expand All @@ -181,7 +181,7 @@ public static function disableFullGroupByMode() {
*/
public static function reenableFullGroupByMode() {
$currentModes = CRM_Utils_SQL::getSqlModes();
if (CRM_Utils_SQL::supportsFullGroupBy() && !in_array('ONLY_FULL_GROUP_BY', $currentModes) && CRM_Utils_SQL::isGroupByModeInDefault()) {
if (!in_array('ONLY_FULL_GROUP_BY', $currentModes) && CRM_Utils_SQL::isGroupByModeInDefault()) {
$currentModes[] = 'ONLY_FULL_GROUP_BY';
CRM_Core_DAO::executeQuery("SET SESSION sql_mode = %1", array(1 => array(implode(',', $currentModes), 'String')));
}
Expand Down
231 changes: 139 additions & 92 deletions CRM/Logging/ReportSummary.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ class CRM_Logging_ReportSummary extends CRM_Report_Form {
*/
protected $currentLogTable;

/**
* Clause used in the final run of buildQuery but now when doing preliminary work.
*
* We do this to all the api to run this report.
*
* @var string
*/
protected $logTypeTableClause;

/**
* Class constructor.
*/
Expand Down Expand Up @@ -227,100 +236,10 @@ public function postProcess() {
$this->beginPostProcess();
$rows = array();

$tempColumns = "id int(10), log_civicrm_entity_log_grouping varchar(32)";
if (!empty($this->_params['fields']['log_action'])) {
$tempColumns .= ", log_action varchar(64)";
}
$tempColumns .= ", log_type varchar(64), log_user_id int(10), log_date timestamp";
if (!empty($this->_params['fields']['altered_contact'])) {
$tempColumns .= ", altered_contact varchar(128)";
}
$tempColumns .= ", altered_contact_id int(10), log_conn_id varchar(17), is_deleted tinyint(4)";
if (!empty($this->_params['fields']['display_name'])) {
$tempColumns .= ", display_name varchar(128)";
}

// temp table to hold all altered contact-ids
$sql = "CREATE TEMPORARY TABLE civicrm_temp_civireport_logsummary ( {$tempColumns} ) ENGINE=HEAP";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);

$logTypes = CRM_Utils_Array::value('log_type_value', $this->_params);
unset($this->_params['log_type_value']);
if (empty($logTypes)) {
foreach (array_keys($this->_logTables) as $table) {
$type = $this->getLogType($table);
$logTypes[$type] = $type;
}
}
$this->buildTemporaryTables();
$sql = $this->buildQuery();

$logTypeTableClause = '(1)';
if ($logTypeTableValue = CRM_Utils_Array::value("log_type_table_value", $this->_params)) {
$logTypeTableClause = $this->whereClause($this->_columns['log_civicrm_entity']['filters']['log_type_table'],
$this->_params['log_type_table_op'], $logTypeTableValue, NULL, NULL);
unset($this->_params['log_type_table_value']);
}

foreach ($this->_logTables as $entity => $detail) {
if ((in_array($this->getLogType($entity), $logTypes) &&
CRM_Utils_Array::value('log_type_op', $this->_params) == 'in') ||
(!in_array($this->getLogType($entity), $logTypes) &&
CRM_Utils_Array::value('log_type_op', $this->_params) == 'notin')
) {
$this->currentLogTable = $entity;
$sql = $this->buildQuery(FALSE);
$sql = str_replace("entity_log_civireport.log_type as", "'{$entity}' as", $sql);
$sql = "INSERT IGNORE INTO civicrm_temp_civireport_logsummary {$sql}";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
}
}

$this->currentLogTable = '';

// add computed log_type column so that we can do a group by after that, which will help
// alterDisplay() counts sync with pager counts
$sql = "SELECT DISTINCT log_type FROM civicrm_temp_civireport_logsummary";
$dao = CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
$replaceWith = array();
while ($dao->fetch()) {
$type = $this->getLogType($dao->log_type);
if (!array_key_exists($type, $replaceWith)) {
$replaceWith[$type] = array();
}
$replaceWith[$type][] = $dao->log_type;
}
foreach ($replaceWith as $type => $tables) {
if (!empty($tables)) {
$replaceWith[$type] = implode("','", $tables);
}
}

$sql = "ALTER TABLE civicrm_temp_civireport_logsummary ADD COLUMN log_civicrm_entity_log_type_label varchar(64)";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
foreach ($replaceWith as $type => $in) {
$sql = "UPDATE civicrm_temp_civireport_logsummary SET log_civicrm_entity_log_type_label='{$type}', log_date=log_date WHERE log_type IN('$in')";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
}

// note the group by columns are same as that used in alterDisplay as $newRows - $key
$this->limit();
$this->orderBy();
$sql = "{$this->_select}
FROM civicrm_temp_civireport_logsummary entity_log_civireport
WHERE {$logTypeTableClause}
GROUP BY log_civicrm_entity_log_date, log_civicrm_entity_log_type_label, log_civicrm_entity_log_conn_id, log_civicrm_entity_log_user_id, log_civicrm_entity_altered_contact_id, log_civicrm_entity_log_grouping
{$this->_orderBy}
{$this->_limit} ";
$sql = str_replace('modified_contact_civireport.display_name', 'entity_log_civireport.altered_contact', $sql);
$sql = str_replace('modified_contact_civireport.id', 'entity_log_civireport.altered_contact_id', $sql);
$sql = str_replace(array(
'modified_contact_civireport.',
'altered_by_contact_civireport.',
), 'entity_log_civireport.', $sql);
$this->buildRows($sql, $rows);
$this->addToDeveloperTab($sql);

Expand Down Expand Up @@ -438,4 +357,132 @@ public function getEntityAction($id, $connId, $entity, $oldAction) {
return NULL;
}

/**
* Build the temporary tables for the query.
*
* @return array
*/
protected function buildTemporaryTables() {
$tempColumns = "id int(10), log_civicrm_entity_log_grouping varchar(32)";
if (!empty($this->_params['fields']['log_action'])) {
$tempColumns .= ", log_action varchar(64)";
}
$tempColumns .= ", log_type varchar(64), log_user_id int(10), log_date timestamp";
if (!empty($this->_params['fields']['altered_contact'])) {
$tempColumns .= ", altered_contact varchar(128)";
}
$tempColumns .= ", altered_contact_id int(10), log_conn_id varchar(17), is_deleted tinyint(4)";
if (!empty($this->_params['fields']['display_name'])) {
$tempColumns .= ", display_name varchar(128)";
}

// temp table to hold all altered contact-ids
$sql = "CREATE TEMPORARY TABLE civicrm_temp_civireport_logsummary ( {$tempColumns} ) ENGINE=HEAP";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);

$logTypes = CRM_Utils_Array::value('log_type_value', $this->_params);
unset($this->_params['log_type_value']);
if (empty($logTypes)) {
foreach (array_keys($this->_logTables) as $table) {
$type = $this->getLogType($table);
$logTypes[$type] = $type;
}
}

$logTypeTableClause = '(1)';
if ($logTypeTableValue = CRM_Utils_Array::value("log_type_table_value", $this->_params)) {
$logTypeTableClause = $this->whereClause($this->_columns['log_civicrm_entity']['filters']['log_type_table'],
$this->_params['log_type_table_op'], $logTypeTableValue, NULL, NULL);
unset($this->_params['log_type_table_value']);
}

foreach ($this->_logTables as $entity => $detail) {
if ((in_array($this->getLogType($entity), $logTypes) &&
CRM_Utils_Array::value('log_type_op', $this->_params) == 'in') ||
(!in_array($this->getLogType($entity), $logTypes) &&
CRM_Utils_Array::value('log_type_op', $this->_params) == 'notin')
) {
$this->currentLogTable = $entity;
$sql = $this->buildQuery(FALSE);
$sql = str_replace("entity_log_civireport.log_type as", "'{$entity}' as", $sql);
$sql = "INSERT IGNORE INTO civicrm_temp_civireport_logsummary {$sql}";
CRM_Core_DAO::disableFullGroupByMode();
CRM_Core_DAO::executeQuery($sql);
CRM_Core_DAO::reenableFullGroupByMode();
$this->addToDeveloperTab($sql);
}
}

$this->currentLogTable = '';

// add computed log_type column so that we can do a group by after that, which will help
// alterDisplay() counts sync with pager counts
$sql = "SELECT DISTINCT log_type FROM civicrm_temp_civireport_logsummary";
$dao = CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
$replaceWith = array();
while ($dao->fetch()) {
$type = $this->getLogType($dao->log_type);
if (!array_key_exists($type, $replaceWith)) {
$replaceWith[$type] = array();
}
$replaceWith[$type][] = $dao->log_type;
}
foreach ($replaceWith as $type => $tables) {
if (!empty($tables)) {
$replaceWith[$type] = implode("','", $tables);
}
}

$sql = "ALTER TABLE civicrm_temp_civireport_logsummary ADD COLUMN log_civicrm_entity_log_type_label varchar(64)";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
foreach ($replaceWith as $type => $in) {
$sql = "UPDATE civicrm_temp_civireport_logsummary SET log_civicrm_entity_log_type_label='{$type}', log_date=log_date WHERE log_type IN('$in')";
CRM_Core_DAO::executeQuery($sql);
$this->addToDeveloperTab($sql);
}
$this->logTypeTableClause = $logTypeTableClause;
}

/**
* Common processing, also via api/unit tests.
*/
public function beginPostProcessCommon() {
parent::beginPostProcessCommon();
$this->buildTemporaryTables();
}

/**
* Build the report query.
*
* We override this in order to be able to run from the api.
*
* @param bool $applyLimit
*
* @return string
*/
public function buildQuery($applyLimit = TRUE) {
if (!$this->logTypeTableClause) {
return parent::buildQuery($applyLimit);
}
// note the group by columns are same as that used in alterDisplay as $newRows - $key
$this->limit();
$this->orderBy();
$sql = "{$this->_select}
FROM civicrm_temp_civireport_logsummary entity_log_civireport
WHERE {$this->logTypeTableClause}
GROUP BY log_civicrm_entity_log_date, log_civicrm_entity_log_type_label, log_civicrm_entity_log_conn_id, log_civicrm_entity_log_user_id, log_civicrm_entity_altered_contact_id, log_civicrm_entity_log_grouping
{$this->_orderBy}
{$this->_limit} ";
$sql = str_replace('modified_contact_civireport.display_name', 'entity_log_civireport.altered_contact', $sql);
$sql = str_replace('modified_contact_civireport.id', 'entity_log_civireport.altered_contact_id', $sql);
$sql = str_replace(array(
'modified_contact_civireport.',
'altered_by_contact_civireport.',
), 'entity_log_civireport.', $sql);
return $sql;
}

}
6 changes: 6 additions & 0 deletions CRM/Report/Form/Contact/LoggingSummary.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
* @copyright CiviCRM LLC (c) 2004-2018
*/
class CRM_Report_Form_Contact_LoggingSummary extends CRM_Logging_ReportSummary {

public $optimisedForOnlyFullGroupBy = FALSE;
/**
* Class constructor.
*/
Expand Down Expand Up @@ -299,6 +301,10 @@ public function alterDisplay(&$rows) {
* Generate From Clause.
*/
public function from() {
if (!$this->currentLogTable) {
// From has already been built in this case.
return;
}
$entity = $this->currentLogTable;

$detail = $this->_logTables[$entity];
Expand Down
3 changes: 0 additions & 3 deletions CRM/Utils/SQL.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,6 @@ public static function disableFullGroupByMode() {
* @return bool
*/
public static function isGroupByModeInDefault() {
if (!self::supportsFullGroupBy()) {
return FALSE;
}
$sqlModes = explode(',', CRM_Core_DAO::singleValueQuery('SELECT @@global.sql_mode'));
if (!in_array('ONLY_FULL_GROUP_BY', $sqlModes)) {
return FALSE;
Expand Down
12 changes: 9 additions & 3 deletions tests/phpunit/api/v3/ReportTemplateTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public function testReportTemplateGetRowsContactSummary() {
}

/**
* Tet api to get rows from reports.
* Test api to get rows from reports.
*
* @dataProvider getReportTemplates
*
Expand All @@ -149,12 +149,20 @@ public function testReportTemplateGetRowsContactSummary() {
* @throws \PHPUnit_Framework_IncompleteTestError
*/
public function testReportTemplateGetRowsAllReports($reportID) {
//$reportID = 'logging/contact/summary';
if (stristr($reportID, 'has existing issues')) {
$this->markTestIncomplete($reportID);
}
if (substr($reportID, 0, '7') === 'logging') {
Civi::settings()->set('logging', 1);
}

$this->callAPISuccess('report_template', 'getrows', array(
'report_id' => $reportID,
));
if (substr($reportID, 0, '7') === 'logging') {
Civi::settings()->set('logging', 0);
}
}

/**
Expand Down Expand Up @@ -189,8 +197,6 @@ public static function getReportTemplates() {
$reportsToSkip = array(
'activity' => 'does not respect function signature on from clause',
'event/income' => 'I do no understand why but error is Call to undefined method CRM_Report_Form_Event_Income::from() in CRM/Report/Form.php on line 2120',
'logging/contact/summary' => '(likely to be test related) probably logging off Undefined index: Form/Contact/LoggingSummary.php(231): PHP',
'logging/contribute/summary' => '(likely to be test related) probably logging off DB Error: no such table',
'contribute/history' => 'Declaration of CRM_Report_Form_Contribute_History::buildRows() should be compatible with CRM_Report_Form::buildRows($sql, &$rows)',
'activitySummary' => 'We use temp tables for the main query generation and name are dynamic. These names are not available in stats() when called directly.',
);
Expand Down

0 comments on commit 1b25f59

Please sign in to comment.