Moodle
  1. Moodle
  2. MDL-32225 add plagiarism api hooks to other areas of Moodle code
  3. MDL-34133

Plagiarism -> get_file_results is not sufficient for modules that are content based

    Details

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

      No core code uses these events or functions so testing options are limited. - it should be sufficient to check to make sure that uploading a file to an assignment occurs without any warnings. (old assignment upload/uploadsingle and new assign)

      Show
      No core code uses these events or functions so testing options are limited. - it should be sufficient to check to make sure that uploading a file to an assignment occurs without any warnings. (old assignment upload/uploadsingle and new assign)
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      master_MDL-34133
    • Rank:
      42461

      Description

      function get_file_results($cmid, $userid, stored_file $file)

      We need to modify this function to work with modules that don't use stored_file like online assignment

        Issue Links

          Activity

          Show
          Dan Marsden added a comment - possible solution from Kanika https://github.com/kanikagoyal/moodle/commit/5f3d74b2094dba76691fa0bbb04f5400fdd3a738
          Hide
          Dan Marsden added a comment -

          Hi Kanika - I'm not sure if I like this or not - I think we should either add an extra param to the function $content - for passing from places like online assignment or we should create a new function get_content_results() and move the majority of the existing get_file_results() into a new function which is called by both get_content_results() and get_file_results() - I'll think on this a bit and let you know.

          Show
          Dan Marsden added a comment - Hi Kanika - I'm not sure if I like this or not - I think we should either add an extra param to the function $content - for passing from places like online assignment or we should create a new function get_content_results() and move the majority of the existing get_file_results() into a new function which is called by both get_content_results() and get_file_results() - I'll think on this a bit and let you know.
          Hide
          Dan Marsden added a comment -

          yeah - this is really the best option - Note to integrator - this is to go into master only. - we will be releasing new plagiarism plugins for 2.4 that use this param in a better way. existing plagiarism plugins will throw strict standard warnings with this change so I don't want it to go into stable branches.

          Show
          Dan Marsden added a comment - yeah - this is really the best option - Note to integrator - this is to go into master only. - we will be releasing new plagiarism plugins for 2.4 that use this param in a better way. existing plagiarism plugins will throw strict standard warnings with this change so I don't want it to go into stable branches.
          Hide
          Dan Poltawski 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
          Dan Poltawski 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 -

          Hi guys,

          What branch is this on sorry?
          I tried master in case it was there, and couldn't find a branch for MDL-34133.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, What branch is this on sorry? I tried master in case it was there, and couldn't find a branch for MDL-34133 . Cheers Sam
          Hide
          Dan Marsden added a comment -

          heh - not sure either! - can you cherry-pick or would you like me to create a new branch and cherry pick it onto that branch? - thanks!

          Show
          Dan Marsden added a comment - heh - not sure either! - can you cherry-pick or would you like me to create a new branch and cherry pick it onto that branch? - thanks!
          Hide
          Sam Hemelryk added a comment -

          I'm happy to cherry-pick, but in order to fetch the changes I need the branch as well.
          Assuming it is on a branch somewhere it would be possible to cherry-pick without knowing the branch by adding the repository as a remote.
          Dan if you have the repo as a remote would you be able to turn it into a branch for me, otherwise I will do it myself later on today.

          Cheers btw

          Show
          Sam Hemelryk added a comment - I'm happy to cherry-pick, but in order to fetch the changes I need the branch as well. Assuming it is on a branch somewhere it would be possible to cherry-pick without knowing the branch by adding the repository as a remote. Dan if you have the repo as a remote would you be able to turn it into a branch for me, otherwise I will do it myself later on today. Cheers btw
          Hide
          Dan Marsden added a comment -

          sure - give me 5min and will add details here.

          Show
          Dan Marsden added a comment - sure - give me 5min and will add details here.
          Hide
          Dan Marsden added a comment -

          done - thanks Sam!

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

          Cool thanks Dan, this has been integrated to master now.

          Show
          Sam Hemelryk added a comment - Cool thanks Dan, this has been integrated to master now.
          Hide
          David Monllaó added a comment -

          It works as expected, no warnings / notices when creating both mod_assign instances and mod_assignment instances (single & advanced upload)

          Show
          David Monllaó added a comment - It works as expected, no warnings / notices when creating both mod_assign instances and mod_assignment instances (single & advanced upload)
          Hide
          Dan Poltawski added a comment -

          *Notice*: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26

          Congratulations

          {tracker.user.name}

          !

          You've made into Moodle

          {tracker.fixversion-1}

          +

          I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world.

          cheers!

          {tracker.friendlyintegrator}
          Show
          Dan Poltawski added a comment - * Notice *: Undefined variable: friendlyintegrator in /Users/danp/git/tokenintegrationthanks.php on line 26 Congratulations {tracker.user.name} ! You've made into Moodle {tracker.fixversion-1} + I would like to personally thank you for this contribution on behalf of all Moodle users throughout the world. cheers! {tracker.friendlyintegrator}

            People

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

              Dates

              • Created:
                Updated:
                Resolved: