Moodle
  1. Moodle
  2. MDL-9802

capability moodle/site:manageblocks used in myMoodle for block editing allows users to see "Turn editing on" button on the site home page

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.8.1, 1.9
    • Component/s: Roles / Access
    • Labels:
      None
    • Environment:
      n/a
    • Database:
      Any
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE, MOODLE_19_STABLE
    • Rank:
      24548

      Description

      Change the existing capability check in myMoodle from moodle/site:manageblocks to moodle/my:manageblocks so that the users of myMoodle don't see the "Turn editing on" button on the site home page when you give them this capability.

      What I did was add this capability to the default role for all users so that anyone could see the editing blocks on the myMoodle page. As I said above this had the unfortunate side affect of allowing them to see the "Turn editing on" button on the site home page, even though if this is clicked on nothing happens.

      So all I would like is to have a new capability and change the check, it's that simple

        Activity

        Hide
        Jenny Gray added a comment -

        Just to be clear, what we're proposing is to add a new capability to sit beside moodle/site:manageblocks, so you can control separately for users whether they can edit blocks on the main site and in myMoodle. See this discussion: http://moodle.org/mod/forum/discuss.php?d=44530 which shows a number of people needing this functionality.

        I have CVS check-in rights, so if James does the work, I'm happy to commit it for him. Helen Foster suggested that we get you (Yu) to glance over the proposal and check you're happy before we proceed?

        Show
        Jenny Gray added a comment - Just to be clear, what we're proposing is to add a new capability to sit beside moodle/site:manageblocks, so you can control separately for users whether they can edit blocks on the main site and in myMoodle. See this discussion: http://moodle.org/mod/forum/discuss.php?d=44530 which shows a number of people needing this functionality. I have CVS check-in rights, so if James does the work, I'm happy to commit it for him. Helen Foster suggested that we get you (Yu) to glance over the proposal and check you're happy before we proceed?
        Hide
        Yu Zhang added a comment -

        Hi guys,

        I have spoken with Martin and this seems good, so please go ahead. The new capability should go into lob/db/access.php and should be a system level capability. The code on myMoodle page should check the system context for this capability.

        Just one thing that's a bit tricky though, because the default role for logged in user is Authenticated User, it has no legacy flags set, so we can not assign capabilities to it automatically during the upgrade. You would need to lookup $CFG->defaultuserroleid and assign moodle/my:manageblocks (set to allow) to whatever role $CFG->defaultuserroleid is pointing to so that the existing behaviour (anyone can add blocks on myMoodle page) is preserved. =)

        Cheers,

        Yu

        Show
        Yu Zhang added a comment - Hi guys, I have spoken with Martin and this seems good, so please go ahead. The new capability should go into lob/db/access.php and should be a system level capability. The code on myMoodle page should check the system context for this capability. Just one thing that's a bit tricky though, because the default role for logged in user is Authenticated User, it has no legacy flags set, so we can not assign capabilities to it automatically during the upgrade. You would need to lookup $CFG->defaultuserroleid and assign moodle/my:manageblocks (set to allow) to whatever role $CFG->defaultuserroleid is pointing to so that the existing behaviour (anyone can add blocks on myMoodle page) is preserved. =) Cheers, Yu
        Hide
        James Brisland added a comment -

        Hi Yu,

        I've made all the changes and can get it working on an upgrade, but when I run a fresh install in puts the capability into the <prefix>capabilities table (that's the part in lib/db/access.php) but I can't seem to find the correct place to put in the assign_capability() function for clean install (it's currently only in lib/db/upgrade.php)

        Please could you point me in the right direction as all I have to do is get it working on a clean install and then I can commit.

        Thanks,

        James.

        Show
        James Brisland added a comment - Hi Yu, I've made all the changes and can get it working on an upgrade, but when I run a fresh install in puts the capability into the <prefix>capabilities table (that's the part in lib/db/access.php) but I can't seem to find the correct place to put in the assign_capability() function for clean install (it's currently only in lib/db/upgrade.php) Please could you point me in the right direction as all I have to do is get it working on a clean install and then I can commit. Thanks, James.
        Hide
        James Brisland added a comment - - edited

        Hi again Yu,

        I think I have figured it out, but what to confirm what I am doing is correct.

        The code in access.php was as follows

        'moodle/my:manageblocks' => array(
        'captype' => 'write',
        'contextlevel' => CONTEXT_SYSTEM
        )

        But I found out that if I don't but the legacy array in it doesn't actually assign the capability to any role, so I changed it to the following.

        'moodle/my:manageblocks' => array(
        'captype' => 'write',
        'contextlevel' => CONTEXT_SYSTEM,
        'legacy' => array(
        'user' => CAP_ALLOW
        )
        )

        This seems to work correctly, but is it the correct way to assign the new capability to the default user role (which is "user"), and is it ok to assume that on install the role "user" will always be the default role, or can I get the id of the default user role as I do in the upgrade.php script through $CFG->defaultuserroleid?

        Hope this all makes sense!

        Regards,

        James

        Show
        James Brisland added a comment - - edited Hi again Yu, I think I have figured it out, but what to confirm what I am doing is correct. The code in access.php was as follows 'moodle/my:manageblocks' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM ) But I found out that if I don't but the legacy array in it doesn't actually assign the capability to any role, so I changed it to the following. 'moodle/my:manageblocks' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM, 'legacy' => array( 'user' => CAP_ALLOW ) ) This seems to work correctly, but is it the correct way to assign the new capability to the default user role (which is "user"), and is it ok to assume that on install the role "user" will always be the default role, or can I get the id of the default user role as I do in the upgrade.php script through $CFG->defaultuserroleid? Hope this all makes sense! Regards, James
        Hide
        Yu Zhang added a comment - - edited

        Hi James,

        I don't think you can hard code the 'user' role because although that is the default role for new sites, it might not be the default role for existing installations (i.e. some people might have it point to some other customized role). I think getting the role from $CFG->defaultuserroleid is the right thing to do here, and you are right that only that role needs to have this capability assigned.

        You said "or can I get the id of the default user role as I do in the upgrade.php script through $CFG->defaultuserroleid? " I am a bit unsure why you need to modify the upgrade.php script? By changing access.php this capability should be added for both new installations and upgrades.

        Cheers,

        Yu

        Edit:

        Sorry, I think

        'moodle/my:manageblocks' => array(
        'captype' => 'write',
        'contextlevel' => CONTEXT_SYSTEM,
        'legacy' => array(
        'user' => CAP_ALLOW
        )
        )

        is enough because it relies on the legacy:user flag that's only set for the default authenticated user role. So there is no need to use $CFG->defaultrolerole, and it should handle both upgrade and new install if it's in lib/db/access.php.

        Show
        Yu Zhang added a comment - - edited Hi James, I don't think you can hard code the 'user' role because although that is the default role for new sites, it might not be the default role for existing installations (i.e. some people might have it point to some other customized role). I think getting the role from $CFG->defaultuserroleid is the right thing to do here, and you are right that only that role needs to have this capability assigned. You said "or can I get the id of the default user role as I do in the upgrade.php script through $CFG->defaultuserroleid? " I am a bit unsure why you need to modify the upgrade.php script? By changing access.php this capability should be added for both new installations and upgrades. Cheers, Yu Edit: Sorry, I think 'moodle/my:manageblocks' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM, 'legacy' => array( 'user' => CAP_ALLOW ) ) is enough because it relies on the legacy:user flag that's only set for the default authenticated user role. So there is no need to use $CFG->defaultrolerole, and it should handle both upgrade and new install if it's in lib/db/access.php.
        Hide
        James Brisland added a comment -

        Hi Yu,

        Right I need to confirm a few more things after testing.

        You are correct in saying that when the code below is added to access.php it works for both upgrades and fresh installs, but as you said before the problem with this is that if Moodle is doing an upgrade and the Moodle default role has been changed from the "Auth. User" role then the capability will only be set for the "Auth. User" role and not the newly chosen default role. This is the reason I added in assign_capability into upgrade.php (see below), it would check if the default role was anything other than "Auth. User" and then set the capability to that role if needed.

        ---- access.php ----
        'moodle/my:manageblocks' => array(
        'captype' => 'write',
        'contextlevel' => CONTEXT_SYSTEM,
        'legacy' => array(
        'user' => CAP_ALLOW
        )
        )
        ---- /access.php ----

        ---- upgrade.php ----
        if ($result && $oldversion < [THEVERSIONNUMBER]) {
        // Get the role id of the "Auth. User" role and check if the default role id is different
        $userrole = get_record( 'role', 'shortname', 'user' );
        $defaultroleid = $CFG->defaultuserroleid;

        if( $defaultroleid != $userrole->id )

        { // Add in the new moodle/my:manageblocks capibility to the default user role $context = get_context_instance(CONTEXT_SYSTEM, SITEID); assign_capability('moodle/my:manageblocks',CAP_ALLOW,$defaultroleid,$context->id); }

        }
        ---- /upgrade.php ----

        Show
        James Brisland added a comment - Hi Yu, Right I need to confirm a few more things after testing. You are correct in saying that when the code below is added to access.php it works for both upgrades and fresh installs, but as you said before the problem with this is that if Moodle is doing an upgrade and the Moodle default role has been changed from the "Auth. User" role then the capability will only be set for the "Auth. User" role and not the newly chosen default role. This is the reason I added in assign_capability into upgrade.php (see below), it would check if the default role was anything other than "Auth. User" and then set the capability to that role if needed. ---- access.php ---- 'moodle/my:manageblocks' => array( 'captype' => 'write', 'contextlevel' => CONTEXT_SYSTEM, 'legacy' => array( 'user' => CAP_ALLOW ) ) ---- /access.php ---- ---- upgrade.php ---- if ($result && $oldversion < [THEVERSIONNUMBER] ) { // Get the role id of the "Auth. User" role and check if the default role id is different $userrole = get_record( 'role', 'shortname', 'user' ); $defaultroleid = $CFG->defaultuserroleid; if( $defaultroleid != $userrole->id ) { // Add in the new moodle/my:manageblocks capibility to the default user role $context = get_context_instance(CONTEXT_SYSTEM, SITEID); assign_capability('moodle/my:manageblocks',CAP_ALLOW,$defaultroleid,$context->id); } } ---- /upgrade.php ----
        Hide
        James Brisland added a comment -

        Just another note to tell you I have tested this in 1.8 Stable code branch and the current dev code (HEAD). With the code both in access.php and upgrade.php it works for both new installations and upgrades with the default role set as "Auth. User" or something different. If the default user is not "Auth. User" then the capability is added to both the "Auth. User" role and whatever the default user role is currently set to.

        Show
        James Brisland added a comment - Just another note to tell you I have tested this in 1.8 Stable code branch and the current dev code (HEAD). With the code both in access.php and upgrade.php it works for both new installations and upgrades with the default role set as "Auth. User" or something different. If the default user is not "Auth. User" then the capability is added to both the "Auth. User" role and whatever the default user role is currently set to.
        Hide
        Yu Zhang added a comment -

        Hi James,

        Ah thanks... I think these changes will ensure both new installations and upgrades get taken care of =)

        Cheer,

        Yu

        Show
        Yu Zhang added a comment - Hi James, Ah thanks... I think these changes will ensure both new installations and upgrades get taken care of =) Cheer, Yu
        Hide
        Petr Škoda added a comment -

        Could you please attach final patch here before commit? I would like to help with review.

        thanks

        Show
        Petr Škoda added a comment - Could you please attach final patch here before commit? I would like to help with review. thanks
        Hide
        Jenny Gray added a comment -

        Too late. Committed already (when I marked it fixed earlier today)

        Show
        Jenny Gray added a comment - Too late. Committed already (when I marked it fixed earlier today)
        Hide
        Petr Škoda added a comment -

        oki, I did not notice the status

        Show
        Petr Škoda added a comment - oki, I did not notice the status
        Hide
        Petr Škoda added a comment -

        hmm, the default user role was originally in 1.7 the guest role, but we should not enable this capability for guests - it is OK in this case, because users can not use the my page anyway. But we should not IMO use this type of capability upgrade/installation for any other similar capabilities of default user because it would assign it to guest role too during the upgrade from 1.7.

        My +1 to do only changes in access.php next time and let the admins deal with it manually during upgrades.

        Show
        Petr Škoda added a comment - hmm, the default user role was originally in 1.7 the guest role, but we should not enable this capability for guests - it is OK in this case, because users can not use the my page anyway. But we should not IMO use this type of capability upgrade/installation for any other similar capabilities of default user because it would assign it to guest role too during the upgrade from 1.7. My +1 to do only changes in access.php next time and let the admins deal with it manually during upgrades.
        Hide
        Jenny Gray added a comment -

        That's true, but now there are a lot more default role settings in the user policies table, including notloggedinroleid and guestroleid which both default to guest on install. I think defaultuserroleid now installs as the new 'user' role that we've used. If I'm mistaken, apologies.

        Show
        Jenny Gray added a comment - That's true, but now there are a lot more default role settings in the user policies table, including notloggedinroleid and guestroleid which both default to guest on install. I think defaultuserroleid now installs as the new 'user' role that we've used. If I'm mistaken, apologies.
        Hide
        Petr Škoda added a comment -

        The defaultuserroleid should be changed from "guest role" to new "user role" during upgrade from 1.7, but it is possible that it was not - people experimented a lot with roles in 1.7 and there was no way to reset them back to defaults.

        What I wanted to say that till today we have relied mostly on legacy capabilities during installation of new capabilities. That means it would be ok to just add the new cap into access.php.

        Anyway this patch should IMO work for all properly configured sites, thanks for working on this issue

        Show
        Petr Škoda added a comment - The defaultuserroleid should be changed from "guest role" to new "user role" during upgrade from 1.7, but it is possible that it was not - people experimented a lot with roles in 1.7 and there was no way to reset them back to defaults. What I wanted to say that till today we have relied mostly on legacy capabilities during installation of new capabilities. That means it would be ok to just add the new cap into access.php. Anyway this patch should IMO work for all properly configured sites, thanks for working on this issue

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: