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

Problem with user profile fields of type Menu in newer versions of Moodle

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.1.9, 3.1.10, 3.1.11, 3.1.12, 3.2.6, 3.2.7, 3.2.8, 3.3.3, 3.3.4, 3.3.5, 3.4, 3.4.1, 3.4.2
    • Fix Version/s: None
    • Component/s: Unknown
    • Labels:
      None
    • Affected Branches:
      MOODLE_31_STABLE, MOODLE_32_STABLE, MOODLE_33_STABLE, MOODLE_34_STABLE

      Description

      This problem starts in Moodle 2.9 and above. And can't be detected unless you worked with it through web services since 2.8 version.

      Background

      We created a custom registration form that can be hosted on different websites (Wordpress, static, . . . etc). The form sends its data to an API endpoint which handles data checks and sanitization then send them to central Moodle instance to `core_user_create_users` web services function to create and register a new user.

      In this registration form, we have some custom fields, and most of them are of `Menu` type. In this type, data is a list of items put together and separated by a newline character (that's what Moodle imply). So when the user field render to a form (whether it was in edit profile form or register user) it will show as a dropdown list (a select input). Each input has the content formatted to the selected language and the value will be the key to that info when Moodle explode the field data with a `\n` character. Hence this code in Moodle 2.8

       

      public function profile_field_menu($fieldid = 0, $userid = 0) {
         // First call parent constructor.
         $this->profile_field_base($fieldid, $userid);
         // Param 1 for menu type is the options.
         if (isset($this->field->param1)) {
           $options = explode("\n", $this->field->param1);
         } else {
           $options = array();
         }
         $this->options = array();
         if (!empty($this->field->required)) {
           $this->options[''] = get_string('choose').'...';
         }
         foreach ($options as $key => $option) {
           $this->options[$key] = format_string($option); // Multilang formatting.
         }
         // Set the data key.
         if ($this->data !== null) {
           $this->datakey = (int)array_search($this->data, $this->options);
         }
       }
      

       

      File link in GitHub here

      1- It fetches the data from the database in the parent class `profile_field_base`.

      2- It explodes the data of the menu field.

      3- It constructs the list of options and put it in the data field `this->options`.

      In this version, the construction of the options list happens in this line

       

      foreach ($options as $key => $option) {
         $this->options[$key] = format_string($option); // Multilang formatting.
      }

       

      After that, it retrieves the user data if it's provided, like in our case or the case of edit profile form. It will search for it using the following line:

       

      // Set the data key.
      if ($this->data !== null) {
         $this->datakey = (int)array_search($this->data, $this->options);
      }

       

      This way Moodle won't be able to retrieve the data if he provides the key because the `array_search` function will lookup for the provided data in the options values and not in the keys.

      In our case, I don't know if I can call it luck or someone else changed it. When we moved to 3.0 the options construction kept the 2.8 version (which I consider the correct one). But taking the data search changes of 3.0. So when we wrote our custom registration form and other tools as well. Sending the key of the data or option worked perfectly and it helped us in both backend and frontend.

      The problem

      Now we are moving registration instance to 3.2 (fresh installation) and while doing QA on the registration form we found that web service function `core_user_create_users` doesn't work and it returns invalid parameters exceptions. After debugging we found this messed up situation. In Moodle 2.9 and above this is what's wrong:

       

      foreach ($options as $key => $option) {
        $this->options[$option] = format_string($option); 
      }

       

      The options construction use $options value as the key for options data field of the profile field object. While it should be the $key as in 2.8. If you look at the user data retrieval code you can see that it checks the data against the key and if it doesn't return anything it searches in the values as follow:

       

      // Set the data key.
      if ($this->data !== null) {
        $key = $this->data;
        if (isset($this->options[$key]) || ($key = array_search($key, $this->options))   !== false) {
          $this->data = $key;
          $this->datakey = $key;
        }
      }

       

      Conclusion

      I don't think that this is a major perhaps you won't even consider it a minor bug because no one reported it. So maybe no one used it this way as we did. But it certainly needs a fix in newer version to avoid conflicts.

      Possible Fix (tested)

      We fixed this bug by changing the following lines in `moodle/user/profile/field/menu/field.class.php`

      • Fix options construction using the $key => $options for options keys and options values:

       foreach ($options as $key => $option) {
         $this->options[$key] = format_string($option); // Multilang formatting with filters.
       }

      * Fix the data retrieval using the options key as follows

      // Set the data key.
      if ($this->data !== null) {
        $key = $this->data;
        if (isset($this->options[$key]) || ($key = array_search($key, $this->options)) !== false) {
          $this->data = $this->options[$key]; // Our update
          $this->datakey = $key;
        }
      }

      In this solution, it doesn't matter which data we provide. If it's the value we will retrieve its key and use it to get the right data. If it's the key it works.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              mohessaid Mohammed Essaid MEZERREG
              Participants:
              Component watchers:
              Amaia Anabitarte, Bas Brands, Carlos Escobedo, Sara Arjona (@sarjona), Víctor Déniz Falcón
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: