Moodle
  1. Moodle
  2. MDL-35255

plagiarism_update_status should be called after header printed

    Details

      Description

      in mod/assign view_grading_table, the function plagiarism_update_status should be called after the page header has been printed as this function adds further admin links to the screen when required (such as a link to login to Turnitin as a teacher, buttons to regenerate plagiarism content for the assignment etc.)

        Gliffy Diagrams

          Activity

          Hide
          Dan Marsden added a comment -

          Hi Damyon - can you please check to make sure you're happy with this change? - thanks.

          Show
          Dan Marsden added a comment - Hi Damyon - can you please check to make sure you're happy with this change? - thanks.
          Hide
          Damyon Wiese added a comment -

          This looks fine to me - it's a small change and not likely to introduce any regressions.

          The lines affected by this patch are too long - but that is not related to this change and should be addressed in a separate issue when there is not much queued for integration (a general cleanup for mod_assign would be good just after the 2.4 release).

          Thanks Dan

          Show
          Damyon Wiese added a comment - This looks fine to me - it's a small change and not likely to introduce any regressions. The lines affected by this patch are too long - but that is not related to this change and should be addressed in a separate issue when there is not much queued for integration (a general cleanup for mod_assign would be good just after the 2.4 release). Thanks Dan
          Hide
          Eloy Lafuente (stronk7) 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
          Eloy Lafuente (stronk7) 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 -

          Thanks Dan, this has been integrated now

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

          Is it possible to test this at unit tests? If so, were unit tests provided with the patch?

          Show
          Tim Barker added a comment - Is it possible to test this at unit tests? If so, were unit tests provided with the patch?
          Hide
          Tim Barker added a comment -

          Nothing has failed but I would like to query something before we give this the nod. Please see my comments about unit tests.

          Show
          Tim Barker added a comment - Nothing has failed but I would like to query something before we give this the nod. Please see my comments about unit tests.
          Hide
          Dan Marsden added a comment -

          I don't plan to write any unit tests at this stage - this is really a regression caused by new mod_assign code - it was fine in mod_assignment. If you want to write some unit tests feel free.

          Show
          Dan Marsden added a comment - I don't plan to write any unit tests at this stage - this is really a regression caused by new mod_assign code - it was fine in mod_assignment. If you want to write some unit tests feel free.
          Hide
          Dan Marsden added a comment -

          can unit tests even check this sort of thing? - if a function is called before or after a page's output?

          Show
          Dan Marsden added a comment - can unit tests even check this sort of thing? - if a function is called before or after a page's output?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          yeah, don't think this is (phpunit) testable at all. Only having some "mockup/fake" (or real) plagiarism plugin installed would enable visual testing.

          I'm sending this back to testing for Tim to know and decide if it's ok to pass without testing or plagiarism + visual testing is needed. As an integrator, I think the code (just a move), has sense.

          Show
          Eloy Lafuente (stronk7) added a comment - yeah, don't think this is (phpunit) testable at all. Only having some "mockup/fake" (or real) plagiarism plugin installed would enable visual testing. I'm sending this back to testing for Tim to know and decide if it's ok to pass without testing or plagiarism + visual testing is needed. As an integrator, I think the code (just a move), has sense.
          Hide
          Tim Barker added a comment -

          I'm happy with that then, if there are no tests specific to the issue then regression testing would be the only requirement. Regression testing of Moodle is now fairly continuous, with coverage continuing to improve. So I am happy to let it go in having been reviewed by Eloy's experienced eyes! Well done

          Show
          Tim Barker added a comment - I'm happy with that then, if there are no tests specific to the issue then regression testing would be the only requirement. Regression testing of Moodle is now fairly continuous, with coverage continuing to improve. So I am happy to let it go in having been reviewed by Eloy's experienced eyes! Well done
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: