Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-37535

Fix block_section_links use of config_plugins

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.4, 2.4.1
    • Fix Version/s: 2.5
    • Component/s: Blocks
    • Labels:
    • Testing Instructions:
      Hide

      Before upgrade...

      1. Log in as an admin.
      2. Browse to Settings > Plugins > Blocks > Section links setting.
      3. Change the default values and remember what you changed them to.
      4. Browse to a course.
      5. Add an instance of the section links block.

      After upgrade...

      1. Log in as an admin
      2. Browse to Settings > Plugins > Blocks > Section links setting.
      3. Make sure the values you selected are still there.
      4. Browse to the course.
      5. Make sure the section links block still works.
      Show
      Before upgrade... Log in as an admin. Browse to Settings > Plugins > Blocks > Section links setting. Change the default values and remember what you changed them to. Browse to a course. Add an instance of the section links block. After upgrade... Log in as an admin Browse to Settings > Plugins > Blocks > Section links setting. Make sure the values you selected are still there. Browse to the course. Make sure the section links block still works.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-37535-m25

      Description

      While reviewing MDL-34344 it was detected that the block_section_links block:

      1) Is storing its plugin settings into blocks/section_links, while it should be using proper component name. So we must move those settings (upgrade) and switch to component.

      2) Surely because of the exceptional (incorrect) plugin name used... the block has implemented the before_delete() method. Once moved to proper component, it should be not needed any more. Standard plugin uninstallation takes rid of related config automatically.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            samhemelryk Sam Hemelryk added a comment -

            Putting this up for peer-review now.

            Show
            samhemelryk Sam Hemelryk added a comment - Putting this up for peer-review now.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Sam,

            Just minor change needed for the upgrade.php. I think the value for the 3rd param in upgrade_block_savepoint() should be 'section_links' instead of 'block_section_links'.

            Other than that, the code looks great.

            [y] Syntax
            [-] Output
            [y] Whitespace
            [-] Language
            [y] Databases
            [-] Testing
            [-] Security
            [-] Documentation
            [y] Git
            [y] Sanity check

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Sam, Just minor change needed for the upgrade.php. I think the value for the 3rd param in upgrade_block_savepoint() should be 'section_links' instead of 'block_section_links'. Other than that, the code looks great. [y] Syntax [-] Output [y] Whitespace [-] Language [y] Databases [-] Testing [-] Security [-] Documentation [y] Git [y] Sanity check
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Rosie, I've fixed up that upgrade call and am putting this up for integration review now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Rosie, I've fixed up that upgrade call and am putting this up for integration review now.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            poltawski Dan Poltawski added a comment -

            This is blocking MDL-37615 so taking it.

            Show
            poltawski Dan Poltawski added a comment - This is blocking MDL-37615 so taking it.
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated, thanks Sam

            (i'd have just rm'd the block :-P )

            Show
            poltawski Dan Poltawski added a comment - Integrated, thanks Sam (i'd have just rm'd the block :-P )
            Hide
            dmonllao David Monllaó added a comment -

            Tested with latest stable and integration, it passes

            Show
            dmonllao David Monllaó added a comment - Tested with latest stable and integration, it passes
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

            (and won't be revisiting it unless some regression is found)

            Thanks and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/13