Uploaded image for project: '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

          Attachments

            Activity

            Hide
            danmarsden Dan Marsden added a comment -

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

            Show
            danmarsden Dan Marsden added a comment - Hi Damyon - can you please check to make sure you're happy with this change? - thanks.
            Hide
            damyon 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 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
            stronk7 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
            stronk7 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks Dan, this has been integrated now

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

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

            Show
            timb Tim Barker added a comment - Is it possible to test this at unit tests? If so, were unit tests provided with the patch?
            Hide
            timb 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
            timb 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
            danmarsden 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
            danmarsden 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
            danmarsden 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
            danmarsden 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
            stronk7 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
            stronk7 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
            timb 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
            timb 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  12/Nov/12