ndunand / moodle-mod_choicegroup

Moodle "Group Choice" plugin
https://moodle.org/plugins/mod_choicegroup
35 stars 63 forks source link

mod_choicegroup: Add option to filter out expired/suspended users #151

Closed tonyjbutler closed 3 years ago

tonyjbutler commented 3 years ago

This commit adds an option to filter users with expired or suspended enrolments out of the response data (in the same way as the corresponding option in mod_choice). This has been suggested by teaching staff in my institution as a quick way to tell which of the students in the 'Not answered yet' column they actually need to chase up.

ndunand commented 3 years ago

Thanks @tonyjbutler for contributing again! I'll be looking into this shortly. Everything looks straightforward so I shouldn't be too long, but feel free to poke should I let things slide.

ndunand commented 3 years ago

Alright, just tested this and it works as expected for users not having made a group selection.

Just one unexpected side effect that I noticed by mistakingly suspending a user that had actually made a choice:

Screen Shot 2021-05-07 at 11 45 37

The user is suspended, but still a member of the chose group. For this reason, his choice is still visible as a member of group A (see above), but the top links reports "0 responses by 0 participants". Moreover, clicking this link yields the following:

Screen Shot 2021-05-07 at 11 51 54

, giving the impression that group A is empty, which is not the case.

Note: using the current release of the plugin, these inconsistencies do not appear – however the inconsistency of showing suspended users's choices (or lack of), which you're solving here, is obviously present.

I'm not sure how we should address this, let me know what you think.

tonyjbutler commented 3 years ago

Thanks Nicolas, I'll have a look.

tonyjbutler commented 3 years ago

Just to clarify @ndunand, of the 2 users shown in the screenshot, is Portal_t the only one who is suspended (so Etufbm2 is still an active user)? Or is that the same user (first and last names)?

ndunand commented 3 years ago

Sorry, "Etufbm2 Portal_t" is out test user, displayed as "lastname, firstname" 😊 , which is suspended.

tonyjbutler commented 3 years ago

I've added another commit which should fix this.

tonyjbutler commented 3 years ago

@ndunand I've added another commit to improve the efficiency of the previous fix, and rebased all 3 commits onto the latest master. This all seems to work fine now.

tonyjbutler commented 3 years ago

The latest commit also enables the full names of students to be displayed according to the site admin setting/string 'fullnamedisplay' while still ordering by last name first.

ndunand commented 3 years ago

Hi @tonyjbutler ,

We made other improvements in the meantime, addressing other issues. I am now ready to resume reviewing on this.

Could you please rebase your branch on master?

tonyjbutler commented 3 years ago

I've now rebased my master branch @ndunand.

ndunand commented 3 years ago

Thanks @tonyjbutler for sharing your work! This is now merged and will be released asap.