Moodle
  1. Moodle
  2. MDL-36755

block/course overview:addinstance capability unnecessary

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.1
    • Component/s: Blocks
    • Labels:
    • Testing Instructions:
      Hide

      1) Upgrade to latest int.
      2) Visit a course and turn editing on
      3) Make sure you can not see the course overview block in the drop down
      4) Make sure there is no missing cap complaint

      Show
      1) Upgrade to latest int. 2) Visit a course and turn editing on 3) Make sure you can not see the course overview block in the drop down 4) Make sure there is no missing cap complaint
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36755_master

      Description

      As it's only possible for the course overview block to be added to My Moodle pages, it seems that the capability block/course overview:addinstance is unnecessary.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Helen Foster added a comment -

            Mark, I hope you don't mind being assigned this issue, since you were very efficient in removing unnecessary myaddinstance block capabilities!

            Show
            Helen Foster added a comment - Mark, I hope you don't mind being assigned this issue, since you were very efficient in removing unnecessary myaddinstance block capabilities!
            Hide
            Helen Foster added a comment -

            I also noticed that the login block can't be added to course pages, so am wondering whether the capability block/login:addinstance is necessary?

            Show
            Helen Foster added a comment - I also noticed that the login block can't be added to course pages, so am wondering whether the capability block/login:addinstance is necessary?
            Hide
            Helen Foster added a comment -

            Another one: the main menu block can't be added to course pages, so am wondering whether the capability block/site main menu:addinstance is necessary?

            Show
            Helen Foster added a comment - Another one: the main menu block can't be added to course pages, so am wondering whether the capability block/site main menu:addinstance is necessary?
            Hide
            Mark Nelson added a comment -

            Hi Helen, the addinstance capability is used for any page other than the My Moodle page, so it is necessary for this capability to be defined for a block if you want to avoid the warning. Regarding block/course_overview, I think we could get rid of the addinstance capability as return array('my-index' => true); is the only applicable format. I will get onto this soon, once I have finished my currently open bugs/peer reviews/tests etc.

            Show
            Mark Nelson added a comment - Hi Helen, the addinstance capability is used for any page other than the My Moodle page, so it is necessary for this capability to be defined for a block if you want to avoid the warning. Regarding block/course_overview, I think we could get rid of the addinstance capability as return array('my-index' => true); is the only applicable format. I will get onto this soon, once I have finished my currently open bugs/peer reviews/tests etc.
            Hide
            Frédéric Massart added a comment -

            Noticed that error on several pages, not sure how to reproduce it:

            The block site_main_menu does not define the standard capability block/site_main_menu:myaddinstance
             
                line 597 of /blocks/moodleblock.class.php: call to debugging()
                line 570 of /blocks/moodleblock.class.php: call to block_base->has_add_block_capability()
                line 1082 of /lib/blocklib.php: call to block_base->user_can_addto()
                line 1043 of /lib/blocklib.php: call to block_manager->user_can_delete_block()
                line 248 of /blocks/moodleblock.class.php: call to block_manager->edit_controls()
                line 953 of /lib/blocklib.php: call to block_base->get_content_for_output()
                line 1005 of /lib/blocklib.php: call to block_manager->create_block_contents()
                line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created()
                line 6 of /theme/base/layout/general.php: call to block_manager->region_has_content()
                line 804 of /lib/outputrenderers.php: call to include()
                line 734 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
                line 153 of /my/index.php: call to core_renderer->header()
            

            It also reports the error on other blocks than site_main_menu.

            Show
            Frédéric Massart added a comment - Noticed that error on several pages, not sure how to reproduce it: The block site_main_menu does not define the standard capability block/site_main_menu:myaddinstance   line 597 of /blocks/moodleblock.class.php: call to debugging() line 570 of /blocks/moodleblock.class.php: call to block_base->has_add_block_capability() line 1082 of /lib/blocklib.php: call to block_base->user_can_addto() line 1043 of /lib/blocklib.php: call to block_manager->user_can_delete_block() line 248 of /blocks/moodleblock.class.php: call to block_manager->edit_controls() line 953 of /lib/blocklib.php: call to block_base->get_content_for_output() line 1005 of /lib/blocklib.php: call to block_manager->create_block_contents() line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created() line 6 of /theme/base/layout/general.php: call to block_manager->region_has_content() line 804 of /lib/outputrenderers.php: call to include() line 734 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 153 of /my/index.php: call to core_renderer->header() It also reports the error on other blocks than site_main_menu.
            Hide
            Mark Nelson added a comment -

            Hmm, the user context is used on several pages besides the My Moodle page. Currently it performs a check on the context level to see which capability to use (addinstance or myaddinstance), so on these pages it uses the myaddinstance, which is incorrect. I am going to create a patch that will perform a check on the page type to ensure it is only ever performed on the My Moodle page.

            Show
            Mark Nelson added a comment - Hmm, the user context is used on several pages besides the My Moodle page. Currently it performs a check on the context level to see which capability to use (addinstance or myaddinstance), so on these pages it uses the myaddinstance, which is incorrect. I am going to create a patch that will perform a check on the page type to ensure it is only ever performed on the My Moodle page.
            Hide
            Mark Nelson added a comment -

            Actually, I am not sure this is a good idea. One of the scenarios this was going to solve was preventing a student from adding the online users block to the My Moodle page so they could not see everyone online. If we only perform the check on 'myaddinstance' on the My Moodle page, then the user can visit their private file area and add the block there.

            Show
            Mark Nelson added a comment - Actually, I am not sure this is a good idea. One of the scenarios this was going to solve was preventing a student from adding the online users block to the My Moodle page so they could not see everyone online. If we only perform the check on 'myaddinstance' on the My Moodle page, then the user can visit their private file area and add the block there.
            Hide
            Helen Foster added a comment -

            Hi Mark,

            There is no add a block dropdown menu on the My private files page for ordinary users, so I don't think it's a problem.

            Show
            Helen Foster added a comment - Hi Mark, There is no add a block dropdown menu on the My private files page for ordinary users, so I don't think it's a problem.
            Hide
            Mark Nelson added a comment -

            Good point Helen. set_blocks_editing_capability('moodle/my:manageblocks') is only present in the My Moodle page, so this is the only place we should perform a check on this capability. I was logged in as an administrator on the private files, hence could add blocks.

            Show
            Mark Nelson added a comment - Good point Helen. set_blocks_editing_capability('moodle/my:manageblocks') is only present in the My Moodle page, so this is the only place we should perform a check on this capability. I was logged in as an administrator on the private files, hence could add blocks.
            Hide
            Adrian Greeve added a comment -

            for such a simple change I won't bother with the peer review template.

            This looks fine, tests fine, no problems found.

            Feel free to send to integration.

            Show
            Adrian Greeve added a comment - for such a simple change I won't bother with the peer review template. This looks fine, tests fine, no problems found. Feel free to send to integration.
            Hide
            Dan Poltawski added a comment -

            Its generally not recommended to delete capabilities on the stable branches, but since this was added to 2.4.0, I am integrating it to the 2.4 stable branch in the on-sync period to get this uncessary capability out ASAP (reducing work for translators, documenters etc).

            Show
            Dan Poltawski added a comment - Its generally not recommended to delete capabilities on the stable branches, but since this was added to 2.4.0, I am integrating it to the 2.4 stable branch in the on-sync period to get this uncessary capability out ASAP (reducing work for translators, documenters etc).
            Hide
            Dan Poltawski added a comment -

            Integrated, thanks Mark.

            Show
            Dan Poltawski added a comment - Integrated, thanks Mark.
            Hide
            Dan Poltawski added a comment -

            aye aye aye! No testing instructions

            Show
            Dan Poltawski added a comment - aye aye aye! No testing instructions
            Hide
            Dan Poltawski added a comment -

            That was conflicting witht he version number on master as it hadn't been rebased. I then completed screwed up the fix to it, so yuk!

            Show
            Dan Poltawski added a comment - That was conflicting witht he version number on master as it hadn't been rebased. I then completed screwed up the fix to it, so yuk!
            Hide
            Frédéric Massart added a comment -

            Updated test instructions for Mark.

            Show
            Frédéric Massart added a comment - Updated test instructions for Mark.
            Hide
            Frédéric Massart added a comment -

            Passing. But when going to My Home and enabling editing the following notice appears:

            The block site_main_menu does not define the standard capability block/site_main_menu:myaddinstance
            line 598 of /blocks/moodleblock.class.php: call to debugging()
            line 571 of /blocks/moodleblock.class.php: call to block_base->has_add_block_capability()
            line 1082 of /lib/blocklib.php: call to block_base->user_can_addto()
            line 1043 of /lib/blocklib.php: call to block_manager->user_can_delete_block()
            line 248 of /blocks/moodleblock.class.php: call to block_manager->edit_controls()
            line 953 of /lib/blocklib.php: call to block_base->get_content_for_output()
            line 1005 of /lib/blocklib.php: call to block_manager->create_block_contents()
            line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created()
            line 6 of /theme/base/layout/general.php: call to block_manager->region_has_content()
            line 804 of /lib/outputrenderers.php: call to include()
            line 734 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
            line 153 of /my/index.php: call to core_renderer->header()
            

            I am raising a separate issue for that.

            Show
            Frédéric Massart added a comment - Passing. But when going to My Home and enabling editing the following notice appears: The block site_main_menu does not define the standard capability block/site_main_menu:myaddinstance line 598 of /blocks/moodleblock.class.php: call to debugging() line 571 of /blocks/moodleblock.class.php: call to block_base->has_add_block_capability() line 1082 of /lib/blocklib.php: call to block_base->user_can_addto() line 1043 of /lib/blocklib.php: call to block_manager->user_can_delete_block() line 248 of /blocks/moodleblock.class.php: call to block_manager->edit_controls() line 953 of /lib/blocklib.php: call to block_base->get_content_for_output() line 1005 of /lib/blocklib.php: call to block_manager->create_block_contents() line 353 of /lib/blocklib.php: call to block_manager->ensure_content_created() line 6 of /theme/base/layout/general.php: call to block_manager->region_has_content() line 804 of /lib/outputrenderers.php: call to include() line 734 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 153 of /my/index.php: call to core_renderer->header() I am raising a separate issue for that.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

            Closing, ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao
            Hide
            Helen Foster added a comment -

            Thanks Mark for fixing this issue. I have deleted the documentation page for the block/course overview:addinstance capability.

            Show
            Helen Foster added a comment - Thanks Mark for fixing this issue. I have deleted the documentation page for the block/course overview:addinstance capability.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: