Moodle
  1. Moodle
  2. MDL-37470

block_manager::add_blocks() ignores block weight parameter and offset

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.2.6, 2.3.3, 2.4
    • Fix Version/s: STABLE backlog
    • Component/s: Blocks
    • Labels:
    • Testing Instructions:
      Hide

      Testing for admin_bookmarks positioning:

      Check the weight of the admin_bookmarks block (it will be 0): SELECT * FROM

      {block_instances} WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks';
      Apply patch.
      Create new Moodle installation.
      Check the weight of the admin_bookmarks block (it will be 2): SELECT * FROM {block_instances}

      WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks';

      Testing for my use case:

      Add this line to config.php: $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming';
      Create new course and see participants block between navigation and settings blocks.
      Replace with this line in config.php: $CFG->defaultblocks_override = ',,participants,calendar_month:activity_modules,news_items,calendar_upcoming';
      Attempt to create new course and get errors saying that the block '' does not exist.
      Apply patch.
      Create new course and see participants and calendar_month blocks after the navigation and settings blocks.

      Show
      Testing for admin_bookmarks positioning: Check the weight of the admin_bookmarks block (it will be 0): SELECT * FROM {block_instances} WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks'; Apply patch. Create new Moodle installation. Check the weight of the admin_bookmarks block (it will be 2): SELECT * FROM {block_instances} WHERE parentcontextid = 1 AND blockname = 'admin_bookmarks'; Testing for my use case: Add this line to config.php: $CFG->defaultblocks_override = 'participants,calendar_month:activity_modules,news_items,calendar_upcoming'; Create new course and see participants block between navigation and settings blocks. Replace with this line in config.php: $CFG->defaultblocks_override = ',,participants,calendar_month:activity_modules,news_items,calendar_upcoming'; Attempt to create new course and get errors saying that the block '' does not exist. Apply patch. Create new course and see participants and calendar_month blocks after the navigation and settings blocks.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Rank:
      47108

      Description

      When you use the add_blocks() function, the weight (block positioning) is ignored in the two ways that it can be provided.

        Activity

        Hide
        Jonathan Champ added a comment -

        Here is a patch that uses the initial weight from the weight parameter (fixes the admin_bookmarks positioning that Sam H. added in 2009 to blocks_add_default_system_blocks()) and allows you to customize the weighting offset between blocks so that default_block_override and similar lines can avoid conflicting with the navigation and settings default page blocks.

        Currently, when you create a new course with blocks specified for the left column, the order jumbles the first non-default block in with the two default blocks (navigation and settings).

        Show
        Jonathan Champ added a comment - Here is a patch that uses the initial weight from the weight parameter (fixes the admin_bookmarks positioning that Sam H. added in 2009 to blocks_add_default_system_blocks()) and allows you to customize the weighting offset between blocks so that default_block_override and similar lines can avoid conflicting with the navigation and settings default page blocks. Currently, when you create a new course with blocks specified for the left column, the order jumbles the first non-default block in with the two default blocks (navigation and settings).
        Hide
        Jonathan Champ added a comment -

        Another implementation that I had considered was to set the weight function parameter to null and only if it was set to a non-null value would I specify the weight. Otherwise, if it were possible to simple append the blocks to the page region, that seems like the best bet. Unfortunately, the code from "add_block_at_end_of_default_region()" did not work the same way in the context of populating the initial page blocks during my testing.

        Show
        Jonathan Champ added a comment - Another implementation that I had considered was to set the weight function parameter to null and only if it was set to a non-null value would I specify the weight. Otherwise, if it were possible to simple append the blocks to the page region, that seems like the best bet. Unfortunately, the code from "add_block_at_end_of_default_region()" did not work the same way in the context of populating the initial page blocks during my testing.
        Hide
        Michael de Raadt added a comment -

        Thanks for spotting that and providing a patch.

        It would be really helpful if you could add a set of testing steps.

        Show
        Michael de Raadt added a comment - Thanks for spotting that and providing a patch. It would be really helpful if you could add a set of testing steps.
        Hide
        Michael de Raadt added a comment -

        Sam: I've added you as a watcher on this issue as you seem to have worked on this code in the past. Perhaps you could have a look at the patch provided.

        Show
        Michael de Raadt added a comment - Sam: I've added you as a watcher on this issue as you seem to have worked on this code in the past. Perhaps you could have a look at the patch provided.

          People

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

            Dates

            • Created:
              Updated: