Moodle
  1. Moodle
  2. MDL-39148

Notices in plugins overview page after uninstalling selfcompletion block

    Details

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

      Testing difficulty: medium (requires access to disk)

      1. Login as administrator and browse to Settings > Site administration > Plugins > Plugins overview.
      2. Click uninstall link next to any block (e.g. self_completion) and uninstall the block data. Do not remove the block folder from the disk.
      3. Return back to the plugins overview page
      4. TEST: Make sure there are no notices/warnings reported.
      5. TEST: Make sure there is no "Uninstall" link displayed for this to-be-installed block.
      6. TEST: Make sure the uninstalled block is now reported as "To be installed" and visiting the main administration page re-installs the block again.
      7. Rename, move or remove some block folder from the disk (e.g. rename "section_links" to ".section_links" or move it out of the $CFG->dirroot or remove it completely).
      8. Purge caches.
      9. Browse to Settings > Site administration > Plugins > Plugins overview.
      10. TEST: Make sure the block is reported as "Missing from disk".
      Show
      Testing difficulty: medium (requires access to disk) Login as administrator and browse to Settings > Site administration > Plugins > Plugins overview. Click uninstall link next to any block (e.g. self_completion) and uninstall the block data. Do not remove the block folder from the disk. Return back to the plugins overview page TEST: Make sure there are no notices/warnings reported. TEST: Make sure there is no "Uninstall" link displayed for this to-be-installed block. TEST: Make sure the uninstalled block is now reported as "To be installed" and visiting the main administration page re-installs the block again. Rename, move or remove some block folder from the disk (e.g. rename "section_links" to ".section_links" or move it out of the $CFG->dirroot or remove it completely). Purge caches. Browse to Settings > Site administration > Plugins > Plugins overview. TEST: Make sure the block is reported as "Missing from disk".
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-39148-uninstall
    • Rank:
      49746

      Description

      As reported by Séverin Terrier (https://tracker.moodle.org/browse/MDLQA-5428?focusedCommentId=215688&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-215688) after uninstalling selfcompletion block and accessing again to Site administration -> Plugins -> Plugins overview there is a couple of PHP notices.

      ( ! ) Notice: Undefined index: selfcompletion in /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/pluginlib.php on line 2684
      Call Stack
      #	Time	Memory	Function	Location
      1	0.0002	244368	{main}( )	../plugins.php:0
      2	0.8274	44382576	core_admin_renderer->plugin_management_page( )	../plugins.php:67
      3	0.9454	45082240	core_admin_renderer->plugins_control_panel( )	../renderer.php:368
      4	1.5291	55318632	plugininfo_block->get_uninstall_url( )	../renderer.php:1137
      
      ( ! ) Notice: Trying to get property of non-object in /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/pluginlib.php on line 2684
      Call Stack
      #	Time	Memory	Function	Location
      1	0.0002	244368	{main}( )	../plugins.php:0
      2	0.8274	44382576	core_admin_renderer->plugin_management_page( )	../plugins.php:67
      3	0.9454	45082240	core_admin_renderer->plugins_control_panel( )	../renderer.php:368
      4	1.5291	55318632	plugininfo_block->get_uninstall_url( )	../renderer.php:1137
      
      1. bootstrap.png
        12 kB
      2. standard.png
        39 kB

        Issue Links

          Activity

          Hide
          Rajesh Taneja added a comment -

          I am not sure if this is a bug, as steps to uninstall plugin ask user to remove files from disk. Anyway this being chicken-n-egg situation I have added if-else to avoid these notices.

          Further looking at plugin_overview page, it seems we are not showing proper information about uninstalled (but on disk), plugins.

          1. Click uninstall for any plugin (Don't delete plugin from disk)
          2. Go back to plugin_overview page
          3. You can still see "uninstall" link. This is misleading as plugin is not installed.

          We considered putting a "Install" link if plugin is not installed, but with multiple uninstalled plugins install seems to loose it's meaning.
          Any link next to plugin (uninstall/settings), is specific to that plugin. But there is no way to install a specific plugin, without going though the whole site upgrade/install process.

          Have added David and Apu as watchers as they can comment further on this.

          Show
          Rajesh Taneja added a comment - I am not sure if this is a bug, as steps to uninstall plugin ask user to remove files from disk. Anyway this being chicken-n-egg situation I have added if-else to avoid these notices. Further looking at plugin_overview page, it seems we are not showing proper information about uninstalled (but on disk), plugins. Click uninstall for any plugin (Don't delete plugin from disk) Go back to plugin_overview page You can still see "uninstall" link. This is misleading as plugin is not installed. We considered putting a "Install" link if plugin is not installed, but with multiple uninstalled plugins install seems to loose it's meaning. Any link next to plugin (uninstall/settings), is specific to that plugin. But there is no way to install a specific plugin, without going though the whole site upgrade/install process. Have added David and Apu as watchers as they can comment further on this.
          Hide
          David Mudrak added a comment -

          Let me recommend to wait for MDL-39087 landing in moodle.git and then trying to reproduce again. The patchset there fixes missing cache invalidation (that causes the notice) and I believe it is a better solution than the if/then hack.

          And yes, the link "Uninstall" is displayed for plugins that are present on the disk. I realized it recently as well. I'm not sure if it is a bug, too. We definitely should think about consequences. If all plugins were required to ship with version.php and thence be registered in the database, we would be able to distinguish between "deployed" and "installed" plugins. But themes are not required to declare their versions and this exception makes things a bit more complicated.

          Show
          David Mudrak added a comment - Let me recommend to wait for MDL-39087 landing in moodle.git and then trying to reproduce again. The patchset there fixes missing cache invalidation (that causes the notice) and I believe it is a better solution than the if/then hack. And yes, the link "Uninstall" is displayed for plugins that are present on the disk. I realized it recently as well. I'm not sure if it is a bug, too. We definitely should think about consequences. If all plugins were required to ship with version.php and thence be registered in the database, we would be able to distinguish between "deployed" and "installed" plugins. But themes are not required to declare their versions and this exception makes things a bit more complicated.
          Hide
          Rajesh Taneja added a comment -

          Thanks David,

          I can see MDL-39087 has been integrated, I tested this on Integration Master and still getting notices

          Notice:  Undefined index: admin_bookmarks in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php
          Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php
          1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php
          2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php
          3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php
          4. plugin_manager->can_uninstall_plugin() /var/www/im/admin/renderer.php:1221, referer: http://rajesh.moodle.local/im/admin/blocks.php
          5. plugin_manager->common_uninstall_check() /var/www/im/lib/pluginlib.php:490, referer: http://rajesh.moodle.local/im/admin/blocks.php
          6. plugininfo_block->get_uninstall_url() /var/www/im/lib/pluginlib.php:951, referer: http://rajesh.moodle.local/im/admin/blocks.php
          [Thu Apr 18 10:13:20 2013] [error] [client 192.168.100.33] PHP Notice:  Trying to get property of non-object in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php
          
          Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php
          1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php
          2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php
          3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php
          4. plugin_manager->can_uninstall_plugin() /var/www/im/admin/renderer.php:1221, referer: http://rajesh.moodle.local/im/admin/blocks.php
          5. plugin_manager->common_uninstall_check() /var/www/im/lib/pluginlib.php:490, referer: http://rajesh.moodle.local/im/admin/blocks.php
          6. plugininfo_block->get_uninstall_url() /var/www/im/lib/pluginlib.php:951, referer: http://rajesh.moodle.local/im/admin/blocks.php
          
          PHP Notice:  Undefined index: admin_bookmarks in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php
          PHP Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php
          1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php
          2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php
          3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php
          4. plugininfo_block->get_uninstall_url() /var/www/im/admin/renderer.php:1222, referer: http://rajesh.moodle.local/im/admin/blocks.php
          
          PHP Notice:  Trying to get property of non-object in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php
          PHP Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php
          1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php
          2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php
          3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php
          4. plugininfo_block->get_uninstall_url() /var/www/im/admin/renderer.php:1222, referer: http://rajesh.moodle.local/im/admin/blocks.php
          
          Show
          Rajesh Taneja added a comment - Thanks David, I can see MDL-39087 has been integrated, I tested this on Integration Master and still getting notices Notice: Undefined index: admin_bookmarks in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php 1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php 2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php 3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php 4. plugin_manager->can_uninstall_plugin() /var/www/im/admin/renderer.php:1221, referer: http://rajesh.moodle.local/im/admin/blocks.php 5. plugin_manager->common_uninstall_check() /var/www/im/lib/pluginlib.php:490, referer: http://rajesh.moodle.local/im/admin/blocks.php 6. plugininfo_block->get_uninstall_url() /var/www/im/lib/pluginlib.php:951, referer: http://rajesh.moodle.local/im/admin/blocks.php [Thu Apr 18 10:13:20 2013] [error] [client 192.168.100.33] PHP Notice: Trying to get property of non-object in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php 1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php 2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php 3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php 4. plugin_manager->can_uninstall_plugin() /var/www/im/admin/renderer.php:1221, referer: http://rajesh.moodle.local/im/admin/blocks.php 5. plugin_manager->common_uninstall_check() /var/www/im/lib/pluginlib.php:490, referer: http://rajesh.moodle.local/im/admin/blocks.php 6. plugininfo_block->get_uninstall_url() /var/www/im/lib/pluginlib.php:951, referer: http://rajesh.moodle.local/im/admin/blocks.php PHP Notice: Undefined index: admin_bookmarks in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php PHP Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php 1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php 2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php 3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php 4. plugininfo_block->get_uninstall_url() /var/www/im/admin/renderer.php:1222, referer: http://rajesh.moodle.local/im/admin/blocks.php PHP Notice: Trying to get property of non-object in /var/www/im/lib/pluginlib.php on line 3056, referer: http://rajesh.moodle.local/im/admin/blocks.php PHP Stack trace:, referer: http://rajesh.moodle.local/im/admin/blocks.php 1. {main}() /var/www/im/admin/plugins.php:0, referer: http://rajesh.moodle.local/im/admin/blocks.php 2. core_admin_renderer->plugin_management_page() /var/www/im/admin/plugins.php:164, referer: http://rajesh.moodle.local/im/admin/blocks.php 3. core_admin_renderer->plugins_control_panel() /var/www/im/admin/renderer.php:368, referer: http://rajesh.moodle.local/im/admin/blocks.php 4. plugininfo_block->get_uninstall_url() /var/www/im/admin/renderer.php:1222, referer: http://rajesh.moodle.local/im/admin/blocks.php
          Hide
          David Mudrak added a comment -

          I have prepared a patchset that is IMHO more robust solution to this.

          This was reported for blocks due to the way they are loaded by the plugin manager but the essence of the problem is more general and it lies elsewhere:

          I believe that we should not display the Uninstall link for plugins that were only deployed to disk but not installed. We should completely prohibit uninstall procedure for them and it should also block other related plugins from being uninstalled, too - until the new plugin is either fully installed or removed from disk.

          So the patchset consists of two commits that address issues mentioned above. The first patch blocks uninstallation of deployed-only plugins (unit tests attached). The second patch improves the Plugin overview screen so that it displays the status of the plugin as well (to prevent confusion of admins who just uninstalled some plugin as still see it listed there).

          Show
          David Mudrak added a comment - I have prepared a patchset that is IMHO more robust solution to this. This was reported for blocks due to the way they are loaded by the plugin manager but the essence of the problem is more general and it lies elsewhere: I believe that we should not display the Uninstall link for plugins that were only deployed to disk but not installed. We should completely prohibit uninstall procedure for them and it should also block other related plugins from being uninstalled, too - until the new plugin is either fully installed or removed from disk. So the patchset consists of two commits that address issues mentioned above. The first patch blocks uninstallation of deployed-only plugins (unit tests attached). The second patch improves the Plugin overview screen so that it displays the status of the plugin as well (to prevent confusion of admins who just uninstalled some plugin as still see it listed there).
          Hide
          David Mudrak added a comment -

          Attaching screenshots of expected result of the patch.

          Show
          David Mudrak added a comment - Attaching screenshots of expected result of the patch.
          Hide
          Dan Poltawski added a comment -

          Integrated to master, thanks David.

          Show
          Dan Poltawski added a comment - Integrated to master, thanks David.
          Hide
          Mark Nelson added a comment -

          Works as expected, passing. Thanks.

          Show
          Mark Nelson added a comment - Works as expected, passing. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I feel myself really alone tonight! So was time to push your fixes upstream!

          "Lest we forget. We will remember them."

          Thanks and ciao!

          Show
          Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: