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
    • Rank:
      46266

      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.

        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: