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

Process unique visitor metric for Roll-ups (set enable_processing_unique_visitors_multiple_sites = 1) #15450

Merged
merged 2 commits into from Mar 9, 2020

Conversation

mattab
Copy link
Member

@mattab mattab commented Jan 24, 2020

Alternatively, we could remove this setting completely and always assume we want to process unique visitors across websites. I'm not sure there is any reason not to want this enabled?

@mattab mattab added the Needs Review PRs that need a code review label Jan 24, 2020
@mattab mattab added this to the 4.0.0 milestone Jan 24, 2020
@mattab mattab changed the title Process unique visitor metric for Roll-ups (set enable_processing_uni… Process unique visitor metric for Roll-ups (set enable_processing_unique_visitors_multiple_sites = 1) Jan 24, 2020
@tsteur tsteur changed the base branch from 3.x-dev to 4.x-dev January 26, 2020 23:43
@tsteur tsteur changed the base branch from 4.x-dev to 3.x-dev January 26, 2020 23:44
@tsteur
Copy link
Member

tsteur commented Jan 26, 2020

@mattab can you recreate the PR against 4.x-dev and then also fix the UI test (there is a diagnostics config test failing)

@diosmosis diosmosis force-pushed the enable_processing_unique_visitors_multiple_sites branch from 3e9d89a to a05d910 Compare February 7, 2020 03:23
@diosmosis diosmosis changed the base branch from 3.x-dev to 4.x-dev February 7, 2020 03:24
mattab and others added 2 commits March 2, 2020 11:13
…que_visitors_multiple_sites = 1)

Alternatively, we could remove this setting completely and always assume we want to process unique visitors across websites. I'm not sure there is any reason not to want this enabled?
@sgiehl sgiehl force-pushed the enable_processing_unique_visitors_multiple_sites branch from 4a714c7 to 92827e4 Compare March 2, 2020 10:16
@diosmosis diosmosis merged commit 65b1fcd into 4.x-dev Mar 9, 2020
@diosmosis diosmosis deleted the enable_processing_unique_visitors_multiple_sites branch March 9, 2020 16:34
@tsteur
Copy link
Member

tsteur commented Mar 17, 2020

@mattab this PR will very likely cause issues because it will only work when enable_fingerprinting_across_websites is also enabled. This is a privacy issue though. As a result archiving will break with roll up reporting etc. I reckon this PR should be reverted.

tsteur added a commit that referenced this pull request Mar 17, 2020
…sing_unique_visitors_multiple_sites = 1) (#15450)"

This reverts commit 65b1fcd.
diosmosis pushed a commit that referenced this pull request Mar 18, 2020
…sing_unique_visitors_multiple_sites = 1) (#15450)" (#15701)

This reverts commit 65b1fcd.
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Sep 29, 2020
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 not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants