Moodle
  1. Moodle
  2. MDL-15183

changes in lib/graphlib are causing notices in overview graph

    Details

    • Type: Bug Bug
    • Status: 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.

        Gliffy Diagrams

          Activity

          Hide
          Petr Skoda added a comment -

          please fix before review Tuesday, thanks

          Show
          Petr Skoda added a comment - please fix before review Tuesday, thanks
          Hide
          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
          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
          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
          Jenny Gray added a comment -

          Defaults added as Jamie suggested.

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

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: