itamart / moodle-block_mhaairs

0 stars 5 forks source link

Plugin uses role shortnames instead of permissions #14

Open frederickOtus opened 6 years ago

frederickOtus commented 6 years ago

In the get_sso_url function in block_mhaairs_util.php, there is this snipped of code to determine if a user is a teacher or a student:

        // Normalize the user's role name to insructor or student.
        if ($roles = get_user_roles($context, $USER->id)) {
            foreach ($roles as $role) {
                $rolename = empty($role->name) ? $role->shortname : $role->name;
                if ($rolename == 'teacher' || $rolename == 'editingteacher') {
                    $rolename = 'instructor';
                    break;
                }
            }
            if ($rolename != null && $rolename != 'instructor') {
                $rolename = 'student';
            }
        }

I'm managing a site where the editingteacher shortname got switched to 'faculty', so now teachers are showing up as students on McGraw Hill's side. Wouldn't it make more sense for this to check if they have the 'block/mhaairs:viewteacherdoc' permission instead (or make a new permission and add it to the teacher archetype)?

Kemmotar83 commented 5 years ago

I'm also interested in this issue.

I don't understand why control the name or the shortname of the roles, when they can be edited, duplicated, ecc...

It could be resolved as Frederick suggested or defining a setting used to select which one of the existing roles (admin_setting_pickroles) should be considered instructor.

Thanks, Giorgio

itamart commented 5 years ago

I agree that this could be more flexible. I'll forward it to the consideration of MHE.

itamart commented 5 years ago

The current behavior looks for roles by archetype not by their short name. So instructor roles are assumed to be roles whose archetype is editingteacher or teacher. Student roles are assumed to be roles whose archetype is student. This means that if new roles use those archetypes they should work.

The 3.6 release will allow admin to specify in the plugin settings the student roles and instructor roles by shortname. The archetype behavior remains as default.