Moodle

This code works but still requires some fine tuning and more testing. I thought I would gage the Moodle community's interest before spending more time on it.

would be great to see the ability to have conditional based on group or cohort. Many use groupings for this, but a conditional would make it a lot easier and straight forward

Sounds good, Mark.

The idea sounds very interesting. Also, the proposal of Julian (adding conditional support for cohort and groups) may lead to very interesting scenarios.My +1

Good idea Mark!

Julian, I am not sure about adding the ability to lock it by cohorts. Cohorts are a site wide group used to enrol users into courses and does not really apply to individual courses. I could limit the cohorts to those that apply to the individual course, but in that cause I may as well be using groups. However, if I did add the functionality to make it conditional via groups, it would make groupings redundant which I am not sure the Moodle HQ wants. Btw, I have only touched base on cohorts, groups and groupings so may be missing something.

Mark, while having a way to assign users to cohort based on user profile filed could be a powerful tool in several scenarios, reflecting on this in my opinion you are right, as cohort are created in higher contexts than activity context and are intended to enroll user into courses.

I can't see HQ having issues.

Groupings was always meant for groups of groups. It is a silly hack that teachers use to create a group and stick it in a grouping just to enable a resource for just a group. I can't see HQ arguing usability on this one. ANyone from HQ able to comment. I would hate for a perceived fear of change to stop what could be a really useful development.

Julian

This for me is absolutely brilliant and thank you Mark! Perfect for what I am needing at the moment.
I'm going to go try as soon as I can and give you full feedback.
Kay

Actually is there anyway i can try this mod. I am at the start of specifying and testing viability of moodle for a project that I propose to use in a rather creative way and just today (while in the bath - a proper eureka moment!) realised that what I need to do is what you are doing here and then I found this.

However I have come back to moodle after a lot of years away and i have no idea how this tracker area works etc.

Short version of this ramble - is there anyway I can get hold of your work as you progress as I'd love to thrash it about for you.

Kay

Agree with Julian that having to use groupings to perform a 'selective release' (in the old WebCT terminology) is one of the dumbest work arounds I have to explain to new users. It would make far more sense to be able to restrict availability based purely on a group, and if an extension of this patch lets that happen then it would get my vote.

As for the 'true meaning' of groupings, I've always explained them as primarily existing to allow lecturers to run multiple types of groups within a course (tutorials and prac groups for example) without tripping up on the filtering of activities to the different group types. Using it to restrict to individual groups is, like Julian said, a bit of a silly functional hack to achieve something which should be able to be done much more simply I reckon.

Locking by cohorts? I can't understand the argument that this shouldn't be done because cohorts are set in a different context, since user fields exist in a different context too - unless I'm missing something else in the argument

Thanks to Mark N for the idea and code regardless of the nuances - nice work

Having seen systems like ELIS and Totara, I think this possibly would be best split into 2 aspects:

a) having membership of a cohort depend on user fields
b) having conditional access based on cohort (or better an automatic group created in the course for each cohort)

I think that perhaps linking conditionals to a user related aspect than a course aspect is not the best approach for abstraction, but that is just how I think about it.

I think it may be better approach to keep conditionals based on course related (like completion, grade, time, or a group (in this case)) than a user aspect (user field)

+1 to Julian and Mark D's comments on why it is important to be able to set conditionals for groups within a course without having to resort to the groupings/dimensions of groups hack.

Thanks Mark!

I have been off sick the past week, so have had no time to look into this any further. I will hopefully add the ability to make events conditional via cohorts and groups by the end of next week. I will keep you updated here.

Thanks for the update Mark and building in the suggested changes.

The comments appears all quite sensible to me, if cohort support can be added into the conditional activities, then OK. However, a separate feature allowing also to assign users to cohort according to user fields (either standard and custom) would be really useful too and satisfy broader usage scenarios. Maybe for this it is needed another tracker issue if not already present ?

Thanks for your work on this feature. It looks good. I haven't had time to check it yet, maybe ask me for review in a few weeks. At least based on the functionality/screenshots I would be happy to submit it for integration.

Regarding coding standards, I would suggest that at least on the trivial level (whitespace, naming, etc.) new code should generally follow coding standards rather than try to match other existing code in a file.

If you do implement cohort/group support (which I suspect is a bad idea) please do it separately. It's best to keep different changes apart.

Hi Kay,

Missed your question. This isn't exactly a mod, as it requires editing core code.

If you visit the link https://github.com/markn86/moodle/compare/MOODLE_21_STABLE...userconditions you will be given a list of all the files that were edited to make this possible, as well as the list of changes on each file. Please make sure you know what you are doing before editing these files as inserting in the incorrect place could cause your site to fail.

Hi Sam,

Any idea when the code will be reviewed for core submission?

Regards,

Mark

+1 for Julian's suggestion as I have always found Groupings overly complicated. Having the condition... Member of group ...would be great for me. Could conditions be made into plugins? If they were plugins I would enable Group and Userfield but disable Cohort!

Seems as different issue exists (MDL-30554) for adding conditions based on groups...

+1 for simplification system of groups/grouping/cohorts.

Hi Guys, I created a separate tracker issue for groups. Please visit the link http://tracker.moodle.org/browse/MDL-30554 to show your interest.

I'm sorry it took me so long but I have finally done a code review.

Because the code review is long (I try to be thorough with these things, which may be why it takes me forever to get around to them) I am attaching it as a text file rather than inline.

Some notes:

1) The code is very high quality, well done!

2) I did not actually try to run it yet

3) While you fix the problems, could you also rebase on current master (this feature should go into 2.3).

4) I didn't check - are there existing unit tests for conditionlib, like for the grade conditions for instance? If there are, could you add at least a basic unit test for this new functionality. If there aren't, I guess you're off the hook.

5) Talking of testing, could you write a brief (eg ~10 point) step-by-step script for manual testing of the feature, these can go in the testing instructions field of this bug (uh if you can access it - I think 'request peer review' might give you that box) or just in a comment and I can move them.

There are quite a lot of issues to address but really it looks like it can be a solid feature. If the problems identified in code review are fixed I will certainly submit for integration into 2.3.

Thanks for all your work on this so far.

(ahem - just thought I should probably put the bug through the right stages for peer review...)

Hi Sam,

Thanks for that.

I will get round to this eventually, however, currently my development time is spent on work and re-writing the certificate module. I will probably have to address this when I have finished with the certificate changes (hopefully by the end of the month).

Great code review, maybe you could do it for the certificate module as well when it is ready for testing!

Regards,

Mark

any movements on getting this into the integration stream for 2.3?

Yeh, I am in the process of doing it now actually.

I just did a rebase on my branch with master - that was a headache in itself. I feel sorry for Eloy who has to manage the central repo all the time.

Going to address the issues that Sam mentioned. Not sure when deadline for core code is coming up but I do want to make it (lets hope that Sam can test it with such little notice)

Hi Sam,

I believe I have finished the changes you suggested, are you able to re-review before the 2.3 release in the hopes it can be merged? Sorry for the late notice.

Regarding the modinfolib.php file - I just copied the logic used by the conditionsgrade array, which states that declaring the availability and completion fields, even if these settings are turned off, saves database space. These are the only changes I made because that is all there were for the conditionsgrade array. Isn't the caching done in lib/conditionlib.php? There is a call in modinfolib.php to is_available, which uses the data (if it exists) in the usersfieldcache.

Regards,

Mark

Any chance of this making it in 2.3 Sam?

This looks really useful, has been peer reviewed, and has been tested in production for a long time.

My +1 to integrate if possible ...

Mark can you

1) Link the user docs (some page in docs.moodle.org/dev)
2) Write some QA tests we can add to MDLQA-1

Moved docs link to http://docs.moodle.org/dev/Conditional_user_fields

It's not a 2.2 feature and should not be in those docs!

Hi guys,

I've been discussing this issue with Mark, Eloy and just now Martin.
The code is 99% perfect now, just one remaining backup and restore issue.
Mark has done an awesome job giving this every opportunity available to get into 2.3 but unfortunately this will be delayed. Sorry all. Its been delayed in the interests of safety given this collided with other new features, introduced new database structure, and was looked at late in the picture for 2.3.
Rest assured as soon as master is accepting changes again after the release of 2.3 this will be interated and will be one of the first new features we can be certain about for 2.4.

A big thank you to Mark and everyone else involved here and I will see this integrated once possible (I believe we will be coming out of master freeze round the end of June).

Cheers
Sam