Cascade cohort sync fixes

amd/src/studyplan-editor-components.js

@@ -58,8 +58,8 @@ export default {
                 studyline_editmode: 'studyline_editmode',
                 editmode_modules_hidden:'editmode_modules_hidden',
                 studyline_add: 'studyline_add',
-                add$core: 'add',
+                add$core: 'add$core',
-                edit$core: 'edit',
+                edit$core: 'edit$core',
                 studyline_name: 'studyline_name',
                 studyline_name_ph: 'studyline_name_ph',
                 studyline_shortname: 'studyline_shortname',
@@ -83,8 +83,8 @@ export default {
                 advanced_tools: 'advanced_tools',
                 confirm_cancel: 'confirm_cancel',
                 confirm_ok: 'confirm_ok',
-                success$core: 'success',
+                success$core: 'success$core',
-                error$core: 'failed',
+                error$core: 'failed$core',
                 advanced_converted: 'advanced_converted',
                 advanced_skipped: 'advanced_skipped',
                 advanced_failed: 'advanced_failed',
@@ -110,6 +110,9 @@ export default {
                 advanced_import_from_file: 'advanced_import_from_file',
                 advanced_purge: "advanced_purge",
                 advanced_purge_expl: "advanced_purge_expl",
+                advanced_cascade_cohortsync_title: "advanced_cascade_cohortsync_title",
+                advanced_cascade_cohortsync_desc: "advanced_cascade_cohortsync_desc",
+                advanced_cascade_cohortsync: "advanced_cascade_cohortsync",
             },
             studyplan_edit: {
                 studyplan_edit: 'studyplan_edit',
@@ -711,6 +714,19 @@ export default {
                         
                     }).fail(notification.exception);  
                 },
+                cascade_cohortsync(){
+                    const self = this;
+                    call([{
+                        methodname: 'local_treestudyplan_cascade_cohortsync',
+                        args: {
+                            studyplan_id:  this.value.id,
+                        },
+                    }])[0].done(function(response){
+                        self.$bvModal.msgBoxConfirm(self.text.cascade_cohortsync + "\n"
+                            + (response.success?self.text.success$core:self.text.error$core)
+                             + "\n" + response.msg);
+                    }).fail(notification.exception);  
+                },
                 modal_close(){
                     this.force_scales.result = [];
                 }
@@ -739,6 +755,16 @@ export default {
                         <b-tab :title="text.advanced_course_manipulation_title" >
                             <b-container>
                             <b-row><b-col cols="*">
+                                <h3>{{ text.advanced_cascade_cohortsync_title}}</h3>
+                                    {{ text.advanced_cascade_cohortsync_desc}}
+                            </b-col></b-row>
+                            <b-row><b-col cols="*">
+                                <b-button 
+                                    variant="info"
+                                    @click="cascade_cohortsync"
+                                >{{ text.advanced_cascade_cohortsync}}</b-button>
+                            </b-col></b-row>
+                            <b-row class="mt-3"><b-col cols="*">
                                 <h3>{{ text.advanced_force_scale_title}}</h3>
                                 {{ text.advanced_force_scale_desc}}
                             </b-col></b-row>
@@ -757,7 +783,7 @@ export default {
                                 </b-col>
 
                             </b-row>
-                            <b-row><b-col cols="*">
+                            <b-row class="mt-3"><b-col cols="*">
                                 <ul class='t-advanced-scrollable' v-if="force_scales.result.length > 0">
                                     <li v-for="c in force_scales.result">
                                         <span class='t-advanced-coursename'>{{c.course.fullname}}</span>
@@ -2187,7 +2213,7 @@ export default {
             },
             methods: {
                 hasGrades() {
-                    if(this.value.course.grades && this.value.course.grades > 0){
+                    if(this.value.course.grades && this.value.course.grades.length > 0){
                         for(const g of this.value.course.grades){
                             if(g.selected){
                                 return true;
@@ -2364,14 +2390,6 @@ export default {
             },
             template: `
             <div>
-                <b-form-group v-if="useItemConditions" 
-                    :label="text.select_conditions"
-                    ><b-form-select size="sm" 
-                        @input="updateConditions" 
-                        v-model="value.conditions" 
-                        :options="condition_options"
-                    ></b-form-select>
-                </b-form-group>
                 <b-form-group
                     :label="text.select_grades"
                     ><ul class="t-item-module-children">

classes/associationservice.php

@@ -461,16 +461,15 @@ class associationservice extends \external_api
     }
 
 	// Actual functions
-	public static function casdade_cohortsync($studyplan_id) 
+	public static function cascade_cohortsync($studyplan_id) 
 	{
         $studyplan = studyplan::findById($studyplan_id);
         webservicehelper::require_capabilities(self::CAP_EDIT,$studyplan->context());
 
+        $enroller = new cascadecohortsync($studyplan);
+        $enroller->sync();
 
-        $enroller = new enrolcohortsync($studyplan);
+        return success::success()->model();
-        $result = $enroller->sync();
-
-        return $result?success::success():success::fail();
 
     }
 

classes/cascadecohortsync.php

@@ -3,42 +3,80 @@ namespace local_treestudyplan;
 require_once($CFG->libdir.'/externallib.php');
 
 use \local_treestudyplan\studyplan;
+use \local_treestudyplan\local\helpers\debugger;
 
-class enrolcohortsync {
+class cascadecohortsync {
     private const METHOD = "cohort";
     private $studyplan;
     private $studyplanid;
+    private $debug;
 
-    function __construct($studyplan){
+    function __construct(studyplan $studyplan){
         $this->studyplan = $studyplan;
         $this->studyplanid = $studyplan->id();
+        $this->debug = new debugger("/tmp/treestudyplan.log","ccsync/{$studyplan->shortname()}/{$this->studyplanid}");
     }
 
-    static private function array_remove_value(&$array,$value){
+    static private function array_remove_value($array,$value){
-        while (($key = array_search($value, $array)) !== false) {
+        $a = [];
-            unset($array[$key]);
+        foreach($array as $v){
+            if($v != $value){
+                $a[] = $v;
+            }
         }
+        return $a;
+    }
+
+    
+    static function uploadenrolmentmethods_get_group($courseid, $groupname) {
+        // Function shamelessly copied from tool/uploadenrolmentmethods/locallib.php
+        global $DB, $CFG;
+    
+        require_once($CFG->dirroot.'/group/lib.php');
+    
+        // Check to see if the group name already exists in this course.
+        if ($DB->record_exists('groups', array('name' => $groupname, 'courseid' => $courseid))) {
+            $group = $DB->get_record('groups', array('name' => $groupname, 'courseid' => $courseid));
+            return $group->id;
+        }
+        // The named group doesn't exist, so create a new one in the course.
+        $groupdata = new \stdClass();
+        $groupdata->courseid = $courseid;
+        $groupdata->name = $groupname;
+        $groupid = groups_create_group($groupdata);
+    
+        return $groupid;
     }
 
     function sync(){
         global $DB;
-        // TODO: Determine if it would be better to add a database table of our own to store references between studyplan and cohort sync enrolment
+
-        // instead of using customtext4 for this
+        /*  Explainer:
+            This script uses {enrol}.customtext4 to store a json array of all studyplans that need this cohort sync to exist.
+            Since the cohort-sync enrolment method uses only customint1 and customint2, this is a safe place to store the data.
+            (Should the cohortsync script at any future time be modified to use customtext fields, it is still extremely unlikely that 
+            customtext4 will be used.)
+            Because of the overhead involved in keeping an extra table up to date and clean it up if cohort syncs are removed outside of this script,
+            it was determined to be the simplest and cleanest solution.
+        */
 
         $enrol = enrol_get_plugin(self::METHOD);
+        $this->debug->dump($enrol,"Enrol");
         // Find the courses that need to be synced to the associated cohorts
         $courseids = $this->studyplan->get_linked_course_ids();
-
+        $this->debug->dump($courseids,"courseids");
         // And find the cohorts that are linked to this studyplan.
         $cohortids = $this->studyplan->get_linked_cohort_ids();
+        $this->debug->dump($cohortids,"cohortids");
 
         // Next, for each course that is linked:
         foreach($courseids as $courseid){
             $course = get_course($courseid);
+            $this->debug->write("Processing Course {$courseid} {$course->shortname}");
             // first create any nonexistent links
             foreach($cohortids as $cohortid){
                 $cohort = $DB->get_record('cohort',['id'=>$cohortid]);
-
+                $this->debug->write("Processing cohort {$cohortid} {$cohort->shortname}");
 
                 $instanceparams = [
                     'courseid' => $courseid,
@@ -53,56 +91,75 @@ class enrolcohortsync {
                     'roleid' => get_config("local_treestudyplan","csync_roleid"),
                 ];
 
-
+                $this->debug->dump($instanceparams,"instanceparams");
+                $this->debug->dump($instancenewparams,"instancenewparams");
                 // Create group: 
 
                 // 1:   check if a link exists
                 //      If not, make it (maybe use some of the custom text to list the studyplans involved)
                 if ($instance = $DB->get_record('enrol', $instanceparams)) {
+                    $this->debug->write("Instance exists");
                     // it already exists
                     // check if this studyplan is already referenced in customtext4 in json format
-                    if(empty($instance->customtext4)){
+
-                        // Cohort sync was already done, but without studyplan info
+                    //TODO: Check this code - Maybe add option to not remember manually added stuff 
-                        // make sure we add a "manual" link to the plan list
+                    $plans = json_decode($instance->customtext4);
-                        $plans = ["manual"];
+                    if($plans == false || !is_array(($plans))){
-                    }
+                        // If the data was not an array (null or garbled), count it as manually added
-                    else{
+                        // This will prevent it's deletion upon
-                        $plans = json_decode($instance->customtext4);
+                        if(get_config("local_treestudyplan","csync_remember_manual_csync")){
+                            $plans = ["manual"];
+                        } else {
+                            $plans = [];
+                        }
                     }
+
+                    $this->debug->dump($plans,"plans");
                     if(!in_array($this->studyplanid ,$plans)){
                         // if not, add it to the reference
+                        $this->debug->write("Adding this plan to the list");
                         $plans[] = $this->studyplanid;
-                        $enrol->update_instance($instance,["customtext4"=>json_encode($plans)]);
+                        $enrol->update_instance($instance,(object)["customtext4"=>json_encode($plans)]);
                     }
 
                 } else {
+                    $this->debug->write("New instance should be made");
                     // If method members should be added to a group, create it or get its ID.
 
                     if (get_config("local_treestudyplan","csync_creategroup")) {
                         // Make or get new new cohort group - but only on creating of instances
-                        $groupname = $cohort->name." ".strtolower(get_string('defaultgroupname', 'core'));
+                        $groupname = $cohort->name." ".strtolower(\get_string('defaultgroupname', 'core_group'));
+                        $this->debug->write("Adding group {$groupname} for this method");
                         // and make sure the 
-                        $instancenewparams['customint2'] = uploadenrolmentmethods_get_group($courseid, $groupname);
+                        $instancenewparams['customint2'] = self::uploadenrolmentmethods_get_group($courseid, $groupname);
                     }
                     
                     if ($instanceid = $enrol->add_instance($course, $instancenewparams)) {
                         // also record the (as of yet only) studyplans id requiring this association
                         // in the customtext4 field in json format
-                        $enrol->update_instance($instance,["customtext4"=>json_encode([$this->studyplanid])]);
+                        $this->debug->write("Instance ({$instanceid} created. Updateing instance with studyplan id");
+                        $instance = $DB->get_record('enrol', array('id' => $instanceid));
+                        $enrol->update_instance($instance,(object)["customtext4"=>json_encode([$this->studyplanid])]);
                         
+                        $this->debug->write("Synchronize the enrolment");
                         // Successfully added a valid new instance, so now instantiate it.
                         // First synchronise the enrolment.
                         $cohorttrace = new \null_progress_trace();
-                        enrol_cohort_sync($cohorttrace, $cohortid);
+                        $result = enrol_cohort_sync($cohorttrace, $cohortid);
+                        if($result > 0){
+                            $this->debug->write("Error during 'enrol_cohort_sync': code {$result}");
+                        }
                         $cohorttrace->finished();
 
                     } else {
                         // Instance not added for some reason, so report an error somewhere
                         // (or not)
+                        $this->debug->write("Error - instance not added for some reason");
                     }
                 } 
             }
 
+
             // 2:   Check if there are cohort links for this studyplan in this course that should be removed
             //      A: Check if there are cohort links that are no longer related to this studyplan
             //      B: Check if these links are valid through another studyplan...
@@ -112,7 +169,7 @@ class enrolcohortsync {
 
             if(get_config("local_treestudyplan","csync_autoremove")){
                 // Only try the autoremove if the option is enabled
-
+                $this->debug->write("Autoremove scan for course {$courseid} {$course->shortname}");
                 // find all cohort syncs for this course
                 $searchparams = [
                     'courseid' => $courseid,
@@ -125,21 +182,27 @@ class enrolcohortsync {
                     if(!empty($instance->customtext4)){ // only check the records that have studyplan information in the customtext4 field
                         // first check if the cohort is not one of the cohort id's we have associated
                         if(!in_array($instance->customint1,$cohortids)){
+                            $this->debug->write("Found cohort sync instance that is not currently liked to the studyplan: {$instance->id}");
                             // So it may or may not need to be removed
                             $plans = json_decode($instance->customtext4);
+                            $this->debug->dump($plans,"Plans attachted to instance {$instance->id}");
                             if($plans !== null && is_array($plans)){
                                 //if a valid array is not returned, better leave it be, we don't want to mess with it
                                 // otherwise, check if we should remove it
                                 if(in_array($this->studyplanid,$plans)){
+                                    $this->debug->write("Found this studyplan in the id list - removing from list");
                                     //if this plan was referenced before
                                     // first remove the link
-                                    self::array_remove_value($plans,$this->studyplanid);
+                                    $fplans = self::array_remove_value($plans,$this->studyplanid);
-                                    if(count($plans) == 0){
+                                    $this->debug->dump($fplans,"Plans proposed to attach to instance {$instance->id}");
+                                    if(count($fplans) == 0){
                                         // delete the sync if there are no studyplan references left
+                                        $this->debug->write("No references are left, removing instance");
                                         $enrol->delete_instance($instance);
                                     } else {
                                         // otherwise just update the references so this studyplan is no longer linked
-                                        $enrol->update_instance($instance,["customtext4"=>json_encode($plans)]);
+                                        $this->debug->write("Still references left in the list, updating list...");
+                                        $enrol->update_instance($instance,(object)["customtext4"=>json_encode($fplans)]);
                                     }
                                 }
                             }
@@ -148,5 +211,6 @@ class enrolcohortsync {
                 }
             }
         }
+        $this->debug->write("Cascading complete");
     }
 }

classes/local/aggregators/bistate_aggregator.php

@@ -58,7 +58,7 @@ class bistate_aggregator extends \local_treestudyplan\aggregator {
                 }
             }
         } else {
-            debug::msg("ARRAY is NOT CONFIG","");
+            
         }
     }
 

classes/local/aggregators/core_aggregator.php

@@ -35,7 +35,7 @@ class core_aggregator extends \local_treestudyplan\aggregator {
                 }
             }
         } else {
-            debug::msg("ARRAY is NOT CONFIG","");
+            
         }
     }
 

classes/local/helpers/debugger.php

@@ -22,10 +22,12 @@ class debugger {
 
     }
 
-    public function dump($object)
+    public function dump($object,string $tag="")
     {
-        global $CFG;
+        if(strlen($tag) > 0){
-        $this->writeblock(print_r($object,true));
+            $tag .= ":\n";
+        }
+        $this->writeblock($tag.print_r($object,true));
 
     }
 

classes/studyplan.php

@@ -771,7 +771,7 @@ class studyplan {
 
         $sql = "SELECT DISTINCT j.cohort_id FROM {local_treestudyplan_cohort} j
                 WHERE j.studyplan_id = :studyplan_id";
-        $fields = $DB->get_field_sql($sql, ['studyplan_id' => $this->id]);
+        $fields = $DB->get_fieldset_sql($sql, ['studyplan_id' => $this->id]);
         return $fields;
     }
 

classes/task/autocohortsync.php

@@ -2,7 +2,7 @@
 namespace local_treestudyplan\task;
 require_once($CFG->dirroot.'/course/externallib.php');
 use local_treestudyplan\studyplan;
-use local_treestudyplan\enrolcohortsync;
+use local_treestudyplan\cascadecohortsync;
 
 class autocohortsync extends \core\task\scheduled_task  {
 
@@ -23,7 +23,7 @@ class autocohortsync extends \core\task\scheduled_task  {
             $studyplans = studyplan::find_all();
 
             foreach($studyplans as $studyplan) {
-                $enroller = new enrolcohortsync($studyplan);
+                $enroller = new cascadecohortsync($studyplan);
                 $enroller->sync();
             }
         }

lang/en/local_treestudyplan.php

@@ -67,6 +67,8 @@ $string['setting_csync_creategroup_field'] = 'Create groups';
 $string['settingdesc_csync_creategroup_field'] = 'Create a group in the course for each cohort sync';
 $string['setting_csync_role_field'] = 'Role';
 $string['settingdesc_csync_role_field'] = 'The role to use for automatic cohort sync enrolment created in courses';
+$string['setting_csync_remember_manual_csync_field'] = 'Remember existing cohort-syncs';
+$string['settingdesc_csync_remember_manual_csync_field'] = 'Mark cohort syncs that were manually created earlier, so they won\'t be removed during autosync if cohorts are removed from the studyplan';
 
 $string['studyplan_add'] = 'Add study plan';
 $string['studyplan_edit'] = 'Edit study plan';
@@ -244,11 +246,15 @@ $string["advanced_import_export"] = 'Backup and restore';
 $string["advanced_import"] = 'Restore studylines from backup';
 $string["advanced_export"] = 'Backup studyplan';
 $string["advanced_export_csv"] = 'Export as CSV';
-
 $string["advanced_import_from_file"] = "Open from file";
+
 $string["advanced_purge"] = "Delete";
 $string["advanced_purge_expl"] = "Delete the entire studyplan. This is not recoverable";
 
+$string["advanced_cascade_cohortsync_title"] = "Cascade cohort sync";
+$string["advanced_cascade_cohortsync_desc"] = "Add cohort sync enrolment to each course in this studyplan for all cohorts linked to this studyplan";
+$string["advanced_cascade_cohortsync"] = "Cascade cohort sync";
+
 $string["advanced_course_manipulation_title"] = 'Course manipulation';
 $string["advanced_disable_autoenddate_title"] = 'Disable automatic end date';
 $string["advanced_disable_autoenddate_desc"] = 'Disable the default on automatic end date function weekly topics have set in all courses in the studyplan';

lang/nl/local_treestudyplan.php

@@ -69,7 +69,8 @@ $string['setting_csync_creategroup_field'] = 'Groepen aanmaken';
 $string['settingdesc_csync_creategroup_field'] = 'Maak voor elke site-groep synchronisatie een groep aan';
 $string['setting_csync_role_field'] = 'Synchronisatierol';
 $string['settingdesc_csync_role_field'] = 'Welke rol te gebruiken voor automatische site-groep sync koppelingen';
-
+$string['setting_csync_remember_manual_csync_field'] = 'Bewaar bestaande site-group syncs';
+$string['settingdesc_csync_remember_manual_csync_field'] = 'Markeer site-group synchronisaties die eerder handmatig zijn aangemaakt, zodat deze niet worden verwijderd als ze uit het studieplan worden gehaald.';
 
 $string['studyplan_add'] = 'Nieuw studieplan';
 $string['studyplan_edit'] = 'Studieplan bewerken';
@@ -249,9 +250,13 @@ $string["advanced_import"] = 'Studieplan invullan vanuit backup';
 $string["advanced_export"] = 'Backup downloaden';
 $string["advanced_export_csv"] = 'Export als CSV';
 $string["advanced_import_from_file"] = "Vanuit bestand";
+
 $string["advanced_purge"] = "Verwijderen";
 $string["advanced_purge_expl"] = "Dit hele studieplan permanent verwijderen.";
 
+$string["advanced_cascade_cohortsync_title"] = "Site-groep synchronisatie doorzetten";
+$string["advanced_cascade_cohortsync_desc"] = "Voeg een site-group synchronisatie aanmelding toe aan alle cursussen in dit studieplan, voor alle gekoppelde site-groepen";
+$string["advanced_cascade_cohortsync"] = "Site-groep sync doorzetten";
 
 $string["advanced_course_manipulation_title"] = 'Cursussen aanpassen';
 $string["advanced_disable_autoenddate_title"] = 'Automatische einddatum uitschakelen';

settings.php

@@ -178,6 +178,13 @@ if ($hassiteconfig) {
 		true
 	));
 
+	// Enable/disable remembering previously added cohort syncs so they're not automatically deleted
+	$page_csync->add(new admin_setting_configcheckbox('local_treestudyplan/csync_remember_manual_csync',
+		get_string('setting_csync_remember_manual_csync_field', 'local_treestudyplan'),
+		get_string('settingdesc_csync_remember_manual_csync_field', 'local_treestudyplan'),
+		true
+	));
+
 	// Enable/disable group creation
 	$page_csync->add(new admin_setting_configcheckbox('local_treestudyplan/csync_creategroup',
 		get_string('setting_csync_creategroup_field', 'local_treestudyplan'),

version.php

@@ -1,6 +1,6 @@
 <?php
 $plugin->component = 'local_treestudyplan';  // Recommended since 2.0.2 (MDL-26035). Required since 3.0 (MDL-48494)
-$plugin->version = 2023061601;  // YYYYMMDDHH (year, month, day, iteration)
+$plugin->version = 2023062602;  // YYYYMMDDHH (year, month, day, iteration)
 $plugin->requires = 2021051700; // YYYYMMDDHH (This is the release version for Moodle 3.11)
 
 $plugin->dependencies = [