Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-12133

problems with changed number of parameters of validate($date, $files)

    Details

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

      Description

      Tim reported some weirdness related to new parameter files in mform->validate(),

      I can not reproduce it on my PHP 5.2, the is_validated is calling the validate() with two parameters now, maybe the problem is in method overriding

      Tim could you please test following code snippet on your server that shows those errors/warnings:
      <?php
      require ('config.php');

      class a {
      function go($c, $d)

      { echo "a: $c $d <br />"; }


      }

      class b {
      function go($c)

      { echo "b: $c <br />"; }


      }

      $a = new a();
      $a->go(1, 2);

      $b = new b();
      $b->go(1, 2);

      die;

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            skodak Petr Skoda added a comment -

            Adding Tim...

            Show
            skodak Petr Skoda added a comment - Adding Tim...
            Hide
            timhunt Tim Hunt added a comment -

            The problem is this: Start with this code:

            class a {
            function go($c)

            { echo "a: $c<br />"; }


            }

            class b extends a {
            function go($c)

            { psrent::go($c); echo "b: $c <br />"; }


            }

            Then change it to be:

            class a {
            function go($c, $d)

            { echo "a: $c $d <br />"; }


            }

            but don't chage class b. Then you get errors/notices (missing second parameter) when you call b::go and it tries to call a::go.

            That is basically what you did when you changed the signature of the validate method in the base class moodleform, but not in any of the other methods that override it.

            By the way, following good object oriented design, every subclass of moodleform should start with

            $errors = parent::validate($data);

            instead of

            $errors = array();

            That way, if we ever want a global validation hook we have got one. However, only the question type classes (and one or two others if I remember the results of my search) actually do this.

            Show
            timhunt Tim Hunt added a comment - The problem is this: Start with this code: class a { function go($c) { echo "a: $c<br />"; } } class b extends a { function go($c) { psrent::go($c); echo "b: $c <br />"; } } Then change it to be: class a { function go($c, $d) { echo "a: $c $d <br />"; } } but don't chage class b. Then you get errors/notices (missing second parameter) when you call b::go and it tries to call a::go. That is basically what you did when you changed the signature of the validate method in the base class moodleform, but not in any of the other methods that override it. By the way, following good object oriented design, every subclass of moodleform should start with $errors = parent::validate($data); instead of $errors = array(); That way, if we ever want a global validation hook we have got one. However, only the question type classes (and one or two others if I remember the results of my search) actually do this.
            Hide
            skodak Petr Skoda added a comment -

            The call to parent is a bit problematic because we expected mixed result true/array of errors. I think we did not call parent::validate() in main moodle codebase, so there should be no warnings.

            I agree this was not a nice hack, unfortunately I did not find better solution yet. I will go through all of them and fix it. Working on this today...

            Show
            skodak Petr Skoda added a comment - The call to parent is a bit problematic because we expected mixed result true/array of errors. I think we did not call parent::validate() in main moodle codebase, so there should be no warnings. I agree this was not a nice hack, unfortunately I did not find better solution yet. I will go through all of them and fix it. Working on this today...
            Hide
            timhunt Tim Hunt added a comment -

            I have just checked in a fix for all calls to parent::validation from the question type editing forms. I hope this is a step in the right direction.

            Show
            timhunt Tim Hunt added a comment - I have just checked in a fix for all calls to parent::validation from the question type editing forms. I hope this is a step in the right direction.
            Hide
            timhunt Tim Hunt added a comment -

            And I have also just checked in a change so that the base class method returns array() instead of true, so you can start your overridden method with

            $errors = parent::validation($data, $files);

            Show
            timhunt Tim Hunt added a comment - And I have also just checked in a change so that the base class method returns array() instead of true, so you can start your overridden method with $errors = parent::validation($data, $files);
            Hide
            skodak Petr Skoda added a comment -

            fix in cvs, thanks for cooperation

            Show
            skodak Petr Skoda added a comment - fix in cvs, thanks for cooperation
            Hide
            skodak Petr Skoda added a comment -

            Docs in wiki updated too

            Show
            skodak Petr Skoda added a comment - Docs in wiki updated too

              People

              • Assignee:
                skodak Petr Skoda
                Reporter:
                skodak Petr Skoda
                Tester:
                Nobody
                Participants:
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  3/Mar/08