Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#183 Convert Logging report summary report to using CRM_Utils… #15825

Merged
merged 1 commit into from
Nov 14, 2019

Conversation

seamuslee001
Copy link
Contributor

…_SQL_TempTable

Overview

This converts the temporary table created in the logging summary report to using CRM_Utils_SQL_TempTable

Before

Legacy Method used

After

CRM_Utils_SQL_TempTable interface used

ping @eileenmcnaughton @totten the comments suggest there is a unit test that backs up this report

@civibot
Copy link

civibot bot commented Nov 11, 2019

(Standard links)

@civibot civibot bot added the master label Nov 11, 2019
@seamuslee001
Copy link
Contributor Author

Test fails relate there and they are:

api_v3_ReportTemplateTest.testReportTemplateGetRowsAllReports with data set #37
api_v3_ReportTemplateTest.testReportTemplateGetRowsAllReportsACL with data set #37
api_v3_ReportTemplateTest.testReportTemplateGetStatisticsAllReports with data set #37
api_v3_ReportTemplateTest.testReportsCustomDataOrderBy with data set #1
api_v3_ReportTemplateTest.testReportsCustomDataOrderBy with data set #2
api_v3_ReportTemplateTest.testReportsCustomDataOrderBy with data set #3
api_v3_ReportTemplateTest.testLoggingReportWithHookRemovalOfCustomDataTable

Copy link
Contributor

@pfigel pfigel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

  • General standards
    • (r-explain) PASS
    • (r-user) PASS: No user-visible changes
    • (r-doc) PASS
    • (r-run) PASS: Applied various contact changes and confirmed the report still renders correctly. Revert still works. Test with separate CIVICRM_LOGGING_DSN was a pass as well.
  • Developer standards

@eileenmcnaughton
Copy link
Contributor

Thanks @pfigel

@eileenmcnaughton eileenmcnaughton merged commit 61e836b into civicrm:master Nov 14, 2019
@eileenmcnaughton eileenmcnaughton deleted the dev_core_183_logging branch November 14, 2019 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants