Moodle
  1. Moodle
  2. MDL-27880

moodle_user_get_course_participants_by_id

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.1
    • Component/s: Web Services
    • Labels:
      None
    • Rank:
      17528

      Description

      it should behave as same as moodle_user_get_users_by_id, but use course context instead

        Issue Links

          Activity

          Hide
          Dongsheng Cai added a comment - - edited
          capability fields
          moodle/user:viewdetails userid, fullname, insterestss, all customfields, roles
          moodle/course:viewhiddenuserfields description, descriptionformat, country, city, address, phone1, phone2, email, url, icq, skype, yahoo, aim, msn, firstaccess, lastaccess
          moodle/site:viewfullnames firstname, lastname
          moodle/user:update auth, confirmed, idnumber, lang,theme, timezone, mailformat
          moodle/site:accessallgroups groups
          admin or current user institution, department
          Show
          Dongsheng Cai added a comment - - edited capability fields moodle/user:viewdetails userid, fullname, insterestss, all customfields , roles moodle/course:viewhiddenuserfields description, descriptionformat, country, city, address, phone1, phone2, email, url, icq, skype, yahoo, aim, msn, firstaccess, lastaccess moodle/site:viewfullnames firstname, lastname moodle/user:update auth, confirmed, idnumber, lang,theme, timezone, mailformat moodle/site:accessallgroups groups admin or current user institution, department
          Hide
          Sam Hemelryk added a comment -

          Hi Dongsheng,

          I've just been looking at this now, in general it looks good
          Most of the points below relate to performance, many of them were/are in fact present in the get_users_by_id method as well I think when I pointed it out to Jerome he choose to create issues so you could if this is urgent as well just link to those issues and comment in them that this new method needs to be cleaned up as well.

          General notes:

          1. Please fix the indenting of the get_course_participants_by_id_returns, could you please also fix the indenting of get_users_by_id_returns at the same time (I asked Jerome too but obviously forgot to check again before integrating).
          2. The if (empty($user->deleted)) should check check !empty and continue, it should also be at the top of the function before any other calls. Alternativily if you switch to fetching uses manually as suggested below in the perf notes you could just check it as a database condition.

          Things that could be done to improve performance:

          1. Rather than fetching all user contexts and then all users through user_get_users_by_id you should just use a recordset to retrieve users AND their contexts at the same time (using context_instance_preload* functions).
          2. Rather than fetching the course and context for each user within foreach you should really be fetching all courses (unique($courseids)) and could be doing the same as above and preloading the contexts at the same time.
          3. When fetching the custom fields the code is fetching all the categories and foreach category fetching all fields, certainly this could be one query rather than queries within a foreach, just fetch all fields sorting by category.sortorder, then field sortorder.

          So 2 minor points + 3 performance options. All looks good to me otherwise

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Dongsheng, I've just been looking at this now, in general it looks good Most of the points below relate to performance, many of them were/are in fact present in the get_users_by_id method as well I think when I pointed it out to Jerome he choose to create issues so you could if this is urgent as well just link to those issues and comment in them that this new method needs to be cleaned up as well. General notes: Please fix the indenting of the get_course_participants_by_id_returns, could you please also fix the indenting of get_users_by_id_returns at the same time (I asked Jerome too but obviously forgot to check again before integrating). The if (empty($user->deleted)) should check check !empty and continue, it should also be at the top of the function before any other calls. Alternativily if you switch to fetching uses manually as suggested below in the perf notes you could just check it as a database condition. Things that could be done to improve performance: Rather than fetching all user contexts and then all users through user_get_users_by_id you should just use a recordset to retrieve users AND their contexts at the same time (using context_instance_preload* functions). Rather than fetching the course and context for each user within foreach you should really be fetching all courses (unique($courseids)) and could be doing the same as above and preloading the contexts at the same time. When fetching the custom fields the code is fetching all the categories and foreach category fetching all fields, certainly this could be one query rather than queries within a foreach, just fetch all fields sorting by category.sortorder, then field sortorder. So 2 minor points + 3 performance options. All looks good to me otherwise Cheers Sam
          Hide
          Dongsheng Cai added a comment -

          A)

          1. Fixed indentation
          2. Added !empty() and continue check to both get_users_by_id and get_course_participants_by_id

          B)

          1. Fixed
          2. Fixed
          3. Fixed
          Show
          Dongsheng Cai added a comment - A) Fixed indentation Added !empty() and continue check to both get_users_by_id and get_course_participants_by_id B) Fixed Fixed Fixed
          Hide
          Sam Hemelryk added a comment -

          Hi DS,

          Looks great to me!

          Thanks for going to the effort of fixing up those performance issues as well (plus fixing them in the get_users_by_id), fantastic to get them out of the way while this is all fresh.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi DS, Looks great to me! Thanks for going to the effort of fixing up those performance issues as well (plus fixing them in the get_users_by_id), fantastic to get them out of the way while this is all fresh. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Thanks DS this has been integrated now.

          Show
          Sam Hemelryk added a comment - Thanks DS this has been integrated now.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          passing, nobody tested this.

          Show
          Eloy Lafuente (stronk7) added a comment - passing, nobody tested this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          U.P.S.T.R.E.A.M

          Show
          Eloy Lafuente (stronk7) added a comment - U.P.S.T.R.E.A.M

            People

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

              Dates

              • Created:
                Updated:
                Resolved: