Skip to content

add actions goal metrics to processed reports in metadata and report data #20817

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

Merged
merged 8 commits into from
Jun 26, 2023

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Jun 1, 2023

Description:

Special goal metrics added in AddColumnsProcessedMetricsGoal are not currently displaying in processed report output, but they need to be for API consumers like the Looker Studio connector to be able to use them. This PR makes sure they are in report metadata for the appropriate Actions reports, and are calculated properly when requesting processed report data.

Changes:

  • Move the code to determine what processOnlyIdGoal in AddColumnsProcessedMetricsGoal should be from the Goals visualization to AddColumnsProcessedMetricsGoal.
  • Use this new method in ProcessedReport to compute the correct methods for Actions processed reports.
  • In Goals.php in the getReportMetadataEnd hook, make sure the different metrics computed by AddColumnsProcessedMetricsGoal are included in report metadata for the appropriate Actions reports.
  • Bug fix: I noticed that this metric was using the wrong value for the translated name: https://github.com/matomo-org/matomo/blob/5.x-dev/plugins/Goals/Columns/Metrics/GoalSpecific/ConversionsEntry.php#L32
  • Bug fix: I noticed that the wrong idGoal override appears to be used in the existing code on 5.x-dev: https://github.com/matomo-org/matomo/blob/5.x-dev/plugins/Goals/Visualizations/Goals.php#L48. The condition checks for an Actions page report, but sets the idGoal value to GOALS_ENTRY_PAGES_ECOMMERCE instead of GOALS_PAGES_ECOMMERCE.
  • Add a processed report test for an Actions page group w/ goal metrics.

Requested by @mattab for the Looker Studio connector.

Review

@diosmosis diosmosis marked this pull request as draft June 1, 2023 12:22
@diosmosis diosmosis changed the title actions goal metrics in processed reports add actions goal metrics to processed reports in metadata and report data Jun 8, 2023
@diosmosis diosmosis marked this pull request as ready for review June 8, 2023 14:50
@diosmosis diosmosis added this to the 5.0.0 milestone Jun 8, 2023
@diosmosis diosmosis added the Needs Review PRs that need a code review label Jun 8, 2023
@github-actions
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Jun 16, 2023
@sgiehl sgiehl requested a review from bx80 June 16, 2023 08:45
@sgiehl sgiehl force-pushed the actions-goals-processed-report branch from 629d713 to 2079b8d Compare June 20, 2023 12:38
Copy link
Contributor

@bx80 bx80 left a comment

Choose a reason for hiding this comment

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

This all look okay to me, I've checked through the code and compared a few reports without seeing any issues. All tests are passing 👍

@sgiehl sgiehl merged commit 860026e into 5.x-dev Jun 26, 2023
@sgiehl sgiehl deleted the actions-goals-processed-report branch June 26, 2023 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review Stale The label used by the Close Stale Issues action
Development

Successfully merging this pull request may close these issues.

3 participants