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
    • Rank:
      42528

      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.

        Activity

        Hide
        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
        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
        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
        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
        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
        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 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 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
        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
        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
        Dan Poltawski added a comment -

        Integrated to master, 24 and 23. Thanks Raj

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

        Test passed on 2.3 onwards. Thanks!

        Show
        Frédéric Massart added a comment - Test passed on 2.3 onwards. Thanks!
        Hide
        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
        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: