GibbonEdu / core

Gibbon is a flexible, open source school management platform designed to make life better for teachers, students, parents and leaders.
https://gibbonedu.org
GNU General Public License v3.0
479 stars 304 forks source link

Remove role restrictions for hooks #1709

Closed fvlasie closed 1 year ago

fvlasie commented 1 year ago

Having the hook restricted by role causes issues when you have custom roles. I had to comment out these lines to get a module working.

yookoala commented 1 year ago

Wouldn't it better if the permission is added to the custom roles?

fvlasie commented 1 year ago

My fear is that module developers may not know about this. Also anyone creating custom roles in Gibbon also needs to know that some modules might not work. Maybe we could base permissions on a core permission such as being able to view lessons rather than role?

fvlasie commented 1 year ago

Hmmm, I am now thinking we do not need the role permission functionality because the page that contains the hook has already been through the permissions control. So I think the hook permissions are redundant.

SKuipers commented 1 year ago

Hi FV, I've had a look at this and wonder if perhaps the hook you were using had incorrectly set these permissions, resulting in the issue you were seeing. It's important for the hook to be able to determine the permission required to see it, as this enables more flexibility: a hook could add functionality that only teachers can see, or separate hooks could display information differently for different users. If you'd like to ensure that all users looking at the page can see the hook, be sure to use the same permission as the page itself. Eg: module = Planner, action = Lesson Planner_viewAllEditMyClasses,Lesson Planner_viewEditAllClasses (action names can be comma separated for grouped actions). Hope this helps!

fvlasie commented 1 year ago

Thanks Sandra! That makes sense!

fvlasie commented 1 year ago

Hello Sandra! 😄

How do I apply your instructions to the module mainfest.php?

'defaultPermissionAdmin' => 'Y', // Default permission for built in role Admin 'defaultPermissionTeacher' => 'Y', // Default permission for built in role Teacher 'defaultPermissionStudent' => 'N', // Default permission for built in role Student 'defaultPermissionParent' => 'N', // Default permission for built in role Parent 'defaultPermissionSupport' => 'Y', // Default permission for built in role Support 'categoryPermissionStaff' => 'Y', // Should this action be available to user roles in the Staff category? 'categoryPermissionStudent' => 'Y', // Should this action be available to user roles in the Student category? 'categoryPermissionParent' => 'Y', // Should this action be available to user roles in the Parent category? 'categoryPermissionOther' => 'Y', // Should this action be available to user roles in the Other category?

SKuipers commented 1 year ago

Hi FV, this actually depends on what permission the hook has been attached to. In your manifest, you should see something like: (this is an example from Free Learning)

$array = array();
$array['sourceModuleName'] = 'Free Learning';
$array['sourceModuleAction'] = 'Unit History By Student_all';
$array['sourceModuleInclude'] = 'hook_studentProfile_unitHistory.php';
$hooks[1] = "INSERT INTO `gibbonHook` (`gibbonHookID`, `name`, `type`, `options`, gibbonModuleID) VALUES (NULL, 'Free Learning', 'Student Profile', '".serialize($array)."', (SELECT gibbonModuleID FROM gibbonModule WHERE name='$name'));";

The sourceModuleName and sourceModuleAction are what determine the permission that the hook is attached to. If you want it to be the same as the planner itself, you could set module = Planner, action = Lesson Planner_viewAllEditMyClasses. The sourceModuleAction can also be a comma separated list of actions, for permissions that are grouped with multiple actions (eg ActionName_all,ActionName_myChildren, etc.)

fvlasie commented 1 year ago

Like this?:

$array['sourceModuleName'] = 'Planner';
$array['sourceModuleAction'] = 'Lesson Planner_viewAllEditMyCLasses';
fvlasie commented 1 year ago

That did not work...🤔

//Hooks
$array = array();
$array['sourceModuleName'] = 'Planner';
$array['sourceModuleAction'] = 'Lesson Planner_viewAllEditMyClasses,Lesson Planner_viewEditAllClasses';
$array['sourceModuleInclude'] = 'hook_lessonPlannerView.php';
$hooks[0] = "INSERT INTO `gibbonHook` (`gibbonHookID`, `name`, `type`, `options`, gibbonModuleID) VALUES (NULL, 'BigBlueButton', 'Lesson Planner', '".serialize($array)."', (SELECT gibbonModuleID FROM gibbonModule WHERE name='$name'));";
SKuipers commented 1 year ago

Hi FV, if you update the manifest file, this will only affect new installations of the module. For modules already installed, you can add SQL entries to the CHANGEDB file to make these changes (a bit more involved), or uninstall and reinstall the module (provided you don't have any data in it).

fvlasie commented 1 year ago

Thanks for the tip Sandra! Hmm now the video does not load for anyone...it used to load to Staff and Students but not a new Auditor role....

fvlasie commented 1 year ago

Maybe it's not Edit ... is there a Lesson Planner_viewAllViewMyClasses ?

SKuipers commented 1 year ago

Yes, you can see the permissions in User Admin > Manage Permissions. Lesson Planner happens to have a number of compound actions:

Screen Shot 2023-02-10 at 10 24 02 AM
fvlasie commented 1 year ago

Thank you for your help Sandra!