ncstate-delta / moodle-mod_zoom

Moodle plugin for Zoom meeting
https://moodle.org/plugins/mod_zoom
61 stars 107 forks source link

Bug: Respect switched role on activity overview page #191

Open abias opened 3 years ago

abias commented 3 years ago

Steps to reproduce:

Expected result:

Actual result:

As this can disturb teachers, I would propose to fix this flaw.

abias commented 3 years ago

Hi @rlorenzo and @jrchamp ,

I have been thinking about this issue and have had to see that there are several obstacles.

mod_zoom does not decide which widgets to show based on the Moodle course role or Moodle capabilities. It decides based on the fact if I am a Zoom host / alternative host or not. Because of that, the standard procedures to handle an active role switch do not work here.

Additionally, the Moodle role switch feature just switches the Moodle course role. It does not change the Moodle user. Thus, if I switch the role to the student role, I will still be the same user and as this use I will still be the meeting host.

I still think that the current behaviour is at least unexpected for teacher, maybe it could even be called wrong. But I think there are at least three distinct ways to improve this:

  1. The easy way: On view.php, we test if the user has switched his role. If yes, we pretend that he is another user and not the host user anymore and show only widgets for non-hosts.

  2. The even more simple way: On view.php, we test if the user has switched his role. If yes, we just show a notification banner at the top of the page and inform the user that the role switch does not have an effect on this page.

  3. The complicated but understandable way:

    • We introduce a new capability mod/zoom:roleswitchashost and add this capability to the teacher archetype role.
    • On view.php, we test if the user has switched his role.
    • If yes, we check if the switched role has the mod/zoom:roleswitchashost or not. If no, we show the page in the non-host view and show (optionally but ideally) a notification banner to inform the user that he is now viewing the page in non-host view. If yes, we check of the user is the current host or alternative host of the meeting. If no, we show the page in the non-host view and show (optionally but ideally) a notification banner to inform the user that he is now viewing the page in non-host view. If yes, we show the page in the host view and show (optionally but ideally) a notification banner to inform the user that he is now viewing the page in host view. [This case would be a unusual case as the user would extend his permissions by switching his role, but it should be supported anyway if the admin configures any role like this]

@rlorenzo and @jrchamp , what do you think, which approach would make most sense to you or do you have other proposals?

jrchamp commented 3 years ago

Option 1 seems like the simplest. In the two places where we calculate $userishost / $userisrealhost, we can check the if there is a roleswitch in progress (and thus, set those to false).

The other idea that I've been considering for a bit now is removing the special treatments for $userishost and instead using Moodle's permissions. This seems like "the Moodle way" of doing things, but I wonder what kinds of problems we might cause with that change.

jrchamp commented 3 years ago

Looks like function is_role_switched($courseid) is the correct function to use for detecting a role switch.