Moodle
  1. Moodle
  2. MDL-40515

Selecting a section name in the logs report causes a DB error

        Details

        • Database:
          PostgreSQL
        • Testing Instructions:
          Hide

          1. Go to a course with activities in more than one section.
          2. From the settings block, go to the Log report for the course (Reports > Log). This will be in the navigation block for earlier versions of moodle.
          3. Verify the activities selector lists topics as bolded groups that you can't select.
          4. Select an activity and click the button to generate the report, verify the results come up as expected.

          Show
          1. Go to a course with activities in more than one section. 2. From the settings block, go to the Log report for the course (Reports > Log). This will be in the navigation block for earlier versions of moodle. 3. Verify the activities selector lists topics as bolded groups that you can't select. 4. Select an activity and click the button to generate the report, verify the results come up as expected.
        • Difficulty:
          Easy
        • Affected Branches:
          MOODLE_24_STABLE, MOODLE_25_STABLE
        • Fixed Branches:
          MOODLE_24_STABLE, MOODLE_25_STABLE
        • Pull from Repository:
        • Pull 2.4 Branch:
        • Pull 2.4 Diff URL:
        • Pull 2.5 Branch:
        • Pull 2.5 Diff URL:
        • Pull Master Branch:
        • Pull Master Diff URL:
        • Rank:
          51330

          Description

          When using the log report (Site pages / ► Reports / ► Logs), and picking a course to report logs on, if you then use the activity drop down, but choose a section heading in there (i.e. '-- Topic 1 --') you get a fatal db error on postgres.

          Debug info: ERROR: invalid input syntax for integer: "section1"
SELECT COUNT(*)
FROM mdl_log l
WHERE l.cmid = $1 AND l.time > $2 AND l.time < $3
[array (
0 => 'section1',
1 => 1372946400,
2 => 1373032800,
)]

          Given the underlying code only deals with a single modid being passed from this form, I believe the topic headings should instead be optgroups that aren't actually meant to be selectable.

          It appears mysql silently doesn't care that you're trying to compare a string on an int field... The error doesn't come up on mysql, but the results displayed are incorrect (you'd expect to maybe see all the logs for that topic, you dont).

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            Michael de Raadt added a comment -

            Thanks for spotting this and sharing a fix.

            Show
            Michael de Raadt added a comment - Thanks for spotting this and sharing a fix.
            Hide
            Permalink
            Adrian Greeve added a comment -

            Hello Adam,

            I agree with the approach that you have taken with solving this issue. The only thing that I think needs looking at is report_log_print_selector_form() (around line 408) This needs similar code to what you have provided further up in the file.

            Thanks.

            Show
            Adrian Greeve added a comment - Hello Adam, I agree with the approach that you have taken with solving this issue. The only thing that I think needs looking at is report_log_print_selector_form() (around line 408) This needs similar code to what you have provided further up in the file. Thanks.
            Hide
            Permalink
            Adam Olley added a comment -

            Applied to both functions and rebased branches.

            Show
            Adam Olley added a comment - Applied to both functions and rebased branches.
            Hide
            Permalink
            Adrian Greeve added a comment -

            Thanks Adam,

            This looks good. Please submit for integration review.

            Show
            Adrian Greeve added a comment - Thanks Adam, This looks good. Please submit for integration review.
            Hide
            Permalink
            Dan Poltawski added a comment -

            Integrated to master, 25 and 24 - thanks Adam.

            Show
            Dan Poltawski added a comment - Integrated to master, 25 and 24 - thanks Adam.
            Hide
            Permalink
            Frédéric Massart added a comment -

            Works as described. Thanks.

            Show
            Frédéric Massart added a comment - Works as described. Thanks.
            Hide
            Permalink
            Damyon Wiese added a comment -

            Here lies 52 bugs.
            All fixed or swept under a rug.
            If they come back one day,
            To our dismay,
            We all will feel quite un-smug.

            Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

            Show
            Damyon Wiese added a comment - Here lies 52 bugs. All fixed or swept under a rug. If they come back one day, To our dismay, We all will feel quite un-smug. Thanks for the reporting/fixing/testing on this issue. It has been sent upstream.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: