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

Self enrolment by group key does not work - until group settings are saved again

    Details

    • Testing Instructions:
      Hide

      Test upgrade from 1.9 => 2.x

      1. Install Moodle 1.9 and create a course and users (don't enrol users)
      2. Add group to course and set enrolment key (don't add any user)
      3. Upgrade to 2.2 and then to master
      4. Check self enrolment method is set for course and "Use group enrolment keys" is set to NO.
      5. Repeat upgrade test for 26, 25 and 24 and make sure it works fine.

      Test upgrade from 2.x to 2.x

      1. Install Moodle 2.2 and and create a course and users (don't enrol users)
      2. Add group to course and set enrolment key (don't add any user)
      3. Add self enrolment method and set "Use group enrolment keys" to yes
      4. Create another course and set "Use group enrolment keys" to No
      5. Upgrade to mater
      6. Check self enrolment method for course and check "Use group enrolment keys" is not modified
      Show
      Test upgrade from 1.9 => 2.x Install Moodle 1.9 and create a course and users (don't enrol users) Add group to course and set enrolment key (don't add any user) Upgrade to 2.2 and then to master Check self enrolment method is set for course and "Use group enrolment keys" is set to NO. Repeat upgrade test for 26, 25 and 24 and make sure it works fine. Test upgrade from 2.x to 2.x Install Moodle 2.2 and and create a course and users (don't enrol users) Add group to course and set enrolment key (don't add any user) Add self enrolment method and set "Use group enrolment keys" to yes Create another course and set "Use group enrolment keys" to No Upgrade to mater Check self enrolment method for course and check "Use group enrolment keys" is not modified
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      wip-mdl-42904
    • Sprint:
      BACKEND Sprint 7
    • Story Points (Obsolete):
      8
    • Sprint:
      BACKEND Sprint 7

      Description

      We have a customer running on Moodle 2.5.2. They are setting up courses using group enrolment keys.

      I have checked their configuration and it seems correct.

      If a user attempts to self-enrol on one of these courses, using the correct group key, the receive the error "Incorrect enrolment key, please try again".

      But the key is correct. I have tested this myself.

      The weird thing is that if you go into the groups page, select the relevant group and choose "edit group settings", then press save (without changing anything at all) then the user can self-enrol.

      It is as if the group key details have not been properly saved the first time they were set up...although the key has certainly been saved, as you can click the "unmask" checkbox, and the correct key is displayed. The customer can't go through the whole site, hundreds of courses, editing and clicking save on every group in every course.

      I cannot find anything wrong with the site configuration, the self-enrolment plugin configuration, or the group configuration. It seems to me that this is a bug, although I'd be happy to be proved wrong as long as an explanation of this behaviour is forthcoming.

      I'd appreciate someone taking a look at this as a matter of urgency - I can arrange access to the site and server if required.

        Gliffy Diagrams

          Activity

          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks for reporting this Sean,

          I am looking at this now. Not sure but it can be related to MDL-42536

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks for reporting this Sean, I am looking at this now. Not sure but it can be related to MDL-42536
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          MDL-42536 was just a backport for behat test.

          It seems to be working fine for me, is there any way you can provide access to site &/ server.

          Thanks.

          Show
          rajeshtaneja Rajesh Taneja added a comment - MDL-42536 was just a backport for behat test. It seems to be working fine for me, is there any way you can provide access to site &/ server. Thanks.
          Hide
          keoghs Sean Keogh added a comment -

          Yes, if you can mail me on sean.keogh@remote-learner.co.uk, I'll give you the details you need.

          Show
          keoghs Sean Keogh added a comment - Yes, if you can mail me on sean.keogh@remote-learner.co.uk, I'll give you the details you need.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Hello Sean,

          I have sent a mail from my personal email, you can either share details on that or rajesh@moodle.com

          FYI: I will look at this on Monday morning.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Hello Sean, I have sent a mail from my personal email, you can either share details on that or rajesh@moodle.com FYI: I will look at this on Monday morning.
          Hide
          tsala Helen Foster added a comment -

          Is a course enrolment key set in addition to the group enrolment keys?

          Show
          tsala Helen Foster added a comment - Is a course enrolment key set in addition to the group enrolment keys?
          Hide
          keoghs Sean Keogh added a comment -

          Yes, it looks like it

          Show
          keoghs Sean Keogh added a comment - Yes, it looks like it
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks Helen and Sean,

          I have mailed Dan Leighton, (as suggested by Sean) for more details.

          FYI:
          On friday I tried enrolling student in course (suggested by Sean) and it worked fine.
          Dan might have fixed that specific course, but will wait for his response.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Helen and Sean, I have mailed Dan Leighton, (as suggested by Sean) for more details. FYI: On friday I tried enrolling student in course (suggested by Sean) and it worked fine. Dan might have fixed that specific course, but will wait for his response.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks Sean and Dan,

          I looked at the site and "customint1" is not set in this case. Not sure how this is possible as enrolment instance defaults are set with 0/1.

          Can you please confirm, how you manage to create self-enrolments, as it should be saved as 0/1 during form submission.

          FYI: enrolment was working after saving the enrolment form, because it sets value of customint1 as 0 (yes). We can fix this wrong visual behaviour on self enrolment edit form, but as mentioned earlier it should not be happening at first place. This value is always set.

          Awaiting more information.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Sean and Dan, I looked at the site and "customint1" is not set in this case. Not sure how this is possible as enrolment instance defaults are set with 0/1. Can you please confirm, how you manage to create self-enrolments, as it should be saved as 0/1 during form submission. FYI: enrolment was working after saving the enrolment form, because it sets value of customint1 as 0 (yes). We can fix this wrong visual behaviour on self enrolment edit form, but as mentioned earlier it should not be happening at first place. This value is always set. Awaiting more information.
          Hide
          aaronfulton Aaron Fulton added a comment - - edited

          I had a similar issue to solve today. All of the customint fields in the enrol table were NULL for the self enrollment method. I've recently upgraded this site from 1.9 through to the latest 2.5+ so I suspect in the upgrade path these fields were not populated with default values. I also had to set "use group enrollment keys" to "Yes" against all of the self enrolment instances.

          Show
          aaronfulton Aaron Fulton added a comment - - edited I had a similar issue to solve today. All of the customint fields in the enrol table were NULL for the self enrollment method. I've recently upgraded this site from 1.9 through to the latest 2.5+ so I suspect in the upgrade path these fields were not populated with default values. I also had to set "use group enrollment keys" to "Yes" against all of the self enrolment instances.
          Hide
          danleighton Dan Leighton added a comment -

          Hi Rajesh

          No idea how it happened!

          We were on 1.9 till a week or so ago.

          Upgraded to 2.5 (via 2.4 I believe, Sean can confirm)

          After this, we used the 'update assignments' tool.

          Not sure whether this helps. Sean may have more info

          Show
          danleighton Dan Leighton added a comment - Hi Rajesh No idea how it happened! We were on 1.9 till a week or so ago. Upgraded to 2.5 (via 2.4 I believe, Sean can confirm) After this, we used the 'update assignments' tool. Not sure whether this helps. Sean may have more info
          Hide
          keoghs Sean Keogh added a comment -

          Hmmm... I wonder if it was an upgrade issue then. If it is a matter of just flipping a field, perhaps we could do it in the database with a suitable query?

          Show
          keoghs Sean Keogh added a comment - Hmmm... I wonder if it was an upgrade issue then. If it is a matter of just flipping a field, perhaps we could do it in the database with a suitable query?
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks Dan, Sean and Aaron,

          I am going to try clean upgrade from 1.9 -> 2.5 and see what will happen.
          Seems like null is stored for customint1, while upgrade from 1.9 -> 2.5. If needed I can add upgrade code to set null => 0, which will reflect the correct status of group enrolment.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Dan, Sean and Aaron, I am going to try clean upgrade from 1.9 -> 2.5 and see what will happen. Seems like null is stored for customint1, while upgrade from 1.9 -> 2.5. If needed I can add upgrade code to set null => 0, which will reflect the correct status of group enrolment.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          I was able to reproduce the problem, and it is happening because in 1.9 enrolment used to get set in course table and 2.0 it is set in enrol table.
          The upgrade code just migrated course fields to enrol fields and didn't consider group enrolmentkey.

          Looking at the history of issue and discussing with Petr, it seems best solution is to set customint1 to 0 and disable group enrolment key if value is null in table.
          As setting value to 1 for existing group enrolment key might cause security problem.

          Show
          rajeshtaneja Rajesh Taneja added a comment - I was able to reproduce the problem, and it is happening because in 1.9 enrolment used to get set in course table and 2.0 it is set in enrol table. The upgrade code just migrated course fields to enrol fields and didn't consider group enrolmentkey. Looking at the history of issue and discussing with Petr, it seems best solution is to set customint1 to 0 and disable group enrolment key if value is null in table. As setting value to 1 for existing group enrolment key might cause security problem.
          Hide
          keoghs Sean Keogh added a comment -

          Thanks Rajesh,

          Can you suggest any suitable query that we could run to fix the existing problem with Dan's site?

          Show
          keoghs Sean Keogh added a comment - Thanks Rajesh, Can you suggest any suitable query that we could run to fix the existing problem with Dan's site?
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Hello Sean,

          You can use

          UPDATE mdl_enrol e LEFT JOIN mdl_groups g ON e.courseid = g.courseid SET e.customint1 = 1 WHERE e.customint1 IS NULL AND e.enrol = 'self' AND g.enrolmentkey IS NOT NULL;
          UPDATE mdl_enrol e SET e.customint1 = 0 WHERE e.customint1 IS NULL AND e.enrol = 'self';

          First one will set group enrolmentkey to yes for all courses having group with enrolmentkey and second one will set rest of the values to no for group enrolmentkey.

          Hope this helps.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Hello Sean, You can use UPDATE mdl_enrol e LEFT JOIN mdl_groups g ON e.courseid = g.courseid SET e.customint1 = 1 WHERE e.customint1 IS NULL AND e.enrol = 'self' AND g.enrolmentkey IS NOT NULL; UPDATE mdl_enrol e SET e.customint1 = 0 WHERE e.customint1 IS NULL AND e.enrol = 'self'; First one will set group enrolmentkey to yes for all courses having group with enrolmentkey and second one will set rest of the values to no for group enrolmentkey. Hope this helps.
          Hide
          keoghs Sean Keogh added a comment -

          Hi Rajesh, many thanks for that.

          Show
          keoghs Sean Keogh added a comment - Hi Rajesh, many thanks for that.
          Hide
          skodak Petr Skoda added a comment -

          The upgrade code itself looks ok.

          The enrol/upgrade.txt is not intended for version history of individual enrol plugins, I do not think you should add anything there. This change should be imo documented in the relevant release notes instead.

          Show
          skodak Petr Skoda added a comment - The upgrade code itself looks ok. The enrol/upgrade.txt is not intended for version history of individual enrol plugins, I do not think you should add anything there. This change should be imo documented in the relevant release notes instead.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks Petr,

          I have removed upgrade.txt changes.

          Pushing it for integration.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Petr, I have removed upgrade.txt changes. Pushing it for integration.
          Hide
          damyon Damyon Wiese added a comment -

          Thanks Raj,

          Integrated to 24, 25, 26 and master.

          Note I had to fix the version numbers on the backports:
          http://docs.moodle.org/dev/Moodle_versions#How_to_increment_version_numbers_in_core

          Off to testing.

          Show
          damyon Damyon Wiese added a comment - Thanks Raj, Integrated to 24, 25, 26 and master. Note I had to fix the version numbers on the backports: http://docs.moodle.org/dev/Moodle_versions#How_to_increment_version_numbers_in_core Off to testing.
          Hide
          rajeshtaneja Rajesh Taneja added a comment -

          Thanks Damyon.

          Show
          rajeshtaneja Rajesh Taneja added a comment - Thanks Damyon.
          Hide
          poltawski Dan Poltawski added a comment -
          Show
          poltawski Dan Poltawski added a comment - Ping Jérôme Mouneyrac .
          Hide
          skodak Petr Skoda added a comment -

          no regressions found during the upgrade, thanks, passing

          Show
          skodak Petr Skoda added a comment - no regressions found during the upgrade, thanks, passing
          Hide
          poltawski Dan Poltawski added a comment -

          Thanks for your contributions, this change is now upstream!

          “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

          Show
          poltawski Dan Poltawski added a comment - Thanks for your contributions, this change is now upstream! “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

            People

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

              Dates

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

                Agile