Disable features that are inaccurate if visit data is not profilable #16773

Description:

Introduces a new core metric, nb_profilable calculated with VisitsSummary metrics as the sum of visits that are profilable. If less than 1% of visits for a period are not profilable features that depend on profilable data are hidden or disabled. This includes:

• reports that rely on detecting returning visitors (everywhere in the UI)
• the nb_uniq_visitors metric (in the UI and API)
• segments for dimensions that rely on detecting returning visitors (hidden in the UI only)
• visitor log features that rely on detecting returning visitors

Other changes:

• nb_profilable is stored even if it's value is 0, this is to be able to differentiate between archives that are old and do not have the metric vs ones that are new and have 0 profilable visits. There is an alternative to check whether ts_archived is < a certain time, but getting this information w/ numeric data is a bit complicated. It might be do-able though.

Fixes #16363

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

@tsteur / @sgiehl this is ready for an initial review

@tsteur @sgiehl this review is ready for review. could use a general review of the strategy used first.

wondering generally instead of removing some reports to maybe rather show a notice like "This report won't show any data because no profilable visitors" (would need to word it better).

:+1: this makes sense to me. It would be strange to just click a report and suddenly not be able to see a report. I'll make this change?

I'll make this change?
sounds great

@mattab can you provide a UI review + look at the comments on the review and provide your thoughts?

@tsteur looked over this pr again. In it I'm adding nb_profilable as a core metric, but it is saved even if the value is 0. This is so we can tell when it was never computed. An alternative is to not save if 0 like every other metric, but in that case also save has_computed_profilable=1 as another metric. Do you think this is a better approach?

@diosmosis thought about it for a while and I reckon we can simply always store the column and no other column needed. Meaning we treat the values before it was not tracked as not profilable. It be worse if it was the other way around and by default it would assume a visitor was profilable.

@diosmosis is this ready for a review? I know it has the label but not sure when it was added.

@tsteur it's been waiting for answers from @mattab, I'll remove the Needs Review tag.

I'll review this later today

More feedback:

• Nice work, this will be very valuable!
• Can we return profilable element in the response of Live.getLastVisitsDetails - it would be helpful to troubleshoot which visit is profilable or not, for example to validate value of <nb_profilable>, or to notice why the API output may not include some elements such as visitorType, visitorTypeIcon, visitCount, daysSinceFirstVisit, daysSinceLastEcommerceOrder etc.
• Getting error The following error just broke Matomo (v4.4.1): Call to a member function isRequiresProfilableData() on null on /home/matt/dev/matomo/core/Plugin/Report.php(1013) at URL index.php?forceView=1&viewDataTable=VisitorLog&module=Live&action=getLastVisitsDetails&small=1&idSite=1&period=day&date=2021-08-27&segment=&showtitle=1&random=6365
• i wasn't able to trigger the display of the text in plugins/CoreHome/templates/_nonProfilableDataWarning.twig -> were you able to get this to show? (it would be great to have UI test for it if not done yet)
• Currently when there is no profilable data at all (cookie-less enforced), it doesn't hide all of the reports in "Behavior > engagement". It only hides some of them (the Returning Visits Over Time and the Frequency Overview are correctly hidden, but the Visits by Visit Number and Visits by Days Since Last Visit are still displayed (expected them not to be displayed in this case?)

• also when these reports show in the report footer: This report requires profilable data, but the current period of 2021-08-27 does not have any, so this report will not have useful information and should not be relied upon. (see screenshot above)
  • Could we change it to This report is inaccurate should not be relied upon. This report requires profilable data (tracking cookies enabled), but none of the visits did in the current period of 2021-08-27.
  • Also, could we show the notice much more visible, using a more visible "yellow notice" /warning style? just changing the CSS of another similar notice to warning it looks like this, which would be acceptable i think (at least visible and can't really miss it)
    

Segments are removed in the UI but nowhere else. We can also disable them and provide a tooltip saying why it doesn't make sense to use them on the current period. But when creating the segment, the period is arbitrary (eg, maybe they want it to be applied to a period w/ profilable data). Seems odd to see it greyed out and realize you have to change the period to be able to use it.

Could we leave the segments available, and not change the Segment Editor UI, but whenever they apply / use a segment that doesn't make in cookie-less (so when they are viewing the reports), then we could display a Warning notification explaining that the segment relies on data that is inaccurate because visits are not profilable?

@tsteur / @mattab this is ready for another review

in the original issue only unique visitors is requested. I'm not sure if there are others we'd also want to remove. Fingerprints could be useful. Can user IDs be specified without a visitor ID?

@diosmosis Here is the full scope we know so far, it is found with some more details in this FAQ: https://matomo.org/faq/general/faq_156/

Here is the full list of reports we need to disable or warn about inaccurate data:

• Unique Visitors and New VS Returning visitors metrics will be inaccurate
• Goals and Ecommerce conversions will be attributed to the channel used in the visit that converts
• Multi Attribution and Cohort reports won’t show data
• A small limited number of other reports will be inaccurate, these are:
  • Days since last visit
  • Visits by visit count
  • Visits to Conversion
  • Days to Conversion

@mattab

Unique Visitors and New VS Returning visitors metrics will be inaccurate

The whole engagement sparklines widget is disabled so no need to specify _returning specifically.

Goals and Ecommerce conversions will be attributed to the channel used in the visit that converts

Added a warning to goals & ecommerce pages.

Multi Attribution and Cohort reports won’t show data

Created PRs for this.

Other reports are marked as requiring profile data.

@mattab was this one reviewed and good to merge once tests pass?

Not yet, I will review it as soon as possible (hopefully this week)

what's the latest here @mattab ?