Moodle
  1. Moodle
  2. MDL-34657

When searching for users, list any exact matches first

    Details

    • Testing Instructions:
      Hide

      Please test this, in particular, on Oracle and MS SQL server.

      1. On Users > Permissions > User policies, add idnumber to the selected Show user identity fields.

      2. Create a user with idnumber s, and full name Students ZZZ. Then create a lot of other accounts with idnumbers s1, ... s100, with last names that come in alphabetical order before ZZZ.

      (Actually, 100 users here is excessive. A few is enough to see the effect of the change.)

      3. Now, try to enrol the user with idnumber s, by searching for 's' in the manual enrolment pop-up.

      4. Verify that Students ZZZ is listed first in the list of matching users.

      You need to do similar checks in other places where users are searched for or just displayed sorted (obviously fewer checks to do when there is no search):

      5. Both user selectors on the assign roles page at system level.

      6. Both user selectors on the assign other users page at course level.

      7. Both user selectors on the assign roles page at activity level.

      8. Both user selectors on the assign admins screen.

      9. In the web service admin UI, on the form where you create a token for a user.

      10. Both user selectors on the manage cohort members screen.

      11. Both user selectors on the non-javascript manage enrolments page (enrol/manual/manage.php, go to Manage enrolments, then click on the two face icon for the manual enrolment plugin.)

      12. Check the list of teachers is still correctly displayed beside each course in the course list.

      13. Check that mnet enrolment still works. (Sorry, I am not sure of how this is supposed to work.)

      14. For self-enrolment, check that the from address of the welcome email is one of the teachers in the course.

      15. In gradebook import:
      15a. Create an import file with the idnumbers of some users who do exist, but who are not enrolled in the course.
      15b. Try to import this file. At some point in the process, you should see a list of users who are not enrolled. Make sure that is displayed correctly, and that the users are in the right order.

      16. On the front page of the groups UI, the list of users where you select one group.

      17. Lists of users on the groups overview page.

      18. Both user selectors on the page where you add/remove users from a group.

      19. Find at least one admin setting using admin_setting_users_with_capability, e.g. the list of users to notify about course requests on Site administration, Courses, Course request. Test that.

      20. I don't know what mnet/service/enrol/course.php does, but I changed it, so it should be tested. In fact, there are several user selectors in the mnet UI. Wherever they are, they need to be tested.

      21. Sort order where users are listed in the choice module.

      22. Forum show and manage subscribers page. That is, turn editing on when looking at the list of subscribers. Also check a forum with forced subscription on.

      23. Lesson module essay qtype UI. (Sorry, this is another area I don't know much about.)

      24. Sorting of lesson reports.

      25. Quiz module, when viewing user overrides, check the sort order. When editing them the list of users in the form select menu.

      26. Lots of places users are listed in the workshop module.

      27. Logs report.

      28. Security report.

      29. Stats report.

      Show
      Please test this, in particular, on Oracle and MS SQL server. 1. On Users > Permissions > User policies, add idnumber to the selected Show user identity fields. 2. Create a user with idnumber s, and full name Students ZZZ. Then create a lot of other accounts with idnumbers s1, ... s100, with last names that come in alphabetical order before ZZZ. (Actually, 100 users here is excessive. A few is enough to see the effect of the change.) 3. Now, try to enrol the user with idnumber s, by searching for 's' in the manual enrolment pop-up. 4. Verify that Students ZZZ is listed first in the list of matching users. You need to do similar checks in other places where users are searched for or just displayed sorted (obviously fewer checks to do when there is no search): 5. Both user selectors on the assign roles page at system level. 6. Both user selectors on the assign other users page at course level. 7. Both user selectors on the assign roles page at activity level. 8. Both user selectors on the assign admins screen. 9. In the web service admin UI, on the form where you create a token for a user. 10. Both user selectors on the manage cohort members screen. 11. Both user selectors on the non-javascript manage enrolments page (enrol/manual/manage.php, go to Manage enrolments, then click on the two face icon for the manual enrolment plugin.) 12. Check the list of teachers is still correctly displayed beside each course in the course list. 13. Check that mnet enrolment still works. (Sorry, I am not sure of how this is supposed to work.) 14. For self-enrolment, check that the from address of the welcome email is one of the teachers in the course. 15. In gradebook import: 15a. Create an import file with the idnumbers of some users who do exist, but who are not enrolled in the course. 15b. Try to import this file. At some point in the process, you should see a list of users who are not enrolled. Make sure that is displayed correctly, and that the users are in the right order. 16. On the front page of the groups UI, the list of users where you select one group. 17. Lists of users on the groups overview page. 18. Both user selectors on the page where you add/remove users from a group. 19. Find at least one admin setting using admin_setting_users_with_capability, e.g. the list of users to notify about course requests on Site administration, Courses, Course request. Test that. 20. I don't know what mnet/service/enrol/course.php does, but I changed it, so it should be tested. In fact, there are several user selectors in the mnet UI. Wherever they are, they need to be tested. 21. Sort order where users are listed in the choice module. 22. Forum show and manage subscribers page. That is, turn editing on when looking at the list of subscribers. Also check a forum with forced subscription on. 23. Lesson module essay qtype UI. (Sorry, this is another area I don't know much about.) 24. Sorting of lesson reports. 25. Quiz module, when viewing user overrides, check the sort order. When editing them the list of users in the form select menu. 26. Lots of places users are listed in the workshop module. 27. Logs report. 28. Security report. 29. Stats report.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      43102

      Description

      1. On Users > Permissions > User policies, add idnumber to the selected Show user identity fields.

      2. Create a user with idnumber s, and full name Students ZZZ. Then create a lot of other accounts with idnumbers s1, ... s100, with last names that come in alphabetical order before ZZZ.

      3. Now, try to enrol the user with idnumber s, by searching for 's' in the manual enrolment pop-up.

      Current behaviour: Students ZZZ is somewhere off the bottom of the list.

      Desired behaviour: List exact matches first.

      1. danp-enrolment-screen.png
        211 kB
      2. danp-role-screen.png
        255 kB
      3. danp-weirdsort-forum.png
        22 kB
      4. danp-weirdsort-group.png
        18 kB
      5. mdl34657.png
        44 kB

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Petr, I was surprised how small the patch to implement this was. Please could you take a quick look.

          If you are happy, would it be OK to cherry-pick this to stable branches? That would make the admins here at the OU very happy.

          Show
          Tim Hunt added a comment - Petr, I was surprised how small the patch to implement this was. Please could you take a quick look. If you are happy, would it be OK to cherry-pick this to stable branches? That would make the admins here at the OU very happy.
          Hide
          Petr Škoda added a comment -

          +1 if it works in all supported databases

          Show
          Petr Škoda added a comment - +1 if it works in all supported databases
          Hide
          Tim Hunt added a comment -

          I tested Postgres and MySQL, and it works on both of those. I am not aware of any cross-db issues with ORDER BY

          {expression}
          Show
          Tim Hunt added a comment - I tested Postgres and MySQL, and it works on both of those. I am not aware of any cross-db issues with ORDER BY {expression}
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm reopening this:

          0) Code looks perfect and I've checked it works as expected, so np with that.

          But:

          1) I'm not sure if that "magical" order is intuitive enough or it can lead to confusion under some circumstances. It has my +0.5 functional vote, but would be great to have other integrators/MD here to decide. Both about the functionality and about the target branches. IMO it's master-only. Of course, will need docs if landing.

          2) Only "ajax" manual enrolments are covered by the patch, if you disable javascript or you go to "enrol/manual/manage.php" from the enrolment methods page... the search there continues ordering in alphabetical order. It needs to be clarified if the same change is needed there.

          3) I've added one test to verify CASE statements in ORDER BY clauses. Pass under the big four (sqlsrv missing here, but I bet will pass). Surely should be added wherever this finally lands (and test instructions updated to verify 'test_get_records_sql_complicated' does not fail under any RDBMS). It's @:

          https://github.com/stronk7/moodle/commit/b2ae0a627742dee417f8e5589a0e349ac4e847ae

          That is, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm reopening this: 0) Code looks perfect and I've checked it works as expected, so np with that. But: 1) I'm not sure if that "magical" order is intuitive enough or it can lead to confusion under some circumstances. It has my +0.5 functional vote, but would be great to have other integrators/MD here to decide. Both about the functionality and about the target branches. IMO it's master-only. Of course, will need docs if landing. 2) Only "ajax" manual enrolments are covered by the patch, if you disable javascript or you go to "enrol/manual/manage.php" from the enrolment methods page... the search there continues ordering in alphabetical order. It needs to be clarified if the same change is needed there. 3) I've added one test to verify CASE statements in ORDER BY clauses. Pass under the big four (sqlsrv missing here, but I bet will pass). Surely should be added wherever this finally lands (and test instructions updated to verify 'test_get_records_sql_complicated' does not fail under any RDBMS). It's @: https://github.com/stronk7/moodle/commit/b2ae0a627742dee417f8e5589a0e349ac4e847ae That is, ciao
          Hide
          Tim Hunt added a comment -

          Right, the same sort logic needs to be used in all user-selectors, as well as in the enrol UI. I will do that. We still need a +1 from someone.

          Show
          Tim Hunt added a comment - Right, the same sort logic needs to be used in all user-selectors, as well as in the enrol UI. I will do that. We still need a +1 from someone.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Tim Hunt added a comment -

          Picking up on Eloy's suggestion that we should use the same sort order everywhere, I did a search for "order by.*lastname".

          Amazingly, that generates 51 matches!

          So, if we are going to clean up that mess, we really a function to generate the right sort SQL, that is then called everywhere. I have written such a function: users_order_by_sql, https://github.com/timhunt/moodle/commit/cbd5a8c58488546036055a66ca2efd8dadffaccf

          I was hoping I could get a peer-review of that (look, in particular, at the PHP doc comment and the unit tests) before I think about converting all 51 places to call this method.

          Show
          Tim Hunt added a comment - Picking up on Eloy's suggestion that we should use the same sort order everywhere, I did a search for "order by.*lastname". Amazingly, that generates 51 matches! So, if we are going to clean up that mess, we really a function to generate the right sort SQL, that is then called everywhere. I have written such a function: users_order_by_sql, https://github.com/timhunt/moodle/commit/cbd5a8c58488546036055a66ca2efd8dadffaccf I was hoping I could get a peer-review of that (look, in particular, at the PHP doc comment and the unit tests) before I think about converting all 51 places to call this method.
          Hide
          Tim Hunt added a comment -

          I have started converting places that need to call the new function. So far admin, cohort and enrol done, and another commit push. No problems found yet while doing this.

          Show
          Tim Hunt added a comment - I have started converting places that need to call the new function. So far admin, cohort and enrol done, and another commit push. No problems found yet while doing this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Side point about "prefix" perhaps better being named "alias" or "tablealias" and not including the dot (to be consistent with other function/methods like get_extra_user_fields_sql() for example.

          Show
          Eloy Lafuente (stronk7) added a comment - Side point about "prefix" perhaps better being named "alias" or "tablealias" and not including the dot (to be consistent with other function/methods like get_extra_user_fields_sql() for example.
          Hide
          Tim Hunt added a comment -

          Thanks Eloy. Good suggestion, I will update things.

          Show
          Tim Hunt added a comment - Thanks Eloy. Good suggestion, I will update things.
          Hide
          Tim Hunt added a comment -

          Argh! groups_get_members_by_role takes a $sort parameter. Those calls need to be fixed too.

          Show
          Tim Hunt added a comment - Argh! groups_get_members_by_role takes a $sort parameter. Those calls need to be fixed too.
          Hide
          Tim Hunt added a comment -

          I think I have now converted everything.

          I chose not to fix the deprecated get_sorted_contexts function.

          Order is not important in mod/lesson/essay.php in the 'email' case (look at how $users) is used), so I just removed the order-by.

          I removed the option for SQL_PARAMS_QM. That would not be needed anywhere in core code, and it is highly dangerous. Other things, like enrol_sql, do not provide this option.

          Both the calls to groups_get_members_by_role were ordering by u.id, followed by other stuff. Of course, since u.id is a primary key, the other stuff will have no effect, so that was easy.

          There are a number of places where the sort for reports is controlled by the report table, and the UI it provides. I am not changing those.

          A good grep, to ensure I have not missed anything, is

          order by.*lastname|users_order_by_sql
          

          However, that does not catch everything. Grepping for "u.lastname" finds some more, including the following functions that pass through a $sort parameter.

          • groups_get_members_by_role
          • get_role_users
          • get_users_by_capability // Problem. See below
          • get_progress_all -> get_tracked_users
          • get_enrolled_users

          Apart from get_users_by_capability, these are all either OK, or easily fixable.

          The problem with gubc is that by default it sorts by user lastaccess, so if you want the users in name order ('firstname ASC, lastname ASC' == users_order_by_sql()) you have to pass that through, but there is no option to pass through any parameter values. This affects admin_setting_users_with_capability::load_choices in adminlib.php, and mod/quiz/override_form.php.

          Now, as it happens, we know that users_order_by_sql with no search will not use any parameters. I will just do a simple conversion that relies on this, but throw a coding exception if users_order_by_sql does return any params, so if things change in the future, it will be clear what the problem is.

          Show
          Tim Hunt added a comment - I think I have now converted everything. I chose not to fix the deprecated get_sorted_contexts function. Order is not important in mod/lesson/essay.php in the 'email' case (look at how $users) is used), so I just removed the order-by. I removed the option for SQL_PARAMS_QM. That would not be needed anywhere in core code, and it is highly dangerous. Other things, like enrol_sql, do not provide this option. Both the calls to groups_get_members_by_role were ordering by u.id, followed by other stuff. Of course, since u.id is a primary key, the other stuff will have no effect, so that was easy. There are a number of places where the sort for reports is controlled by the report table, and the UI it provides. I am not changing those. A good grep, to ensure I have not missed anything, is order by.*lastname|users_order_by_sql However, that does not catch everything. Grepping for "u.lastname" finds some more, including the following functions that pass through a $sort parameter. groups_get_members_by_role get_role_users get_users_by_capability // Problem. See below get_progress_all -> get_tracked_users get_enrolled_users Apart from get_users_by_capability, these are all either OK, or easily fixable. The problem with gubc is that by default it sorts by user lastaccess, so if you want the users in name order ('firstname ASC, lastname ASC' == users_order_by_sql()) you have to pass that through, but there is no option to pass through any parameter values. This affects admin_setting_users_with_capability::load_choices in adminlib.php, and mod/quiz/override_form.php. Now, as it happens, we know that users_order_by_sql with no search will not use any parameters. I will just do a simple conversion that relies on this, but throw a coding exception if users_order_by_sql does return any params, so if things change in the future, it will be clear what the problem is.
          Hide
          Tim Hunt added a comment -

          Final issues in forum:

          • forum_subscribed_users - bizarrely, this uses order by u.email. The only place in Moodle to do so. I don't plan to change that here, but someone might want to think about that.
          • forum_potential_subscriber_selector was doing weird filtering in PHP that should have been in SQL. I rewrote its find_users method.

          There are other methods that might be relevant, but mostly they sort on things like time posted.

          • forum_get_discussions
          • forum_get_all_discussion_posts
          • forum_get_potential_subscribers
          • forum_print_latest_discussions

          So, I think I am finally done here. Just need to tidy up my branch.

          Show
          Tim Hunt added a comment - Final issues in forum: forum_subscribed_users - bizarrely, this uses order by u.email. The only place in Moodle to do so. I don't plan to change that here, but someone might want to think about that. forum_potential_subscriber_selector was doing weird filtering in PHP that should have been in SQL. I rewrote its find_users method. There are other methods that might be relevant, but mostly they sort on things like time posted. forum_get_discussions forum_get_all_discussion_posts forum_get_potential_subscribers forum_print_latest_discussions So, I think I am finally done here. Just need to tidy up my branch.
          Hide
          Tim Hunt added a comment -

          Right. Branch re-based. This is now ready for peer review.

          Having just tried to write the testing instructions, I am really not sure this is a good idea.

          On the other hand, in terms of removing duplication in the code, and improving usability, I am still sure it is a good idea.

          I guess we have a lot of time to shake-out regressions before Moodle 2.4

          Show
          Tim Hunt added a comment - Right. Branch re-based. This is now ready for peer review. Having just tried to write the testing instructions, I am really not sure this is a good idea. On the other hand, in terms of removing duplication in the code, and improving usability, I am still sure it is a good idea. I guess we have a lot of time to shake-out regressions before Moodle 2.4
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just don't forget this needs to be decided if it an "intuitive" feature to be applied everywhere or no. I'm not 100% convinced, nor 100% against it. Just to open the possibilities: admin setting, cap...?

          That apart from the implementation analysis, it would be great to hear from MD here.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just don't forget this needs to be decided if it an "intuitive" feature to be applied everywhere or no. I'm not 100% convinced, nor 100% against it. Just to open the possibilities: admin setting, cap...? That apart from the implementation analysis, it would be great to hear from MD here. Ciao
          Hide
          Martin Dougiamas added a comment -

          As long as it is only affecting the UI in places where SEARCHING occurs then my +1 to just make this happen all the time without adding new caps/settings etc. I think people are used to exact matches coming at the top.

          Show
          Martin Dougiamas added a comment - As long as it is only affecting the UI in places where SEARCHING occurs then my +1 to just make this happen all the time without adding new caps/settings etc. I think people are used to exact matches coming at the top.
          Hide
          Tim Hunt added a comment -

          Thanks Martin.

          Technically it does affect many places where lists of users are sorted, because in the past there were different places doing

          • ORDER BY u.lastname
          • ORDER BY u.lastname, u.firstname
          • ORDER BY u.lastname, u.firstname, u.id
            and, as part of my changes, everywhere now consistently does the third of those. (The u.id is a meaningless order, but it does mean that the order is always 100% predictable, even when two users have identical names.)

          Also, if someone wants to customise the order, then now they only have to edit one function. The next logical step would be to add an admin configuration option for the user sort order, but I think that is a separate issue.

          We also still need a peer review of the code.

          Show
          Tim Hunt added a comment - Thanks Martin. Technically it does affect many places where lists of users are sorted, because in the past there were different places doing ORDER BY u.lastname ORDER BY u.lastname, u.firstname ORDER BY u.lastname, u.firstname, u.id and, as part of my changes, everywhere now consistently does the third of those. (The u.id is a meaningless order, but it does mean that the order is always 100% predictable, even when two users have identical names.) Also, if someone wants to customise the order, then now they only have to edit one function. The next logical step would be to add an admin configuration option for the user sort order, but I think that is a separate issue. We also still need a peer review of the code.
          Hide
          Nadav Kavalerchik added a comment -

          And if you can add a "quick filter" textbox above any UI that display a list of users, which uses JS (or AJAX) to to filter out only the users which correspond to the "quick filter" text, that could be very helpful too

          Show
          Nadav Kavalerchik added a comment - And if you can add a "quick filter" textbox above any UI that display a list of users, which uses JS (or AJAX) to to filter out only the users which correspond to the "quick filter" text, that could be very helpful too
          Hide
          Tim Hunt added a comment -

          Nadav, That also is a separate feature request.

          Show
          Tim Hunt added a comment - Nadav, That also is a separate feature request.
          Hide
          Nadav Kavalerchik added a comment -

          Tim, got it. (thanks)

          Show
          Nadav Kavalerchik added a comment - Tim, got it. (thanks)
          Hide
          Tim Hunt added a comment -

          Right, I have just done a final review of the code, and some more testing, which found a few odd things, but I think this is now ready for integration.

          Dear integrators, what can I say, other than "I'm sorry to inflict this on you, love Tim."

          Show
          Tim Hunt added a comment - Right, I have just done a final review of the code, and some more testing, which found a few odd things, but I think this is now ready for integration. Dear integrators, what can I say, other than "I'm sorry to inflict this on you, love Tim."
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Here we go, integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Here we go, integrated, thanks!
          Hide
          Tim Hunt added a comment -

          Thank you super-integrator.

          /me crosses fingers and hopes there are no regressions.

          Show
          Tim Hunt added a comment - Thank you super-integrator. /me crosses fingers and hopes there are no regressions.
          Hide
          Frédéric Massart added a comment - - edited

          I am having troubles with this issue, I must be doing something wrong but for now I will fail the test.

          On both MySQL and PostgreSQL the test fails at step 4. After investigation on MySQL I noticed that the query generated is right and orders the user in the right order, but it is not displayed like that in the Enrol popup, nor in the 'Assign system roles' page.

          I haven't tested this on Oracle or MSSQL.

          // The query from SQL logs
          mysql> SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email,u.idnumber,u.username,u.lastaccess FROM mdl_user u
              ->             LEFT JOIN mdl_user_enrolments ue ON (ue.userid = u.id AND ue.enrolid = '1')
              ->                 WHERE u.id <> '1' AND u.deleted = 0 AND u.confirmed = 1 AND (LOWER(idnumber) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\' OR LOWER(email) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\' OR LOWER(CONCAT(u.firstname, ' ', u.lastname)) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\')
              ->                       AND ue.id IS NULL ORDER BY CASE WHEN CONCAT(u.firstname, ' ', u.lastname) = 'f' OR u.firstname = 'f' OR u.lastname = 'f' OR u.idnumber = 'f' OR u.email = 'f' THEN 0 ELSE 1 END, u.lastname, u.firstname, u.id LIMIT 0, 25;
          +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+
          | id | picture | firstname | lastname   | imagealt | email                  | idnumber | username | lastaccess |
          +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+
          | 19 |       0 | Students  | ZZZ        |          | x1@fred.moodle.local   | f        | x1       |          0 |
          | 16 |      53 | Officer   | Barbady    | NULL     | m1@localhost           |          | m1       |          0 |
          |  5 |       9 | Kyle      | Broflovski |          | s3@localhost           | f3       | s3       |          0 |
          | 20 |       0 | Ken       | Brov       |          | x2@fred.moodle.local   | f6       | x2       |          0 |
          |  3 |       1 | Eric      | Cartman    |          | s1@localhost           | f1       | s1       |          0 |
          |  4 |       5 | Stan      | Marsh      |          | s2@localhost           | f2       | s2       |          0 |
          |  6 |      13 | Kenny     | McCormick  |          | s4@localhost           | f4       | s4       |          0 |
          |  2 |       0 | Admin     | User       |          | fred@fred.moodle.local |          | admin    | 1349240956 |
          +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+
          8 rows in set (0.00 sec)
          

          See attachment for UI display.

          Show
          Frédéric Massart added a comment - - edited I am having troubles with this issue, I must be doing something wrong but for now I will fail the test. On both MySQL and PostgreSQL the test fails at step 4. After investigation on MySQL I noticed that the query generated is right and orders the user in the right order, but it is not displayed like that in the Enrol popup, nor in the 'Assign system roles' page. I haven't tested this on Oracle or MSSQL. // The query from SQL logs mysql> SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email,u.idnumber,u.username,u.lastaccess FROM mdl_user u -> LEFT JOIN mdl_user_enrolments ue ON (ue.userid = u.id AND ue.enrolid = '1') -> WHERE u.id <> '1' AND u.deleted = 0 AND u.confirmed = 1 AND (LOWER(idnumber) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\' OR LOWER(email) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\' OR LOWER(CONCAT(u.firstname, ' ', u.lastname)) LIKE LOWER('%f%') COLLATE utf8_bin ESCAPE '\\') -> AND ue.id IS NULL ORDER BY CASE WHEN CONCAT(u.firstname, ' ', u.lastname) = 'f' OR u.firstname = 'f' OR u.lastname = 'f' OR u.idnumber = 'f' OR u.email = 'f' THEN 0 ELSE 1 END, u.lastname, u.firstname, u.id LIMIT 0, 25; +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+ | id | picture | firstname | lastname | imagealt | email | idnumber | username | lastaccess | +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+ | 19 | 0 | Students | ZZZ | | x1@fred.moodle.local | f | x1 | 0 | | 16 | 53 | Officer | Barbady | NULL | m1@localhost | | m1 | 0 | | 5 | 9 | Kyle | Broflovski | | s3@localhost | f3 | s3 | 0 | | 20 | 0 | Ken | Brov | | x2@fred.moodle.local | f6 | x2 | 0 | | 3 | 1 | Eric | Cartman | | s1@localhost | f1 | s1 | 0 | | 4 | 5 | Stan | Marsh | | s2@localhost | f2 | s2 | 0 | | 6 | 13 | Kenny | McCormick | | s4@localhost | f4 | s4 | 0 | | 2 | 0 | Admin | User | | fred@fred.moodle.local | | admin | 1349240956 | +----+---------+-----------+------------+----------+------------------------+----------+----------+------------+ 8 rows in set (0.00 sec) See attachment for UI display.
          Hide
          Tim Hunt added a comment -

          That is weird. This certainly was working for me when I wrote the code. Also, if the query is returning the records in the right order, why are they being displayed in a different order?

          I am confused, but I am also going to a meeting today, so won't be able to look at this until mid-afternoon today (Wednesday, UK time). I will certainly look then. Sorry about the bad timing.

          Show
          Tim Hunt added a comment - That is weird. This certainly was working for me when I wrote the code. Also, if the query is returning the records in the right order, why are they being displayed in a different order? I am confused, but I am also going to a meeting today, so won't be able to look at this until mid-afternoon today (Wednesday, UK time). I will certainly look then. Sorry about the bad timing.
          Hide
          Tim Hunt added a comment -

          Hmm. I just quickly tried to reproduce this.

          1. Starting with last week's master, I went to the enrol users page for a course, and searched. The uses were listed in alphabetical order, with Student ZZZ last.

          2. Upgraded to integration/master.

          3. Reloaded the enrol/users.php page, and did another search. Student ZZZ was listed first.

          So, I don't understand what you were seeing Fréd. Please can you try again, and do all the silly tests like "Have you plugged it in?".

          Show
          Tim Hunt added a comment - Hmm. I just quickly tried to reproduce this. 1. Starting with last week's master, I went to the enrol users page for a course, and searched. The uses were listed in alphabetical order, with Student ZZZ last. 2. Upgraded to integration/master. 3. Reloaded the enrol/users.php page, and did another search. Student ZZZ was listed first. So, I don't understand what you were seeing Fréd. Please can you try again, and do all the silly tests like "Have you plugged it in?".
          Hide
          Frédéric Massart added a comment -

          I have tried many things, is there a setting that I missed somewhere?

          Although, I just noticed that this does work on Firefox for some reason. At least the steps 4 and 5. But Chromium still doesn't... I suspect an ordering issue while reading the JSON object. Could you check on your side?

          Show
          Frédéric Massart added a comment - I have tried many things, is there a setting that I missed somewhere? Although, I just noticed that this does work on Firefox for some reason. At least the steps 4 and 5. But Chromium still doesn't... I suspect an ordering issue while reading the JSON object. Could you check on your side?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          (I bet some PHP-sorting is happening there....)

          Show
          Eloy Lafuente (stronk7) added a comment - (I bet some PHP-sorting is happening there....)
          Hide
          Dan Poltawski added a comment -

          Just tested this too, and i'm seeing the same behaviour as fred - on Chrome its not working, on firefox it is.

          Show
          Dan Poltawski added a comment - Just tested this too, and i'm seeing the same behaviour as fred - on Chrome its not working, on firefox it is.
          Hide
          Dan Poltawski added a comment -

          Oh, but for me, its only failing on the enrol screen, and not on the role assignment screen!!?!

          Show
          Dan Poltawski added a comment - Oh, but for me, its only failing on the enrol screen, and not on the role assignment screen!!?!
          Hide
          Dan Poltawski added a comment - - edited

          Attaching screenshot of enrolment screen and assignment screen, firefox on left, chrome on right. Both same install, different results!

          Note that freds chromium isn't working on the role assignment screen either!

          Show
          Dan Poltawski added a comment - - edited Attaching screenshot of enrolment screen and assignment screen, firefox on left, chrome on right. Both same install, different results! Note that freds chromium isn't working on the role assignment screen either!
          Hide
          Tim Hunt added a comment -

          Since this depends on browser, it cannot be a PHP issue.

          I am suspicious of web browser JSON parsing. Or, possibly the way they enumerate the fields of objects. I wonder if the order we are seeing is the order of field names / array keys?

          Show
          Tim Hunt added a comment - Since this depends on browser, it cannot be a PHP issue. I am suspicious of web browser JSON parsing. Or, possibly the way they enumerate the fields of objects. I wonder if the order we are seeing is the order of field names / array keys?
          Hide
          Tim Hunt added a comment -

          Confirmed. Chrome, either when it parses JSON to an object, or when it iterates through the properties of an object, does it in the order of the array keys, not in the order in the input.

          To see this, in the Chrome equivalent of Firebug, in the Network panel, for the ajax request to get the list of users, compare the raw Response to what is shown in the Preview tab.

          Therefore, it is an existing bug that the users are displayed in an effectively random order in Chrome, and I have filed it as MDL-35776. I'm working on a fix.

          Show
          Tim Hunt added a comment - Confirmed. Chrome, either when it parses JSON to an object, or when it iterates through the properties of an object, does it in the order of the array keys, not in the order in the input. To see this, in the Chrome equivalent of Firebug, in the Network panel, for the ajax request to get the list of users, compare the raw Response to what is shown in the Preview tab. Therefore, it is an existing bug that the users are displayed in an effectively random order in Chrome, and I have filed it as MDL-35776 . I'm working on a fix.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Reading MDL-35776 it seems this can be sent back to testing, ignore Chrome disorders, plz.

          Show
          Eloy Lafuente (stronk7) added a comment - Reading MDL-35776 it seems this can be sent back to testing, ignore Chrome disorders, plz.
          Hide
          Frédéric Massart added a comment - - edited

          Result of testing on PostgreSQL (Firefox)

          (Improvement) The exact ordering is case sensitive for information such as first and last name, IMHO it should be case insensitive.

          (Minor bug) When viewing the users which are site administrators, when doing a search which does not include the real admin account, the user below "Main administator" is no longer the real admin user.

          (Minor bug) When adding/removing users from a specific group, the left area does not order by exact match (the right does). (Javascript ON or OFF)

          (SQL bug) When going to Home ► Courses ► Miscellaneous ► MYCOURSE ► General ► Workshop ► Submission allocation > Manual allocation

          Debug info: ERROR: missing FROM-clause entry for table "u"
          LINE 17: ... je ON (je.id = u.id AND u.deleted = 0) ORDER BY u.lastname...
          ^
          SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email
          FROM mdl_user u
          JOIN (SELECT DISTINCT eu3_u.id
          FROM mdl_user eu3_u
          JOIN mdl_role_assignments eu3_ra3 ON (eu3_ra3.userid = eu3_u.id AND eu3_ra3.roleid IN (5) AND eu3_ra3.contextid IN (1,3,89,113))
          JOIN mdl_user_enrolments eu3_ue ON eu3_ue.userid = eu3_u.id
          JOIN mdl_enrol eu3_e ON (eu3_e.id = eu3_ue.enrolid AND eu3_e.courseid = $1)
          WHERE eu3_u.deleted = 0 AND eu3_u.id <> $2 AND eu3_ue.status = $3 AND eu3_e.status = $4 AND eu3_ue.timestart < $5 AND (eu3_ue.timeend = 0 OR eu3_ue.timeend > $6)) je ON (je.id = u.id AND u.deleted = 0)
          UNION
          SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email
          FROM mdl_user u
          JOIN (SELECT DISTINCT eu4_u.id
          FROM mdl_user eu4_u
          JOIN mdl_role_assignments eu4_ra3 ON (eu4_ra3.userid = eu4_u.id AND eu4_ra3.roleid IN (5) AND eu4_ra3.contextid IN (1,3,89,113))
          JOIN mdl_user_enrolments eu4_ue ON eu4_ue.userid = eu4_u.id
          JOIN mdl_enrol eu4_e ON (eu4_e.id = eu4_ue.enrolid AND eu4_e.courseid = $7)
          WHERE eu4_u.deleted = 0 AND eu4_u.id <> $8 AND eu4_ue.status = $9 AND eu4_e.status = $10 AND eu4_ue.timestart < $11 AND (eu4_ue.timeend = 0 OR eu4_ue.timeend > $12)) je ON (je.id = u.id AND u.deleted = 0) ORDER BY u.lastname, u.firstname, u.id LIMIT 10 OFFSET 0
          [array (
          0 => '6',
          1 => '1',
          2 => 0,
          3 => 0,
          4 => 1349323600,
          5 => 1349323600,
          6 => '6',
          7 => '1',
          8 => 0,
          9 => 0,
          10 => 1349323600,
          11 => 1349323600,
          )]
          Error code: dmlreadexception
          Stack trace:
          
              line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
              line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
              line 734 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
              line 506 of /mod/workshop/locallib.php: call to pgsql_native_moodle_database->get_records_sql()
              line 212 of /mod/workshop/allocation/manual/lib.php: call to workshop->get_participants()
              line 79 of /mod/workshop/allocation.php: call to workshop_manual_allocator->ui()
          

          (SQL bug) When viewing Reports > Statistics (Detailed (user) view)

          Debug info: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list
          LINE 4: ... $1 AND timeend >= $2 ORDER BY u.lastname, u.firstname, u.id
          ^
          SELECT DISTINCT s.userid, u.firstname, u.lastname, u.idnumber
          FROM mdl_stats_user_daily s
          JOIN mdl_user u ON u.id = s.userid
          WHERE courseid = $1 AND timeend >= $2 ORDER BY u.lastname, u.firstname, u.id
          [array (
          0 => '6',
          1 => 1346860800,
          )]
          Error code: dmlreadexception
          Stack trace:
          
              line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown
              line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
              line 734 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
              line 128 of /report/stats/locallib.php: call to pgsql_native_moodle_database->get_records_sql()
              line 87 of /report/stats/index.php: call to report_stats_report()
          

          Please raise issues accordingly.

          I have already spent about one day to test this issue (yesterday and today), I am putting this test on hold until integrators/wise men decide what should be done.

          Show
          Frédéric Massart added a comment - - edited Result of testing on PostgreSQL (Firefox) (Improvement) The exact ordering is case sensitive for information such as first and last name, IMHO it should be case insensitive. (Minor bug) When viewing the users which are site administrators, when doing a search which does not include the real admin account, the user below "Main administator" is no longer the real admin user. (Minor bug) When adding/removing users from a specific group, the left area does not order by exact match (the right does). (Javascript ON or OFF) (SQL bug) When going to Home ► Courses ► Miscellaneous ► MYCOURSE ► General ► Workshop ► Submission allocation > Manual allocation Debug info: ERROR: missing FROM-clause entry for table "u" LINE 17: ... je ON (je.id = u.id AND u.deleted = 0) ORDER BY u.lastname... ^ SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email FROM mdl_user u JOIN (SELECT DISTINCT eu3_u.id FROM mdl_user eu3_u JOIN mdl_role_assignments eu3_ra3 ON (eu3_ra3.userid = eu3_u.id AND eu3_ra3.roleid IN (5) AND eu3_ra3.contextid IN (1,3,89,113)) JOIN mdl_user_enrolments eu3_ue ON eu3_ue.userid = eu3_u.id JOIN mdl_enrol eu3_e ON (eu3_e.id = eu3_ue.enrolid AND eu3_e.courseid = $1) WHERE eu3_u.deleted = 0 AND eu3_u.id <> $2 AND eu3_ue.status = $3 AND eu3_e.status = $4 AND eu3_ue.timestart < $5 AND (eu3_ue.timeend = 0 OR eu3_ue.timeend > $6)) je ON (je.id = u.id AND u.deleted = 0) UNION SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email FROM mdl_user u JOIN (SELECT DISTINCT eu4_u.id FROM mdl_user eu4_u JOIN mdl_role_assignments eu4_ra3 ON (eu4_ra3.userid = eu4_u.id AND eu4_ra3.roleid IN (5) AND eu4_ra3.contextid IN (1,3,89,113)) JOIN mdl_user_enrolments eu4_ue ON eu4_ue.userid = eu4_u.id JOIN mdl_enrol eu4_e ON (eu4_e.id = eu4_ue.enrolid AND eu4_e.courseid = $7) WHERE eu4_u.deleted = 0 AND eu4_u.id <> $8 AND eu4_ue.status = $9 AND eu4_e.status = $10 AND eu4_ue.timestart < $11 AND (eu4_ue.timeend = 0 OR eu4_ue.timeend > $12)) je ON (je.id = u.id AND u.deleted = 0) ORDER BY u.lastname, u.firstname, u.id LIMIT 10 OFFSET 0 [array ( 0 => '6', 1 => '1', 2 => 0, 3 => 0, 4 => 1349323600, 5 => 1349323600, 6 => '6', 7 => '1', 8 => 0, 9 => 0, 10 => 1349323600, 11 => 1349323600, )] Error code: dmlreadexception Stack trace: line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end() line 734 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 506 of /mod/workshop/locallib.php: call to pgsql_native_moodle_database->get_records_sql() line 212 of /mod/workshop/allocation/manual/lib.php: call to workshop->get_participants() line 79 of /mod/workshop/allocation.php: call to workshop_manual_allocator->ui() (SQL bug) When viewing Reports > Statistics (Detailed (user) view) Debug info: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list LINE 4: ... $1 AND timeend >= $2 ORDER BY u.lastname, u.firstname, u.id ^ SELECT DISTINCT s.userid, u.firstname, u.lastname, u.idnumber FROM mdl_stats_user_daily s JOIN mdl_user u ON u.id = s.userid WHERE courseid = $1 AND timeend >= $2 ORDER BY u.lastname, u.firstname, u.id [array ( 0 => '6', 1 => 1346860800, )] Error code: dmlreadexception Stack trace: line 407 of /lib/dml/moodle_database.php: dml_read_exception thrown line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end() line 734 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 128 of /report/stats/locallib.php: call to pgsql_native_moodle_database->get_records_sql() line 87 of /report/stats/index.php: call to report_stats_report() Please raise issues accordingly. I have already spent about one day to test this issue (yesterday and today), I am putting this test on hold until integrators/wise men decide what should be done.
          Hide
          Tim Hunt added a comment -

          Thanks for all the testing Fred. Coding this, and rebasing it several times, was a lot of work too.

          I will look into these issues later today. Hopefully most of them are easy fixes.

          Show
          Tim Hunt added a comment - Thanks for all the testing Fred. Coding this, and rebasing it several times, was a lot of work too. I will look into these issues later today. Hopefully most of them are easy fixes.
          Hide
          Tim Hunt added a comment -

          1. Case insensitive exact match detection: DONE

          2. Correct identification of main admin: DONE (that was an unrelated existing bug.)

          3. Group members not sorted: DONE (that was an unrelated existing bug.)

          but note that the groups are sorted by role, and I have not attempted to move the group containing the exact match to be the first group. I don't think it is worth the effort, but if anyone else feels they are up to the challenge, please go for it.

          4. Workshop bug: DONE.

          I did not know UNIONS worked like that. Even though both the selects that make up the UNION use the same table alias 'u', the column in the result of the UNION has column names without the prefix.

          5. Stats report bug: DONE.

          Although I failed to test this very well. I could not work out how to get my server to generate any stats data. However, I commented out enough print_error('nostatstodisplay' calls that I could be sure that the SQL that was giving a fatal error was being run. With this fix, that SQL now executes, and logically the new query should give the same results as the old query. However, I have been unable to verify that.

          All ready for integration: https://github.com/timhunt/moodle/compare/master...MDL-34657_fixup

          Show
          Tim Hunt added a comment - 1. Case insensitive exact match detection: DONE 2. Correct identification of main admin: DONE (that was an unrelated existing bug.) 3. Group members not sorted: DONE (that was an unrelated existing bug.) but note that the groups are sorted by role, and I have not attempted to move the group containing the exact match to be the first group. I don't think it is worth the effort, but if anyone else feels they are up to the challenge, please go for it. 4. Workshop bug: DONE. I did not know UNIONS worked like that. Even though both the selects that make up the UNION use the same table alias 'u', the column in the result of the UNION has column names without the prefix. 5. Stats report bug: DONE. Although I failed to test this very well. I could not work out how to get my server to generate any stats data. However, I commented out enough print_error('nostatstodisplay' calls that I could be sure that the SQL that was giving a fatal error was being run. With this fix, that SQL now executes, and logically the new query should give the same results as the old query. However, I have been unable to verify that. All ready for integration: https://github.com/timhunt/moodle/compare/master...MDL-34657_fixup
          Hide
          Tim Hunt added a comment -

          To INTEGRATORS: MDL-35776 is now also ready for integration, and it might be good to get that in and test it at the same time as testing this.

          Show
          Tim Hunt added a comment - To INTEGRATORS: MDL-35776 is now also ready for integration, and it might be good to get that in and test it at the same time as testing this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          1) The fixup commits have been integrated.

          2) MDL-35776 won't be integrated this week (it's Friday and involves stables too).

          3) This is ready for testing again. Ignore the Chrome disorders thing, plz. They will be fixed next week.

          4) Please report all remaining problems (hopefully they won't exist), as new linked (regression) issues. Then pass this (after all it's master and we can accept minor-buggy code there). So unless there is something really blocking development (install, every-page-problem...), I think we can "jump forward" that way.

          Thanks Tim and Fred (et all)!

          Show
          Eloy Lafuente (stronk7) added a comment - 1) The fixup commits have been integrated. 2) MDL-35776 won't be integrated this week (it's Friday and involves stables too). 3) This is ready for testing again. Ignore the Chrome disorders thing, plz. They will be fixed next week. 4) Please report all remaining problems (hopefully they won't exist), as new linked (regression) issues. Then pass this (after all it's master and we can accept minor-buggy code there). So unless there is something really blocking development (install, every-page-problem...), I think we can "jump forward" that way. Thanks Tim and Fred (et all)!
          Hide
          Tim Hunt added a comment -

          Great. Thanks Eloy.

          Show
          Tim Hunt added a comment - Great. Thanks Eloy.
          Hide
          Filter Manager added a comment - - edited

          (added one more commit fixing unit tests to verify LOWER() is used)

          Edited: Filter Manager was Eloy, just in case.

          Show
          Filter Manager added a comment - - edited (added one more commit fixing unit tests to verify LOWER() is used) Edited: Filter Manager was Eloy, just in case.
          Hide
          Frédéric Massart added a comment -

          I will test this issue on Oracle, but I won't be able to do it on MSSQL and would prefer having someone helping for MySQL. Cheers!

          Show
          Frédéric Massart added a comment - I will test this issue on Oracle, but I won't be able to do it on MSSQL and would prefer having someone helping for MySQL. Cheers!
          Hide
          Dan Poltawski added a comment -

          I can do mssql + mysql

          Show
          Dan Poltawski added a comment - I can do mssql + mysql
          Hide
          Frédéric Massart added a comment -

          Successful on Oracle. Please note that I haven't tested the MNet functionalities, and I skipped the check of the teacher name in the email sent on self-enrolment (this worked on PostgreSQL) because I was not sure how to set up the emails thing or debug on Windows.

          I noticed:

          • Assigning other users on a course level does not filter by id number;
          • Teachers displayed on the list of courses are not ordered by last name;
          • The participants in the dropdown on the logs page are ordered by first name;

          Tim, about the statistics, it took me some time to figure out how to generate them too. I actually hacked the code to force the cron to run, but I think if you set those settings then you should be able to have the stats generated during the cron:

          • statsfirstrun: All
          • statsmaxruntime: Until complete
          • statsruntimedays: 99999999
          • statsruntimestarthour: The your server time (ish) at which you'll launch the cron
          • statslast*: Set to 0 to force regeneration
          Show
          Frédéric Massart added a comment - Successful on Oracle . Please note that I haven't tested the MNet functionalities, and I skipped the check of the teacher name in the email sent on self-enrolment (this worked on PostgreSQL) because I was not sure how to set up the emails thing or debug on Windows. I noticed: Assigning other users on a course level does not filter by id number; Teachers displayed on the list of courses are not ordered by last name; The participants in the dropdown on the logs page are ordered by first name; Tim, about the statistics, it took me some time to figure out how to generate them too. I actually hacked the code to force the cron to run, but I think if you set those settings then you should be able to have the stats generated during the cron: statsfirstrun: All statsmaxruntime: Until complete statsruntimedays: 99999999 statsruntimestarthour: The your server time (ish) at which you'll launch the cron statslast*: Set to 0 to force regeneration
          Hide
          Dan Poltawski added a comment -

          [will turn this into a new bug if necessary]
          On MSSQL, if I go to the 'enrol other users' part of the course page:
          http://dan.moodle.local/integration/enrol/otherusers.php?id=2
          and search for s, I get no results.

          Show
          Dan Poltawski added a comment - [will turn this into a new bug if necessary] On MSSQL, if I go to the 'enrol other users' part of the course page: http://dan.moodle.local/integration/enrol/otherusers.php?id=2 and search for s, I get no results.
          Hide
          Frédéric Massart added a comment -

          Yes Dan, that's what I noticed as well, I think it is just because the filters for this field are not enabled, the ID number does not even show up in the list.

          Show
          Frédéric Massart added a comment - Yes Dan, that's what I noticed as well, I think it is just because the filters for this field are not enabled, the ID number does not even show up in the list.
          Hide
          Dan Poltawski added a comment -

          Fred: for future reference, I believe this should work for emails:

          $CFG->smtphosts = 'smtp.highway1.com.au';
          $CFG->divertallemailsto = 'fred@moodle.com';

          Show
          Dan Poltawski added a comment - Fred: for future reference, I believe this should work for emails: $CFG->smtphosts = 'smtp.highway1.com.au'; $CFG->divertallemailsto = 'fred@moodle.com';
          Hide
          Dan Poltawski added a comment -

          MSSQL:

          • Problem with 'Enrol other users' search listed above
          • In security report, getting weird "The default user role "" is incorrectly defined!""
          • I noticed that the sorting was weird on the 'existing user' screens of groups screen and forum screen (and maybe others). See the attached screenshots.
          Show
          Dan Poltawski added a comment - MSSQL: Problem with 'Enrol other users' search listed above In security report, getting weird "The default user role "" is incorrectly defined!"" I noticed that the sorting was weird on the 'existing user' screens of groups screen and forum screen (and maybe others). See the attached screenshots.
          Hide
          Dan Poltawski added a comment -

          MySQL looks good, though I skimped a bit on this db. That sort problem exists on mysql too, seems like the default sort is weird.

          (I had a minor panic thinking it wasn't working at all, but it was chrome ).

          Show
          Dan Poltawski added a comment - MySQL looks good, though I skimped a bit on this db. That sort problem exists on mysql too, seems like the default sort is weird. (I had a minor panic thinking it wasn't working at all, but it was chrome ).
          Hide
          Tim Hunt added a comment -

          Thanks Dan and Fred for your continuing labours on this.

          I am not acutally sure what some of dan't screen-grabs are meant to show, particularly the left/right comparison ones.

          And, some of the places were Dan says the sorting is weird, I don't get it. Excluding exact matches, it sorts by last name. Taking the forum example, 1 < U < V < Z, so what is wrong there, or just that 's' has not been moved to the top?

          Anyway, I will work on fixes for the remaining glitches in a few hours. You can decide whether to integrate them this week or next week.

          Show
          Tim Hunt added a comment - Thanks Dan and Fred for your continuing labours on this. I am not acutally sure what some of dan't screen-grabs are meant to show, particularly the left/right comparison ones. And, some of the places were Dan says the sorting is weird, I don't get it. Excluding exact matches, it sorts by last name. Taking the forum example, 1 < U < V < Z, so what is wrong there, or just that 's' has not been moved to the top? Anyway, I will work on fixes for the remaining glitches in a few hours. You can decide whether to integrate them this week or next week.
          Hide
          Dan Poltawski added a comment -

          Tim: the left/right screens are old screengrabs, please disregard.

          The sorting is by lastname! That was what I was missing, so disregard what I said there too.

          Show
          Dan Poltawski added a comment - Tim: the left/right screens are old screengrabs, please disregard. The sorting is by lastname! That was what I was missing, so disregard what I said there too.
          Hide
          Tim Hunt added a comment -

          6. The Enrol other users thing is another example of an existing bug that no-one had noticed before. In enrol/locallib.php, get_potential_users (and various other places) call get_extra_user_fields, but search_other_users does not. Even thought this is an unrelated issue, I will fix it for next week. MDL-35802

          7. I cannot reproduce the Security report issue, and anyway that bit of the report has nothing to do with user sorting. So, if you can reproduce that, please file another issue.

          8. As already identified, the 'funny sort order' is just order by lastname, given that, I don't think there are any other outstanding problems. If I have missed anything, please let me know.

          By the way. Now that (almost) all sorting of users goes through users_order_by_sql, the next logical step is to introduce an admin setting for what the order should be. It could go on User -> Permissions -> User policies, next to the 'fields to display' setting.

          Show
          Tim Hunt added a comment - 6. The Enrol other users thing is another example of an existing bug that no-one had noticed before. In enrol/locallib.php, get_potential_users (and various other places) call get_extra_user_fields, but search_other_users does not. Even thought this is an unrelated issue, I will fix it for next week. MDL-35802 7. I cannot reproduce the Security report issue, and anyway that bit of the report has nothing to do with user sorting. So, if you can reproduce that, please file another issue. 8. As already identified, the 'funny sort order' is just order by lastname, given that, I don't think there are any other outstanding problems. If I have missed anything, please let me know. By the way. Now that (almost) all sorting of users goes through users_order_by_sql, the next logical step is to introduce an admin setting for what the order should be. It could go on User -> Permissions -> User policies, next to the 'fields to display' setting.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your awesome collaboration.

          Show
          Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.
          Hide
          Dan Poltawski added a comment -

          FYI: this issue caused a regression discovered in MDL-36086

          Show
          Dan Poltawski added a comment - FYI: this issue caused a regression discovered in MDL-36086
          Hide
          Tim Hunt added a comment -
          Show
          Tim Hunt added a comment - Dev docs added at http://docs.moodle.org/dev/User-related_APIs which is linked from http://docs.moodle.org/dev/Core_APIs#Other_General_APIs .

            People

            • Votes:
              20 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: