Feature/grouping restriction over multiple instances

[NinaHerrmann]

This pull request implements the solution to #184.
Short Summary:

• Choices over multiple mod_choicegroup instances can be restricted by membership in the same grouping.
  Example:
• We have Timeslots and multiple Events in the same timeslot. Example
• Grouping 14:00-15:00 has Groups 1) Ninas 14:00-15:00 2) Alex 14:00-15:00
• Grouping 15:00-16:00 has 1) Ninas 15:00-16:00 2) Alex 15:00-16:00
• All events from Nina are in one choicegroup activity, and all events from Alex are in a choicegroup activity.
• When a student picks one timeslot from Alex, he/she is not able to be a member of a group at the same timeslot

To be discussed

• corner cases
• design decisions
• View of teacher

[NinaHerrmann]

New Settings:

Depending on the way the choices should be displayed, the student sees the choices in different ways.
Hide from group list:

Show group as dimmed:

Show group with limitation notification

Show group with conflicting group information

[NinaHerrmann]

Error Message in case a conflict regarding groupings occurs:
(e.g. two tabs are open - the user submits Nina 14:00-15:00 in one tab and then Alex 14:00-15:00 in the next tab)

[NinaHerrmann]

If we have a conflict (assigned to multiple groups in the restricted grouping), throw an error notification.

[NinaHerrmann]

@ndunand Thank you for your Feedback! This pull request is now ready to review! Please do not hesitate to contact me if you have any questions!

[NinaHerrmann]

@ndunand @abias with the behat fixes this pull request is ready to review! 🥳
Please contact me for any questions, feedback etc!

[ndunand]

Thanks @NinaHerrmann !

@NicoAlexH will be able to review this next week hopefully. I'll have a look myself whenever I can as well.

[NinaHerrmann]

Thank you so much - squashed everything, and CI should (hopefully) run successfully now!

[NicoAlexH]

Hello Nina,

I am a colleague of @ndunand, I am currently reviewing your PR and I'd like to ask you a few questions.

Regarding the implementation :

1 – I have created the following:
Grouping A which contains groups A1, B1, C1
Grouping B which contains groups A2, B2, C2
Grouping C which contains groups A3, B3, C3
Then, I set a restriction based on groupings (any of them). In this specific case, I chose “Hide group from group list”.

As a student, if I select group A1, then save my choice, the groups B1 and C1 are removed from the list as expected.
If I do the same with group A2, the groups from grouping B are also removed, however I cannot select a group from the third grouping anymore.
Am I doing something wrong or is this the expected behavior?

2 – As a student, if the grouping limitation is on, I can still select groups from the same grouping if I select them all at once.
It’s only after saving my choices that I will get the warning message telling me that I cannot participate to this activity because of conflicts.
I was just wondering, wouldn’t it be better to prevent the student from making the wrong choices before submitting the form ?

Regarding the code :

1 - It’s insignificant, but I think it would be relevant to replace lines 204 to 208 and 334 to 338 of lib.php by this simpler expression :
$choicegroup->restrictbygrouping = $choicegroup->restrictbygrouping ?? false;

2 - Also, maybe we can create a method from lines 230 to 245 and 319 to 333 of lib.php, as they are duplicates ; I think it would make future maintenance easier.

I must say I don't have much experience in reviewing PRs, so I hope I'm not asking too much !
In any case, thank you very much for your work, it is greatly appreciated :)

Best regards,
Nicolas

[abias]

Hi @NicoAlexH ,

many thanks for your feedback to @NinaHerrmann .

We are currently waiting for final feedback from the customer who asked us to build this enhancement. As soon as we have it, @NinaHerrmann and I will come back to you to answer your questions and to hopefully see this integrated afterwards.

Cheers,
Alex

[abias]

Hi @ndunand and @NicoAlexH ,

I just came across this PR while reviewing the list of our pending issues here.
Unfortunately, the customer who asked us to build this enhancement lost interest in it last year and we didn't get any final feedback from him anymore.

If I remember correctly, the code in this PR which was built by Nina was working well for us during our last tests in early 2023, however you, @NicoAlexH , raised some valid questions.

I already told you, @ndunand , orally some months ago that you can finally push this PR over the line anytime and anyhow you like if you can solve your remaining questions yourself. Do you think this would be feasible or would you need some more help by @NinaHerrmann in any case?

Thanks,
Alex