Moodle
  1. Moodle
  2. MDL-8119

review the use of $course object in mnet related code in course/report/log/index.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.8
    • Fix Version/s: 1.8.4
    • Component/s: Course, MNet
    • Labels:
      None
    • Affected Branches:
      MOODLE_18_STABLE
    • Fixed Branches:
      MOODLE_18_STABLE
    • Rank:
      29332

      Description

      • optional_param() must have 3 parameter - missing default value for $id and $host_course
      • can not work without mnet

        Activity

        Hide
        Donal McMullan added a comment -

        Reworked some elements of this page that may have changed it's function.
        Pretty sure it's ok though.

        Show
        Donal McMullan added a comment - Reworked some elements of this page that may have changed it's function. Pretty sure it's ok though.
        Hide
        Petr Škoda added a comment -

        Definitely not ok, reopening:

        1/ throws many warnings when mnet log is empty
        2/ the constructed mnet $course object is used in access control checks, local logs, etc

        Either the course is local and then it can be fetched from local course table or it is remove and all use of $course->xxx must be fixed.
        I do not understand the mnet code much yet, I am studying it now and finding bugs there...

        Show
        Petr Škoda added a comment - Definitely not ok, reopening: 1/ throws many warnings when mnet log is empty 2/ the constructed mnet $course object is used in access control checks, local logs, etc Either the course is local and then it can be fetched from local course table or it is remove and all use of $course->xxx must be fixed. I do not understand the mnet code much yet, I am studying it now and finding bugs there...
        Hide
        Martin Dougiamas added a comment -

        Is this still an issue?

        Show
        Martin Dougiamas added a comment - Is this still an issue?
        Hide
        Donal McMullan added a comment -

        Hi Petr - I'm not getting the warnings that you mention... I'm working on the $course problem.

        Show
        Donal McMullan added a comment - Hi Petr - I'm not getting the warnings that you mention... I'm working on the $course problem.
        Hide
        Petr Škoda added a comment -

        Hi, set debugging to max level, set up fresh mnet and go to this page using link in the configuration page - the log should be still empty and you should get many warnings. I do not remember it exactly, but I can try to replicate it once later. Unfortunately I am a bit in a hurry - 1.8.1 should be released really soon and I am going to be in Seville this/next week Anyway the warnings are not a priority for 1.8.1 release IMO. I would prefer to see MDL-9288 fixed instead

        Show
        Petr Škoda added a comment - Hi, set debugging to max level, set up fresh mnet and go to this page using link in the configuration page - the log should be still empty and you should get many warnings. I do not remember it exactly, but I can try to replicate it once later. Unfortunately I am a bit in a hurry - 1.8.1 should be released really soon and I am going to be in Seville this/next week Anyway the warnings are not a priority for 1.8.1 release IMO. I would prefer to see MDL-9288 fixed instead
        Hide
        Donal McMullan added a comment - - edited

        Yup - I've been working on MDL-9288 this pm, and I think all the changes in your patch are required. I'm hoping to apply that in the next hour or so.

        Cheers Petr

        Show
        Donal McMullan added a comment - - edited Yup - I've been working on MDL-9288 this pm, and I think all the changes in your patch are required. I'm hoping to apply that in the next hour or so. Cheers Petr
        Hide
        Martin Dougiamas added a comment -

        Donal, is this checked in yet? What is the status of this?

        Show
        Martin Dougiamas added a comment - Donal, is this checked in yet? What is the status of this?
        Hide
        Donal McMullan added a comment -

        Although I couldn't reproduce Petr's issue, I did fix some problems with logging where mnet hosts are involved.
        It's a bit hackish in places and relies on the course->id being zero (in the mdl_log table) when the mdl_log table is recording that we've looked at the logs for a remote course.

        I think it improves behaviour though.

        Show
        Donal McMullan added a comment - Although I couldn't reproduce Petr's issue, I did fix some problems with logging where mnet hosts are involved. It's a bit hackish in places and relies on the course->id being zero (in the mdl_log table) when the mdl_log table is recording that we've looked at the logs for a remote course. I think it improves behaviour though.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: