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

backport meta course sync plugin fixes and improvements from master to 2.2

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.2.2
    • Component/s: Enrolments
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      1/ enable enrol meta sync
      2/ set up a few links and test every kind of modifications via enrol UI
      3/ edit code and disable propagation of events (add return true; in sync_with_parent_course()) and now try modification via UI followed by CLI sync, again try all combinations of settings and operations
      4/ try cron executes the sync too (once should be enough if somebody reviews the change in version.php)

      Show
      1/ enable enrol meta sync 2/ set up a few links and test every kind of modifications via enrol UI 3/ edit code and disable propagation of events (add return true; in sync_with_parent_course()) and now try modification via UI followed by CLI sync, again try all combinations of settings and operations 4/ try cron executes the sync too (once should be enough if somebody reviews the change in version.php)
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE

      Description

      I think we should backport MDL-29684 to stable branches.

      I blogged about this here as well:
      http://danmarsden.com/blog/2012/01/23/meta-course-enrolments-and-failed-moodle-2-upgrades-oh-my/

      People who have roles listed in 'nosyncroleids' are still synchronised in child courses (without any role, but still synchronised).

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dougiamas Martin Dougiamas added a comment -

            +1 to backport this. Lots of people used to use metacourses this way apparently.

            Show
            dougiamas Martin Dougiamas added a comment - +1 to backport this. Lots of people used to use metacourses this way apparently.
            Hide
            danmarsden Dan Marsden added a comment -

            great - submitting for integration. - thanks.

            Show
            danmarsden Dan Marsden added a comment - great - submitting for integration. - thanks.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Backported, thanks!

            To testers, please test everything twice! It's a big change and it's stable (22_STABLE).

            Also, I've added the docs_required label because sure this needs new details in 2.2 Docs (both enrol and release notes).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Backported, thanks! To testers, please test everything twice! It's a big change and it's stable (22_STABLE). Also, I've added the docs_required label because sure this needs new details in 2.2 Docs (both enrol and release notes). Ciao
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This is working great.

            Test passed.

            Show
            rwijaya Rossiani Wijaya added a comment - This is working great. Test passed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers.

            Many thanks & ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - A bit later this week, but finally your changes have been accepted and are now available in all the upstream git/cvs servers. Many thanks & ciao
            Hide
            craigdr Craig Drayton added a comment -

            Hi all,

            Could this patch be easily backported to 2.1 also? This is a major issue for us and we're not ready to move to 2.2.X yet.

            Thanks,
            Craig

            Show
            craigdr Craig Drayton added a comment - Hi all, Could this patch be easily backported to 2.1 also? This is a major issue for us and we're not ready to move to 2.2.X yet. Thanks, Craig
            Hide
            brianking Brian King added a comment - - edited

            This doesn't seem to fix the issue mentioned in http://danmarsden.com/blog/2012/01/23/meta-course-enrolments-and-failed-moodle-2-upgrades-oh-my/ .

            There's this comment in lib/db/upgrade.php, and the code following it removes all manually assigned roles for metacourses. As far as I can see though, there's no effort to preserve the assignments that are not synced according to the config variable nosyncroleids.

            // nuke any old role assignments+enrolments in previous meta courses, we have to start from scratch

            We recently upgraded from the latest 1.9.x to version 2.2.2+ (Build: 20120329).

            A workaround is to run this sql before the upgrade:

            create table upgd2012_role_assignments as 
              select * from mdl_role_assignments 
              where contextid in 
                (SELECT ctx.id 
                 FROM mdl_context ctx JOIN mdl_course c ON (c.id = ctx.instanceid AND ctx.contextlevel = 50 AND c.metacourse = 1))
              and enrol='manual';

            And then this sql after the upgrade:

            INSERT into mdl_role_assignments (roleid, contextid, userid, timemodified, modifierid)
               SELECT roleid, contextid, userid, timemodified, 0 from upgd2012_role_assignments
               WHERE roleid in 
                 (select value from mdl_config_plugins where plugin = 'enrol_meta' and name = 'nosyncroleids');

            Show
            brianking Brian King added a comment - - edited This doesn't seem to fix the issue mentioned in http://danmarsden.com/blog/2012/01/23/meta-course-enrolments-and-failed-moodle-2-upgrades-oh-my/ . There's this comment in lib/db/upgrade.php, and the code following it removes all manually assigned roles for metacourses. As far as I can see though, there's no effort to preserve the assignments that are not synced according to the config variable nosyncroleids. // nuke any old role assignments+enrolments in previous meta courses, we have to start from scratch We recently upgraded from the latest 1.9.x to version 2.2.2+ (Build: 20120329). A workaround is to run this sql before the upgrade: create table upgd2012_role_assignments as select * from mdl_role_assignments where contextid in (SELECT ctx.id FROM mdl_context ctx JOIN mdl_course c ON (c.id = ctx.instanceid AND ctx.contextlevel = 50 AND c.metacourse = 1)) and enrol='manual'; And then this sql after the upgrade: INSERT into mdl_role_assignments (roleid, contextid, userid, timemodified, modifierid) SELECT roleid, contextid, userid, timemodified, 0 from upgd2012_role_assignments WHERE roleid in (select value from mdl_config_plugins where plugin = 'enrol_meta' and name = 'nosyncroleids');
            Hide
            danmarsden Dan Marsden added a comment -

            Hi Brian,

            you'll need to open a new bug for that if you'd like it to be fixed - please provide all the information on how to reproduce the issue you still have - thanks!

            Show
            danmarsden Dan Marsden added a comment - Hi Brian, you'll need to open a new bug for that if you'd like it to be fixed - please provide all the information on how to reproduce the issue you still have - thanks!
            Hide
            marycooch Mary Cooch added a comment - - edited

            I am just looking at the docs_required label for this issue so I can document where necessary. As I understand it, previously if a teacher (for example) was selected as not synchronised, they were still given access to the course but with no role in there (useless and annoying) This has been fixed - so do I assume that the case now is that they are not enrolled in the course at all? (ie, what would be the expected behaviour?) Or have I misunderstood? Thanks for the clarification anyone

            Show
            marycooch Mary Cooch added a comment - - edited I am just looking at the docs_required label for this issue so I can document where necessary. As I understand it, previously if a teacher (for example) was selected as not synchronised, they were still given access to the course but with no role in there (useless and annoying) This has been fixed - so do I assume that the case now is that they are not enrolled in the course at all? (ie, what would be the expected behaviour?) Or have I misunderstood? Thanks for the clarification anyone
            Hide
            marycooch Mary Cooch added a comment -

            (Housekeeping) Removing docs_required label as I presume I understood correctly that it is now expected behaviour.

            Show
            marycooch Mary Cooch added a comment - (Housekeeping) Removing docs_required label as I presume I understood correctly that it is now expected behaviour.

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Mar/12