Groups assignation #37
Conversation
|
Hello Camille @Pedagotheque , Thanks for sharing this, we'll look into it asap. |
|
|
||
| $DB->delete_records('enrol_attributes_groups', array('enrolid' => $instance->id)); | ||
|
|
||
| foreach ($groups as $value) { |
There was a problem hiding this comment.
For clarity, $value here should better be called something like $groupid.
There was a problem hiding this comment.
Hello again Camille @Pedagotheque ,
I haven't tried your feature yet, but:
I had a first look at your modifications. These look straightforward enough. I have just made a couple suggestions about cosmetics.
Also, my choice (instead of having to rely on a new enrol_groups table) would have been to serialize group data into $enrol->customtext2 (since customtext1 is already used to store the rule. Then you could get rid of the upgrade DB process and install.xml files. It's a little bit less elegant, but would yield to less code in the end, which I tend to always prefer.
Anyway, none of these are integration stoppers for me, and I am very thankful to you for sharing this.
Let me know what you think, and in the meantime I will try to find some time to test the feature soon.
Cheers,
Nicolas
|
|
||
| // Start modification | ||
| $groups = $data->groupselect; | ||
| foreach ($groups as $value) { |
There was a problem hiding this comment.
Same here, $value should be named $groupid.
| $object->enrolid = $id; | ||
| $DB->insert_record('enrol_attributes_groups', $object); | ||
| } | ||
| // End modification |
There was a problem hiding this comment.
"Start|End modification" comments can be removed.
| $groups = groups_get_all_groups($courseid); | ||
|
|
||
| $groups2 = array(); | ||
| foreach ($groups as $value) { |
There was a problem hiding this comment.
Here, $value should be called $group.
…rrections suggested by @ndunand
|
This is now being continued in #38 |
Integration of pr #37, unit tests and some bug fixes
Hello,
We made modifications to add the following feature:
This feature may interest you or other users?
If you want so you can contact me for further information.
Regards
Camille