Moodle
  1. Moodle
  2. MDL-40662

setting up the default for My Home page produces block/course_overview:addinstance notice

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.5, 2.5
    • Fix Version/s: 2.4.6, 2.5.2
    • Component/s: Blocks, My home
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in as admin.
      2. Ensure debugging is enabled (Site admin > Development > Debugging).
      3. Visit the default setting for 'My home page' (Site admin > Appearance > Default my home page).
      4. Click "Customise this page".
      5. Ensure no debugging message is shown.
      6. Visit '<yoursite>/admin/tool/capability/index.php' and ensure no debugging is shown about a missing string.
      7. In the search box type 'course_over' to display the block/course_overview:addinstance capability and ensure there is a readable description.
      Show
      Log in as admin. Ensure debugging is enabled (Site admin > Development > Debugging). Visit the default setting for 'My home page' (Site admin > Appearance > Default my home page). Click "Customise this page". Ensure no debugging message is shown. Visit '<yoursite>/admin/tool/capability/index.php' and ensure no debugging is shown about a missing string. In the search box type 'course_over' to display the block/course_overview:addinstance capability and ensure there is a readable description.
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull 2.4 Branch:
      MDL-40662_m24
    • Pull 2.5 Branch:
      MDL-40662_m25
    • Pull Master Branch:
      MDL-40662_master
    • Story Points:
      8
    • Rank:
      51510
    • Sprint:
      BACKEND Sprint 3

      Description

      Reproduction steps:

      1. Log in as admin
      2. Ensure debugging is enabled (Site admin > Development > Debugging)
      3. Visit the default setting for 'My home page' (Site admin > Appearance > Default my home page)
      4. Click "Customise this page"

      Notice the following error:

      The block course_overview does not define the standard capability block/course_overview:addinstance
      
          line 606 of /blocks/moodleblock.class.php: call to debugging()
          line 584 of /blocks/moodleblock.class.php: call to block_base->has_add_block_capability()
          line 1090 of /lib/blocklib.php: call to block_base->user_can_addto()
          line 1047 of /lib/blocklib.php: call to block_manager->user_can_delete_block()
          line 256 of /blocks/moodleblock.class.php: call to block_manager->edit_controls()
          line 951 of /lib/blocklib.php: call to block_base->get_content_for_output()
          line 1003 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 3068 of /lib/outputrenderers.php: call to block_manager->region_has_content()
          line 3104 of /lib/outputrenderers.php: call to core_renderer->body_css_classes()
          line 49 of /theme/clean/layout/columns3.php: call to core_renderer->body_attributes()
          line 847 of /lib/outputrenderers.php: call to include()
          line 777 of /lib/outputrenderers.php: call to core_renderer->render_page_layout()
          line 99 of /my/indexsys.php: call to core_renderer->header()
      

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          I think the capability is correctly set by the block, and the pages where the block is allowed to appear is set correctly, but clearly this page is not working with the block capability check somehow.

          Show
          Michael de Raadt added a comment - I think the capability is correctly set by the block, and the pages where the block is allowed to appear is set correctly, but clearly this page is not working with the block capability check somehow.
          Hide
          Michael de Raadt added a comment -

          Mark: I've added you as a watcher as you were involved in the original issue.

          Show
          Michael de Raadt added a comment - Mark: I've added you as a watcher as you were involved in the original issue.
          Hide
          Mark Nelson added a comment -

          I am going to take this for next sprint if you don't mind Michael. Should be relatively painless to fix (though I haven't looked into it atm).

          Show
          Mark Nelson added a comment - I am going to take this for next sprint if you don't mind Michael. Should be relatively painless to fix (though I haven't looked into it atm).
          Hide
          Mark Nelson added a comment -

          I do realise this has been assigned to frontend, but I will be taking it since I am the most knowledgeable in this area. This fix will need to be applied from 2.4 onwards.

          Show
          Mark Nelson added a comment - I do realise this has been assigned to frontend, but I will be taking it since I am the most knowledgeable in this area. This fix will need to be applied from 2.4 onwards.
          Hide
          Mark Nelson added a comment -

          The issue is that the page is not being treated as a 'My Moodle' page. The course overview block should only ever be added to a user's My Moodle page, however on the 'Default My home page' the system context is being used, as this is the location the administrator sets up how the My Moodle page should appear for other users.

          There are two options.

          Change the context being used on this page, or add the addinstance capability.

          Show
          Mark Nelson added a comment - The issue is that the page is not being treated as a 'My Moodle' page. The course overview block should only ever be added to a user's My Moodle page, however on the 'Default My home page' the system context is being used, as this is the location the administrator sets up how the My Moodle page should appear for other users. There are two options. Change the context being used on this page, or add the addinstance capability.
          Hide
          Rajesh Taneja added a comment -

          Thanks for fixing this Mark,

          Patch looks good, just one question about capability.
          Why editingteacher is given addinstance capability as this is admin page, it might only be applicable to manager.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Mark, Patch looks good, just one question about capability. Why editingteacher is given addinstance capability as this is admin page, it might only be applicable to manager.
          Hide
          Mark Nelson added a comment -

          Hi Raj, I just copied the structure the other blocks use. Really I do not think there is any harm as the page calls "require_capability('moodle/my:configsyspages', $context);", and if the user has this capability I don't see why they can't add this block.

          Submitting to integration.

          Show
          Mark Nelson added a comment - Hi Raj, I just copied the structure the other blocks use. Really I do not think there is any harm as the page calls "require_capability('moodle/my:configsyspages', $context);", and if the user has this capability I don't see why they can't add this block. Submitting to integration.
          Hide
          Mark Nelson added a comment -

          After some discussion (verbal threats) with Raj I have decided to make the change.

          Show
          Mark Nelson added a comment - After some discussion (verbal threats) with Raj I have decided to make the change.
          Hide
          Rajesh Taneja added a comment -

          I think it was just discussion about why not all roles and just two roles (Manager and editingteacher)

          Show
          Rajesh Taneja added a comment - I think it was just discussion about why not all roles and just two roles (Manager and editingteacher)
          Hide
          Sam Hemelryk added a comment - - edited

          Thanks Mark + Raj - this has been integrated now.

          Show
          Sam Hemelryk added a comment - - edited Thanks Mark + Raj - this has been integrated now.
          Hide
          Sam Hemelryk added a comment -

          Test failed - we all missed that you are missing the string required for your new capability.
          Marina picked it up running the behat suite overnight.
          Please create a fix that I can pull in and then amend the test instructions to cover testing it.

          Show
          Sam Hemelryk added a comment - Test failed - we all missed that you are missing the string required for your new capability. Marina picked it up running the behat suite overnight. Please create a fix that I can pull in and then amend the test instructions to cover testing it.
          Hide
          Mark Nelson added a comment - - edited

          Sorry guys. I didn't notice this because when I added the capability and then visited the page that throws the debugging message regarding a missing capability, the debugging message was gone, and there was no debugging message about a missing string. I guess it's lucky we had Behat. Marina, any chance you can provide the location the debugging message is thrown so I can add this into the testing instructions?

          Show
          Mark Nelson added a comment - - edited Sorry guys. I didn't notice this because when I added the capability and then visited the page that throws the debugging message regarding a missing capability, the debugging message was gone, and there was no debugging message about a missing string. I guess it's lucky we had Behat. Marina, any chance you can provide the location the debugging message is thrown so I can add this into the testing instructions?
          Hide
          Mark Nelson added a comment -

          Ah, nvm Sam sent me the debugging message. Of course it would be displayed on the page listing the capabilities. Ok, creating patch and will update testing instructions.

          Show
          Mark Nelson added a comment - Ah, nvm Sam sent me the debugging message. Of course it would be displayed on the page listing the capabilities. Ok, creating patch and will update testing instructions.
          Hide
          Sam Hemelryk added a comment -

          Back it goes thanks Mark

          Show
          Sam Hemelryk added a comment - Back it goes thanks Mark
          Hide
          Sam Hemelryk added a comment -

          and passed

          Show
          Sam Hemelryk added a comment - and passed
          Hide
          Sam Hemelryk added a comment -

          Huzzah, your code made it into Moodle. Perhaps now things are ever so slightly better!

          "The ship can't take this much pressure. Sometimes it falls apart just sitting in the hangar."
          ~ Professor Farnsworth

          Show
          Sam Hemelryk added a comment - Huzzah, your code made it into Moodle. Perhaps now things are ever so slightly better! "The ship can't take this much pressure. Sometimes it falls apart just sitting in the hangar." ~ Professor Farnsworth

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Agile