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

Select default button on course reset page should select 'Student' from unenrol users list

    Details

    • Testing Instructions:
      Hide
      1. Log in as admin/teacher
      2. Select a course and click reset
      3. Go to course reset page by clicking on reset (Settings -> Course administration -> Reset)
      4. Click "Select defaults" and make sure Student is selected in "Unenrol users" list

      Test 2

      1. Log in as admin
      2. Go to define roles (Home ► Site administration ► Users ► Permissions ► Define roles)
      3. Edit teacher role and set "Role archetype" = student
      4. Repeat Test 1 and this time "Student" and "Teacher" should be selected in "unenrol user" list.
      Show
      Log in as admin/teacher Select a course and click reset Go to course reset page by clicking on reset (Settings -> Course administration -> Reset) Click "Select defaults" and make sure Student is selected in "Unenrol users" list Test 2 Log in as admin Go to define roles (Home ► Site administration ► Users ► Permissions ► Define roles) Edit teacher role and set "Role archetype" = student Repeat Test 1 and this time "Student" and "Teacher" should be selected in "unenrol user" list.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      wip-mdl-34190

      Description

      The [Select default] button on the course reset page should select Student from unenrol users list like it did in Moodle 1. This no longer happens.

        Gliffy Diagrams

          Activity

          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks for reporting this.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          ccaajoa Jo Matthews added a comment -

          In function load_defaults()
          replace
          $defaults = array ('reset_events'=>1, 'reset_logs'=>1, 'reset_roles_local'=>1, 'reset_gradebook_grades'=>1, 'reset_notes'=>1);
          with
          $defaults = array ('reset_events'=>1, 'reset_logs'=>1, 'reset_roles_local'=>1, 'reset_gradebook_grades'=>1, 'reset_notes'=>1, 'unenrol_users' =>5);

          Show
          ccaajoa Jo Matthews added a comment - In function load_defaults() replace $defaults = array ('reset_events'=>1, 'reset_logs'=>1, 'reset_roles_local'=>1, 'reset_gradebook_grades'=>1, 'reset_notes'=>1); with $defaults = array ('reset_events'=>1, 'reset_logs'=>1, 'reset_roles_local'=>1, 'reset_gradebook_grades'=>1, 'reset_notes'=>1, 'unenrol_users' =>5);
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks for the patch Jo,

          I have created a git branch on your behalf and pushing it for peer review.

          FYI: You have used hard-coded value for student role = 5, which is probably not true for some installations as they might change Default role or have multiple roles to define student. I have modified the patch to consider if defined roles have student archetype then select them by default.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks for the patch Jo, I have created a git branch on your behalf and pushing it for peer review. FYI: You have used hard-coded value for student role = 5, which is probably not true for some installations as they might change Default role or have multiple roles to define student. I have modified the patch to consider if defined roles have student archetype then select them by default.
          Hide
          damyon Damyon Wiese added a comment -

          Thanks everyone,

          Peer Review Checklist:

          [Y] Syntax
          [-] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [Y] Testing
          [-] Security
          [-] Documentation
          [Y] Git
          [Y] Sanity check

          This patch looks great to me - sending for integration review.

          Show
          damyon Damyon Wiese added a comment - Thanks everyone, Peer Review Checklist: [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check This patch looks great to me - sending for integration review.
          Hide
          stronk7 Eloy Lafuente (stronk7) 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
          stronk7 Eloy Lafuente (stronk7) 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
          poltawski Dan Poltawski added a comment -

          Integrated to master, 24 and 23. Thanks Raj

          Show
          poltawski Dan Poltawski added a comment - Integrated to master, 24 and 23. Thanks Raj
          Hide
          fred Frédéric Massart added a comment -

          Test passed on 2.3 onwards. Thanks!

          Show
          fred Frédéric Massart added a comment - Test passed on 2.3 onwards. Thanks!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

          (and won't be revisiting it unless some regression is found)

          Thanks and ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/Jan/13