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

Adding some metrics requested by customers #22511

Closed
wants to merge 13 commits into from

Conversation

snake14
Copy link
Contributor

@snake14 snake14 commented Aug 16, 2024

Description:

Customers have been requesting some metrics be made available in CustomReports, so this PR is trying to add some of those.

Metrics being added:

  • Average product quantity
  • Average product price
  • Product Revenue (Total & avg)
  • Total click outlinks
  • Product Category (Dimension)
  • Total content interactions (Entries with Content Interaction)
  • Content interaction rate

Internal ticket: PG-3658

Review

@snake14 snake14 added the Needs Review PRs that need a code review label Aug 19, 2024
@snake14 snake14 requested a review from a team August 19, 2024 04:00
@snake14 snake14 removed the Needs Review PRs that need a code review label Aug 19, 2024
@snake14 snake14 removed the request for review from a team August 19, 2024 07:05
@snake14 snake14 marked this pull request as draft August 19, 2024 07:05
@snake14
Copy link
Contributor Author

snake14 commented Aug 19, 2024

Converting to draft because it looks like my latest commit broke some tests.

@sgiehl
Copy link
Member

sgiehl commented Aug 19, 2024

@snake14 In general it would be good to split up the PR into one PR per metric you want to add and where possible add a test for it. That makes it easier to review and merge it step by step.

Note: Time on page won't work that way, as that metric isn't tacked as a normal dimension. Instead it also stores the id of the previous action the stored time is actually valid for. This can't be achieved with a SQL query.

@snake14
Copy link
Contributor Author

snake14 commented Aug 19, 2024

@snake14 In general it would be good to split up the PR into one PR per metric you want to add and where possible add a test for it. That makes it easier to review and merge it step by step.

Note: Time on page won't work that way, as that metric isn't tacked as a normal dimension. Instead it also stores the id of the previous action the stored time is actually valid for. This can't be achieved with a SQL query.

Thank you @sgiehl . I'll remove the time on page metrics. I've been using my CustomReports PR to test most of the metrics.

@snake14
Copy link
Contributor Author

snake14 commented Aug 19, 2024

@sgiehl I'm not finding any good examples of testing new metrics in this repo. Could you please point me in the right direction? Do I mock a new report or something?

@snake14
Copy link
Contributor Author

snake14 commented Aug 23, 2024

@snake14 snake14 closed this Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants