Problem/Motivation

Seeing The following warnings:

  • Deprecated function: Implicit conversion from float 19.5 to int loses precision in bawstats_create_map_image() (line 256 of /bawstats/modules/render_map.inc.php)
  • Warning: Increment on type bool has no effect, this will change in the next major version of PHP in baw_render_htmlchart() (line 50 of /bawstats/modules/render_htmlchart.inc.php)
  • Warning: Undefined variable $site in bawstats_get_sites() (line 253 of /bawstats/bawstats.data.inc)

Steps to reproduce

Simply go to admin/reports/bawstats

Proposed resolution

Will provide a patch shortly…

Comments

bisonbleu created an issue. See original summary.

bisonbleu’s picture

Status: Active » Needs review
StatusFileSize
new4.01 KB

Here's the promised patch. It applies cleanly to 2.x-dev when added to composer.json.

egfrith’s picture

Many thanks for the report and the patch.

I'm very happy to accept the patch for the files render_map.inc.php and render_htmlchart.inc.php.

However, the changes to bawstats.data.inc have alerted me to other changes I should make to prevent file names associated with sites other than the current site (i.e. current host) being cached, unless the bawstats_admin_access permission is enabled. I think I'd like to work on that in a separate issue. Are you OK with me committing just the changes you've suggested for render_map.inc.php and render_htmlchart.inc.php, or would you prefer to re-roll the patch with just those changes?

egfrith’s picture

Status: Needs review » Needs work
bisonbleu’s picture

I'm ok with providing a new patch. Can it excludes just this ?

@@ -250,8 +250,7 @@ function bawstats_get_site_files_yearmonth($regenerate_cache=FALSE, $site=NULL)
* If TRUE, clear the cache and regenerate it
*/
function bawstats_get_sites($regenerate_cache=FALSE) {
- $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, $site);
+ // Pass NULL instead of undefined $site variable.
+ $site_files_year_month = bawstats_get_site_files_year_month($regenerate_cache, NULL);
return(array_unique(array_keys($site_files_year_month['files'])));
}
-
-?>

bisonbleu’s picture

Status: Needs work » Needs review
StatusFileSize
new3.74 KB

Here a new patch where I've only removed the last change in bawstats.data.inc (see #5). Does that work for you?

  • egfrith committed ae47f863 on 2.x
    Issue #3531406 by bisonbleu: Deprecated function: Implicit conversion...
egfrith’s picture

Great - I have committed the float-to-int conversion bits, and I've created a separate issue for the issues I wanted to fix in bawstats.data.inc, which your patch alerted me to: see separate issue: #3531663: data files caching fixes and improvements.

egfrith’s picture

I've discovered a problem with the part of the patch in render_htmlchart.inc.php . The change from
$top_x++
to

// Ensure $top_x is numeric before incrementing.
$top_x = is_numeric($top_x) ? $top_x + 1 : 1;

has resulted in the monthly totals being displayed incorrectly.

Before patch:
Monthly totals

After patch:
Monthly totals

Thoughts welcome!

bisonbleu’s picture

Hm… I'm running the dev version of the module from a few days ago with only my 2 patches (not yours yet):

- implicit-conversion-from-float-to-int-3531406.patch
- timezone-must-be-explicitly-set-to-utc-3531531-2.patch

What I see:

  • Whether I have $top_x++ or $top_x = is_numeric($top_x) ? $top_x + 1 : 1;, Monthly History displays correctly (I don't see what you see in capture 2);
  • When I use $top_x++, I get «Warning: Increment on type bool has no effect», but not so with $top_x = is_numeric($top_x) ? $top_x + 1 : 1;.

Did you try flushing all caches?
Could it be because of fixes you introduced?
Any particular errors in the watchdog?

An then there may be a problem with -3531531-6.patch. I'll check tomorrow.

Note - Maybe unrelated, but in render_htmlchart.inc.php there are quite a few occurrences of $some_var++ where $some_var might be set to false. So these will eventually need to be fix as well I guess…

bisonbleu’s picture

StatusFileSize
new80.17 KB

Now having a look at the capture labelled «After patch:» which shows not the Monthly History (which displays fine with the latest dev version) but rather Days of month. Ok, I see what's wrong. Let's see if I can figure why that is…

Days of Month

bisonbleu’s picture

StatusFileSize
new145.99 KB

A patch will follow shortly.

Days of Month - Fixed!

bisonbleu’s picture

Assigned: Unassigned » bisonbleu
StatusFileSize
new2.31 KB

Here's the patch… It applies cleanly to the latest 2.x-dev when added to composer.json.
Similar work needs to be done for the following:

  • Days of week (Averages)
  • Hours (Averages)

I suggest we do this in another ticket.

bisonbleu’s picture

@egfrith I sent you an email a few days ago using the contact form of your d.o account, did you get it?

bisonbleu’s picture

Created a separate issue for Days of Week & Hours charts: #3532354: Days of Week & Hours charts are broken

egfrith’s picture

Many thanks for this patch. However, I think the hard-coding of 30 is causing problems for months with 31 days: see below, where March has 30 days and then "others".
30 days in March

I feel that the root cause of this patch is in render_htmlchart.inc.php, which is (I think) designed to deal with a variable number of rows in the chart when $top_x==false. I'm proposing an alternative patch that only increments $top_x if it's numeric. If it's false, it remains so throughout the rest of the function, which is important later in the function. I've tried to document the function at the top, which should explain how it's working. (I didn't write this function, so I've inferred the docs from the code.)

Could you test this patch please?

bisonbleu’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new150.69 KB
new105.04 KB
new124.01 KB

Oh… looks like you found the Holy Grail… the patch that patches them all!

Tested just this patch (removed days-of-week-and-hours-charts-3532354.patch) and now all charts render perfectly. Even Days of Month is rolling!

Days-of-month--rolling
Days-of-Week--chart
Hours--chart

I spent time in render_htmlchart.inc.php but could not quite untangle its intricacies. And so my patches are «localized» i.e. Zork grade, at flashlight point, deep down in a dark cave. ;^)

So I also marked #3532354: Days of Week & Hours charts are broken as Closed (outdated).

  • egfrith committed 40c576d0 on 2.x
    Issue #3531406 by bisonbleu, egfrith: Fixed implicit conversion from...
egfrith’s picture

Thank you for testing! (And for your kind words.) baw_render_htmlchart() is quite complex... I will mark this as fixed and release an updated version.

egfrith’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.