Moodle

changes in lib/graphlib are causing notices in overview graph

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.9.2
  • Fix Version/s: 1.9.2
  • Component/s: Libraries
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

Before changes in the graphlib I had the following lines in the overview graph :

//following two lines seem to silence notice warnings from graphlib.php
$line->y_tick_labels = null;
$line->offset_relation = null;

Since changes in graphlib these no longer work but now these lines silence the warnings :

//following two lines seem to silence notice warnings from graphlib.php
$line->parameter['y_tick_labels'] = null;
$line->offset_relation = null;

I think other graphs need to be checked to see if they are also broken. The library really needs fixing so that these properties are set to a sensible default or something and don't need to be set if they are not used.

Assigning this to Jenny since according to CVS Jenny made the changes to graphlib.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Petr Škoda (skodak) added a comment -

please fix before review Tuesday, thanks

Show
Petr Škoda (skodak) added a comment - please fix before review Tuesday, thanks
Hide
Permalink
Jenny Gray added a comment -

Well this is what you get for trying to do something quickly that looked simple I guess. A big SORRY for me, this 1 line change has been a pain all the way.

I've just reverted the code in graphlib, so all core graphs should go back to working now.

What I was trying to do was resolve an issue for some-one else at the OU who had used $graph->parameter['y_tick_labels'] rather than the way Jamie has defaulted the values above. She's on maternity leave so sadly I can't ask why! I've altered her code to match Jamie's instead so the contrib module should work now with the core code - which is better.

What I wonder though - and why I'm not closing this yet, is whether we should add to the class init to default offset_relation and y_tick_labels to null, rather than expect every new graph() to do it?

Show
Jenny Gray added a comment - Well this is what you get for trying to do something quickly that looked simple I guess. A big SORRY for me, this 1 line change has been a pain all the way. I've just reverted the code in graphlib, so all core graphs should go back to working now. What I was trying to do was resolve an issue for some-one else at the OU who had used $graph->parameter['y_tick_labels'] rather than the way Jamie has defaulted the values above. She's on maternity leave so sadly I can't ask why! I've altered her code to match Jamie's instead so the contrib module should work now with the core code - which is better. What I wonder though - and why I'm not closing this yet, is whether we should add to the class init to default offset_relation and y_tick_labels to null, rather than expect every new graph() to do it?
Hide
Permalink
Jamie Pratt added a comment -

Hi Jenny,

Thanks for fixing this.

My +1 to set default properties of class graph :

var $y_tick_labels = null;
var $offset_relation = null;

Jamie

Show
Jamie Pratt added a comment - Hi Jenny, Thanks for fixing this. My +1 to set default properties of class graph : var $y_tick_labels = null; var $offset_relation = null; Jamie
Hide
Permalink
Jamie Pratt added a comment -

Jenny's patch looks good to me. Tested in overview report and it looks OK. No need for :

//following two lines seem to silence notice warnings from graphlib.php
$line->parameter['y_tick_labels'] = null;
$line->offset_relation = null;

Anymore.

Show
Jamie Pratt added a comment - Jenny's patch looks good to me. Tested in overview report and it looks OK. No need for : //following two lines seem to silence notice warnings from graphlib.php $line->parameter['y_tick_labels'] = null; $line->offset_relation = null; Anymore.
Hide
Permalink
Jenny Gray added a comment -

Defaults added as Jamie suggested.

Show
Jenny Gray added a comment - Defaults added as Jamie suggested.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.