Moodle
  1. Moodle
  2. MDL-26177

Can't edit profiles when there are custom fields

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.2
    • Component/s: Administration
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16208

      Description

      To reproduce:

      0. Logged in as admin.
      1. Create a Text input custom user profile field that must be unique (not sure if the unique bit is really necessary)
      2. Go to browse list of users, and click to edit someone's profile.
      3. Type something in the custom field box, and click save.

      Result:

      Comparisons of text column conditions are not allowed. Please use sql_compare_text() in your query.

      Stack trace:
      line 513 of /lib/dml/moodle_database.php: dml_exception thrown
      line 1288 of /lib/dml/moodle_database.php: call to moodle_database->where_clause()
      line 134 of /user/profile/lib.php: call to moodle_database->get_field()
      line 399 of /user/profile/lib.php: call to profile_field_base->edit_validate_field()
      line 159 of /user/editadvanced_form.php: call to profile_validation()
      line 438 of /lib/formslib.php: call to user_editadvanced_form->validation()
      line 397 of /lib/formslib.php: call to moodleform->validate_defined_fields()
      line 484 of /lib/formslib.php: call to moodleform->is_validated()
      line 128 of /user/editadvanced.php: call to moodleform->get_data()

        Activity

        Hide
        Tim Hunt added a comment -

        Eloy, I don't understand why we don't support TEXT columns in where_clause. Why don't we do this: https://github.com/timhunt/moodle/commit/2ef6807d70f92087c122f470f1d3df11a46e1794

        Show
        Tim Hunt added a comment - Eloy, I don't understand why we don't support TEXT columns in where_clause. Why don't we do this: https://github.com/timhunt/moodle/commit/2ef6807d70f92087c122f470f1d3df11a46e1794
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Hi Tim,

        this was implemented that way since day 0 and later (recently) we added the exception to clearly show the prohibition.

        The main cause is that TEXT exact searches aren't 100% cross-db, nor the the creation of indexes for those columns is so, also, they are awfully slow (FULL scans).

        So, after various discussions, we decided to force any TEXT comparison to be performed EXPLICITLY, by forcing developers to use exclusively the get_xxx_sql() alternatives, with the sql_compare_text() helper function, avoiding any "dark magic" SQL transformation in the simpler get_recordXXX() functions.

        That way, we have all the TEXT comparison (sql_compare_text) uses properly detected, that could be interesting in the future, who knows. See MDL-24863 for some background.

        Simply, it was the decision taken, I'd keep it alive and close this as won't fix. Just that.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Hi Tim, this was implemented that way since day 0 and later (recently) we added the exception to clearly show the prohibition. The main cause is that TEXT exact searches aren't 100% cross-db, nor the the creation of indexes for those columns is so, also, they are awfully slow (FULL scans). So, after various discussions, we decided to force any TEXT comparison to be performed EXPLICITLY, by forcing developers to use exclusively the get_xxx_sql() alternatives, with the sql_compare_text() helper function, avoiding any "dark magic" SQL transformation in the simpler get_recordXXX() functions. That way, we have all the TEXT comparison (sql_compare_text) uses properly detected, that could be interesting in the future, who knows. See MDL-24863 for some background. Simply, it was the decision taken, I'd keep it alive and close this as won't fix. Just that. Ciao
        Hide
        Tim Hunt added a comment -

        Well, no we can't close this bug.

        Custom user profile fields with the 'Unique' option are broken, and someone needs to fix that.

        Show
        Tim Hunt added a comment - Well, no we can't close this bug. Custom user profile fields with the 'Unique' option are broken, and someone needs to fix that.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Ah, yes, sorry, just was going to edit that, lol, obviously the bug itself needs fixing, hehe. Only the "dark magic" patch is the one I think we shouldn't apply.

        Sorry for the confusion! Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Ah, yes, sorry, just was going to edit that, lol, obviously the bug itself needs fixing, hehe. Only the "dark magic" patch is the one I think we shouldn't apply. Sorry for the confusion! Ciao
        Hide
        Tim Hunt added a comment -

        Eloy, please review https://github.com/timhunt/moodle/commit/25f44c72bdf3b524cda704ab02d380a5fded69b5 and issue a pull request if you think it is OK. Thanks.

        Show
        Tim Hunt added a comment - Eloy, please review https://github.com/timhunt/moodle/commit/25f44c72bdf3b524cda704ab02d380a5fded69b5 and issue a pull request if you think it is OK. Thanks.
        Hide
        Petr Škoda added a comment -

        Tim: your last patch looks ok.

        Show
        Petr Škoda added a comment - Tim: your last patch looks ok.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Resolving this as fixed for 2.0.2 and created PULL-191 for integration. Thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Resolving this as fixed for 2.0.2 and created PULL-191 for integration. Thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: