- Issue created by @bisonbleu
- 🇬🇧United Kingdom egfrith
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?
- 🇨🇦Canada bisonbleu
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'])));
}
-
-?> - 🇨🇦Canada bisonbleu
Here a new patch where I've only removed the last change in bawstats.data.inc. Does that work for you?
- 🇬🇧United Kingdom egfrith
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: 🐛 data files caching fixces and improvments Needs work .
- 🇬🇧United Kingdom egfrith
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:
After patch:
Thoughts welcome!
- 🇨🇦Canada bisonbleu
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.patchWhat 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 ?
Note - Maybe unrelated, but in render_htmlchart.inc.php there are quite a few occurrences of
$some_var_x++
where $var might be set to false. So these will eventually need to be fix as well I guess… - 🇨🇦Canada bisonbleu
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…
- 🇬🇧United Kingdom egfrith
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".
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?
- 🇨🇦Canada bisonbleu
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 displays render perfectly. Even Days of Month is rolling!
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 guess you can mark as -3532354.patch : )
- 🇬🇧United Kingdom egfrith
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.