@tsteur
opened this Pull Request on October 7th 2020

Fixes eg such an error

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.

totals are formatted:

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

I think the test failures are unrelated.

@diosmosis
commented
on October 8th 2020

relevant test failure here: https://travis-ci.org/github/matomo-org/matomo/jobs/733830035#L809

@tsteur
commented
on October 8th 2020

tests should be fixed now.

This Pull Request was closed on October 8th 2020

Powered by GitHub Issue Mirror