Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0.2
    • Component/s: Messages
    • Labels:
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Rank:
      16126

      Description

      Logged in as an admin viewing Navigation > Users > Sam Student > Activity reports > All logs it seems messaging activity is no longer included in the logs. It was in 1.9.

        Activity

        Hide
        Andrew Davis added a comment -

        Ive added logging when a user sends a message. Note that nothing is logged in /user/messageselect.php or /admin/user/user_bulk_message.php but it wasnt in 1.9 either. Not sure if I should add it.

        repo: git://github.com/andyjdavis/moodle.git
        branch: MDL-26119_message_logging
        diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26119_message_logging

        Show
        Andrew Davis added a comment - Ive added logging when a user sends a message. Note that nothing is logged in /user/messageselect.php or /admin/user/user_bulk_message.php but it wasnt in 1.9 either. Not sure if I should add it. repo: git://github.com/andyjdavis/moodle.git branch: MDL-26119 _message_logging diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26119_message_logging
        Hide
        Sam Hemelryk added a comment -

        Hi Andrew
        The changes look good, I haven't look at the logging going on in 1.9 however if it was only the one place then this has my +1.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Andrew The changes look good, I haven't look at the logging going on in 1.9 however if it was only the one place then this has my +1. Cheers Sam
        Hide
        Andrew Davis added a comment -

        PULL-219

        Show
        Andrew Davis added a comment - PULL-219
        Hide
        Sam Hemelryk added a comment -

        Hi Andrew,
        The PULL request has been rejected because the URL it is generating is broken. You also need to check whether there is an entry in lib/db/log.php for this log event.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Andrew, The PULL request has been rejected because the URL it is generating is broken. You also need to check whether there is an entry in lib/db/log.php for this log event. Cheers Sam
        Hide
        Andrew Davis added a comment -

        Ok, I've fixed the issue with logged URL. Not sure what was up with me/that URL before. To get a URL that takes you to the specific message I had to alter some messaging functions to return a message ID instead of just boolean success flag.

        More through good luck than good design lib/db/log.php was in fact correct so I haven't had to change it.

        Show
        Andrew Davis added a comment - Ok, I've fixed the issue with logged URL. Not sure what was up with me/that URL before. To get a URL that takes you to the specific message I had to alter some messaging functions to return a message ID instead of just boolean success flag. More through good luck than good design lib/db/log.php was in fact correct so I haven't had to change it.
        Hide
        Andrew Davis added a comment -

        Also, there is one little issue with this I havent been able to iron out. Although the message URL is being stored unescaped in the database its being escaped prior to display. For some reason Firefox (3.6.13) unescapes the ampersands correctly but leaves the hash (#) escaped so it doesnt jump down into the history to the correct point.

        Show
        Andrew Davis added a comment - Also, there is one little issue with this I havent been able to iron out. Although the message URL is being stored unescaped in the database its being escaped prior to display. For some reason Firefox (3.6.13) unescapes the ampersands correctly but leaves the hash (#) escaped so it doesnt jump down into the history to the correct point.
        Hide
        Sam Hemelryk added a comment -

        Hi Andrew,
        Other than that one point that you raised above it works fine now (tested myself as well this time).
        In regards to that one point it is because make_log_url() in course/lib.php doesn't respect #'s on the URL, it processes it as part of the last argument.
        I think it is probably best to talk to Petr about that to find out whether that function needs to be fixed or whether you shouldn't log the #.

        Also before you create your PULL request you should probably rebase the patch, I did in order to get a diff against current master and hit a small conflict.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Hi Andrew, Other than that one point that you raised above it works fine now (tested myself as well this time). In regards to that one point it is because make_log_url() in course/lib.php doesn't respect #'s on the URL, it processes it as part of the last argument. I think it is probably best to talk to Petr about that to find out whether that function needs to be fixed or whether you shouldn't log the #. Also before you create your PULL request you should probably rebase the patch, I did in order to get a diff against current master and hit a small conflict. Cheers Sam
        Hide
        Andrew Davis added a comment -

        Rebased. Ive also asked Petr for some feedback on the # issue.

        Show
        Andrew Davis added a comment - Rebased. Ive also asked Petr for some feedback on the # issue.
        Hide
        Petr Škoda added a comment -

        The logging needs a rewrite pretty badly, it is very old code. My +1 for any brute force hack that solves the # trouble there. Please file a separate issue for that. Thanks.

        Show
        Petr Škoda added a comment - The logging needs a rewrite pretty badly, it is very old code. My +1 for any brute force hack that solves the # trouble there. Please file a separate issue for that. Thanks.
        Hide
        Andrew Davis added a comment -

        Ive raised MDL-26386 to deal with the hash escaping issue.

        Show
        Andrew Davis added a comment - Ive raised MDL-26386 to deal with the hash escaping issue.
        Hide
        Andrew Davis added a comment -

        As well as the hash issue mentioned above Ive raised MDL-26387 to deal with another issue you'll probably spot when you look at the diff for this issue.

        repo: git://github.com/andyjdavis/moodle.git
        branch: MDL-26119_message_logging
        diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26119_message_logging

        PULL-294

        Show
        Andrew Davis added a comment - As well as the hash issue mentioned above Ive raised MDL-26387 to deal with another issue you'll probably spot when you look at the diff for this issue. repo: git://github.com/andyjdavis/moodle.git branch: MDL-26119 _message_logging diff: https://github.com/andyjdavis/moodle/compare/master...MDL-26119_message_logging PULL-294

          People

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

            Dates

            • Created:
              Updated:
              Resolved: