@tsteur opened this Pull Request on October 7th 2020 Member

Fixes eg such an error
image

Since merging https://github.com/matomo-org/matomo/pull/16520 the custom reports UI tests fail. I have been able to reproduce this and it seems to be eg caused by this PR https://github.com/matomo-org/matomo/pull/15304/files

To be specific it is happening here where eg reportTotal=494,836.4. Basically, the total values are formatted and then it throws an error.

image

totals are formatted:
image

Not sure how to best fix it. Was first thinking maybe we need to store also totalsUnformatted as a metadata in ReportTotalsCalculator class and then use it there. This be possible to do but not sure if it has any downside when calculating the percentages. On the other side I think it should not calculate percentage values where one part is formatted and the other part is unformatted so this might be the solution.

We also can't simply do if (is_numeric($value) && is_numeric($reportTotal)) { because this will prevent calculating the percentage for any metric that has values > 1000 (as it would be 1,000 and therefore not a number). Maybe the problem is that it partially needs to be smarter and only calculate percentages for certain metrics that are actually needing percentages but this alone won't fix it either because it also needs to use additionally the unformatted total metrics. Otherwise the same issue persists.

It seems to me using the unformatted total metric values might be the best solution so it actually always compares unformatted values.

I was reproducing it with an "avg" metric (what I did was actually creating a report that adds all metrics like below but note some metrics might not be available... below metrics I added to a "table" custom reports in the custom_reports table directly in the metrics column):

{code:java}
["avg_sum_actions_per_visits","avg_sum_corehome_visitordayssincefirst_per_visits","avg_sum_corehome_visitordayssinceorder_per_visits","avg_sum_visitorinterest_visitordayssincelast_per_visits","avg_sum_events_totalevents_per_visits","avg_sum_actions_visittotalinteractions_per_visits","avg_sum_actions_visittotalsearches_per_visits","avg_sum_corehome_profilable_per_visits","avg_sum_corehome_visitorsecondssincefirst_per_visits","avg_sum_corehome_visitorsecondssinceorder_per_visits","avg_sum_visitorinterest_visitorsecondssincelast_per_visits","avg_sum_corehome_visittotaltime_per_visits","bounce_rate","bounce_count","visits_conversion_rate","max_actions","max_corehome_visitordayssincefirst","max_corehome_visitordayssinceorder","max_visitorinterest_visitordayssincelast","max_events_totalevents","max_actions_visittotalinteractions","max_actions_visittotalsearches","max_corehome_visitorsecondssincefirst","max_corehome_visitorsecondssinceorder","max_visitorinterest_visitorsecondssincelast","max_corehome_visittotaltime","sum_actions","sum_corehome_visitordayssincefirst","sum_corehome_visitordayssinceorder","sum_visitorinterest_visitordayssincelast","sum_events_totalevents","sum_actions_visittotalinteractions","sum_actions_visittotalsearches","sum_corehome_profilable","sum_corehome_visitorsecondssincefirst","sum_corehome_visitorsecondssinceorder","sum_visitorinterest_visitorsecondssincelast","sum_corehome_visittotaltime","nb_uniq_devicesdetection_browserengine","nb_uniq_devicesdetection_browserversion","nb_uniq_devicesdetection_browsername","nb_uniq_devicesdetection_clienttype","nb_uniq_devicesdetection_devicebrand","nb_uniq_devicesdetection_devicemodel","nb_uniq_devicesdetection_devicetype","nb_uniq_customdimension_customdimension3","nb_uniq_corehome_idsite","nb_uniq_devicesdetection_osversion","nb_uniq_devicesdetection_os","nb_uniq_resolution_resolution","nb_uniq_customdimension_customdimension2","nb_uniq_corehome_userid","nb_uniq_corehome_visitgoalbuyer","nb_uniq_corehome_visitorreturning","nb_uniq_corehome_visitip","nb_uniq_visitors","nb_uniq_visitors_fingerprints","nb_visits","nb_visits_converted","nb_uniq_usercountry_city","nb_uniq_usercountry_continent","nb_uniq_usercountry_country","nb_uniq_userlanguage_language","nb_uniq_usercountry_latitude","nb_uniq_usercountry_longitude","nb_uniq_usercountry_region","avg_time_dom_completion","avg_time_dom_processing","avg_page_generation_time","avg_time_network","avg_time_on_load","avg_time_server","avg_time_transfer","hits","max_time_dom_completion","max_time_dom_processing","max_time_network","max_time_on_load","max_actions_pagegenerationtime","max_actions_pageviewposition","max_time_server","max_time_transfer","min_time_dom_completion","min_time_dom_processing","min_time_network","min_time_on_load","min_time_server","min_time_transfer","pageviews","pageviews_with_time_dom_completion","pageviews_with_time_dom_processing","pageviews_with_generation_time","pageviews_with_time_network","pageviews_with_time_on_load","pageviews_with_time_server","pageviews_with_time_transfer","sum_time_dom_completion","sum_time_dom_processing","sum_time_network","sum_time_on_load","sum_actions_pagegenerationtime","sum_actions_pageviewposition","sum_time_server","sum_time_transfer","nb_uniq_actions_actionurl","nb_uniq_actions_searchcategory","nb_uniq_actions_clickedurl","nb_uniq_contents_contentinteraction","nb_uniq_contents_contentname","nb_uniq_contents_contentpiece","nb_uniq_contents_contenttarget","nb_uniq_actions_downloadurl","nb_uniq_actions_entrypagetitle","nb_uniq_actions_entrypageurl","nb_uniq_actions_exitpagetitle","nb_uniq_actions_exitpageurl","nb_uniq_actions_searchcount","nb_uniq_actions_searchkeyword","nb_uniq_actions_pagetitle","nb_uniq_actions_pageurl","nb_uniq_customdimension_customdimension1","avg_event_value","events_with_event_value","max_events_eventvalue","min_events_eventvalue","sum_events_eventvalue","nb_uniq_events_eventaction","nb_uniq_events_eventcategory","nb_uniq_events_eventname","nb_uniq_events_eventurl","nb_uniq_marketingcampaignsreporting_campaigncontent","nb_uniq_marketingcampaignsreporting_campaignid","nb_uniq_marketingcampaignsreporting_campaignkeyword","nb_uniq_marketingcampaignsreporting_campaignmedium","nb_uniq_marketingcampaignsreporting_campaignname","nb_uniq_marketingcampaignsreporting_campaignsource","nb_uniq_referrers_referrertype","nb_uniq_referrers_keyword","nb_uniq_referrers_referrername","nb_uniq_referrers_referrerurl","avg_sum_ecommerce_revenuediscount_per_uniq_orders","avg_sum_ecommerce_items_per_uniq_orders","avg_sum_ecommerce_revenue_per_uniq_orders","avg_sum_ecommerce_revenueshipping_per_uniq_orders","avg_sum_ecommerce_revenuesubtotal_per_uniq_orders","avg_sum_ecommerce_revenuetax_per_uniq_orders","max_ecommerce_revenuediscount","max_ecommerce_items","max_ecommerce_revenue","max_ecommerce_productprice","max_ecommerce_productquantity","max_ecommerce_revenueshipping","max_ecommerce_revenuesubtotal","max_ecommerce_revenuetax","max_ecommerce_productviewprice","nb_uniq_orders","sum_ecommerce_revenuediscount","sum_ecommerce_items","sum_ecommerce_revenue","sum_ecommerce_productprice","sum_ecommerce_productquantity","sum_ecommerce_revenueshipping","sum_ecommerce_revenuesubtotal","sum_ecommerce_revenuetax","sum_ecommerce_productviewprice","nb_uniq_ecommerce_productname","nb_uniq_ecommerce_productsku","nb_uniq_ecommerce_productviewcategory","nb_uniq_ecommerce_productviewcategory2","nb_uniq_ecommerce_productviewcategory3","nb_uniq_ecommerce_productviewcategory4","nb_uniq_ecommerce_productviewcategory5","nb_uniq_ecommerce_productviewname","nb_uniq_ecommerce_productviewsku","goal_4_conversion_uniq_visitors_rate","goal_5_conversion_uniq_visitors_rate","goal_2_conversion_uniq_visitors_rate","goal_15_conversion_uniq_visitors_rate","goal_11_conversion_uniq_visitors_rate","goal_29_conversion_uniq_visitors_rate","goal_26_conversion_uniq_visitors_rate","goal_22_conversion_uniq_visitors_rate","goal_23_conversion_uniq_visitors_rate","goal_21_conversion_uniq_visitors_rate","goal_6_conversion_uniq_visitors_rate","goal_8_conversion_uniq_visitors_rate","goal_7_conversion_uniq_visitors_rate","goal_20_conversion_uniq_visitors_rate","goal_19_conversion_uniq_visitors_rate","goal_18_conversion_uniq_visitors_rate","goal_4_conversion","goal_5_conversion","goal_2_conversion","goal_15_conversion","goal_11_conversion","goal_29_conversion","goal_26_conversion","goal_22_conversion","goal_23_conversion","goal_21_conversion","goal_6_conversion","goal_8_conversion","goal_7_conversion","goal_20_conversion","goal_19_conversion","goal_18_conversion","goal_4_daystoconversion","goal_5_daystoconversion","goal_2_daystoconversion","goal_15_daystoconversion","goal_11_daystoconversion","goal_29_daystoconversion","goal_26_daystoconversion","goal_22_daystoconversion","goal_23_daystoconversion","goal_21_daystoconversion","goal_6_daystoconversion","goal_8_daystoconversion","goal_7_daystoconversion","goal_20_daystoconversion","goal_19_daystoconversion","goal_18_daystoconversion","max_goals_revenue","goal_4_revenue","goal_5_revenue","goal_2_revenue","goal_15_revenue","goal_11_revenue","goal_29_revenue","goal_26_revenue","goal_22_revenue","goal_23_revenue","goal_21_revenue","goal_6_revenue","goal_8_revenue","goal_7_revenue","goal_20_revenue","goal_19_revenue","goal_18_revenue","sum_goals_revenue","goal_4_visitstoconversion","goal_5_visitstoconversion","goal_2_visitstoconversion","goal_15_visitstoconversion","goal_11_visitstoconversion","goal_29_visitstoconversion","goal_26_visitstoconversion","goal_22_visitstoconversion","goal_23_visitstoconversion","goal_21_visitstoconversion","goal_6_visitstoconversion","goal_8_visitstoconversion","goal_7_visitstoconversion","goal_20_visitstoconversion","goal_19_visitstoconversion","goal_18_visitstoconversion","avg_form_time_hesitation","avg_form_time_spent","avg_form_time_to_first_submission","form_conversion_rate","nb_form_conversions","form_resubmitters_rate","nb_form_resubmitters","nb_form_starters","form_starters_rate","nb_form_starts","nb_form_submissions","nb_form_submitters","nb_form_viewers","nb_form_views","nb_form_time_hesitation","max_form_views","max_form_time_hesitation","max_form_time_spent","max_form_time_to_first_submission","nb_form_time_spent","nb_form_time_to_first_submission","nb_uniq_paidadvertisingperformance_campaignname","nb_uniq_paidadvertisingperformance_adclickid","nb_uniq_paidadvertisingperformance_adprovider"]
{code}

This patch seems to work for me.

@tsteur commented on October 7th 2020 Member

I think the test failures are unrelated.

@diosmosis commented on October 8th 2020 Member
@tsteur commented on October 8th 2020 Member

tests should be fixed now.

This Pull Request was closed on October 8th 2020
Powered by GitHub Issue Mirror