Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Activity

          Hide
          andyjdavis 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
          andyjdavis 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
          samhemelryk 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
          samhemelryk 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
          andyjdavis Andrew Davis added a comment -

          PULL-219

          Show
          andyjdavis Andrew Davis added a comment - PULL-219
          Hide
          samhemelryk 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
          samhemelryk 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
          andyjdavis 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
          andyjdavis 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
          andyjdavis 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
          andyjdavis 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
          samhemelryk 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
          samhemelryk 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
          andyjdavis Andrew Davis added a comment -

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

          Show
          andyjdavis Andrew Davis added a comment - Rebased. Ive also asked Petr for some feedback on the # issue.
          Hide
          skodak Petr Skoda 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
          skodak Petr Skoda 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
          andyjdavis Andrew Davis added a comment -

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

          Show
          andyjdavis Andrew Davis added a comment - Ive raised MDL-26386 to deal with the hash escaping issue.
          Hide
          andyjdavis 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
          andyjdavis 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:
                Fix Release Date:
                21/Feb/11