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.

Sign up for GitHub

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

Jump to bottom

Hide unique visitors email in VisitsSummary.get if not enabled in config. #17260

Merged
merged 1 commit into from Mar 7, 2021
Merged

Hide unique visitors email in VisitsSummary.get if not enabled in config. #17260

merged 1 commit into from Mar 7, 2021

Conversation

diosmosis
Copy link
Member

Description:

Fixes #16222

The only instance of unique visitors I could find when processing was disabled was in VisitsSummary.get, and I noticed the metric is enabled everywhere in this case. This removes the metric everywhere if processing is disabled.

Note: this will also remove the metric even if there is existing data. So if a user computes it for months initially, then disables it for months, the old data won't be visible. Not sure if this is desired (cc @tsteur/@mattab).

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@diosmosis diosmosis added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Feb 24, 2021
@diosmosis diosmosis added this to the 4.3.0 milestone Feb 25, 2021
@diosmosis
Copy link
Member Author

@mattab / @tsteur can you tell me if this is desired behavior:

Note: this will also remove the metric even if there is existing data. So if a user computes it for months initially, then disables it for months, the old data won't be visible. Not sure if this is desired

@tsteur
Copy link
Member

tsteur commented Mar 2, 2021

@diosmosis I reckon that's fine 👍

@diosmosis diosmosis merged commit 32357a7 into 4.x-dev Mar 7, 2021
@diosmosis diosmosis deleted the 16222-hide-unique-visitors-email branch March 7, 2021 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sgiehl sgiehl sgiehl approved these changes

Assignees
No one assigned
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.

Hide Unique Visitors from email reports when the metric is not activated for this period
3 participants
@diosmosis @tsteur @sgiehl