Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Show
            danmarsden Dan Marsden added a comment - possible solution from Kanika https://github.com/kanikagoyal/moodle/commit/5f3d74b2094dba76691fa0bbb04f5400fdd3a738
            Hide
            danmarsden 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
            danmarsden 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
            danmarsden 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
            danmarsden 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
            poltawski 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
            poltawski 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
            samhemelryk 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
            samhemelryk 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
            danmarsden 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
            danmarsden 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
            samhemelryk 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
            samhemelryk 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
            danmarsden Dan Marsden added a comment -

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

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

            done - thanks Sam!

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

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

            Show
            samhemelryk Sam Hemelryk added a comment - Cool thanks Dan, this has been integrated to master now.
            Hide
            dmonllao 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
            dmonllao 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
            poltawski 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
            poltawski 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:
                  Fix Release Date:
                  3/Dec/12