Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Course
    • Labels:
      None
    • Testing Instructions:
      Hide

      This needs to be tested in multiple databases.
      As teacher

      1. set course layout setting to "show one section per page"
      2. access a course (topic/week format) and view a few section (eg: http://moodle/course/view.php?id=xx&section=3)
      3. check the log report for the course and make sure there's "course view section" entry pointing to the correct section.

      As admin

      1. backup the course and tick on "include course logs"
      2. restore the course as new course
      3. check the log for the new course and make sure there's "course view section" entry which takes you to the right section.
      Show
      This needs to be tested in multiple databases. As teacher set course layout setting to "show one section per page" access a course (topic/week format) and view a few section (eg: http://moodle/course/view.php?id=xx&section=3 ) check the log report for the course and make sure there's "course view section" entry pointing to the correct section. As admin backup the course and tick on "include course logs" restore the course as new course check the log for the new course and make sure there's "course view section" entry which takes you to the right section.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-33465-master

      Description

      Log entries for "course view section" should use sectionid parameter isntead of section parameter while generating action urls.

      The reason being that section number is a variable thing, and changes when section are moved around, which leads to incorrect urls being stored in the log entries.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            ankit_frenz Ankit Agarwal added a comment -

            @integrator
            This must land after MDL-33369 is integrated.
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - @integrator This must land after MDL-33369 is integrated. Thanks
            Hide
            rwijaya Rossiani Wijaya added a comment -

            This looks good Ankit.

            Show
            rwijaya Rossiani Wijaya added a comment - This looks good Ankit.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Thanks for the review Rosie,
            Sending this for integration

            @integrator
            Master only

            Show
            ankit_frenz Ankit Agarwal added a comment - Thanks for the review Rosie, Sending this for integration @integrator Master only
            Hide
            poltawski Dan Poltawski added a comment -

            Note to integrator: this is blocked by MDL-33369

            Show
            poltawski Dan Poltawski added a comment - Note to integrator: this is blocked by MDL-33369
            Hide
            nebgor Aparup Banerjee added a comment -

            Thanks Ankit, this has been integrated into master (as well as its dependant issue).

            They are ready for testing.

            Show
            nebgor Aparup Banerjee added a comment - Thanks Ankit, this has been integrated into master (as well as its dependant issue). They are ready for testing.
            Hide
            poltawski Dan Poltawski added a comment -

            Debug info: ERROR: COALESCE types character varying and bigint cannot be matched
            LINE 1: SELECT COALESCE(name, section) FROM mdl_course_sections WHER...
            ^
            SELECT COALESCE(name, section) FROM mdl_course_sections WHERE id = $1
            [array (
            0 => '3',
            )]
            Error code: dmlreadexception
            Stack trace:
            line 413 of /lib/dml/moodle_database.php: dml_read_exception thrown
            line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
            line 703 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
            line 1341 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql()
            line 1414 of /lib/dml/moodle_database.php: call to moodle_database->get_record_sql()
            line 1395 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql()
            line 1374 of /lib/dml/moodle_database.php: call to moodle_database->get_field_select()
            line 387 of /course/lib.php: call to moodle_database->get_field()
            line 157 of /report/log/index.php: call to print_log()

            Show
            poltawski Dan Poltawski added a comment - Debug info: ERROR: COALESCE types character varying and bigint cannot be matched LINE 1: SELECT COALESCE(name, section) FROM mdl_course_sections WHER... ^ SELECT COALESCE(name, section) FROM mdl_course_sections WHERE id = $1 [array ( 0 => '3', )] Error code: dmlreadexception Stack trace: line 413 of /lib/dml/moodle_database.php: dml_read_exception thrown line 243 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end() line 703 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 1341 of /lib/dml/moodle_database.php: call to pgsql_native_moodle_database->get_records_sql() line 1414 of /lib/dml/moodle_database.php: call to moodle_database->get_record_sql() line 1395 of /lib/dml/moodle_database.php: call to moodle_database->get_field_sql() line 1374 of /lib/dml/moodle_database.php: call to moodle_database->get_field_select() line 387 of /course/lib.php: call to moodle_database->get_field() line 157 of /report/log/index.php: call to print_log()
            Hide
            ankit_frenz Ankit Agarwal added a comment - - edited

            The above error was introduced in MDL-32766, any way I have added a second commit to fix up the issue. Also created MDL-33606 to improve the code to use section number if section name is null.

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - - edited The above error was introduced in MDL-32766 , any way I have added a second commit to fix up the issue. Also created MDL-33606 to improve the code to use section number if section name is null. Thanks
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Ankit for that fix to at least get the logs working on postgres (albeit without section name).

            Show
            poltawski Dan Poltawski added a comment - Thanks Ankit for that fix to at least get the logs working on postgres (albeit without section name).
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Note that this was integrated incorrectly, with one wrong conflict resolution, so this line:

            1109) $this->set_mapping('course_sectionnumber', $oldsection, ....

            was kept at it's leading to broken restore operation each time one restore is tried onto blank course.

            Such merge needs to be reviewed and fixed. It's causing any testing involving restore to fail badly (like MDL-33080).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Note that this was integrated incorrectly, with one wrong conflict resolution, so this line: 1109) $this->set_mapping('course_sectionnumber', $oldsection, .... was kept at it's leading to broken restore operation each time one restore is tried onto blank course. Such merge needs to be reviewed and fixed. It's causing any testing involving restore to fail badly (like MDL-33080 ). Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            My testing is failing due to this problem.

            Show
            poltawski Dan Poltawski added a comment - My testing is failing due to this problem.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I'm reviewing the offending merge right now... stay tuned...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I'm reviewing the offending merge right now... stay tuned...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Done, added commit, deleting the 2 lines incorrectly kept by the merge resolution.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Done, added commit, deleting the 2 lines incorrectly kept by the merge resolution.
            Hide
            poltawski Dan Poltawski added a comment -

            Tested and thanks.

            Show
            poltawski Dan Poltawski added a comment - Tested and thanks.
            Hide
            nebgor Aparup Banerjee added a comment -

            ah noted! thanks for fixing that up!

            Show
            nebgor Aparup Banerjee added a comment - ah noted! thanks for fixing that up!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

            Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

            Many thanks for your collaboration!

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12