Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-32225 add plagiarism api hooks to other areas of Moodle code
  3. MDL-34382

Add new option to module_supports() to flag modules that support the use of plagiarism plugins

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4
    • Component/s: Plagiarism
    • Labels:
    • Testing Instructions:
      Hide

      This can only be reviewed/tested by a developer - no core code uses this - a developer could write their own php file to test that the function returns valid options?
      (Also - no contrib code uses this yet either so you can't use any plagiarism plugins to test this - it won't be added until support is available in core)

      Show
      This can only be reviewed/tested by a developer - no core code uses this - a developer could write their own php file to test that the function returns valid options? (Also - no contrib code uses this yet either so you can't use any plagiarism plugins to test this - it won't be added until support is available in core)
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      master_MDL-34382_add_new_option_to_flag_modules_that_support_plagiarism_plugins

      Description

      now that multiple plugins support the use of the plagiarism API it would be nice to add a new option to the module_supports() functions so we can check this.

      for example - mod/assign/lib.php function assign_supports() should have an item like this:
      case FEATURE_PLAGIARISMAPI: return true;
      (FEATURE_PLAGIARISMAPI should be defined in moodlelib.php like the other FEATURE defines)

      For every module we have added support we should add this to the relevant module_supports function

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              danmarsden Dan Marsden added a comment -

              Hi Kanika,

              can you please implement this for the plugins that have patches already submitted as a separate patch on this bug including the new define in moodlelib.php and for the plugins that haven't been submitted yet can you please add this to the existing patches before we integrate them?

              After doing this I'd like you to implement a setting with the plagiarism plugins that lets the site admin configure which modules they would like the plagiarism plugin to be enabled in - contact me via jabber to discuss further.

              I think we should do this before you do much more work on the essay question stuff - thanks!

              Show
              danmarsden Dan Marsden added a comment - Hi Kanika, can you please implement this for the plugins that have patches already submitted as a separate patch on this bug including the new define in moodlelib.php and for the plugins that haven't been submitted yet can you please add this to the existing patches before we integrate them? After doing this I'd like you to implement a setting with the plagiarism plugins that lets the site admin configure which modules they would like the plagiarism plugin to be enabled in - contact me via jabber to discuss further. I think we should do this before you do much more work on the essay question stuff - thanks!
              Hide
              kanikagoyal Kanika Goyal added a comment -

              Hi Dan,

              I am not sure what is exactly to be done. Is this is what you meant? -
              https://github.com/kanikagoyal/moodle/compare/master_MDL-34382_add_new_option_to_flag_modules_that_support_plagiarism_plugins
              https://github.com/kanikagoyal/moodle/compare/master_add_option_in_assign_support_for_plagiarism_api

              If this is right, then I will create patches for other modules too. Please let me know what changes are required.

              Thanks,
              Kanika

              Show
              kanikagoyal Kanika Goyal added a comment - Hi Dan, I am not sure what is exactly to be done. Is this is what you meant? - https://github.com/kanikagoyal/moodle/compare/master_MDL-34382_add_new_option_to_flag_modules_that_support_plagiarism_plugins https://github.com/kanikagoyal/moodle/compare/master_add_option_in_assign_support_for_plagiarism_api If this is right, then I will create patches for other modules too. Please let me know what changes are required. Thanks, Kanika
              Hide
              danmarsden Dan Marsden added a comment -

              yeah - that's it but lets change this:
              define('FEATURE_PLAGIARISMAPI', 'plagiarismapi');

              to this:
              define('FEATURE_PLAGIARISM', 'plagiarism');

              Can you provide a single patch that adds the define to moodlelib and then to the modules that already support it that have been pushed (forum/assign/assignment) to this bug?

              thanks!

              Dan

              Show
              danmarsden Dan Marsden added a comment - yeah - that's it but lets change this: define('FEATURE_PLAGIARISMAPI', 'plagiarismapi'); to this: define('FEATURE_PLAGIARISM', 'plagiarism'); Can you provide a single patch that adds the define to moodlelib and then to the modules that already support it that have been pushed (forum/assign/assignment) to this bug? thanks! Dan
              Hide
              kanikagoyal Kanika Goyal added a comment -

              Hi Dan,

              Here is the patch - https://github.com/kanikagoyal/moodle/compare/master_MDL-34382_add_new_option_to_flag_modules_that_support_plagiarism_plugins

              Let me know if it requires further modification.

              Thanks,
              Kanika

              Show
              kanikagoyal Kanika Goyal added a comment - Hi Dan, Here is the patch - https://github.com/kanikagoyal/moodle/compare/master_MDL-34382_add_new_option_to_flag_modules_that_support_plagiarism_plugins Let me know if it requires further modification. Thanks, Kanika
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Kanika/Dan,

              This looks good, though you have added the capability to forum/workshop (which I don't think have plagarism capability yet?)

              So i'd propose you only add it only to the core things which support it so you are able to land this change earlier than the other things landing.

              cheers,
              dan

              Show
              poltawski Dan Poltawski added a comment - Hi Kanika/Dan, This looks good, though you have added the capability to forum/workshop (which I don't think have plagarism capability yet?) So i'd propose you only add it only to the core things which support it so you are able to land this change earlier than the other things landing. cheers, dan
              Hide
              danmarsden Dan Marsden added a comment -

              Thanks Dan - Forum is supported already but the workshop patch hasn't gone in yet and I've linked that bug as a blocker for this - Workshop is ready for integration now so I'll submit this one when that has been integrated.

              Show
              danmarsden Dan Marsden added a comment - Thanks Dan - Forum is supported already but the workshop patch hasn't gone in yet and I've linked that bug as a blocker for this - Workshop is ready for integration now so I'll submit this one when that has been integrated.
              Hide
              kanikagoyal Kanika Goyal added a comment -

              Hi Dan,

              I think this can be integrated now?

              Thanks,
              Kanika

              Show
              kanikagoyal Kanika Goyal added a comment - Hi Dan, I think this can be integrated now? Thanks, Kanika
              Hide
              danmarsden Dan Marsden added a comment -

              looks good - Kanika can you rebase this for the integrators just in case it causes conflicts? - thanks!

              Show
              danmarsden Dan Marsden added a comment - looks good - Kanika can you rebase this for the integrators just in case it causes conflicts? - thanks!
              Hide
              kanikagoyal Kanika Goyal added a comment -

              Update the link.

              Thanks,
              Kanika

              Show
              kanikagoyal Kanika Goyal added a comment - Update the link. Thanks, Kanika
              Hide
              danmarsden Dan Marsden added a comment -

              great! - thanks.

              Show
              danmarsden Dan Marsden added a comment - great! - thanks.
              Hide
              nebgor Aparup Banerjee added a comment - - edited

              Thanks Kanika and Dan and Dan

              That's been put thru to integration now. only in master.

              Show
              nebgor Aparup Banerjee added a comment - - edited Thanks Kanika and Dan and Dan That's been put thru to integration now. only in master.
              Hide
              fred Frédéric Massart added a comment -

              Passing. Sorry for the delay!

              Show
              fred Frédéric Massart added a comment - Passing. Sorry for the delay!
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility!

              Many thanks for your collaboration, yay!

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - For the good and the bad... this is now part of Moodle and people around the world will start using it immediately, what a responsibility! Many thanks for your collaboration, yay! Closing, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    3/Dec/12