Moodle
  1. Moodle
  2. MDL-34258

hook to plagiarism_print_disclosure incorrect

    Details

    • Testing Instructions:
      Hide

      No Plagiarism plugins exist in core so it is sufficient to test a user submission to ensure that no errors occur.

      1. Create an assignment with submission files enabled
      2. Login as a student and view the submissions page
      3. Verify that no errors occur.
      4. Login as a teacher, and view the list of submissions page
      5. verify no errors occur.

      If you have access to a plugin (have passed urkund test details to Tim)
      Login as student and check to make sure disclosure statement appears on the initial submission page.

      Show
      No Plagiarism plugins exist in core so it is sufficient to test a user submission to ensure that no errors occur. Create an assignment with submission files enabled Login as a student and view the submissions page Verify that no errors occur. Login as a teacher, and view the list of submissions page verify no errors occur. If you have access to a plugin (have passed urkund test details to Tim) Login as student and check to make sure disclosure statement appears on the initial submission page.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      42607

      Description

      We changed the plagiarism plugin print disclosure functions to return content instead of printing them inside the functions - looks like this change was missed when the new mod/assign was developed.

        Issue Links

          Activity

          Hide
          Dan Marsden added a comment -

          this:

          ob_start();
          plagiarism_print_disclosure($this->get_course_module()->id);
          $o = ob_get_contents();
          ob_end_clean();
          

          should be replaced by this:

          echo plagiarism_print_disclosure($this->get_course_module()->id);
          
          Show
          Dan Marsden added a comment - this: ob_start(); plagiarism_print_disclosure($ this ->get_course_module()->id); $o = ob_get_contents(); ob_end_clean(); should be replaced by this: echo plagiarism_print_disclosure($ this ->get_course_module()->id);
          Hide
          Dan Marsden added a comment -

          and this:

          plagiarism_update_status($this->get_course(), $this->get_course_module());
          

          should be this:

          echo plagiarism_update_status($this->get_course(), $this->get_course_module());
          
          Show
          Dan Marsden added a comment - and this: plagiarism_update_status($ this ->get_course(), $ this ->get_course_module()); should be this: echo plagiarism_update_status($ this ->get_course(), $ this ->get_course_module());
          Hide
          Damyon Wiese added a comment -

          Haven't tested these changes yet - will do shortly.

          Show
          Damyon Wiese added a comment - Haven't tested these changes yet - will do shortly.
          Hide
          Damyon Wiese added a comment -

          In testing this I found the following issue.

          The API may have changed - but only the turn it in contributed plagiarism modules has been updated to use the new API (I assume). Because the function name has not changed - all other modules silently break. I would have preferred an API breaking change here as it is not possible to write one module that supports 2.2 and 2.3.

          Dan can you comment on this?

          Thanks - Damyon

          Show
          Damyon Wiese added a comment - In testing this I found the following issue. The API may have changed - but only the turn it in contributed plagiarism modules has been updated to use the new API (I assume). Because the function name has not changed - all other modules silently break. I would have preferred an API breaking change here as it is not possible to write one module that supports 2.2 and 2.3. Dan can you comment on this? Thanks - Damyon
          Hide
          Dan Marsden added a comment -

          Thanks Damyon - btw Crot isn't really supported anymore as the bing api is no longer free to use and has been merged into Azure - I don't think anyone has done the conversion work yet so unless a different search engine api is found that is free to use it might not get much use anymore.
          The renderer changes for Moodle 2.3 haven't been included either.

          The same team behind Crot have developed an external commercial Crot pro service which looks interesting though.

          in any case - if someone from HQ ends up doing the QA on this I might be able to share some my URKUND test account for the purposes of QA.

          Show
          Dan Marsden added a comment - Thanks Damyon - btw Crot isn't really supported anymore as the bing api is no longer free to use and has been merged into Azure - I don't think anyone has done the conversion work yet so unless a different search engine api is found that is free to use it might not get much use anymore. The renderer changes for Moodle 2.3 haven't been included either. The same team behind Crot have developed an external commercial Crot pro service which looks interesting though. in any case - if someone from HQ ends up doing the QA on this I might be able to share some my URKUND test account for the purposes of QA.
          Hide
          Dan Marsden added a comment -

          Hi Damyon - just saw your update after posting my above comment - Urkund and Turnitin both use the new style - mod/assignment will work with the new changes too.

          Crot is no longer supported - which other plagiarism plugins that support mod/assign still need updating?

          Show
          Dan Marsden added a comment - Hi Damyon - just saw your update after posting my above comment - Urkund and Turnitin both use the new style - mod/assignment will work with the new changes too. Crot is no longer supported - which other plagiarism plugins that support mod/assign still need updating?
          Hide
          Dan Marsden added a comment -

          to clarify further - older plugins (eg crot or older versions of turnitin/urkund) will still work in 2.3 with this change as the output is made at the same time as the function call - only mod/assign introduces issues for older plugins or unsupported plugins.

          also important to note that this api change occured before mod/assign landed in core.

          Show
          Dan Marsden added a comment - to clarify further - older plugins (eg crot or older versions of turnitin/urkund) will still work in 2.3 with this change as the output is made at the same time as the function call - only mod/assign introduces issues for older plugins or unsupported plugins. also important to note that this api change occured before mod/assign landed in core.
          Hide
          Damyon Wiese added a comment -

          OK thanks Dan,

          This is a small change so I'll submit for review and it can get a proper test with urkund by HQ.

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - OK thanks Dan, This is a small change so I'll submit for review and it can get a proper test with urkund by HQ. Cheers, Damyon
          Hide
          Dan Poltawski added a comment -

          Thanks guys, seems ok

          Show
          Dan Poltawski added a comment - Thanks guys, seems ok
          Hide
          Sam Hemelryk added a comment -

          Thanks Damyon, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Damyon, this has been integrated now
          Hide
          Tim Barker added a comment -

          Two things, crot is broken so I had to hack Moodle to get it working.

          2 once it was installed I couldn't get the plagiarism warning to appear at all. I do not know whether this is related to the fix or whether it is related to Crot being broken. I therefore have no way of testing this further.

          Show
          Tim Barker added a comment - Two things, crot is broken so I had to hack Moodle to get it working. 2 once it was installed I couldn't get the plagiarism warning to appear at all. I do not know whether this is related to the fix or whether it is related to Crot being broken. I therefore have no way of testing this further.
          Hide
          Damyon Wiese added a comment -

          Thanks Tim,

          Dan suggested using URKUND for the test.

          (It would be really nice to have a dummy plagiarism plugin for testing that needs no configuration and can be used as a template for people makeing plagiarism plugins).

          Show
          Damyon Wiese added a comment - Thanks Tim, Dan suggested using URKUND for the test. (It would be really nice to have a dummy plagiarism plugin for testing that needs no configuration and can be used as a template for people makeing plagiarism plugins).
          Hide
          Dan Marsden added a comment -

          we have a template already - check the docs or my git repo for the example - but it won't work as a "dummy" plugin for testing.

          Tim - as Damyon mentions, please see my comments above regarding Crot - it's no longer being actively maintained.

          If this must be tested with an active plugin (I'm not sure why it does considering there aren't any actually in core) you are welcome to touch base with me privately and I can share access to an Urkund test account.

          Show
          Dan Marsden added a comment - we have a template already - check the docs or my git repo for the example - but it won't work as a "dummy" plugin for testing. Tim - as Damyon mentions, please see my comments above regarding Crot - it's no longer being actively maintained. If this must be tested with an active plugin (I'm not sure why it does considering there aren't any actually in core) you are welcome to touch base with me privately and I can share access to an Urkund test account.
          Hide
          Tim Barker added a comment -

          Works perfectly nice one!

          Show
          Tim Barker added a comment - Works perfectly nice one!
          Hide
          Dan Marsden added a comment -

          phew - thanks Tim!

          Show
          Dan Marsden added a comment - phew - thanks Tim!
          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:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: