Moodle
  1. Moodle
  2. MDL-31374

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

    Details

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

      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).

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

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

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

          great - submitting for integration. - thanks.

          Show
          Dan Marsden added a comment - great - submitting for integration. - thanks.
          Hide
          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
          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
          Rossiani Wijaya added a comment -

          This is working great.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working great. Test passed.
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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

            People

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

              Dates

              • Created:
                Updated:
                Resolved: