Moodle
  1. Moodle
  2. MDL-32002

No 'manage plagiarism plugins'. It appears to be impossible to un-install a plagiarism plugin

    Details

    • Testing Instructions:
      Hide

      install a couple of plagiarism plugins - Urkund and turnitin are good examples. - you don't need to configure these for testing.
      https://github.com/danmarsden/moodle-plagiarism_turnitin
      https://github.com/danmarsden/moodle-plagiarism_urkund

      Make sure the advanced feature to enable plagiarism plugins is enabled.

      after install - try using the new delete option in admin > plugins > plagiarism > manage plagiarism plugins

      • confirm that the custom plagiarism_plugin tables have been removed.

      important to note that this will only delete the tables created by the plugins - the plugins both use the config_plugins table to store settings information and this information will currently stay in the db - we need to add uninstall.php to the plugins to remove this data as well (will do this at some point.)

      Show
      install a couple of plagiarism plugins - Urkund and turnitin are good examples. - you don't need to configure these for testing. https://github.com/danmarsden/moodle-plagiarism_turnitin https://github.com/danmarsden/moodle-plagiarism_urkund Make sure the advanced feature to enable plagiarism plugins is enabled. after install - try using the new delete option in admin > plugins > plagiarism > manage plagiarism plugins confirm that the custom plagiarism_plugin tables have been removed. important to note that this will only delete the tables created by the plugins - the plugins both use the config_plugins table to store settings information and this information will currently stay in the db - we need to add uninstall.php to the plugins to remove this data as well (will do this at some point.)
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      master_MDL-32002

      Description

      All (most?) other plugin types have a 'manage' screen that allows them to be deleted. Plagiarism plugins do not for some reason.

        Gliffy Diagrams

          Activity

          Hide
          Michael de Raadt added a comment -

          Good point.

          Feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Good point. Feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
          Hide
          Dan Poltawski added a comment -

          Hi Dan,

          Looks good to me, +1 for integration. Two small comments:

          • I'd prefer the default of this to be false rather than an empty string:

            optional_param('confirm', '', PARAM_BOOL);
            

          • This could be a html_writer::link & moodle_url (I see its existing code):

            $settings = "<a href=\"$CFG->wwwroot/plagiarism/$plugin/settings.php\">{$txt->settings}</a>";
            

          Show
          Dan Poltawski added a comment - Hi Dan, Looks good to me, +1 for integration. Two small comments: I'd prefer the default of this to be false rather than an empty string: optional_param('confirm', '', PARAM_BOOL); This could be a html_writer::link & moodle_url (I see its existing code): $settings = "<a href=\"$CFG->wwwroot/plagiarism/$plugin/settings.php\">{$txt->settings}</a>";
          Hide
          Dan Marsden added a comment -

          thanks Dan - yeah, those were both copy/paste of code that I didn't look at too closely - tidied that up now thanks!

          Show
          Dan Marsden added a comment - thanks Dan - yeah, those were both copy/paste of code that I didn't look at too closely - tidied that up now thanks!
          Hide
          Aparup Banerjee 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
          Aparup Banerjee 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
          Sam Hemelryk added a comment -

          Hehe reopening this at the moment sorry, certainly the code looks 99% perfect.
          However there is a return in the page if there are no plagiarism plugins installed.
          Also just a thought but perhaps it would be worth adding a notification to that page when enableplagiarism is off notifying the user of that. Just something simple like "The plagiarism detection system is currently turned off."
          Then again perhaps it is not needed.

          Also it'd be friendlier to use $table->colclasses (mdl-left, mdl-align) rather than $table->align. Just makes things styleable.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hehe reopening this at the moment sorry, certainly the code looks 99% perfect. However there is a return in the page if there are no plagiarism plugins installed. Also just a thought but perhaps it would be worth adding a notification to that page when enableplagiarism is off notifying the user of that. Just something simple like "The plagiarism detection system is currently turned off." Then again perhaps it is not needed. Also it'd be friendlier to use $table->colclasses (mdl-left, mdl-align) rather than $table->align. Just makes things styleable. Cheers Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Dan Marsden added a comment -

          hehe - whoops! - I've fixed that return and now using colclasses as suggested - thanks.

          I'm not sure if we need to add a check to see if plagiarism is enabled as this page won't appear in the nav when plagiarism is disabled - but feel free to bounce back if you think I need to add it for people that type the url in manually?

          Show
          Dan Marsden added a comment - hehe - whoops! - I've fixed that return and now using colclasses as suggested - thanks. I'm not sure if we need to add a check to see if plagiarism is enabled as this page won't appear in the nav when plagiarism is disabled - but feel free to bounce back if you think I need to add it for people that type the url in manually?
          Hide
          Dan Poltawski added a comment -

          Submitting for integration - as I think thats the right state

          Show
          Dan Poltawski added a comment - Submitting for integration - as I think thats the right state
          Hide
          Dan Marsden added a comment -

          thanks Dan!

          Show
          Dan Marsden added a comment - thanks Dan!
          Hide
          Sam Hemelryk added a comment -

          Thanks Dan, code is now 100% perfect! (and has been integrated btw)

          Show
          Sam Hemelryk added a comment - Thanks Dan, code is now 100% perfect! (and has been integrated btw)
          Hide
          Frédéric Massart added a comment -

          Test successful! \o/

          Show
          Frédéric Massart added a comment - Test successful! \o/
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Fixed STOP Closed STOP Thanks STOP

          Yay, imagination! Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Fixed STOP Closed STOP Thanks STOP Yay, imagination! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: