Details

    • Rank:
      48253

      Description

      spotted this in master code - so not sure if it causes any issues (haven't looked to see if it's in stable branches)

      undefined var $grading_info - should be $gradinginfo.

      4515             if (isset($gradinginfo->items[0]->grades[$userid]) &&
      4516                     !$grading_info->items[0]->grades[$userid]->hidden) {
      

        Activity

        Hide
        Damyon Wiese added a comment - - edited

        Thanks Dan,

        I checked and it is in 2.3, 2.4 and 2.5.

        Will add a fix and a unit test.

        Show
        Damyon Wiese added a comment - - edited Thanks Dan, I checked and it is in 2.3, 2.4 and 2.5. Will add a fix and a unit test.
        Hide
        Prateek Sachan added a comment - - edited

        I've fixed this bugs on all the 3 branches. Here are the links of the commits I made on my Github repo:

        master branch: https://github.com/prateeksachan/moodle/compare/master...MDL-38361
        MOODLE_24_STABLE branch: https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38361_24
        MOODLE_23_STABLE branch: https://github.com/prateeksachan/moodle/compare/MOODLE_23_STABLE...MDL-38361_23

        Show
        Prateek Sachan added a comment - - edited I've fixed this bugs on all the 3 branches. Here are the links of the commits I made on my Github repo: master branch: https://github.com/prateeksachan/moodle/compare/master...MDL-38361 MOODLE_24_STABLE branch: https://github.com/prateeksachan/moodle/compare/MOODLE_24_STABLE...MDL-38361_24 MOODLE_23_STABLE branch: https://github.com/prateeksachan/moodle/compare/MOODLE_23_STABLE...MDL-38361_23
        Hide
        Aparup Banerjee added a comment -

        Thanks Prateek, the change looks right.

        For completeness, adding test instructions to replicate the problem and verifying g the fix helps with our weekly testing day.

        Ps: diff urls help for quick reviews - https://github.com/prateeksachan/moodle/compare/master...MDL-38361

        Pps: looking forward to more fixes

        Show
        Aparup Banerjee added a comment - Thanks Prateek, the change looks right. For completeness, adding test instructions to replicate the problem and verifying g the fix helps with our weekly testing day. Ps: diff urls help for quick reviews - https://github.com/prateeksachan/moodle/compare/master...MDL-38361 Pps: looking forward to more fixes
        Hide
        Prateek Sachan added a comment -

        Thanks Aparup, for pointing that out. I didn't understood what you meant earlier. I've updated the links accordingly to show the "compare" versions.

        Show
        Prateek Sachan added a comment - Thanks Aparup, for pointing that out. I didn't understood what you meant earlier. I've updated the links accordingly to show the "compare" versions.
        Hide
        Damyon Wiese added a comment -

        The function is not easily covered by unit tests without hacking $_GET/$_POST - it could be covered with a behat test - but I would rather see an organised list of behat tests than adding lots of tests matching fixes.

        Show
        Damyon Wiese added a comment - The function is not easily covered by unit tests without hacking $_GET/$_POST - it could be covered with a behat test - but I would rather see an organised list of behat tests than adding lots of tests matching fixes.
        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
        Dan Poltawski added a comment -

        Integrated to master, 24 and 23 - thanks Prateek!

        Show
        Dan Poltawski added a comment - Integrated to master, 24 and 23 - thanks Prateek!
        Hide
        Prateek Sachan added a comment -

        Show
        Prateek Sachan added a comment -
        Hide
        Mark Nelson added a comment -

        Works as expected, passing. Thanks for your contribution.

        Show
        Mark Nelson added a comment - Works as expected, passing. Thanks for your contribution.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I feel myself really alone tonight! So was time to push your fixes upstream!

        "Lest we forget. We will remember them."

        Thanks and ciao!

        Show
        Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!
        Hide
        Aparup Banerjee added a comment -

        Thanks Prateek, try to look into writing some unit tests which will help towards writing/maintaining sanity in a large component like http://docs.moodle.org/dev/Global_Search

        Show
        Aparup Banerjee added a comment - Thanks Prateek, try to look into writing some unit tests which will help towards writing/maintaining sanity in a large component like http://docs.moodle.org/dev/Global_Search

          People

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

            Dates

            • Created:
              Updated:
              Resolved: